Add simply connected interior validation and test suite by bretttully · Pull Request #1472 · georust/geo

bretttully · 2025-12-09T10:40:15Z

I agree to follow the project's code of conduct.
I added an entry to CHANGES.md if knowledge of this change could be valuable to users.

Add validation to detect when a polygon's interior is not simply connected, which violates the OGC Simple Feature Specification requirement that "the interior of every Surface is a connected point set."

What This Detects

The validation detects two types of interior disconnection:

Multi-touch: Two rings sharing 2+ touch points at different coordinates
Cycle: Rings forming a cycle through distinct single-touch coordinates

Algorithm

Uses the GeometryGraph edge intersection data to find where rings touch:

Collect intersections: For each edge intersection, record the coordinate, which ring (edge) it belongs to, and whether it's a vertex of that ring
Sort and group: Sort intersections by (x, y, edge_idx) using total_cmp for consistent ordering, then group by coordinate
Build touch map: For coordinates where 2+ rings meet (and at least one has it as a vertex), record the touch for each ring pair
Detect disconnection:
- If any ring pair has 2+ touch coordinates → disconnected
- Build a graph of single-touch ring pairs and DFS for cycles through distinct coordinates

A "touch" requires at least one ring to have the point as an actual vertex (not a mid-segment crossing).

Key Insight

Multiple rings meeting at a single coordinate does NOT disconnect the interior. For example, three triangular holes meeting at one point still have a connected interior around their perimeter. Disconnection only occurs when touches happen at distinct coordinates.

Error Reporting

The error message includes the problematic coordinates to aid debugging:

polygon interior is not simply connected at coordinate(s): (2, 2), (3, 3)

Performance

Uses PreparedGeometry to cache R-tree structures for interior/exterior containment checks. The sorting-based coordinate grouping provides better cache locality than hash-based approaches.

michaelkirk

Hello! Thank you for the pull request. This would be a great feature to incorporate, but I've got some questions to work through.

geo-test-fixtures/src/checkerboard.rs

geo-benches/src/validation.rs

geo/src/algorithm/validation/polygon.rs

geo/CHANGES.md

michaelkirk

Sorry for the slow turn around! I'd like to have this behavior, but I have some questions about the implementation. I haven't done a full review, but wanted to check in before dedicating more time to the review.

I think directionally it makes sense. Can I ask, did you use some kind of reference implementation for the graph algorithm, or did it spring mostly from you and the AI?

geo/src/algorithm/validation/polygon.rs

geo/src/algorithm/bool_ops/tests.rs

geo/src/algorithm/validation/polygon.rs

michaelkirk · 2026-02-04T20:00:09Z

Hi @bretttully - are you able to respond to any of my feedback? As I said, I think this is useful functionality, but I'm not comfortable merging it without understanding more how it works.

bretttully · 2026-02-04T21:59:04Z

@michaelkirk I haven't had a chance to carve out time just yet. I am keen to get the functionality in, so will have something together next week. Sorry for the delay!

…extraction Bumps i_overlay from 4.0 to 4.4 and enables `OverlayOptions::ogc()` for all boolean operations. This ensures output polygons have connected interiors per ISO 19125-1, fixing cases where holes sharing vertices produced invalid geometry. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

bretttully · 2026-02-12T02:17:14Z

@michaelkirk I am just picking this up again. I have started by getting in the OGC changes for iOverlay here: #1500

I'm going to rebase this branch off that one, and possibly split this PR into smaller ones to make review easier.

Cache R-tree structures via PreparedGeometry when validating polygon ring relationships. This avoids rebuilding the geometry graph for each interior containment check against the exterior, and for each interior-interior intersection check. Also adds an early return when no non-empty interiors exist, and skips empty interior rings in pairwise comparisons. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

The prepared_interior_1 construction was moved upward so the exterior_vs_interior relate compares Polygon-vs-Polygon instead of Polygon-vs-LineString. This changes the DE-9IM semantics: a Polygon's boundary is the ring curve (not its interior), so the line-intersection check must use (OnBoundary, OnBoundary) instead of (OnBoundary, Inside). Also applies cargo fmt to bool_ops files. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

With PreparedGeometry wrapping interior rings as Polygons, an interior ring identical to the exterior now passes is_contains() (a polygon contains itself), so only IntersectingRingsOnALine is reported — not InteriorRingNotContainedInExteriorRing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Detect polygons with disconnected interiors per OGC Simple Features (ISO 19125-1, section 6.1.11.1): "The interior of every Surface is a connected point set." The new InteriorNotSimplyConnected(RingRole, RingRole) error fires when: - Two rings share 2+ distinct touch coordinates (creating a corridor) - Rings form a cycle of single-vertex touches through distinct coordinates (enclosing interior area) Ring pairs already reported as IntersectingRingsOnALine or IntersectingRingsOnAnArea are skipped to avoid duplicate errors. Includes: - Checkerboard polygon fixture for stress-testing cycle detection - 9 new tests covering valid and invalid cases - Validation benchmarks (simple polygons, checkerboards, real fixtures) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- Use Vec<Vec<usize>> instead of Vec<HashSet<usize>> for adjacency (only iterated, never queried) - Store edge_coords in one direction with canonical (min, max) keys - Flatten nested if-let chains into tuple-matching patterns - Replace unreachable if-let with .expect() for invariant - Hoist unique_edges allocation outside grouping loop - Minor: hoist coords() call, use |= operator Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Simplifies check_ring_touches_disconnect_interior by replacing the 45-line DFS with entry_coord tracking with a dual Union-Find approach. Also moves Case 1 (pair with 2+ touches) into the caller as an early exit using a HashSet, and builds coord_edges directly instead of going through an intermediate HashMap. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

bretttully · 2026-02-12T11:25:13Z

Hi @michaelkirk

Can I ask, did you use some kind of reference implementation for the graph algorithm, or did it spring mostly from you and the AI?

This is has been a mostly iterative process with Claude; I have included the iOverlay repo in context and have found that referencing its implementation has guided the solution here.

I have been using Claude to validate test cases against shapely as I go, but have shied away from using GEOS/JTS in context given licensing of those libraries. I have also been getting it to generate explanatory images such as this for the union find implementation that I think is easier to understand than what was there previously.

bretttully mentioned this pull request Dec 9, 2025

Boolean operations can produce OGC-invalid polygons with disconnected interiors #1470

Open

bretttully force-pushed the fix-simply-connected-interior-validation branch from dfa491f to 7398dfe Compare December 9, 2025 10:42

michaelkirk requested changes Dec 12, 2025

View reviewed changes

michaelkirk mentioned this pull request Dec 12, 2025

Add failing test for issue iOverlay#27 (disconnected interiors) #1471

Closed

bretttully force-pushed the fix-simply-connected-interior-validation branch from 935d0ad to 15efe63 Compare December 19, 2025 01:56

michaelkirk requested changes Jan 6, 2026

View reviewed changes

bretttully and others added 3 commits February 12, 2026 11:20

georustGH-1470 Add CHANGES.md entry for i_overlay 4.4 update

d1c3493

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

georustGH-1470 Update CHANGES.md with PR link

1bc1a3c

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

bretttully force-pushed the fix-simply-connected-interior-validation branch from bb29608 to 118d5bb Compare February 12, 2026 02:20

bretttully mentioned this pull request Feb 12, 2026

Use PreparedGeometry for polygon validation performance #1501

Open

5 tasks

bretttully force-pushed the fix-simply-connected-interior-validation branch from 118d5bb to 333d345 Compare February 12, 2026 03:47

bretttully force-pushed the fix-simply-connected-interior-validation branch from 75a5028 to d92b4f4 Compare February 12, 2026 10:03

bretttully and others added 5 commits February 12, 2026 21:11

Updated CHANGES.md

dc1fa0d

bretttully force-pushed the fix-simply-connected-interior-validation branch from d92b4f4 to 9400e3a Compare February 12, 2026 11:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add simply connected interior validation and test suite#1472

Add simply connected interior validation and test suite#1472
bretttully wants to merge 10 commits intogeorust:mainfrom
bretttully:fix-simply-connected-interior-validation

bretttully commented Dec 9, 2025 •

edited

Loading

Uh oh!

michaelkirk left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

michaelkirk left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

michaelkirk commented Feb 4, 2026

Uh oh!

bretttully commented Feb 4, 2026

Uh oh!

bretttully commented Feb 12, 2026

Uh oh!

bretttully commented Feb 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

Conversation

bretttully commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What This Detects

Algorithm

Key Insight

Error Reporting

Performance

Uh oh!

michaelkirk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

michaelkirk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

michaelkirk commented Feb 4, 2026

Uh oh!

bretttully commented Feb 4, 2026

Uh oh!

bretttully commented Feb 12, 2026

Uh oh!

bretttully commented Feb 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

bretttully commented Dec 9, 2025 •

edited

Loading