From 4814fc9a66bf36d7beeb341309fe64ed67956beb Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 1 Oct 2026 10:05:34 +0300 Subject: [PATCH] perf(stairs): draw all treads of a stair as one path (#740) A floor with stairs pays for them on every switch to it: the stair layer is emptied on other floors, so Lit recreates every symbol on each return and the browser lays out and paints it again. With 250 stairs (the large-house fixture, the per-floor limit) that was 2,875 SVG elements and about 40 ms per entry locally; each stair carried 3-7 separate tread lines with four bound coordinates each. The treads of one stair are now a single with one `M a L b` subpath per tread, in geometry order and with the numbers the lines carried. Treads of one stair never overlap (straight: parallel, >= 20 cm apart; spiral: inner ends >= 6.7 cm apart at the 3.6 cm stroke), so the path paints the same pixels at any opacity. The outline points and the tread data are built once per cached geometry object (cachedStairMarkup, weak keys), not on every render. The View and plan-editor layers share the strings; outline, hit polygon, trapezoid, arrow, attributes and handlers are unchanged. Floor 1 of the fixture drops from 4,210 to 2,960 elements. Witnesses: the unit test for the path data and its cache, and the smoke_stairs markup checks in View and in the plan editor, are red on dev. The stairs-view-tread-lines mutant restores the View lines. Issue: #740 User-Visible: no Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_018qZfe7YS4rqEMKoVeS3GKd --- demo/smoke_stairs.mjs | 28 ++++++++++++ docs/STAIRS.md | 5 ++- docs/testing-notes/mutation-browser-guards.md | 1 + scripts/mutation-registry.mjs | 12 +++++ scripts/smoke-links.mjs | 8 ++++ src/stairs-editor.ts | 11 +++-- src/stairs-view.ts | 11 +++-- src/stairs.ts | 37 +++++++++++++++ test/stairs.test.mjs | 45 +++++++++++++++++++ 9 files changed, 145 insertions(+), 13 deletions(-) diff --git a/demo/smoke_stairs.mjs b/demo/smoke_stairs.mjs index 677b8b90..8c219dc9 100644 --- a/demo/smoke_stairs.mjs +++ b/demo/smoke_stairs.mjs @@ -96,6 +96,30 @@ const out = await page.evaluate(async () => { }; const activeSpace = () => root().querySelector('[data-hp="space-tab"][aria-current="page"]') ?.getAttribute('data-id'); + // #740 AC2: all treads of one stair are a single path whose `M` subpaths + // count the physical treads — equal intervals closest to 30 cm, ties upward + // (#683); a straight run draws the inner divisions, a spiral every radius. + const intervals = (cm) => { + const lower = Math.max(1, Math.floor(cm / 30)); + const upper = Math.max(1, Math.ceil(cm / 30)); + return Math.abs(cm / upper - 30) <= Math.abs(cm / lower - 30) ? upper : lower; + }; + const physicalTreads = (stair, cellCm) => (stair.kind === 'straight' + ? intervals(stair.length * cellCm * 240) - 1 + : intervals(Math.PI * 2 * (stair.radius * 2 / 3) * cellCm * 240)); + const treadsAreOnePath = (id, trapezoids, layer) => { + const node = stairNode(id); + const stair = stairs().find((item) => item.id === id); + const count = (selector) => node?.querySelectorAll(selector).length ?? -1; + const subpaths = node?.querySelector('path.hp-stair-tread')?.getAttribute('d')?.match(/M/g)?.length ?? 0; + // The View layer marks its symbols; the plan editor's are unmarked. + return !!node && !!stair && node.classList.contains('hp-stair-view') === (layer === 'view') + && count('path.hp-stair-tread') === 1 && count('line.hp-stair-tread') === 0 + && count('.hp-stair-tread') === 1 && count('.hp-stair-trapezoid') === trapezoids + && count('.hp-stair-arrow') === 1 && count('.hp-stair-outline') === 1 + && count('.hp-stair-hit') === 1 + && subpaths > 0 && subpaths === physicalTreads(stair, spaceCfg('f1').cell_cm); + }; const closeTo = (a, b, tolerance = 1e-5) => Math.abs(a - b) <= tolerance; const pathClose = (a, b, tolerance = 1e-5) => { const numbers = (value) => String(value || '').match(/-?\d+(?:\.\d+)?/g)?.map(Number) || []; @@ -162,6 +186,8 @@ const out = await page.evaluate(async () => { result.straightHasTrapezoidAndSpiralDoesNot = stairNode(straight.id)?.querySelectorAll('.hp-stair-trapezoid').length === 3 && stairNode(spiral.id)?.querySelectorAll('.hp-stair-trapezoid').length === 0; + result.editorTreadsAreOnePath = treadsAreOnePath(straight.id, 3, 'editor') + && treadsAreOnePath(spiral.id, 0, 'editor'); // A legacy record remains untouched on read; its first explicit Save writes // the complete visual quartet using the current decor fallback. @@ -442,6 +468,8 @@ const out = await page.evaluate(async () => { await hp.setMode('view'); let linkedNode = stairNode(straight.id); + result.viewTreadsAreOnePath = treadsAreOnePath(straight.id, 3, 'view') + && treadsAreOnePath(spiral.id, 0, 'view'); result.validLinkIsAccessible = linkedNode?.getAttribute('role') === 'link' && linkedNode?.getAttribute('data-target-state') === 'active' && getComputedStyle(linkedNode).cursor === 'pointer'; diff --git a/docs/STAIRS.md b/docs/STAIRS.md index d8c9b414..ec247057 100644 --- a/docs/STAIRS.md +++ b/docs/STAIRS.md @@ -145,7 +145,10 @@ links are cleared; a one-space transfer cannot invent an external target. ## Implementation boundary - `src/stairs.ts` owns validation, drawing geometry and area-subtraction - primitives. + primitives. The View and plan-editor symbols draw all treads of a stair as + one `` (one `M a L b` subpath per tread, #740); + its `d` and the outline points are built once per cached geometry object + (`cachedStairMarkup`), not on every render. - `src/stairs-view.ts` is the eager read-only boundary for symbols, guarded navigation and touch/pointer gesture suppression. - `src/stairs-editor-model.ts` keeps the eager-safe helpers the View runtime diff --git a/docs/testing-notes/mutation-browser-guards.md b/docs/testing-notes/mutation-browser-guards.md index 9599f738..477c7847 100644 --- a/docs/testing-notes/mutation-browser-guards.md +++ b/docs/testing-notes/mutation-browser-guards.md @@ -267,6 +267,7 @@ The invariant crosses Lit/custom-element lifecycle, browser storage/events, lazy - `same-space-room-change-recenters` - `space-create-hidden-display-override` - `stairs-view-pan-opens-target-floor` +- `stairs-view-tread-lines` - `support-invalid-response-leaks-issued-token` - `support-stale-preview-response-revives-consent` - `support-timeout-claims-success` diff --git a/scripts/mutation-registry.mjs b/scripts/mutation-registry.mjs index 468b5beb..1852ebf3 100644 --- a/scripts/mutation-registry.mjs +++ b/scripts/mutation-registry.mjs @@ -119,6 +119,18 @@ const MUTANT_DEFINITIONS = [ replace: ' if (!active\n', }], }, + { + id: 'stairs-view-tread-lines', + guard: 'node demo/smoke_stairs.mjs', + because: '#740 AC2: all treads of one stair are a single path; separate tread lines bring ' + + 'back 3–7 DOM elements per stair that every floor switch rebuilds.', + patches: [{ + file: 'src/stairs-view.ts', + find: ' ${markup.treads ? svg`` : nothing}\n', + replace: ' ${geometry.treads.map((line) => svg``)}\n', + }], + }, { id: 'stairs-fixed-floor-still-navigates', guard: 'node demo/smoke_stairs.mjs', diff --git a/scripts/smoke-links.mjs b/scripts/smoke-links.mjs index adc0fb35..1cb4b65c 100644 --- a/scripts/smoke-links.mjs +++ b/scripts/smoke-links.mjs @@ -590,6 +590,14 @@ export const SMOKE_LINKS = [ + 'isometric smoke inspects door/window/gate bases from the same helper, but neither ' + 'browser bundle exposes the helper name in its test steps', }, + { + symbols: ['cachedStairMarkup', 'stairTreadPath', 'StairMarkup'], + smokes: ['smoke_stairs.mjs'], + because: '#740: the strings of the stair symbol are observed only as the production markup of ' + + 'the View and plan-editor layers — one `path.hp-stair-tread` whose `M` subpaths count the ' + + 'physical treads, the outline and hit polygons, navigation and the editor gestures on them; ' + + 'no smoke names the helpers', + }, { // #234: единый резолвер толщины отрезка цепочки. Смок перехода между // толщинами не называет ни `chainSegmentCms`, ни `_wallChainSegmentCms` — он diff --git a/src/stairs-editor.ts b/src/stairs-editor.ts index c4ab4b75..ad2fe2c6 100644 --- a/src/stairs-editor.ts +++ b/src/stairs-editor.ts @@ -16,7 +16,7 @@ import { convertStairKind, normalizeStairAngle, snapStairToStairs, stairTargetState, } from './stairs-editor-model'; import { - cachedStairRenderGeometry, MAX_STAIRS_PER_SPACE, stairList, stairStyleVars, + cachedStairMarkup, cachedStairRenderGeometry, MAX_STAIRS_PER_SPACE, stairList, stairStyleVars, stairVisualFields, stairVisualStyle, type Stair, type StairVisualStyle, } from './stairs'; import type { SpaceModel } from './types'; @@ -502,7 +502,7 @@ export class StairEditorRuntime { private renderStair(stair: Stair, spaceIds: ReadonlySet, selected: boolean, draft: boolean): TemplateResult { const geometry = cachedStairRenderGeometry(stair, this.owner._cellCm); - const outline = geometry.outline.map((point) => point.join(',')).join(' '); + const markup = cachedStairMarkup(geometry); const targetState = stairTargetState( stair, this.owner._space, spaceIds, this.owner._hasFixedFloor, ); @@ -526,14 +526,13 @@ export class StairEditorRuntime { event.stopPropagation(); if (this.owner._mode === 'plan' && !draft) this.openDialog(stair); }}> - - + this.pointerDown(event, stair, 'move')} @click=${select}> ${geometry.trapezoid.map((line) => svg``)} - ${geometry.treads.map((line) => svg``)} + ${markup.treads ? svg`` : nothing} ` as unknown as TemplateResult; } diff --git a/src/stairs-view.ts b/src/stairs-view.ts index a3833ab3..d06d9e7a 100644 --- a/src/stairs-view.ts +++ b/src/stairs-view.ts @@ -1,6 +1,6 @@ import { nothing, svg, type TemplateResult } from 'lit'; import { - cachedStairRenderGeometry, stairList, stairStyleVars, type Stair, + cachedStairMarkup, cachedStairRenderGeometry, stairList, stairStyleVars, type Stair, } from './stairs'; import { stairTargetState } from './stairs-editor-model'; import type { SpaceModel } from './types'; @@ -68,7 +68,7 @@ export class StairViewRuntime { const interactive = this.owner._mode === 'view'; const items = this.stairs.map((stair) => { const geometry = cachedStairRenderGeometry(stair, this.owner._cellCm); - const outline = geometry.outline.map((point) => point.join(',')).join(' '); + const markup = cachedStairMarkup(geometry); const targetState = stairTargetState( stair, this.owner._space, spaceIds, this.owner._hasFixedFloor, ); @@ -107,13 +107,12 @@ export class StairViewRuntime { navigate(event); } }}> - - + this.pointerDown(event)}> ${geometry.trapezoid.map((line) => svg``)} - ${geometry.treads.map((line) => svg``)} + ${markup.treads ? svg`` : nothing} `; }); diff --git a/src/stairs.ts b/src/stairs.ts index b0006645..b2c2d6a2 100644 --- a/src/stairs.ts +++ b/src/stairs.ts @@ -328,6 +328,43 @@ export function cachedStairRenderGeometry( return geometry; } +/** Attribute strings of one stair symbol, shared by the View and plan-editor layers. */ +export interface StairMarkup { + /** `points` of the outline polygon and of its hit twin. */ + outline: string; + /** `d` of the single tread path; empty when the stair has no treads. */ + treads: string; +} + +/** + * #740: all treads of one stair are a single ``, one `M a L b` subpath + * per tread in geometry order with the numbers the separate ``s carried. + * Treads of one stair never overlap (parallel ≥ 20 cm apart; spiral inner ends + * ≥ 6.7 cm apart at the 3.6 cm stroke), so the path paints the same pixels. + */ +export function stairTreadPath(treads: readonly StairLine[]): string { + return treads.map((line) => `M ${line.a[0]} ${line.a[1]} L ${line.b[0]} ${line.b[1]}`).join(' '); +} + +const MARKUP_CACHE = new WeakMap(); + +/** + * The strings are built once per geometry object, not on every render: the + * render-geometry cache hands out a new object only when the geometry changes, + * and weak keys release the strings together with it. + */ +export function cachedStairMarkup(geometry: StairRenderGeometry): StairMarkup { + let markup = MARKUP_CACHE.get(geometry); + if (!markup) { + markup = { + outline: geometry.outline.map((point) => point.join(',')).join(' '), + treads: stairTreadPath(geometry.treads), + }; + MARKUP_CACHE.set(geometry, markup); + } + return markup; +} + export function stairFootprintGeometry(stair: Stair, scale = NORM_W): Geom { const ring = stairOutline(stair, scale); return (ring.length ? [[[...ring, ring[0]]]] : []) as unknown as Geom; diff --git a/test/stairs.test.mjs b/test/stairs.test.mjs index 4b8adaa6..e6ee93db 100644 --- a/test/stairs.test.mjs +++ b/test/stairs.test.mjs @@ -4,6 +4,7 @@ import test from 'node:test'; import { optimizePlans } from '../test-build/plan-optimizer.js'; import { + cachedStairMarkup, cachedStairRenderGeometry, floorAreaMinusStairs, geometryAreaMinusStairs, @@ -17,6 +18,7 @@ import { stairStrokePrintMm, stairStrokeUnits, stairStyleVars, + stairTreadPath, stairVisualFields, stairVisualStyle, } from '../test-build/stairs.js'; @@ -212,6 +214,49 @@ test('#663 cached render geometry survives live repaints and invalidates only on assert.notEqual(changed, first, 'in-place edits cannot leave a stale cache entry'); }); +test('#740 AC1 all treads of a stair are one path with the separate lines\' numbers', () => { + const joined = (treads) => treads + .map((line) => `M ${line.a[0]} ${line.a[1]} L ${line.b[0]} ${line.b[1]}`).join(' '); + const cases = [ + ['straight forward', straight({ angle: 17 })], + ['straight backward', straight({ direction: 'backward', angle: -33 })], + ['spiral clockwise', spiral({ angle: 30 })], + ['spiral counterclockwise', spiral({ direction: 'counterclockwise' })], + ]; + for (const [label, stair] of cases) { + const geometry = stairRenderGeometry(stair, 5); + assert.ok(geometry.treads.length > 1, `${label}: the fixture has treads`); + assert.equal(stairTreadPath(geometry.treads), joined(geometry.treads), label); + const markup = cachedStairMarkup(geometry); + assert.equal(markup.treads, joined(geometry.treads), `${label}: treads in geometry order`); + assert.equal(markup.treads.match(/M /g).length, geometry.treads.length, + `${label}: one subpath per tread`); + assert.equal(markup.treads.match(/L /g).length, geometry.treads.length, label); + assert.equal(markup.outline, geometry.outline.map((point) => point.join(',')).join(' '), + `${label}: the outline points are the polygon's former string`); + } + const short = stairRenderGeometry(straight({ length: 35 / 1200 }), 5); + assert.equal(short.treads.length, 0, 'a straight stair shorter than 40 cm has one interval'); + assert.equal(stairTreadPath(short.treads), ''); + assert.equal(cachedStairMarkup(short).treads, '', 'no treads, no path data'); +}); + +test('#740 AC1 tread and outline strings are built once per geometry object', () => { + const item = straight(); + const geometry = cachedStairRenderGeometry(item, 5); + const first = cachedStairMarkup(geometry); + assert.equal(cachedStairMarkup(geometry), first, 'the same geometry reuses the built strings'); + assert.equal(cachedStairMarkup(cachedStairRenderGeometry(item, 5)), first, + 'a repaint of an unchanged stair reuses them through the geometry cache'); + item.angle = 45; + const moved = cachedStairMarkup(cachedStairRenderGeometry(item, 5)); + assert.notEqual(moved, first, 'a geometry change builds new strings'); + assert.notEqual(moved.treads, first.treads); + const copy = stairRenderGeometry(item, 5); + assert.notEqual(cachedStairMarkup(copy), moved, 'the cache key is the geometry object'); + assert.deepEqual(cachedStairMarkup(copy), moved, 'equal geometry yields equal strings'); +}); + test('#663 stair magnet covers rectangle/rectangle, rectangle/circle and circle/circle footprints', () => { const rect = straight({ id: 'rect', x: 0.5, y: 0.5, length: 0.2, width: 0.1 }); const circle = spiral({ id: 'circle', x: 0.5, y: 0.5, radius: 0.08 });