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

Fix rounding error between triangle area calculation and intersection calculation #60

Closed
wants to merge 1 commit into from

Conversation

mfogel
Copy link
Contributor

@mfogel mfogel commented Feb 1, 2018

Currently, both the triangle area calculation in signed_area.js and the intersection calculation in segment_intersection.js are used to determine if two line segments are collinear. A signed area of 0 indicates collinearity, while a intersection calculation of 2 also indicates collinearity.

Segment intersection is used here while signed area is used here and here.

It's possible (due to rounding errors) for the signed area calculation to return a non-zero result (indicating the segments are not collinear) while for the same two line segments, the intersection calculation will calculate two intersections (indicating the segments are collinear). When this happens, segments can get inserted into the sweep line but never removed - messing up order of segments in the sweep line which in turn messes up the calculation of whether other line segments should be included in the final result or not.

This PR adds a test case demonstrating that sutiation, and implements a fix.

@rowanwins
Copy link
Collaborator

I've had a quick play and this pull request in conjunction with pull #58 appears to resolve issue #35 .

The only thing that still seems to be broken on it is Difference B - A (see the http://localhost:5000/demo/#overlap_two), this should return an empty array I think, but it's looping to infinity and beyond!

Great work @mfogel

@rowanwins
Copy link
Collaborator

So i just replaced avl with splaytree and this appears to resolve the issue above with Difference B - A

@w8r
Copy link
Owner

w8r commented Feb 11, 2018

Cooool, how are the benchmarks? Splaytree is slightly slower on insertion

@mfogel mfogel closed this Nov 9, 2018
@mfogel mfogel deleted the fix-another-rounding-error branch November 9, 2018 21:28
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.

3 participants