feat: Custom viewshed polygon reconstructor - #67
Merged
Conversation
Collaborator
Author
|
Current state of bencmarks: Correct: This PR: |
tombh
force-pushed
the
tombh/adjacent-polar-viewshed-builder
branch
from
June 19, 2026 09:45
60f7aac to
d343bc7
Compare
At higher angle counts a traditional polygon unioning of all the polar segments suffers from O(N²). For example, even in Rust a 3600 angled viewshed takes ~20s to create its GeoJSON. And many minutes in browser JS. Therefore we have to design an entirely new algorithm that takes advantage of the fact that we are only ever adding new pieces to the growing viewshed in a very specific way. Namely we only ever add polar segments, that are always contained within the arc of a single angle, onto the _side_ of a polygon. This returns polygon reconstruction to a ~200ms for 3600 angled viewsheds.
tombh
force-pushed
the
tombh/adjacent-polar-viewshed-builder
branch
from
June 28, 2026 09:28
d343bc7 to
ac0d147
Compare
tombh
marked this pull request as ready for review
June 28, 2026 09:31
Collaborator
Author
|
Benchmark viewshed is now pixel-perfect with the existing one. Though note that the file size is slightly smaller due to a tidier algorithm. |
ryan-berger
reviewed
Jul 3, 2026
ryan-berger
left a comment
ryan-berger
left a comment
Member
There was a problem hiding this comment.
I'm going to leave this for some changes responses, and dig into the algorithm a lot heavier tomorrow
Adds the following lints: unreachable_pub = "warn" redundant_pub_crate = "allow" This enforces the practice of explicitly defining visibility. Therefore, if a symbol isn't truly public to the outside world then it should be qualified with `pub(crate)`, `pub(super)`, etc. This is slightly more verbose but helps reasoning about API surfaces and also safeguards against unexpected symbols becoming public when parent visibility is changed.
tombh
force-pushed
the
tombh/adjacent-polar-viewshed-builder
branch
from
July 4, 2026 07:36
aab2a56 to
7db051b
Compare
ryan-berger
reviewed
Jul 4, 2026
| } | ||
|
|
||
| /// Join a final polygon to either itself or a starting polygon. | ||
| fn join_final_polygon( |
Member
There was a problem hiding this comment.
This code is nearly identical to join_segment in the next.rs.
The only difference I can see is how maybe_joining_polygon is constructed, and maybe base_polygon?
To me this says that there is some incorrect abstraction here. There should be a function that does finds the start/end pair you are looking for, only referencing its parameters and not behind a &mut self
Collaborator
Author
There was a problem hiding this comment.
Now that we have the openings iterator, these functions look quite different now.
Member
There was a problem hiding this comment.
Things have changed and I think this isn't a problem anymore
ryan-berger
reviewed
Jul 4, 2026
And other review improvemnts
And other improvements from review.
tombh
force-pushed
the
tombh/adjacent-polar-viewshed-builder
branch
from
July 5, 2026 09:20
7db051b to
dfec260
Compare
ryan-berger
reviewed
Jul 5, 2026
ryan-berger
left a comment
ryan-berger
left a comment
Member
There was a problem hiding this comment.
Less comments, but getting close
ryan-berger
reviewed
Jul 12, 2026
This prepares for compiling this code to WASM.
Successfully used in the browser.
ryan-berger
approved these changes
Jul 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
At higher angle counts a traditional polygon unioning of all the polar segments suffers from O(N²). For example, even in Rust a 3600 angled viewshed takes ~20s to create its GeoJSON. And many minutes in browser JS.
Therefore we have to design an entirely new algorithm that takes advantage of the fact that we are only ever adding new pieces to the growing viewshed in a very specific way. Namely we only ever add polar segments, that are always contained within the arc of a single angle, onto the side of a polygon. This returns polygon reconstruction to
200ms80ms for 3600 angled viewsheds. There's actually huuuuge amount of room for speeding up this approach, but seeing as its already so much faster than the old method, this is good enough for now.TODO:
Apologies, this is rather big PR to review. Most of the changes are hidden in collapsed file views in the Github Diff view.