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

Fixes parse error with complex shapes #6976

Merged
merged 1 commit into from Jul 24, 2014

Conversation

Projects
None yet
3 participants
@colings86
Copy link
Member

colings86 commented Jul 23, 2014

The bug reproduces when the point under test for the placement of the hole of the polygon has an x coordinate which only intersects with the ends of edges in the main polygon. The previous code threw out these cases as not relevant but an intersect at 1.0 of the distance from the start to the end of an edge is just as valid as an intersect at any other point along the edge. The fix corrects this and adds a test.

Closes #5773

@colings86 colings86 added the review label Jul 23, 2014

@jpountz

This comment has been minimized.

Copy link
Contributor

jpountz commented Jul 24, 2014

LGTM

It would be nice to also fix the comparison of the outputs of Double.compare as mentioned in the issue.

@jpountz jpountz removed the review label Jul 24, 2014

@colings86

This comment has been minimized.

Copy link
Member Author

colings86 commented Jul 24, 2014

@jpountz Agreed it would be nice to fix the Double.compare. However I am having trouble working out what it is supposed to be achieving. It looks like it was probably put in as a bug fix but there don't seem to be any tests that rely on it. I don't want to just change it or remove it without some understanding since it could break something, although I don't know what.

@jpountz

This comment has been minimized.

Copy link
Contributor

jpountz commented Jul 24, 2014

Let's push this PR and create a new issue for that suspicious line of code?

Geo: Fixes parse error with complex shapes
The bug reproduces when the point under test for the placement of the hole of the polygon has an x coordinate which only intersects with the ends of edges in the main polygon. The previous code threw out these cases as not relevant but an intersect at 1.0 of the distance from the start to the end of an edge is just as valid as an intersect at any other point along the edge.  The fix corrects this and adds a test.

Closes #5773

@colings86 colings86 merged commit 5483c62 into elastic:master Jul 24, 2014

@colings86

This comment has been minimized.

Copy link
Member Author

colings86 commented Jul 24, 2014

opened #7016 for the Double.compare issue

@colings86 colings86 deleted the colings86:fix/5773 branch Aug 13, 2014

@colings86 colings86 self-assigned this Aug 21, 2014

@clintongormley clintongormley changed the title Geo: Fixes parse error with complex shapes Fixes parse error with complex shapes Jun 7, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.