From d61a613cdb27aa7f2ab9c06e91b9660531f149fc Mon Sep 17 00:00:00 2001 From: matthewtrahan Date: Fri, 16 Oct 2026 16:37:30 -0500 Subject: [PATCH] remove fins: key review fixes -- an exact key wins (a #n twin's removal hid its first copy), older and unparseable keys match exactly only, and the net is across 0.4 / along 2.5 mm of the wall plus height within 0.35: a removed fin gone from the rebuild no longer hides the neighbour beside it (Full coverage fill walls sit 0.5 mm apart), at the cost of a few removals coming back (m4 X30 77/84) --- tests/finkeys.test.js | 28 ++++++++++++++++++++-- web/ui/finkeys.js | 56 +++++++++++++++++++++++++++---------------- 2 files changed, 62 insertions(+), 22 deletions(-) diff --git a/tests/finkeys.test.js b/tests/finkeys.test.js index b64690b..9c1c133 100644 --- a/tests/finkeys.test.js +++ b/tests/finkeys.test.js @@ -2,7 +2,8 @@ // moves it a little. The key used to be matched exactly and held the wall top's z // and height, both measured below the part by the support gap, so a gap change // re-keyed every fin and every removed fin came back. Pinned here: -// - a gap change 0.2 -> 0.3 keeps every removal (exact keys kept none); +// - a gap change 0.2 -> 0.3 keeps the removals (exact keys kept none); +// - when the removed fin is gone, its key does NOT hide a neighbour; // - a key matches its own fin, and nothing in another pose; // - two walls stacked at one x/y stay two fins; a reversed line is the same fin; // - an older session's key (taken at the middle VERTEX) still matches. @@ -17,6 +18,10 @@ const records = (topo, res, rot, gap) => assignKeys( buildFins(topo, res, rot, { mode: 'auto', bedPad: true, tunables: { propGap: gap } }) .fins.map((f) => ({ ...f })).filter((f) => f.line?.length)); +// torus: one of its eight walls grows 0.9 mm taller at gap 0.3 (its mirror twin +// doesn't) -- past the height tolerance, so that removal comes back. The safe +// way to miss: a fin returning beats a kept fin silently hidden. +const MISSES = { torus: 1 }; for (const name of ['lowledge', 'tshape', 'torus']) { Deno.test(`fin keys: ${name} X30 -- removals hold across a gap change, and stay in their pose`, () => { const topo = loadModel(name), rot = rotX(30), res = analyze(topo, 45, rot); @@ -25,7 +30,7 @@ for (const name of ['lowledge', 'tshape', 'torus']) { const keys = new Set(A.map((r) => r.sig)); assert(B.filter((r) => keys.has(r.sig)).length < A.length, `${name}: exact keys already survive -- not the 045 case`); const m = matchRemoved(B, keys); - assert(m.size === A.length, `${name}: ${m.size}/${A.length} removals kept at gap 0.3`); + assert(m.size === A.length - (MISSES[name] ?? 0), `${name}: ${m.size}/${A.length} removals kept at gap 0.3`); const self = matchRemoved(A, keys); assert(A.every((r) => self.get(r.id) === r.sig), `${name}: a key matched another fin of its own build`); const up = rotX(0), other = records(topo, analyze(topo, 45, up), up, 0.2); @@ -46,6 +51,25 @@ Deno.test('fin keys: stacked walls at one x/y stay two fins; a reversed line is assert(matchRemoved(turned, new Set([a.sig])).size === 0, 'a crossing wall took the removal'); }); +Deno.test("fin keys: a #n twin's removal hides that twin, not its first copy", () => { + // one midpoint, bearing and height, different lengths: one key text, then `#1` + const twins = assignKeys([rec(0, [[0, 0, 5], [4, 0, 5]]), rec(1, [[-1, 0, 5], [5, 0, 5]])]); + assert(twins[1].sig === `${twins[0].sig}#1`, `twins keyed ${twins.map((r) => r.sig)}`); + const m = matchRemoved(twins, new Set([twins[1].sig])); + assert(m.size === 1 && m.get(1) === twins[1].sig, `the #1 removal matched ${JSON.stringify([...m])}`); +}); + +Deno.test('fin keys: a removed fin gone from the rebuild hides no neighbour', () => { + // Full coverage's short fill walls side by side, 0.5 mm apart: remove one, then + // a rebuild without it -- the other must stay + const a = rec(0, [[0, 0, 10], [2.5, 0, 10]], 3), b = rec(1, [[0, 0.5, 10], [2.5, 0.5, 10]], 3.4); + const [ka] = assignKeys([a, { ...b }]); + assert(matchRemoved(assignKeys([b]), new Set([ka.sig])).size === 0, 'a side-by-side neighbour took the removal'); + // end to end along one line, 1.8 mm on, a mm higher + const c = rec(2, [[1.8, 0, 11], [4.3, 0, 11]], 4); + assert(matchRemoved(assignKeys([c]), new Set([ka.sig])).size === 0, 'an end-to-end neighbour took the removal'); +}); + Deno.test('fin keys: one key per fin, repeats and suffixes parse', () => { const twins = assignKeys([rec(0, [[0, 0, 5], [4, 0, 5]]), rec(1, [[0, 0, 5], [4, 0, 5]])]); assert(twins[0].sig !== twins[1].sig && twins[1].sig.endsWith('#1'), `twins keyed ${twins.map((r) => r.sig)}`); diff --git a/web/ui/finkeys.js b/web/ui/finkeys.js index 8dc6fad..87ef638 100644 --- a/web/ui/finkeys.js +++ b/web/ui/finkeys.js @@ -5,17 +5,29 @@ * survives a rebuild. The key used to be matched EXACTLY, and it held the wall * top's z and height -- both measured below the part by the support gap, so * changing the gap re-keyed every fin and the removed ones came back (local issue - * 045). A rebuild mostly moves a fin up or down, a little sideways at most (gap - * 0.2 -> 0.3: m4 bracket 92/93 fins within 0.05 mm in x/y, the rest within 1 mm), - * so a key now matches the NEAREST fin of the same kind and bearing within a - * tolerance. z stays in the key as a tie-break only: two walls stacked at one x/y - * on different levels are still two fins. + * 045). A rebuild mostly moves a fin up or down, a little sideways at most, so a + * key now matches the NEAREST fin of the same kind and bearing within a tolerance. + * + * The tolerance errs toward a removed fin coming BACK, never toward a kept fin + * vanishing: a fin hidden that the user didn't click is support silently lost. + * Measured over 975 fins (real parts + stress models, Auto and Full, upright and + * X30, gap 0.2 -> 0.3), split along and across each wall: the same fin's middle + * slides ALONG the wall as its ends change (p99 1.5 mm, torus walls 1-1.4) but + * barely moves ACROSS it (p90 0.06), and its height changes 0.1. Neighbours of the + * same bearing sit side by side (>= 0.5 mm across: Full coverage's short fill + * walls) or end to end at another height (>= 1 mm apart) -- so a wide round net + * would, once the removed fin is gone from a rebuild, hide a neighbour. Hence + * across <= 0.4, along <= 2.5, height within 0.35. (One near-duplicate pair on a + * Full-coverage ramp, 0.1 across, can't be told apart.) z is a tie-break too: two + * walls stacked at one x/y on different levels are still two fins. * * Pure (no DOM, no three.js) so tests can run it against real builds. */ -const XY_TOL = 1.5; // mm a fin may move sideways and still be the same fin -const Z_TOL = 2; // mm up or down: a gap step moves a wall top ~0.1-0.2 +const ACROSS_TOL = 0.4; // mm a fin may move across its own line and still be the same fin +const ALONG_TOL = 2.5; // mm its middle may slide along the line (its ends grew or shrank) +const H_TOL = 0.35; // mm its height may change (a gap step: 0.1) +const Z_TOL = 1; // mm up or down: a gap step moves a wall top ~0.1-0.2 const BEARING_TOL = 0.15; // rad const r1 = (v) => Math.round(v * 10) / 10; // 0.1 mm @@ -61,7 +73,7 @@ export function assignKeys(records) { const n = seen.get(base) ?? 0; seen.set(base, n + 1); r.sig = n === 0 ? base : `${base}#${n}`; - r.legacySig = finKey(r, true); // matched too, so an older session's removals hold + r.legacySig = finKey(r, true); // an older session's key, matched exactly } return records; } @@ -70,7 +82,7 @@ export function assignKeys(records) { * suffixes that keep keys unique. */ export function parseKey(key) { const m = /^([^:]+):(-?[\d.]+),(-?[\d.]+),(-?[\d.]+):(-?[\d.]+):(-?[\d.]+)(?:[#~]\d+)*$/.exec(key ?? ''); - return m ? { kind: m[1], x: +m[2], y: +m[3], z: +m[4], bearing: +m[6] } : null; + return m ? { kind: m[1], x: +m[2], y: +m[3], z: +m[4], h: +m[5], bearing: +m[6] } : null; } /** Bearing difference of two undirected lines (a rebuild may run a wall the @@ -81,23 +93,27 @@ function bearingGap(a, b) { } /** - * Match removed keys to a build's records: each key takes the nearest record of - * its kind and bearing within the tolerances, one record per key, closest pairs - * first. A key with no fin near it (another pose, or a fin the new build doesn't - * make) matches nothing and is kept for later. + * Match removed keys to a build's records: a key that names a record exactly + * takes it (a `#n` twin is told apart only by its suffix); otherwise the nearest + * record of its kind and bearing within the tolerances, one record per key, + * closest pairs first. An older session's key (middle vertex), or one that + * doesn't parse, matches exactly only. A key with no fin near it (another pose, + * or a fin the new build doesn't make) matches nothing and is kept for later. * Returns Map record id -> the removed key it matched. */ export function matchRemoved(records, removedKeys) { - const keys = [...removedKeys].map((k) => [k, parseKey(k)]).filter(([, p]) => p); - const recs = []; - for (const r of records) for (const s of [r.sig, r.legacySig]) { const q = parseKey(s); if (q) recs.push([r, q]); } const pairs = []; - for (const [k, p] of keys) { + const recs = records.map((r) => [r, parseKey(r.sig)]).filter(([, q]) => q); + for (const k of removedKeys) { + for (const r of records) if (k === r.sig || k === r.legacySig) pairs.push([-1, k, r]); + const p = parseKey(k); + if (!p) continue; for (const [r, q] of recs) { if (p.kind !== q.kind || bearingGap(p.bearing, q.bearing) > BEARING_TOL) continue; - const dxy = Math.hypot(p.x - q.x, p.y - q.y), dz = Math.abs(p.z - q.z); - if (dxy > XY_TOL || dz > Z_TOL) continue; - pairs.push([Math.hypot(dxy, dz), k, r]); + const dx = q.x - p.x, dy = q.y - p.y, c = Math.cos(p.bearing), s = Math.sin(p.bearing); + const along = Math.abs(dx * c + dy * s), across = Math.abs(dy * c - dx * s), dz = Math.abs(p.z - q.z); + if (across > ACROSS_TOL || along > ALONG_TOL || dz > Z_TOL || Math.abs(p.h - q.h) > H_TOL) continue; + pairs.push([Math.hypot(along, 4 * across, dz), k, r]); // across weighs more: it's the neighbour axis } } pairs.sort((a, b) => a[0] - b[0]);