[TimSC]

Fix dropped polygons in scaleRing; skip shapefile records and rings outside the clipping box

Two changes, both affecting shapefile polygons such as the coastline's water polygons.

1. TileBbox::scaleRing could drop whole polygons

scaleRing removes small loops by comparing each point with the last few added, but it never compares with the first point. If clipping leaves a ring whose first point is repeated (p0 p0 p1 p2 p3 p0), the repeat is kept. When the ring closes on p0, it matches the repeat and is taken for a loop, so the ring is cut down to two points and the polygon is dropped.

Clipping a large polygon to a tile can leave exactly this four-cornered shape. In my renders, large areas of sea disappeared at particular tiles and zooms, for example off the Moroccan coast at z8 and in the Bay of Biscay at z9.

The fix skips any point identical to the one before it, including the first.

2. Faster shapefile reading against a clipping box

• Record boxes: each record's bounding box is read from the first bytes of the record, and records outside the clipping box are skipped without being read in full. The existing check after SHPReadObject stays, as a fallback for records whose box can't be read this way.
• Rings: when building a polygon, only the exterior rings, and their interior rings, that reach the clipping box are kept. The coastline's water polygons can have thousands of holes and hundreds of thousands of points, all of which were otherwise validated and clipped.

This helps renders of a regional extract (--bbox, or an input with bounds) using the worldwide coastline shapefile. A whole-planet render is unaffected.

Testing

I rendered two coastal extracts (--bbox -6,35,-3,37 and --bbox -3,43,-1,45, up to z10) with config-openmaptiles.json, comparing against current master:

• Output: most tiles are identical. The rest differ at the vertex level along the coastline: under 0.02% of a tile's area.
• Speed: about 7% faster with a warm file cache. The first, cold run was 4.1 s against 1.4 s, since far less of the 1 GB coastline shapefile has to be read.

[geoneutrino]

Point 1 is a duplicate of PR #933

I did the same as you but this introduced some new effects so i had to change to a 2-step approach

Don't know which PR will be merged by the maintainer but this should be considered
Thanks

[systemed]

Thanks both! I'm currently waiting for feedback on #939 but once we have that then I'll look at applying the currently outstanding PRs over that.

[TimSC]

I can drop point 1 from this PR if you choose to merge #933

Actually point 2 is contained within #939

[dktue]

@TimSC, a quick status of the three parts of this PR against current master:

• scaleRing fix: same as Fix scaleGeometry dropping polygons whose ring collapses #933, which merges cleanly and is needed now that perf: exact rectangle clip for valid polygons (planet 25% faster, 31% less CPU) #949 is in (see my comment on perf: exact rectangle clip for valid polygons (planet 25% faster, 31% less CPU) #949).
• Skipping shapefile records outside the clipping box via the record header: in master since perf: 3.4x faster planet build (2h10 -> 39 min), less RAM, deterministic output #939.
• Skipping individual rings outside the clipping box: not in master yet. That still helps extracts a lot (coastline water polygons with thousands of islands far outside the area).

Since you said you don't mind what happens to this PR: would it be OK if I open a small follow-up PR with just the ring skipping, rebased on master and credited to you? This one could then be closed once #933 is in.

[TimSC]

@dktue Sounds good!

[dktue]

@TimSC, thanks again for the OK to take the ring skipping from this PR forward. We measured it on current master before opening anything, and it doesn't make a measurable difference anymore, so we're not opening a separate PR for it.

OpenMapTiles profile, 32 threads, 3 interleaved rounds each, master vs. the same commit with ring skipping (British Columbia and Norway: d0ab14b, .pmtiles; Germany: bfe7a2f, .mbtiles):

	shapefile phase, master	with ring skipping
Germany	0.62 / 0.66 / 0.63 s	0.62 / 0.56 / 0.59 s
British Columbia	10.6 / 11.5 / 11.5 s	11.6 / 11.3 / 10.9 s
Norway	52.9 / 57.7 / 60.4 s	57.1 / 56.8 / 59.7 s

Wall times are within noise too, and the output is the same apart from the usual poi run-to-run differences (#942). Our guess is that skipping whole records outside the clipping box via their header bbox, which went into master with #939, already covers most of what ring skipping used to save, but we didn't profile that further.