Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] O(nlog n) time algorithm for computing a visibility polygon #62

Merged
merged 60 commits into from
Feb 26, 2021

Conversation

noinia
Copy link
Owner

@noinia noinia commented Dec 14, 2020

many details still missing, moreover some types of the functions don't match up yet, but at least the main skeleton is there already.

@lemmih lemmih marked this pull request as draft December 15, 2020 01:40
@noinia
Copy link
Owner Author

noinia commented Dec 26, 2020

FYI: I think this works/is complete now, but I should write some proper tests to make sure I didn't make any mistakes.

I also fixed the show/read instances for LineSegment along the way.

@lemmih
Copy link
Collaborator

lemmih commented Dec 26, 2020

FYI: I think this works/is complete now, but I should write some proper tests to make sure I didn't make any mistakes.

I have a visibility polygon implementation that uses the SSSP tree. I'll hammer it into shape and submit a PR. Then we can add a QC property that the two algorithms always give the same result.

I also fixed the show/read instances for LineSegment along the way.

I saw, love it. Good Read/Show instances are worth their weight in gold.

@lemmih lemmih marked this pull request as ready for review December 26, 2020 12:46
@lemmih
Copy link
Collaborator

lemmih commented Dec 26, 2020

I pushed a merge commit so remember to pull before making any new changes.

@noinia
Copy link
Owner Author

noinia commented Dec 26, 2020

FYI: I think this works/is complete now, but I should write some proper tests to make sure I didn't make any mistakes.

I have a visibility polygon implementation that uses the SSSP tree. I'll hammer it into shape and submit a PR. Then we can add a QC property that the two algorithms always give the same result.

Ah nice, that seems useful.

One thing to maybe think a bit about is how we treat "spikes", something like this:
image
on the rightmost edge there is a single point that could be considered visible. My current implementation does not include such spikes in the visibility polygon.

@noinia
Copy link
Owner Author

noinia commented Dec 27, 2020

Fixing some weird degeneracy issues (like the one shown above) lead me down some wild goose chase into computing intersections between points and segments, and points and halflines. In particular, I realized that in 2D we can test if a point lies on a linesegment by using only 3 ccw tests. I'm guessing that that will be more robust (and potentially faster) than the generic onSegment test we were using so far. I'm hoping that this may also fix some of the issues users reported that were using Doubles as their numeric type.

@lemmih
Copy link
Collaborator

lemmih commented Jan 17, 2021

Is this PR done? Done enough to be merged?

@noinia
Copy link
Owner Author

noinia commented Jan 18, 2021

Seems there is still some bug that I need to fix before we merge this.

@lemmih
Copy link
Collaborator

lemmih commented Jan 21, 2021

What bugs? Anything I can help with? There are so many goodies in this PR that I would like to see merged.

@noinia
Copy link
Owner Author

noinia commented Feb 8, 2021

I finally had some time to make some progress on this. Seems like its mostly doing the right thing now. I'm hoping to actually wrap this up tomorrow so that we can also release 0.12 soon :)

@noinia noinia merged commit a971f5e into master Feb 26, 2021
@noinia noinia deleted the feature-visibilityalgo branch October 29, 2021 07:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants