X-GIS
Blog

geometry

The roof that became a hole

A building went missing from the 3D layer. Tile clipping had split its footprint into two pieces; the tiler flattened both outers into one rings array, and the extrusion consumer read everything after rings[0] as a hole — so piece #2 was earcut-punched out of piece #1's roof. Walls survived, roofs vanished, n pieces rendered n-1 roofs. The 2D fill was pixel-perfect the whole time, and zero tests had ever fed the consumer more than one polygon.

By the X-GIS team 6 min read

A user reported a missing building (#1079). Not flickering, not misplaced — absent from the 3D extrusion layer, at every zoom, deterministically. The same feature’s 2D footprint rendered pixel-perfect in the flat fill. And on closer inspection the building wasn’t even fully absent: its walls were standing. Only the roof was gone, and where the roof should have been you could see straight down into the neighbouring piece’s interior.

Walls without a roof is a very specific corpse. Walls are generated per ring edge; roofs are generated by earcut over a ring set. Whatever happened, happened to the ring set.

One record, two readers

The feature crossed a tile boundary, and our clipper’s backtrack repair (splitBoundaryBacktracks) had legitimately split its clipped outer ring into two disjoint sub-outer pieces — that machinery was correct and stays. The tiler then tessellated the 2D fill per piece, correctly, and then did this with the metadata:

const allRingsForFeature: number[][][] = []
for (let si = 0; si < effectiveOuters.length; si++) {
const subRings = [effectiveOuters[si]!, ...subHoles[si]!]
tessellatePolygonToArrays(subRings, fid, scratch.pv, scratch.pi, dedupMap)
for (const r of subRings) allRingsForFeature.push(r)
}
tilePolygons.push({ rings: allRingsForFeature, featId: fid })

One RingPolygon entry, holding [outerA, ...holesA, outerB, ...holesB]. To the producer, rings meant “all the rings this feature owns in this tile.” The flatten looked like bookkeeping.

The extrusion consumer, generateWallMeshExtrudedECEF, reads the same field under a different contract: rings[0] is THE outer, and every ring after it is a hole to punch out of the roof. Feed it the flattened entry and piece B’s outer becomes a hole in piece A’s roof: earcut dutifully removes it. Piece B still gets walls — walls are per-edge and don’t care what kind of ring an edge belongs to — so you get B’s walls standing around B’s roof-shaped hole. N pieces in, N−1 roofs out.

Neither reader was wrong in isolation. The type held rings; it did not say what position means. Two owners, one array, two grammars.

Why the flat path hid it for so long

The order of operations in that producer loop is the whole camouflage: the 2D fill triangles are generated inside the per-piece loop, from subRings, before the flatten. The flat path never re-reads the metadata — it ships baked triangles. Only the extrusion path re-tessellates at runtime from rings, because it needs to build roofs at feature height. So every 2D rendering of the feature — the thing you check first, the thing screenshots compare — was correct, while the record it shipped alongside was quietly unparseable under the consumer’s grammar.

And nothing forced the two grammars to meet: the consumer’s test suite had zero cases with polygons.length > 1. Every existing test handed it one polygon — one outer, optional holes — the exact shape where both readings of rings coincide. The contracts only diverge on multi-piece input, and multi-piece input only exists where clipping splits a ring, and no fixture had ever crossed that path into the consumer.

The fix, twice

The producer now emits what the consumer parses — one RingPolygon per piece, its own outer plus the holes point-in-polygon bucketed into it:

for (let si = 0; si < effectiveOuters.length; si++) {
const subRings = [effectiveOuters[si]!, ...subHoles[si]!]
tessellatePolygonToArrays(subRings, fid, scratch.pv, scratch.pi, dedupMap)
tilePolygons.push({ rings: subRings, featId: fid })
}

One worry dissolved on contact with the code: splitting one entry into N looked like it would require duplicating the per-feature height/base bookkeeping in lockstep. It doesn’t — heights and bases are keyed by featId in a Map, not index-parallel to the polygons array, so every emitted piece sharing the feature’s id resolves the same height for free. The commit message drafted before checking said “duplicated in lockstep”; the code comment corrects it. Worth pinning, which the test does explicitly.

The less comfortable part: this else-branch exists twice — byte- duplicated in polygon-tiler.ts (the runtime single-tile path) and vector-tiler.ts’s processZoomLevelShared (the build-time pyramid path). Both got the identical fix, and each now carries a comment naming its twin. The duplication itself is the familiar smell — the same one that gave the raster globe a 16-gon silhouette this week — but consolidating two hot tiler paths is its own change with its own risks, not a rider on a bug fix. Fix both, name both, refactor separately.

How we know it holds

The witness is real geometry, not a toy: Natural Earth’s South Korea with a synthetic hole, at z7 tile (108,49) — the same canonical fixture our hole-distribution tests use, because that shape genuinely makes the clipped ring backtrack and split into two sub-outers with the hole bucketed into the southern piece. Fail-before, on both producer sites: the tile emitted one polygon entry with 3 rings. Pass-after: two entries, ring counts [1, 2], total still 3 — nothing lost, nothing duplicated.

On the consumer side, a new test hands generateWallMeshExtrudedECEF two disjoint squares under one featId — the post-fix shape — and asserts roof triangle coverage over both centroids. It is, verbatim per its own header, “the FIRST test here exercising polygons.length > 1.” That sentence is the bug’s epitaph: the consumer’s half of the contract had never once been observed on the input class where the contract could break.

What generalizes

A flattened array is a lossy encode. The producer had structure — pieces, each with its holes — and compressed it into a sequence whose piece boundaries existed only in its own memory of having written it. Any consumer then needs a convention to re-derive the structure, and [outer, ...holes] is a convention — just one the producer wasn’t following. If a field’s meaning depends on position, that meaning is part of the type’s contract and belongs where both sides can see it; “we both use rings” is agreement about a name, not about a grammar.

And when one record feeds two pipelines, test coverage on one pipeline says nothing about the other’s parse. The 2D path being provably correct made the metadata look verified — it wasn’t, because the 2D path had stopped reading it before the flatten. The input class that splits the two readings (N > 1) needed exactly one test. It now has three.

References

  1. “Two renderers, one truth” — the same week’s larger case study in duplicated code drifting: four globe bugs, all in a hand-maintained twin.
  2. “The label that lands in the ocean” — a sibling geometry-contract bug: a point that is usually inside is not a point that is always inside.
  3. Regression specs: compiler/src/__tests__/extrude-clip-split-pieces.test.ts (both producer sites, Korea z7 witness) and the multi-piece roof-coverage suite in map/src/core/polygon-mesh-ecef.test.ts.

Read next

testing

4 min read

The passing test that dimmed the wrong thing

A fix for per-feature fill opacity came with a fail-before/pass-after test: 0.25 and 0.75 baked to alpha 64 and 191, two values where there had been one. It was green and I reverted the change — because the alpha it proved feeds the outline, not the fill.