diff --git a/CLAUDE.md b/CLAUDE.md index ab4835a..cfb1fd7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -186,6 +186,7 @@ Renders an interactive hex-grid map for tabletop RPG world-building inside Obsid - Exclude notes whose `basename` starts with `"_"` whenever enumerating vault files for display, dropdowns, or linking. Pattern: `.filter(f => !f.basename.startsWith("_"))`. - **All modals must extend `DuckmageModal`** (`src/DuckmageModal.ts`) rather than Obsidian's `Modal` directly. `DuckmageModal` is the single place for shared modal behaviour. Any functionality needed by more than one modal belongs there, not in `utils.ts` or duplicated across files. - All modals call `this.makeDraggable()` in `onOpen` (inherited from `DuckmageModal`), which adds the `duckmage-editor-modal-drag` class and locks dragging to the title-bar area. (`FileLinkSuggestModal` is the sole exception — it extends `SuggestModal` and cannot inherit from `DuckmageModal`.) +- **NEVER interleave layout reads (`offsetWidth/Height/Left/Top`, `getBoundingClientRect`, `getComputedStyle`) with DOM writes (`createDiv`/`createEl`/`appendChild`/`setCssProps`/style changes) inside a per-hex loop.** Each read after a write forces a full synchronous layout flush → O(n²) thrash. On large maps (chult = 3843 hexes) this stalled the main thread for ~20s on open (regression in `renderCoordLabelsLayer`, commit 10475fd; fixed by batching). **Pattern:** read ALL geometry into a plain array first, THEN do all DOM writes. This is the established pattern in `renderPathOverlay`/`renderFactionOverlay` (build `centerMap` read-only, then build the SVG). The overlay/label builders in `HexMapView` are the hot path — any new one MUST follow read-then-write. Reproduce/validate with `dev/coord-label-thrash-bench.{html,mjs}` (~198× speedup batched vs interleaved). ### Obsidian reviewer-bot conventions (enforced beyond what eslint catches) diff --git a/dev/coord-label-thrash-bench.html b/dev/coord-label-thrash-bench.html new file mode 100644 index 0000000..488b190 --- /dev/null +++ b/dev/coord-label-thrash-bench.html @@ -0,0 +1,131 @@ + + +
+ + + + + + + + diff --git a/dev/coord-label-thrash-bench.mjs b/dev/coord-label-thrash-bench.mjs new file mode 100644 index 0000000..42dd5cf --- /dev/null +++ b/dev/coord-label-thrash-bench.mjs @@ -0,0 +1,36 @@ +#!/usr/bin/env node +/** Reproduces the renderCoordLabelsLayer layout-thrash regression (commit + * 10475fd) and validates the read-then-write fix, at the real chult map's + * hex count, with and without a bg image. Headless Chromium — absolute ms + * differ from the user's machine but the thrash→batch ratio is the signal. + */ +import path from "node:path"; +import { fileURLToPath, pathToFileURL } from "node:url"; +import { withBrowser } from "/mnt/c/Users/markr/work/tools/chromium-driver/lib/browser.mjs"; + +const here = path.dirname(fileURLToPath(import.meta.url)); +const sandbox = path.join(here, "coord-label-thrash-bench.html"); + +const ITERS = 3; +const r = await withBrowser(async ({ page }) => { + await page.goto(pathToFileURL(sandbox).href, { waitUntil: "networkidle0" }); + await page.waitForFunction(() => document.title === "ready", { timeout: 10000 }); + const hexCount = await page.evaluate(() => window.__hexCount); + const m = (which, bg) => page.evaluate((w, b, n) => window.__measure(w, b, n), which, bg, ITERS); + return { + hexCount, + iters: ITERS, + thrash_noBg: await m("thrash", false), + batch_noBg: await m("batch", false), + thrash_bg: await m("thrash", true), + batch_bg: await m("batch", true), + }; +}); + +const f = (n) => n.toFixed(1).padStart(9); +console.log(`\nrenderCoordLabelsLayer cost — hexes=${r.hexCount}, median of ${r.iters} (ms)\n`); +console.log(` no bg image with bg image`); +console.log(`thrash (current) ${f(r.thrash_noBg)} ${f(r.thrash_bg)} ← regression`); +console.log(`batch (fixed) ${f(r.batch_noBg)} ${f(r.batch_bg)}`); +console.log(`\nspeedup no-bg: ${(r.thrash_noBg / r.batch_noBg).toFixed(1)}× with-bg: ${(r.thrash_bg / r.batch_bg).toFixed(1)}×`); +console.log(`bg amplifies thrash by ${(r.thrash_bg / r.thrash_noBg).toFixed(1)}× (vs ${(r.batch_bg / r.batch_noBg).toFixed(1)}× when batched)\n`); diff --git a/src/hex-map/HexMapView.ts b/src/hex-map/HexMapView.ts index 7cf5292..96211d7 100644 --- a/src/hex-map/HexMapView.ts +++ b/src/hex-map/HexMapView.ts @@ -4462,14 +4462,17 @@ export class HexMapView extends ItemView { ?.remove(); const gw = gridContainer.offsetWidth || 1; const gh = gridContainer.offsetHeight || 1; - const layer = gridContainer.createDiv({ - cls: "duckmage-coord-labels-layer", - }); + + // CRITICAL: read every hex's offset geometry FIRST, into a plain array, + // before creating any label DOM. Interleaving offset reads with DOM writes + // forces a full synchronous layout per hex — O(n²) layout thrash that + // stalled opening large maps for ~20s at chult's 3843 hexes (see + // dev/coord-label-thrash-bench). With reads batched ahead of writes the + // browser flushes layout once, dropping this to ~100ms. + const placements: { x: number; y: number; ox: number; oy: number }[] = []; gridContainer .querySelectorAll