Add per-layer invalid_polygon_repair_scope option to repair clipped-and-quantised polygons - #929
Open
geoneutrino wants to merge 2 commits into
Open
Conversation
Large clipped polygons such as coastline/ocean shapefiles are written to tiles invalid: scaling them onto the integer tile grid introduces self-intersections. Until now only SIMPLIFIED geometry was repaired, so those layers stayed invalid and can be mis-rendered by strict renderers - as missing water, or as land hidden behind an ocean polygon. New layer option invalid_polygon_repair_scope: "simplified_only" (default, unchanged behaviour) repairs only invalid polygons that simplification may have broken; "all" is a superset and additionally repairs polygons that were merely clipped and quantised. For those, repair_multi_polygon runs with a strict area guard that also rejects any repair GROWING a polygon - growing means a hole was filled, which would make an island disappear. That bound must not apply to simplified geometry, where legitimate repairs do change area; applying it globally measurably made things worse (5 -> 20 invalid features in one tile). Verified: with the default the output is unchanged (Bangladesh z7 96/55: 166 features, 5 invalid, 319 holes). With "all" on the ocean layers, that tile goes to 4 invalid / 318 holes and the ocean feature behind a reported rendering artefact becomes valid; Denmark z5 16/10 improves 5 -> 1 invalid while Funen correctly stays land; Philippines z5 26/14 improves 7 -> 1 invalid with no land wrongly covered by ocean.
Contributor
Author
|
geom::is_valid() runs full self-intersection detection and was called for every output polygon in every tile. For a layer that is neither simplified nor opted into repair, nothing acts on the answer - the block it guards does nothing, so the check only fed a verbose message that is off by default. Hoist the repair condition into mayRepair and evaluate is_valid() only when that, or verbose, is set. Verified byte-identical to the parent commit on a Bangladesh extract with the option enabled for the water layers (93930 tiles, 25544140 features, --threads 1).
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.
(This code was created with AI assistance)
Problem
Repair polygon PR 908 corrected polygon errors after simplification but still there could be issues im complex geometries like holes in river deltas or coastal regions like Philippines mentioned in discussion 909
Polygons written to a tile can be GEOS-invalid even when the source geometry is perfectly valid and no simplification is applied.
Root cause
It is not the clipper. Feeding real ocean shapefile polygons through
fast_clipand testing the result with GEOS, every sampled polygon came back valid. The invalidity is introduced afterwards, inTileBbox::scaleGeometry/scaleRing:snapping float coordinates onto the integer tile grid (
floor, plus the 4-point backtracking dedup) collapses near-coincident vertices and creates self-intersections and touching rings.Today
repair_multi_polygon()runs only whensimplifyLevel > 0, i.e. only for geometry that simplification may have broken. Layers that are merely clipped and quantised — typically ocean/coastline shapefiles — are never repaired, so thedamage goes straight into the tile.
What this PR adds
As a defnsive approach a new config option per layer is introduced "invalid_polygon_repair_scope" with default "simplified_only" (current code flow) and the new option "all" as superset: additionally repair polygons that were only clipped and quantised
extend the area guard
repair_multi_polygon() rejects a repair whose result is less than 50% of the original area, to avoid the catastrophic collapse that buffer(0) can cause on large multipolygons. For clipped-but-unsimplified geometry an upper bound is needed as well: a repair that grows the polygon has filled a hole, which means an island silently disappears from the map.
That upper bound must not be applied to simplified geometry — simplification legitimately changes area in both directions, and enforcing the bound globally made things worse in testing (one Bangladesh z7 tile went from 5 to 20 invalid features). So repair_multi_polygon() takes a strictArea flag, set only when simplifyLevel == 0.
Results
verified with a complete planet build. All known effects, mostly holes in very complex river deltas, are fixed. no new visual effects introduced. Tests with setting the new option on the ocean layer fixes
Tests with activating the option on ocean + water_polygons fixes also very minor effects like in a harbor in a Switzerland lake
No mentionable effect on performance when active for ocean and water_polygon layers
Scope / limitations
This is a repair pass, not a fix of the quantisation itself. It does not reach full validity in every case — the remaining class of failures needs true snap-rounding at tile-grid precision (GEOS GEOSGeom_setPrecision or an equivalent)
Touching quantisation would have been very big task and alternatively i didn't want to introduce libgeos, - not as a new dependeny and also not because of maybe licence implications as it is lgpl