Update GeoPolygonFilter to handle polygons crossing the dateline #9339
Conversation
This file contains 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
nknize
added
:Analytics/Geo
| } | ||
|
|
||
| @Override | ||
| public boolean equals(Object o) { | ||
| if (this == o) return true; | ||
| if (o == null || getClass() != o.getClass()) return false; | ||
| if (o == null) return false; | ||
| if (o instanceof Coordinate) { |
There was a problem hiding this comment.
Won't this be true for any GeoPoint, so you can never reach the block casting to GeoPoint?
|
@nknize I left some comments. |
|
@rjernst please could you give another review |
| return (points[prev].x > points[next].x); | ||
| } | ||
|
|
||
| private static final int computePolyTop(GeoPoint[] points, int length) { |
There was a problem hiding this comment.
i still feel like we should make this function name "correct" and maybe call it "computePolyBottomLeft"?
|
LGTM. I also think having dedicated unit tests for the utility methods in |
nknize
force-pushed
the
enhancement/9304
branch
from
4111a93
to
49b323f
Compare
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
force-pushed
the
enhancement/9304
branch
from
5155a51
to
06667c6
Compare
clintongormley
changed the title
clintongormley
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
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
GeoPolygonFilteris self contained logic, the fix in #8672 did not address the issue for theGeoPolygonFilter. This was identified in issue #5968This fixes the ambiguous polygon issue in
GeoPolygonFilterby moving the dateline crossing code fromShapeBuildertoGeoUtilsand reusing the logic inside thepointInPolygonmethod. Unit tests are added to ensure support for coordinates specified in either standard lat/lon or great-circle coordinate systems.closes #5968
closes #9304