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

Tessellator: Improve logic when two holes share the same vertex with the polygon #13980

Merged
merged 4 commits into from
Nov 11, 2024

Conversation

iverase
Copy link
Contributor

@iverase iverase commented Nov 7, 2024

One of the situation with David Eberly's algorithm for finding a bridge between a hole and outer polygon was failing was the case of a polygon sharing a vertex with the outer polygon. To fixed that we added a step to find shared vertex between the hole and the polygon.

That worked well except on the situations where more than one hole was sharing the same vertex with the outer polygon. In those cases, we might have more than one vertex to choose from and we need to make sure we pick the right one. The current algorithm test if the line joining the previous vertex if the outer polygon band the next vertex from the hole intersects the polygon. This worked well in many cases but it is not generic as it depends on the topology of the union.

This commit tries to improve the algorithm by:

  1. If there is only one vertex, then there is not further checks and that's the one used.
  2. if there is a common vertex, it first compute the signed area of the join, if they have different sign, it chooses the negative one as that's the convex union. If they have the same sign, it computes the angle of the join and chooses the smallest angle.

I added a few test to check the different combinations and the new algorithm seems resilient to them.

In addition I clean the tessellator class by removing some useless final declarations for static methods and clean up the visibility of some internal classes (no point of protected inner classes on a final class) and remove unused methods. I clean up the tests too.

fixes #13841

Copy link
Contributor

@craigtaverner craigtaverner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, except for the method name fetchSharedVertex which I really feel needs to change.

I did notice that two of the new geojson files were actually WKT, but think that file names are not really important here.

Copy link
Contributor

@craigtaverner craigtaverner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@iverase iverase merged commit 6545722 into apache:main Nov 11, 2024
3 checks passed
@iverase iverase deleted the sharedVertex branch November 11, 2024 12:28
iverase added a commit that referenced this pull request Nov 11, 2024
…the polygon (#13980)

This commit tries to improve the algorithm by:

1.- If there is only one vertex, then there is not further checks and that's the one used.

2.- if there is a common vertex, it first compute the signed area of the join, if they have different sign, 
it chooses the negative one as that's the convex union. If they have the same sign, it computes the angle 
of the join and chooses the smallest angle.
iverase added a commit that referenced this pull request Nov 11, 2024
…the polygon (#13980)

This commit tries to improve the algorithm by:

1.- If there is only one vertex, then there is not further checks and that's the one used.

2.- if there is a common vertex, it first compute the signed area of the join, if they have different sign,
it chooses the negative one as that's the convex union. If they have the same sign, it computes the angle
of the join and chooses the smallest angle.
# Conflicts:
#	lucene/CHANGES.txt
#	lucene/core/src/test/org/apache/lucene/geo/TestTessellator.java
@ChrisHegarty ChrisHegarty added this to the 9.12.1 milestone Nov 12, 2024
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.

Unable to Tessellate shape for a valid Polygon according to GDAL/OGR and PostGIS
3 participants