VoronoiDiagramBuilder: tolerance-based dedup + safe cell construction (stacked on #1212)#3
Open
grootstebozewolf wants to merge 4 commits into
Conversation
…ent points. - Use robust Orientation.index in Vertex.isCCW (raw arith in comments) - Dedup sites with tolerance via KdTree in DelaunayTriangulationBuilder.unique(..., tol) and both builders' create paths - Remove debug println in getVoronoiCellPolygon; ensure >=4 pts for LinearRing ctor always - Add regression test using exact issue locationtech#20 WKB + tol=0.1 asserting valid + correct cell count for unique sites All existing tests pass; repro now succeeds with valid diagram of 4 cells.
…CellPolygon + filter; independent count assert in repro test (hardcoded 4 from known locationtech#20 data). Targeted tests + fresh verif runs pass.
…ech#20 (WKB, theorems, test); update history link
Owner
|
I think this would be better as a separate PR to Dr JTS if/once this PR is merged, so changes remain discrete. |
VoronoiDiagramBuilder.java, DelaunayTriangulationBuilder.java and QuadEdgeSubdivision.java used outdated 'Eclipse Public License v1.0' headers (and epl-v10.html links). Updated to match the canonical header in build-tools/src/main/resources/jts/header.txt: - 'Eclipse Public License 2.0' - 'http://www.eclipse.org/legal/epl-v20.html' This fixes the Maven checkstyle header check failures.
This file contains hidden or 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
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.
Stacked PR on top of micycle1's \fixup\ (which powers locationtech#1212)
Base: \micycle1/fixup\
Summary
This is a stacked PR built directly on top of the great work in micycle1's \fixup\ branch (and locationtech#1212).
PR locationtech#1212 delivers:
This change adds the pieces required to fully resolve the original locationtech#20 report for the public \VoronoiDiagramBuilder\ API:
Why this stack on \ixup?
The predicate improvements are an excellent foundation for the whole triangulation stack.
For the specific scenario reported in locationtech#20 (\VoronoiDiagramBuilder\ + positive tolerance on near-coincident sites), we also need upstream prevention and downstream safety:
This work provides exactly that defense-in-depth on top of the predicate improvements.
Relationship to locationtech#1212
Testing & verification
Request to the author / maintainers
@micycle1 — I really like the direction in your \ixup\ / locationtech#1212. The bounded fast-path incircle predicate is a nice improvement.
I put this together as a stack on top of \ixup\ because the original locationtech#20 issue (and many real-world Voronoi users) specifically exercise \VoronoiDiagramBuilder.setTolerance(...). Would you be open to reviewing this, or folding the relevant parts (dedup + cell safety) into the base work? I'm very happy to rebase, adjust scope, or change anything to make the overall contribution stronger.
The only goal here is to make JTS more robust for everyone.
Happy to chat about any trade-offs.
Thanks again for pushing on this area!
#MakeJTSStronger
High-level diff
How to review/test locally:
\
git fetch https://github.com/grootstebozewolf/jts.git stacked-on-1212-voronoi-deduplication
or from this fork
\\