fix: cull a district by where it stands, not by where its footprint is at sea level
Two independent reviewers found the same defect in `a2a04e0`, which is the only reason it is being fixed rather than shipped. `DetailRange.r` is `reach` from the lattice sweep: the half-diagonal of the district's lat/lng bounding box plus a lot. That is a **plan** radius, and it is a good one — measured over all 59,166 instances, every lot is inside its own district's `r` of the centre with metres to spare. It was then used as the radius of a sphere pinned at `y = 0`, which is a different claim, and one this board does not support: a lot's local y is the ground under it times `verticalExaggeration`, and that is 15 on the merged board. **39 of 97 district ranges had lots outside their own cull sphere**, the worst by 4.78 units — 9.2 km of scene height, in the San Gabriel foothills. Swept over 32,400 poses built the way `scene.ts` builds them, 61 dropped lots that project strictly inside the NDC cube. Hillside buildings vanishing while you look at them. So the range carries `y0`/`y1` — walked once per district at build, over lots that were just pushed — and the test is `Frustum.intersectsBox` instead of `intersectsSphere`. Same six plane tests, and exact for the volume being described rather than right in x/z and wrong in y. Measured, SF 2.5 km: 215,598 → 216,392 triangles. The fix *adds* 794, which is the shape of a correct answer here — it stops discarding geometry that was on screen. The whole-board pose is unchanged and draws are unchanged at 437. The doc comments that asserted the invariant this violated are rewritten rather than deleted; they were the part most likely to survive a fix and mislead. Also corrected there: the pack declares 99 detail districts but ten emit no lot and carry no range, so the quoted reach statistics are over 89, not 99 — the file and the hand-off report disagreed. `blocksDetail.test.ts` gains a peak board, because the existing fixture cannot express this: it exaggerates by 2 with a 200 m hill, so every district's height is a rounding error next to its plan radius and a y=0 volume contains it by accident. The new board is one small district on a 3,000 m peak at 15x — lots at y 36.09-39.28 on a 2.70-unit footprint. Verified to fail against the old volume and pass against the box, rather than assumed to.
This commit is contained in:
+59
-18
@@ -389,16 +389,42 @@ export function createBlocks(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
if (boxes.length > districtStart) {
|
if (boxes.length > districtStart) {
|
||||||
|
/*
|
||||||
|
* The vertical extent, walked here because the horizontal reach is not it.
|
||||||
|
*
|
||||||
|
* `reach` is the half-diagonal of the district's own lat/lng bounding box
|
||||||
|
* plus a lot, and it is a **plan** radius: measured over all 59,166
|
||||||
|
* instances, every lot on the merged board is inside its district's
|
||||||
|
* `reach` of the centre with metres to spare. It was used as a 3-D radius
|
||||||
|
* on a sphere pinned at y=0, and that is a different claim — one this
|
||||||
|
* board does not support. A lot's local y is the ground it stands on
|
||||||
|
* times `verticalExaggeration`, which is 15 here, so a district in the San
|
||||||
|
* Gabriel foothills sits kilometres of scene height above the plane its
|
||||||
|
* cull sphere was centred on. **39 of 97 districts had lots outside their
|
||||||
|
* own sphere**, the worst by 4.78 units — 9.2 km — and the failure that
|
||||||
|
* produces is buildings on a hillside vanishing while they are on screen.
|
||||||
|
*
|
||||||
|
* So the test is an axis-aligned box and not a sphere. It costs the same
|
||||||
|
* — `Frustum.intersectsBox` is the same six plane tests as
|
||||||
|
* `intersectsSphere` — and it is *exact* for the volume being described
|
||||||
|
* rather than conservative in x/z and wrong in y. Walking the slice is one
|
||||||
|
* pass over lots that were just pushed, once per district at build.
|
||||||
|
*/
|
||||||
|
let y0 = Infinity;
|
||||||
|
let y1 = -Infinity;
|
||||||
|
for (let k = districtStart; k < boxes.length; k++) {
|
||||||
|
const box = boxes[k]!;
|
||||||
|
if (box.y < y0) y0 = box.y;
|
||||||
|
if (box.y + box.h > y1) y1 = box.y + box.h;
|
||||||
|
}
|
||||||
districtRanges.push({
|
districtRanges.push({
|
||||||
start: districtStart,
|
start: districtStart,
|
||||||
count: boxes.length - districtStart,
|
count: boxes.length - districtStart,
|
||||||
x: cx,
|
x: cx,
|
||||||
z: cz,
|
z: cz,
|
||||||
// `reach` is the half-diagonal of the district's own bounding box plus a
|
|
||||||
// lot, computed a few lines above to size the lattice sweep and until
|
|
||||||
// now thrown away. It is exactly the radius a sphere around (cx, cz)
|
|
||||||
// needs to contain every lot this district emitted.
|
|
||||||
r: reach,
|
r: reach,
|
||||||
|
y0,
|
||||||
|
y1,
|
||||||
detail: district.detail === true,
|
detail: district.detail === true,
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
@@ -538,17 +564,23 @@ export function createBlocks(
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* One district's lots, where that district is in scene units, and how far it
|
* One district's lots, and the box that contains them.
|
||||||
* reaches.
|
|
||||||
*
|
*
|
||||||
* `r` is the radius of a sphere on the ground plane that contains every lot the
|
* `r` is the **plan** radius: the half-diagonal of the district's own lat/lng
|
||||||
* district emitted, and it is what the frustum test is run against. Measured
|
* bounding box plus a lot. Measured across the 89 detail districts that emit
|
||||||
* across the merged board's 99 detail districts it runs from **1.06 km to
|
* lots it runs from **1.06 km to 15.53 km, median 3.82 km**, and every lot on
|
||||||
* 15.53 km, median 3.82 km** — every one of them an order of magnitude larger
|
* the board is inside its own district's `r` of `(x, z)`. (The pack declares 99
|
||||||
* than the tallest thing standing on it, which is why a sphere centred at y=0
|
* detail districts; ten emit no lot at all — every candidate falls in water, in
|
||||||
* needs no vertical pad for the buildings' own height. The eight base districts
|
* a park or outside the coverage roll — and carry no range. The eight base
|
||||||
* are far larger again; they are the state pack's own, and they are the reason
|
* districts are far larger again, which is why a whole-board pose still draws
|
||||||
* a whole-board pose still draws everything.
|
* everything.)
|
||||||
|
*
|
||||||
|
* `y0`/`y1` are the vertical extent, and they exist because `r` is not it. A
|
||||||
|
* lot's local y is the ground under it times `verticalExaggeration` — 15 on the
|
||||||
|
* merged board — so `r` describes the district's footprint and says nothing
|
||||||
|
* about how far up the mountain it is. The two together are an axis-aligned box
|
||||||
|
* and that box is what the frustum test runs against; see the walk in
|
||||||
|
* `createBlocks` for the measurement that made this necessary.
|
||||||
*/
|
*/
|
||||||
interface DetailRange {
|
interface DetailRange {
|
||||||
start: number;
|
start: number;
|
||||||
@@ -556,6 +588,10 @@ interface DetailRange {
|
|||||||
x: number;
|
x: number;
|
||||||
z: number;
|
z: number;
|
||||||
r: number;
|
r: number;
|
||||||
|
/** Lowest ground any lot stands on, in local scene units. */
|
||||||
|
y0: number;
|
||||||
|
/** Highest roof, in local scene units. */
|
||||||
|
y1: number;
|
||||||
/** Metro detail, revealed by stand-off; `false` for the base pack's own. */
|
/** Metro detail, revealed by stand-off; `false` for the base pack's own. */
|
||||||
detail: boolean;
|
detail: boolean;
|
||||||
}
|
}
|
||||||
@@ -603,7 +639,7 @@ interface DetailStore {
|
|||||||
const DETAIL_FRUSTUM_PAD = 0.02;
|
const DETAIL_FRUSTUM_PAD = 0.02;
|
||||||
|
|
||||||
/** Held rather than allocated: the frustum test runs once per district. */
|
/** Held rather than allocated: the frustum test runs once per district. */
|
||||||
const REACH_SPHERE = new THREE.Sphere();
|
const REACH_BOX = new THREE.Box3();
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Draw the cities near `(x, z)` and inside `view`, and no others.
|
* Draw the cities near `(x, z)` and inside `view`, and no others.
|
||||||
@@ -664,9 +700,14 @@ export function updateBlocksDetail(
|
|||||||
const visible = store.ranges.filter((r) => {
|
const visible = store.ranges.filter((r) => {
|
||||||
if (r.detail && (reachUnits <= 0 || (x - r.x) ** 2 + (z - r.z) ** 2 >= reach2)) return false;
|
if (r.detail && (reachUnits <= 0 || (x - r.x) ** 2 + (z - r.z) ** 2 >= reach2)) return false;
|
||||||
if (view === undefined) return true;
|
if (view === undefined) return true;
|
||||||
REACH_SPHERE.center.set(r.x, 0, r.z);
|
/*
|
||||||
REACH_SPHERE.radius = r.r + pad;
|
* The pad goes on every axis, including the vertical, because the vertical
|
||||||
return view.intersectsSphere(REACH_SPHERE);
|
* is where the camera's own motion is least constrained: a descent changes
|
||||||
|
* altitude far faster than it changes the ground point being looked at.
|
||||||
|
*/
|
||||||
|
REACH_BOX.min.set(r.x - r.r - pad, r.y0 - pad, r.z - r.r - pad);
|
||||||
|
REACH_BOX.max.set(r.x + r.r + pad, r.y1 + pad, r.z + r.r + pad);
|
||||||
|
return view.intersectsBox(REACH_BOX);
|
||||||
});
|
});
|
||||||
const key = visible.map((r) => r.start).join(",");
|
const key = visible.map((r) => r.start).join(",");
|
||||||
if (key === store.key) return;
|
if (key === store.key) return;
|
||||||
|
|||||||
@@ -192,3 +192,82 @@ test("a board with no detail districts is never repacked at all", async () => {
|
|||||||
updateBlocksDetail(blocks, 0, 0, 0, looking(plain, 36.05, -122.95, 0.2), 1);
|
updateBlocksDetail(blocks, 0, 0, 0, looking(plain, 36.05, -122.95, 0.2), 1);
|
||||||
assert.equal(blocks.count, drawn, "a board with no detail districts must not move");
|
assert.equal(blocks.count, drawn, "a board with no detail districts must not move");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A district on high ground is culled by where it *is*, not by where its
|
||||||
|
* footprint would be at sea level.
|
||||||
|
*
|
||||||
|
* This is a regression test for a real defect, and it is written as a separate
|
||||||
|
* board because the fixture above cannot express it: that board exaggerates by
|
||||||
|
* 2 and its tallest hill is 200 m, so every district's height above the ground
|
||||||
|
* plane is a rounding error next to its own plan radius and a cull volume
|
||||||
|
* pinned at y=0 contains it by accident.
|
||||||
|
*
|
||||||
|
* The merged California board exaggerates by 15. A lot's local y is the terrain
|
||||||
|
* under it times that, so a district in the San Gabriel foothills stands
|
||||||
|
* kilometres of scene height above y=0 while its plan radius stays a few units.
|
||||||
|
* Tested against a sphere centred on the ground plane, **39 of the merged
|
||||||
|
* board's 97 district ranges had lots outside their own cull volume** — the
|
||||||
|
* worst by 4.78 units, 9.2 km — and the picture that produces is hillside
|
||||||
|
* buildings vanishing while they are on screen.
|
||||||
|
*
|
||||||
|
* The board below reproduces that geometry rather than that city: one small
|
||||||
|
* detail district on a 3,000 m peak, exaggerated 15x, so its lots sit about
|
||||||
|
* forty units up on a footprint about three units across. A camera at the
|
||||||
|
* summit looking at the summit contains every one of those lots and misses a
|
||||||
|
* y=0 sphere entirely.
|
||||||
|
*/
|
||||||
|
const PEAK_CITY: City = {
|
||||||
|
...CITY,
|
||||||
|
id: "peak-board",
|
||||||
|
verticalExaggeration: 15,
|
||||||
|
districts: [district("summit", 37, -122, 0.02, true)],
|
||||||
|
hills: [{ name: "peak", lat: 37, lng: -122, elevation: 3_000, radius: 0.25 }],
|
||||||
|
};
|
||||||
|
|
||||||
|
test("a district on high ground survives a frustum that contains it", async () => {
|
||||||
|
const world = new World(PEAK_CITY);
|
||||||
|
assert.equal(await world.ready(), true, "the peak board failed to build a heightfield");
|
||||||
|
const blocks = createBlocks(world);
|
||||||
|
const store = blocks.userData.detail as { ranges: { r: number; y0: number; y1: number }[] };
|
||||||
|
assert.ok(store !== undefined && store.ranges.length > 0, "the peak board built no district");
|
||||||
|
|
||||||
|
/*
|
||||||
|
* The fixture is only a test while this holds. If a pack edit ever flattens
|
||||||
|
* this board, the assertion below would pass against a y=0 volume too and
|
||||||
|
* would quietly stop testing anything.
|
||||||
|
*/
|
||||||
|
const range = store.ranges[0]!;
|
||||||
|
assert.ok(
|
||||||
|
range.y0 > range.r,
|
||||||
|
`the fixture is not off the ground plane: y0 ${range.y0.toFixed(2)} against r ${range.r.toFixed(2)}`,
|
||||||
|
);
|
||||||
|
|
||||||
|
/*
|
||||||
|
* Aimed at the summit, and reaching only as far as the summit.
|
||||||
|
*
|
||||||
|
* `looking()` above aims every camera at `y = 0` with a far plane ten times
|
||||||
|
* the stand-off, which is right for that board and useless here: it puts the
|
||||||
|
* ground plane in the middle of the frustum, so a cull volume pinned to the
|
||||||
|
* ground plane is contained whatever the terrain does and the defect cannot
|
||||||
|
* be expressed. This camera hangs eight units over the peak, looks at the
|
||||||
|
* peak, and stops twelve units short — so the lots at y 36.09-39.28 are in
|
||||||
|
* frame and the plane at y = 0, thirty-seven units below them, is not.
|
||||||
|
*/
|
||||||
|
const [x, z] = world.project(37, -122);
|
||||||
|
const summitY = world.groundAt(37, -122);
|
||||||
|
const camera = new THREE.PerspectiveCamera(42, 1, 0.01, 12);
|
||||||
|
camera.position.set(x, summitY + 8, z);
|
||||||
|
camera.lookAt(new THREE.Vector3(x, summitY, z));
|
||||||
|
camera.updateMatrixWorld(true);
|
||||||
|
camera.updateProjectionMatrix();
|
||||||
|
const view = new THREE.Frustum().setFromProjectionMatrix(
|
||||||
|
new THREE.Matrix4().multiplyMatrices(camera.projectionMatrix, camera.matrixWorldInverse),
|
||||||
|
);
|
||||||
|
|
||||||
|
updateBlocksDetail(blocks, x, z, REACH, view, 0);
|
||||||
|
assert.ok(
|
||||||
|
blocks.count > 0,
|
||||||
|
"a district standing on a peak was culled by a frustum aimed straight at it",
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user