Repository navigation
Conversation
overlapsOf now ray-casts a tube whose screen width is under three cells as a tube three cells wide, moved back along the view by reach - radius so its nearest surface stays at its true depth. Thicker tubes and other shapes are measured as before.
rahul05ranjan
left a comment
There was a problem hiding this comment.
Verified with a revert-and-rerun: node --test scripts/audit-thin-tube.test.mjs passes 3/3 on this branch, and the identical test file against main fails 0/3 (wire never registers as overlapping in any of the three cases), so the fix is load-bearing exactly as the changelog claims.
The geometry also checks out. cameraBasis builds cam.V as a unit viewer axis (cosA·cosE, sinA·cosE, sinE — squares sum to 1), so in rasterOf the shift sub3(p, mul3(cam.V, reach − radii[index])) with radii raised to reach moves the centerline back along the view axis while keeping the camera-facing surface at exactly its original depth (dot(p,V) + r is preserved). That matches the doc sentence in references/kit.md: "ray-cast three cells wide with its nearest surface at its true depth". And the threshold reach = step·1.5 / cam.k is exactly the doc's 2·radius·scale < 3·step boundary, so code and prose agree.
One detail I want to call out as correct because it would have been an easy miss: { ...shape, points, radii, _b: undefined } — the spread would carry the pre-widened shape's cached bounds (shape._b from the bounds function around line 262), and clearing it forces a recompute for the new points/radii. (_f doesn't need clearing here: that cache only populates for kind === "body" in bodyDepth, and this branch is tube-only.)
One nit: the gate is Math.max(...shape.radii) >= reach, so a tube with mixed radii — one end thin, one end thick — is skipped entirely and the thin end still vanishes under the close/erode pass. Widening per point (radii.map(r => r < reach ? reach : r) with a matching per-point shift) would cover that middle case; as written it's all-or-nothing per tube.
On the security lens: this is the project's own audit harness rather than an input-trust boundary, so there's no principal at risk — the value here is closing a false-negative class (a wire hidden behind a plate passed the audit) in the tool whose whole job is to catch that.
Signed-off-by: Don Yin <Don_Yin@outlook.com>
|
Thanks for the careful read. You were right about mixed radii, and while checking it I also found an oblique tube fully behind a plate that got ordered in front, because the shift didn't preserve its depth along the ray. So I dropped the widening and instead sample the real geometry more finely for pairs that involve a thin tube; that covers the thin end of a taper too. I added the oblique case as a test. The audit/turn tests and the Raptor audit pass and every example rebuilds unchanged. It isn't free though: the radius-0.1 repro now takes about 40 ms instead of 7 ms. |
What this changes
overlapsOf(and soA.settle) now ray-casts a tube whose screen width is under three cells (2 · radius · scale < 3 · step, 0.75 px at the default step) as a tube three cells wide. Its points move back along the view byreach − radiusat each end, so its nearest surface stays at its true depth. Tubes of three cells or more, and every other shape, are measured as before.The change is in
rasterOfinkit/audit.mjs(8 lines).references/kit.mdgets one sentence underoverlapsOfandCHANGELOG.mdone line under Fixed.scripts/audit-thin-tube.test.mjshas three tests, a plate and a tube 0.05 in radius on a diagonal, camera straight on:tolerance, and the pair vanished again.Why
A tube about a cell wide leaves a one-cell trail in the flagged cells. The close and erode in
regionOfdo not widen it, the erosion removes it, its area is 0 andmeasurereturnsnull. The item then keeps the key it got from its centre point. If that key is wrong, the tube is drawn over a plate it passes behind, and--auditprintsaudit passed, becausedepthOrderonly checks the pairs thatoverlapsOffound. Details and numbers are in the issue (#128).I hit it in a figure with four upright plates and 78 straight wires of radius 0.1 (0.21 px at scale 1.07), each starting on a hidden pin behind its sending plate. I removed the figure's workaround (clamping two wires' keys below the plate's after
settle) and built it againstmainand against this change:mainA.settlew0.0-0,w1.0-0)lines.mjs, dark1 of 1066, start 1.24 px from anything0 of 1066lines.mjs, light1 of 1066, start 1.24 px from anything0 of 1066--auditaudit passedaudit passed, 0 problems on every lineProcess CPU time up to the end of
settleis 2.3 s on both. A smallerstepfinds the pair too (step: 0.1, 267 overlaps, 0 late wires) but costs about 6 times as much (wall time), and a thinner wire needs a smaller step again. Widening the tube costs nothing extra, because the grid does not change.Why three cells: on
main, a plate with a tube behind it, 13 directions times 5 offsets, step 0.25, scale 1.2, found 10/65 at 1 cell wide, 13/65 at 1.5, 56/65 at 2, 65/65 at 2.5 and 65/65 at 3. Three leaves some margin.Every example is unchanged. Only three examples call
A.settle(arcade-cabinet, turning-dial with its stress build, raptor-engine). I counted the tubes this change widens while building all of them in both themes (andturning-dial --light --verify,stress.mjs --light): none.git status --shortafter the builds shows only the files of this change. The Raptor--auditrun was not repeated, because the pages it checks are byte-identical.Not covered: a thin solid (a 0.2 wide box or a thin rod) may have the same problem. I have not tried it, and a flush slab 0.6 thick would need a different test, since its footprint is not thin when seen face on.
Checks
git status --shortshows only the changes I meant. arcade-cabinet, turning-dial (and--verify,stress.mjs) and raptor-engine, dark and light. The other examples do not usekit/audit.mjs.node build.mjs --auditendsaudit passed. Run on arcade-cabinet (1290 overlaps, 0 problems) and on the figure above. Not repeated on raptor-engine or turning-dial, whose pages come out byte-identical.node scripts/lines.mjsprintsline ends: 0on both themes and in every resting pose. On the figure above at rest, dark and light. The examples' pages are unchanged.turning-dialpages are byte-identical; I did not run the full orbit audit.UnreleasedinCHANGELOG.md.node --test scripts/audit-*.test.mjs scripts/turn-*.test.mjs: 107 pass, 0 fail (three of those files drive Chrome and needCHROME_PATHon my machine). The rest ofscripts/*.test.mjsneeds the React install from CI and I did not run it. Without the change torasterOfthe three new tests fail, and without the shift back the third one fails.Screenshots
None. The fault on the page is the first pixel of one 0.35 px wire over a plate, and
lines.mjsreports it better than a close-up. Nothing is drawn differently in any example.Fixes #128.