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

GeoPolygonFilter fails on ambiguous polygons #9304

Closed
nknize opened this issue Jan 15, 2015 · 5 comments · Fixed by #9339
Closed

GeoPolygonFilter fails on ambiguous polygons #9304

nknize opened this issue Jan 15, 2015 · 5 comments · Fixed by #9339
Labels
:Analytics/Geo Indexing, search aggregations of geo points and shapes >enhancement

Comments

@nknize
Copy link
Contributor

nknize commented Jan 15, 2015

Polygon ambiguity was addressed in ShapeBuilder via #8762. GeoPolygonFilter, however, doesn't use ShapeBuilder and instead relies on legacy pointInPolygon code that fails on ambiguous polygons. One use-case example is presented in #5968 The suggested fix is to translate all candidate points and GeoPolygonFilter points to great-circle coordinates. This, however, will cause a failure with any GeoPolygonFilter that crosses the prime meridian.

The correct approach is to update the GeoPolygonFilter to leverage code written in #8762 by refactoring the ambiguous polygon logic to helper methods. This will also remove any legacy code (and rename any parameters) that might cause confusion when using the Java API.

@nknize nknize self-assigned this Jan 15, 2015
@nknize nknize added :Analytics/Geo Indexing, search aggregations of geo points and shapes >enhancement v2.0.0-beta1 v1.5.0 v1.4.3 labels Jan 15, 2015
nknize added a commit to nknize/elasticsearch that referenced this issue Jan 27, 2015
PR elastic#8672 addresses ambiguous polygons - those that either cross the dateline or span the map - by complying with the OGC standard right-hand rule. Since ```GeoPolygonFilter``` is self contained logic, the fix in elastic#8672 did not address the issue for the ```GeoPolygonFilter```. This was identified in issue elastic#5968

This fixes the ambiguous polygon issue in ```GeoPolygonFilter``` by moving the dateline crossing code from ```ShapeBuilder``` to ```GeoUtils``` and reusing the logic inside the ```pointInPolygon``` method.  Unit tests are added to ensure support for coordinates specified in either standard lat/lon or great-circle coordinate systems.

closes elastic#5968
closes elastic#9304
nknize added a commit that referenced this issue Jan 27, 2015
PR #8672 addresses ambiguous polygons - those that either cross the dateline or span the map - by complying with the OGC standard right-hand rule. Since ```GeoPolygonFilter``` is self contained logic, the fix in #8672 did not address the issue for the ```GeoPolygonFilter```. This was identified in issue #5968

This fixes the ambiguous polygon issue in ```GeoPolygonFilter``` by moving the dateline crossing code from ```ShapeBuilder``` to ```GeoUtils``` and reusing the logic inside the ```pointInPolygon``` method.  Unit tests are added to ensure support for coordinates specified in either standard lat/lon or great-circle coordinate systems.

closes #5968
closes #9304
nknize added a commit that referenced this issue Jan 27, 2015
PR #8672 addresses ambiguous polygons - those that either cross the dateline or span the map - by complying with the OGC standard right-hand rule. Since ```GeoPolygonFilter``` is self contained logic, the fix in #8672 did not address the issue for the ```GeoPolygonFilter```. This was identified in issue #5968

This fixes the ambiguous polygon issue in ```GeoPolygonFilter``` by moving the dateline crossing code from ```ShapeBuilder``` to ```GeoUtils``` and reusing the logic inside the ```pointInPolygon``` method.  Unit tests are added to ensure support for coordinates specified in either standard lat/lon or great-circle coordinate systems.

closes #5968
closes #9304
@nknize
Copy link
Contributor Author

nknize commented Jan 28, 2015

Reopening due to #9462

@nknize nknize reopened this Jan 28, 2015
@nknize nknize added v1.4.4 and removed v1.4.3 labels Feb 5, 2015
@spinscale spinscale added v1.4.5 and removed v1.4.4 labels Feb 19, 2015
@s1monw s1monw added v1.6.0 and removed v1.5.0 labels Mar 17, 2015
@clintongormley clintongormley changed the title [GEO] GeoPolygonFilter fails on ambiguous polygons GeoPolygonFilter fails on ambiguous polygons May 29, 2015
@s1monw s1monw removed the v1.5.3 label Jun 3, 2015
@martijnvg martijnvg added v1.7.1 and removed v1.6.1 labels Jul 16, 2015
@martijnvg
Copy link
Member

Bumping the version up to 1.7.1 for the today's release.

@spinscale spinscale added v1.7.2 and removed v1.7.1 labels Jul 28, 2015
mute pushed a commit to mute/elasticsearch that referenced this issue Jul 29, 2015
PR elastic#8672 addresses ambiguous polygons - those that either cross the dateline or span the map - by complying with the OGC standard right-hand rule. Since ```GeoPolygonFilter``` is self contained logic, the fix in elastic#8672 did not address the issue for the ```GeoPolygonFilter```. This was identified in issue elastic#5968

This fixes the ambiguous polygon issue in ```GeoPolygonFilter``` by moving the dateline crossing code from ```ShapeBuilder``` to ```GeoUtils``` and reusing the logic inside the ```pointInPolygon``` method.  Unit tests are added to ensure support for coordinates specified in either standard lat/lon or great-circle coordinate systems.

closes elastic#5968
closes elastic#9304
@spinscale spinscale removed the v1.7.2 label Sep 14, 2015
@spinscale
Copy link
Contributor

removing the 1.x version label here, it just keeps getting bumped since ages. Please remember this when closing, that it might need to be backported

@imotov
Copy link
Contributor

imotov commented Dec 10, 2018

It looks like this was addressed by 1a60e1c#diff-48767b395fcd07ab92b9c05815741b74R221 Can we close this now?

@nknize
Copy link
Contributor Author

nknize commented Feb 4, 2019

+1 to close. I knew this one would eventually resolve itself but I forgot the issue was still open. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/Geo Indexing, search aggregations of geo points and shapes >enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants