fix: eliminate O(n²) layout thrash in renderCoordLabelsLayer (open-time stall)

renderCoordLabelsLayer read each hex's offsetWidth/offsetLeft/offsetTop and created the label DOM inside the SAME loop. Every offset read after a DOM write forces a synchronous layout flush, so the loop was O(n²): on the chult map (3843 hexes) opening stalled the main thread ~21s (regression from commit 10475fd, the coord-label layer). Split into a read phase (collect all hex geometry into an array) then a write phase (create all labels) — matches the read-then-write pattern already used by renderPathOverlay/renderFactionOverlay. ~21s → ~0.1s (dev/coord-label-thrash- bench measures ~198× batched vs interleaved). Bakes in a regression guard: CLAUDE.md convention forbidding interleaved layout-read / DOM-write in per-hex loops, plus the committed benchmark. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-22 06:14:01 +00:00 · 2026-06-30 06:57:16 -04:00 · 2026-06-30 06:57:16 -04:00 · e74bc0d6f6
commit e74bc0d6f6
parent 4cd0550a9a
4 changed files with 196 additions and 12 deletions
--- 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)

--- a/dev/coord-label-thrash-bench.html
+++ b/dev/coord-label-thrash-bench.html
@ -0,0 +1,131 @@
+<!doctype html>
+<html>
+<head>
+<meta charset="utf-8" />
+<style>
+  body { margin: 0; font-family: monospace; }
+  .viewport { position: relative; will-change: transform; }
+  .bg-layer { position: absolute; top: 0; left: 0; transform-origin: 0 0; transform: scale(4.84); z-index: 0; pointer-events: none; }
+  .bg-layer img { display: block; max-width: none; max-height: none; }
+  .grid { position: relative; display: flex; flex-direction: row; align-items: flex-start; }
+  .col { display: flex; flex-direction: column; margin-right: -1.1em; }
+  .col-offset { margin-top: 1.9em; }
+  .hex {
+    width: 4.4em; height: 3.81em; font-size: 8px;
+    clip-path: polygon(25% 0%, 75% 0%, 100% 50%, 75% 100%, 25% 100%, 0% 50%);
+    background-color: #2a4d3a; position: relative;
+  }
+  .coord-labels-layer { position: absolute; inset: 0; pointer-events: none; z-index: 10; }
+  .coord-label-html {
+    position: absolute;
+    left: var(--duckmage-coord-x, 0%); top: var(--duckmage-coord-y, 0%);
+    transform: translate(-50%, -50%);
+  }
+</style>
+</head>
+<body>
+<div id="root"></div>
+<script>
+  const COLS = 63, ROWS = 61;          // chult map dimensions
+  const HEX_COUNT = COLS * ROWS;
+  const root = document.getElementById("root");
+
+  const c = document.createElement("canvas");
+  c.width = 1107; c.height = 1499;
+  const ctx = c.getContext("2d");
+  ctx.fillStyle = "#357"; ctx.fillRect(0, 0, 1107, 1499);
+  const IMG_URL = c.toDataURL("image/png");
+
+  let grid, viewport;
+  function buildGrid(withBg) {
+    root.replaceChildren();
+    viewport = document.createElement("div");
+    viewport.className = "viewport";
+    if (withBg) {
+      const bg = document.createElement("div");
+      bg.className = "bg-layer";
+      const img = document.createElement("img");
+      img.src = IMG_URL;
+      bg.appendChild(img);
+      viewport.appendChild(bg);
+    }
+    grid = document.createElement("div");
+    grid.className = "grid";
+    for (let x = 0; x < COLS; x++) {
+      const col = document.createElement("div");
+      col.className = "col" + (x % 2 ? " col-offset" : "");
+      for (let y = 0; y < ROWS; y++) {
+        const hex = document.createElement("div");
+        hex.className = "hex";
+        hex.dataset.x = x; hex.dataset.y = y;
+        col.appendChild(hex);
+      }
+      grid.appendChild(col);
+    }
+    viewport.appendChild(grid);
+    root.appendChild(viewport);
+    void document.body.offsetHeight; // ensure laid out before we measure
+  }
+
+  // ── CURRENT code: read offset* then write a label, interleaved per hex ──
+  function thrashing() {
+    grid.querySelector(".coord-labels-layer")?.remove();
+    const gw = grid.offsetWidth || 1, gh = grid.offsetHeight || 1;
+    const layer = grid.appendChild(document.createElement("div"));
+    layer.className = "coord-labels-layer";
+    grid.querySelectorAll(".hex").forEach((hexEl) => {
+      let ox = hexEl.offsetWidth / 2, oy = hexEl.offsetHeight / 2;   // READ
+      let cur = hexEl;
+      while (cur && cur !== grid) { ox += cur.offsetLeft; oy += cur.offsetTop; cur = cur.offsetParent; }
+      const label = layer.appendChild(document.createElement("div")); // WRITE
+      label.className = "coord-label-html";
+      label.textContent = hexEl.dataset.x + "," + hexEl.dataset.y;
+      label.style.setProperty("--duckmage-coord-x", ((ox / gw) * 100).toFixed(3) + "%");
+      label.style.setProperty("--duckmage-coord-y", ((oy / gh) * 100).toFixed(3) + "%");
+    });
+  }
+
+  // ── FIXED code: read ALL positions first, then write ALL labels ──
+  function batched() {
+    grid.querySelector(".coord-labels-layer")?.remove();
+    const gw = grid.offsetWidth || 1, gh = grid.offsetHeight || 1;
+    const pos = [];
+    grid.querySelectorAll(".hex").forEach((hexEl) => {                // READ phase
+      let ox = hexEl.offsetWidth / 2, oy = hexEl.offsetHeight / 2;
+      let cur = hexEl;
+      while (cur && cur !== grid) { ox += cur.offsetLeft; oy += cur.offsetTop; cur = cur.offsetParent; }
+      pos.push({ t: hexEl.dataset.x + "," + hexEl.dataset.y, ox, oy });
+    });
+    const layer = grid.appendChild(document.createElement("div"));    // WRITE phase
+    layer.className = "coord-labels-layer";
+    for (const p of pos) {
+      const label = layer.appendChild(document.createElement("div"));
+      label.className = "coord-label-html";
+      label.textContent = p.t;
+      label.style.setProperty("--duckmage-coord-x", ((p.ox / gw) * 100).toFixed(3) + "%");
+      label.style.setProperty("--duckmage-coord-y", ((p.oy / gh) * 100).toFixed(3) + "%");
+    }
+  }
+
+  function time(fn, withBg, iters) {
+    const samples = [];
+    for (let i = 0; i < iters; i++) {
+      buildGrid(withBg);
+      const t0 = performance.now();
+      fn();
+      void document.body.offsetHeight; // force the final flush
+      samples.push(performance.now() - t0);
+    }
+    samples.sort((a, b) => a - b);
+    return samples[Math.floor(samples.length / 2)];
+  }
+
+  window.__hexCount = HEX_COUNT;
+  // One config per call so each puppeteer evaluate stays short (the thrash
+  // path on a bg map can take seconds per iteration).
+  window.__measure = (which, withBg, iters) =>
+    time(which === "thrash" ? thrashing : batched, withBg, iters);
+  document.title = "ready";
+</script>
+</body>
+</html>
--- a/dev/coord-label-thrash-bench.mjs
+++ 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`);
--- 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<HTMLElement>(".duckmage-hex")
      .forEach((hexEl) => {
-        const x = Number(hexEl.dataset.x);
-        const y = Number(hexEl.dataset.y);
        // Hex center in gridContainer-local pixel coords (unaffected by
        // CSS transform; offsetParent walks stop at gridContainer).
        let ox = hexEl.offsetWidth / 2;
@ -4480,15 +4483,28 @@ export class HexMapView extends ItemView {
          oy += cur.offsetTop;
          cur = cur.offsetParent as HTMLElement | null;
        }
-        const label = layer.createDiv({
-          cls: "duckmage-coord-label-html",
-          text: `${x},${y}`,
-        });
-        label.setCssProps({
-          "--duckmage-coord-x": `${((ox / gw) * 100).toFixed(3)}%`,
-          "--duckmage-coord-y": `${((oy / gh) * 100).toFixed(3)}%`,
+        placements.push({
+          x: Number(hexEl.dataset.x),
+          y: Number(hexEl.dataset.y),
+          ox,
+          oy,
        });
      });
+
+    // Write phase: no offset reads here, so layout stays clean throughout.
+    const layer = gridContainer.createDiv({
+      cls: "duckmage-coord-labels-layer",
+    });
+    for (const { x, y, ox, oy } of placements) {
+      const label = layer.createDiv({
+        cls: "duckmage-coord-label-html",
+        text: `${x},${y}`,
+      });
+      label.setCssProps({
+        "--duckmage-coord-x": `${((ox / gw) * 100).toFixed(3)}%`,
+        "--duckmage-coord-y": `${((oy / gh) * 100).toFixed(3)}%`,
+      });
+    }
  }

  private updatePathOverlay(): void {