diff --git a/dev/bg-hex-recalc-bench.html b/dev/bg-hex-recalc-bench.html new file mode 100644 index 0000000..0c91dad --- /dev/null +++ b/dev/bg-hex-recalc-bench.html @@ -0,0 +1,125 @@ + + + + + + + +
+ + + diff --git a/dev/bg-hex-recalc-bench.mjs b/dev/bg-hex-recalc-bench.mjs new file mode 100644 index 0000000..d8d266a --- /dev/null +++ b/dev/bg-hex-recalc-bench.mjs @@ -0,0 +1,32 @@ +#!/usr/bin/env node +/** Headless benchmark: isolates the style-recalc cost of the current + * `:not([style*="background-color"])` attribute-substring selector vs a + * class-based `:not(.has-terrain)` selector, at the real chult map's hex + * count (63×61). A :hover state change over the grid forces exactly this + * recalc, so this measures the per-mousemove jank a bg image introduces. + * + * Relative A/B only — absolute ms will differ from the user's machine, but + * the ratio between the two selectors is what we're after. + */ +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, "bg-hex-recalc-bench.html"); + +const result = await withBrowser(async ({ page }) => { + await page.goto(pathToFileURL(sandbox).href, { waitUntil: "networkidle0" }); + await page.waitForFunction(() => document.title === "ready", { timeout: 10000 }); + return page.evaluate(() => window.__runBench(40)); +}); + +const f = (n) => n.toFixed(2).padStart(8); +console.log(`\nFull grid rebuild + layout flush (the renderGrid open-time cost)`); +console.log(`hexes=${result.hexCount} painted=${result.painted} empty=${result.empty} iters=${result.iters}\n`); +console.log(` median mean p95 (ms)`); +console.log(`no bg image ${f(result.none.median)} ${f(result.none.mean)} ${f(result.none.p95)} baseline`); +console.log(`bg + attr selector ${f(result.attr.median)} ${f(result.attr.mean)} ${f(result.attr.p95)} (current)`); +console.log(`bg + class selector ${f(result.cls.median)} ${f(result.cls.mean)} ${f(result.cls.p95)} (proposed)`); +console.log(`\nbg overhead (attr vs none): ${(result.attr.median - result.none.median).toFixed(2)} ms/render`); +console.log(`selector saving (attr→class): ${(result.attr.median - result.cls.median).toFixed(2)} ms/render\n`); diff --git a/src/frontmatter.ts b/src/frontmatter.ts index e8f4986..2158637 100644 --- a/src/frontmatter.ts +++ b/src/frontmatter.ts @@ -59,9 +59,33 @@ export function getFrontMatter(app: App, path: string) { return frontmatter; } +/** + * Pure extractors that read already-fetched frontmatter. The hex-grid render + * loop fetches frontmatter ONCE per hex (getAbstractFileByPath + getFileCache) + * and feeds it to all three — instead of calling the three `*FromFile` helpers + * below, which each re-fetch the same cache (3× the lookups per hex, ~11.5k + * redundant ops on a 3843-hex map). + */ +export function terrainFromFm(fm: Frontmatter | null | undefined): string | null { + return typeof fm?.terrain === "string" ? fm.terrain : null; +} + +export function iconOverrideFromFm(fm: Frontmatter | null | undefined): string | null { + return typeof fm?.icon === "string" ? fm.icon : null; +} + +export function gmIconsFromFm(fm: Frontmatter | null | undefined): string[] { + if (!fm) return []; + const arr = fm["gm-icons"]; + if (Array.isArray(arr)) { + return arr.filter((v): v is string => typeof v === "string"); + } + const legacy = fm["gm-icon"]; + return typeof legacy === "string" ? [legacy] : []; +} + export function getTerrainFromFile(app: App, path: string): string | null { - const terrain = getFrontMatter(app, path)?.terrain; - return typeof terrain === "string" ? terrain : null; + return terrainFromFm(getFrontMatter(app, path)); } export async function setTerrainInFile( @@ -224,8 +248,7 @@ export async function setRegionStyleInFile( } export function getIconOverrideFromFile(app: App, path: string): string | null { - const icon = getFrontMatter(app, path)?.icon; - return typeof icon === "string" ? icon : null; + return iconOverrideFromFm(getFrontMatter(app, path)); } export async function setIconOverrideInFile( @@ -263,14 +286,7 @@ export function getGmIconFromFile(app: App, path: string): string | null { * singular key `gm-icon` if present so existing notes Just Work. */ export function getGmIconsFromFile(app: App, path: string): string[] { - const fm = getFrontMatter(app, path); - if (!fm) return []; - const arr = fm["gm-icons"]; - if (Array.isArray(arr)) { - return arr.filter((v): v is string => typeof v === "string"); - } - const legacy = fm["gm-icon"]; - return typeof legacy === "string" ? [legacy] : []; + return gmIconsFromFm(getFrontMatter(app, path)); } /** diff --git a/src/hex-map/HexMapView.ts b/src/hex-map/HexMapView.ts index 96211d7..3b4017e 100644 --- a/src/hex-map/HexMapView.ts +++ b/src/hex-map/HexMapView.ts @@ -4,7 +4,10 @@ import { normalizeFolder, getIconUrl, createIconEl } from "../utils"; import { getTerrainFromFile, getIconOverrideFromFile, - getGmIconsFromFile, + getFrontMatter, + terrainFromFm, + iconOverrideFromFm, + gmIconsFromFm, setGmIconsInFile, setTerrainInFile, setIconOverrideInFile, @@ -178,6 +181,10 @@ export class HexMapView extends ItemView { private pendingZoomPivot: { cx: number; cy: number } | null = null; private zoomFrameId: number | null = null; private viewportEl: HTMLElement | null = null; + // Cached bg-image layer reused across renders (see renderBackgroundImage) so + // renderGrid()'s viewportEl.empty() doesn't force an image re-decode each time. + private bgLayerEl: HTMLElement | null = null; + private bgLayerSrc: string | null = null; private drawingMode: | "path" | "terrain" @@ -2037,16 +2044,42 @@ export class HexMapView extends ItemView { const bg = map.backgroundImage; viewportEl.toggleClass("has-bg-image", !!bg?.path); viewportEl.toggleClass("is-bg-calibrating", this.bgCalibrating && !!bg?.path); - if (!bg?.path) return; + if (!bg?.path) { + this.bgLayerEl = null; + this.bgLayerSrc = null; + return; + } const file = this.app.vault.getAbstractFileByPath(bg.path); - if (!(file instanceof TFile)) return; + if (!(file instanceof TFile)) { + this.bgLayerEl = null; + this.bgLayerSrc = null; + return; + } + + const src = this.app.vault.adapter.getResourcePath(bg.path); + + // Reuse the cached layer+img across renders when the image is unchanged. + // renderGrid() calls viewportEl.empty() every render, which detaches the + // layer but leaves our JS reference (and its already-decoded bitmap) alive. + // Re-attaching it avoids recreating the and forcing the browser to + // re-decode + re-rasterise the (large, scaled) image on every single + // render — the churn that makes editing a bg-image map sluggish. We skip + // reuse during calibration so its drag/wheel handlers are wired exactly as + // before (fresh layer each render, as the old code did). + if (this.bgLayerEl && this.bgLayerSrc === src && !this.bgCalibrating) { + viewportEl.prepend(this.bgLayerEl); + this.applyBgLayerVars(this.bgLayerEl, bg); + return; + } const layer = viewportEl.createDiv({ cls: "duckmage-bg-image-layer" }); this.applyBgLayerVars(layer, bg); const img = layer.createEl("img", { cls: "duckmage-bg-image" }); - img.src = this.app.vault.adapter.getResourcePath(bg.path); + img.src = src; img.alt = ""; img.draggable = false; + this.bgLayerEl = layer; + this.bgLayerSrc = src; if (this.bgCalibrating) this.attachCalibrationHandlers(layer, map); } @@ -2818,9 +2851,15 @@ export class HexMapView extends ItemView { const addHex = (parent: HTMLElement, x: number, y: number) => { const path = folder ? `${folder}/${x}_${y}.md` : `${x}_${y}.md`; const exists = existingHexPaths.has(path); + // Fetch this hex's frontmatter ONCE; terrain/icon/gm-icons all derive + // from it. Calling getTerrainFromFile + getIconOverrideFromFile + + // getGmIconsFromFile separately would re-run getAbstractFileByPath + + // getFileCache three times per hex (~11.5k redundant lookups on a + // 3843-hex map). + const fm = getFrontMatter(this.app, path); const terrainKey = terrainOverrides?.has(path) ? terrainOverrides.get(path)! - : getTerrainFromFile(this.app, path); + : terrainFromFm(fm); const terrainEntry = terrainKey != null ? paletteByName.get(terrainKey) : undefined; const hexEl = parent.createDiv({ @@ -2829,11 +2868,11 @@ export class HexMapView extends ItemView { }); hexEl.tabIndex = -1; - if (terrainEntry?.color) hexEl.style.backgroundColor = terrainEntry.color; + this.setHexColor(hexEl, terrainEntry?.color); const iconOverride = iconOverrides?.has(path) ? iconOverrides.get(path)! - : getIconOverrideFromFile(this.app, path); + : iconOverrideFromFm(fm); if (iconOverride) { // Render terrain icon as hidden fallback — shown by CSS when overrides are off if (terrainEntry?.icon) { @@ -2874,7 +2913,7 @@ export class HexMapView extends ItemView { // list lands on the dataset immediately, no race. const gmIcons = gmIconsOverrides?.has(path) ? gmIconsOverrides.get(path)! - : getGmIconsFromFile(this.app, path); + : gmIconsFromFm(fm); if (gmIcons.length > 0) hexEl.dataset.gmIcons = JSON.stringify(gmIcons); if (this.selectedHex?.x === x && this.selectedHex?.y === y) @@ -3232,7 +3271,7 @@ export class HexMapView extends ItemView { `[data-x="${hx}"][data-y="${hy}"]`, ); if (hexEl) { - hexEl.style.backgroundColor = entry?.color ?? ""; + this.setHexColor(hexEl, entry?.color); hexEl.querySelector(".duckmage-hex-icon")?.remove(); hexEl.querySelector(".duckmage-hex-dot")?.remove(); if (entry?.icon) { @@ -3567,6 +3606,24 @@ export class HexMapView extends ItemView { // writes: the in-flight one and then the final value. No writes are lost; no // stale intermediate value can overwrite a newer one. + /** + * Set (or clear) a hex cell's terrain background color, keeping the + * `duckmage-hex-painted` marker class in sync. The bg-image stylesheet uses + * that class — rather than a `:not([style*="background-color"])` + * attribute-substring selector, which is measurably slower for the engine to + * match across the thousands of hexes on a large map — to decide which empty + * hexes fall transparent over the image. + */ + private setHexColor(hexEl: HTMLElement, color: string | undefined | null): void { + if (color) { + hexEl.style.backgroundColor = color; + hexEl.addClass("duckmage-hex-painted"); + } else { + hexEl.style.removeProperty("background-color"); + hexEl.removeClass("duckmage-hex-painted"); + } + } + private applyTerrainToHexEl( hexEl: HTMLElement, terrain: string | null, @@ -3574,7 +3631,7 @@ export class HexMapView extends ItemView { const palette = this.plugin.getMapPalette(this.activeMapName); const entry = terrain != null ? palette.find((p) => p.name === terrain) : undefined; - hexEl.style.backgroundColor = entry?.color ?? ""; + this.setHexColor(hexEl, entry?.color); hexEl.querySelector(".duckmage-hex-icon")?.remove(); hexEl.querySelector(".duckmage-hex-dot")?.remove(); if (entry?.icon) { diff --git a/src/hex-table/HexTableView.ts b/src/hex-table/HexTableView.ts index 08d7c89..75e665a 100644 --- a/src/hex-table/HexTableView.ts +++ b/src/hex-table/HexTableView.ts @@ -5,6 +5,7 @@ import { VIEW_TYPE_HEX_TABLE, VIEW_TYPE_RANDOM_TABLES, } from "../constants"; +import type { TerrainColor } from "../types"; import { getAllSectionData } from "../sections"; import { getTerrainFromFile } from "../frontmatter"; import { normalizeFolder, makeTableTemplate } from "../utils"; @@ -39,6 +40,11 @@ export class HexTableView extends ItemView { private scrollEl: HTMLElement | null = null; private updateTimers = new Map>(); private loadGeneration = 0; + // Memoised name→entry palette maps, keyed by region. fillRow runs once per + // hex per load (skeleton + fill batch) so rebuilding this Map inline cost + // O(rows × palette) Map insertions; memoising makes it O(palette) per region. + // Cleared at the start of every loadTable so palette edits take effect. + private paletteMapCache = new Map>(); // Sort state private sortPrimary: "x" | "y" = "x"; @@ -361,6 +367,7 @@ export class HexTableView extends ItemView { async loadTable(): Promise { if (!this.scrollEl) return; const gen = ++this.loadGeneration; + this.paletteMapCache.clear(); this.scrollEl.empty(); this.scrollEl.createSpan({ @@ -477,12 +484,15 @@ export class HexTableView extends ItemView { slice.map((f) => getAllSectionData(this.app, f.path)), ); if (gen !== this.loadGeneration) return false; + const batchRows: HTMLTableRowElement[] = []; for (let j = 0; j < slice.length; j++) { const { path, x, y, region } = slice[j]; const { text, links } = sectionData[j]; this.fillRow(rows[start + j], path, x, y, region, text, links); + batchRows.push(rows[start + j]); } - this.applyFilters(); + // Only the rows we just filled gained data — re-filter just those. + this.applyFilters(batchRows); return true; }; @@ -555,12 +565,19 @@ export class HexTableView extends ItemView { this.applyFilters(); } - private applyFilters(): void { + /** + * Re-evaluate row visibility against the current filters. Each row's + * visibility depends only on its own dataset, so a `rows` subset can be + * passed to filter just newly-filled rows — `loadTable` does this after each + * fill batch so Phase 2 stays O(rows) total instead of O(rows × batches). + * Called with no argument (all rows) when the filter state itself changes. + */ + private applyFilters(rows?: Iterable): void { if (!this.scrollEl) return; const tbody = this.scrollEl.querySelector("tbody"); if (!tbody) return; - for (const tr of Array.from(tbody.rows)) { + for (const tr of rows ?? Array.from(tbody.rows)) { const x = Number(tr.dataset.hexX); const y = Number(tr.dataset.hexY); const terrain = tr.dataset.terrain ?? ""; @@ -594,6 +611,22 @@ export class HexTableView extends ItemView { // ── Row rendering ───────────────────────────────────────────────────────── + /** + * Name→entry palette map for a region, memoised for the current load. + * First-wins on duplicate names — matches HexMapView's `.find()` behaviour. + */ + private getPaletteMap(region: string): Map { + let m = this.paletteMapCache.get(region); + if (!m) { + m = new Map(); + for (const p of this.plugin.getMapPalette(region)) { + if (!m.has(p.name)) m.set(p.name, p); + } + this.paletteMapCache.set(region, m); + } + return m; + } + private fillRow( tr: HTMLTableRowElement, path: string, @@ -605,13 +638,7 @@ export class HexTableView extends ItemView { ): void { tr.empty(); - const palette = this.plugin.getMapPalette(region); - // First-wins on duplicate-named palette entries — matches HexMapView's - // .find() behaviour. - const paletteMap = new Map(); - for (const p of palette) { - if (!paletteMap.has(p.name)) paletteMap.set(p.name, p); - } + const paletteMap = this.getPaletteMap(region); const terrainName = getTerrainFromFile(this.app, path); const hasTown = (links.get("towns") ?? []).length > 0; diff --git a/styles.css b/styles.css index fd96858..6ca82f2 100644 --- a/styles.css +++ b/styles.css @@ -268,9 +268,12 @@ .duckmage-calibration-handle-se { bottom: -9px; right: -9px; cursor: nwse-resize; transform: scale(var(--duckmage-cal-scale, 1)); } /* When a background image is present, hexes without a terrain become visually - transparent so the underlying image shows through. Hexes with a terrain - color set inline (via style="background-color: …") render normally. */ -.duckmage-hex-map-viewport.has-bg-image .duckmage-hex:not([style*="background-color"]) { + transparent so the underlying image shows through. Hexes with a terrain are + tagged `.duckmage-hex-painted` (see setHexColor) and render normally. A class + selector is used here rather than a `:not([style*="background-color"])` + attribute-substring match because the engine matches it far more cheaply + across the thousands of hexes on a large map. */ +.duckmage-hex-map-viewport.has-bg-image .duckmage-hex:not(.duckmage-hex-painted) { background-color: transparent; } diff --git a/tests/frontmatter.test.ts b/tests/frontmatter.test.ts index 638eeb7..ca12d0b 100644 --- a/tests/frontmatter.test.ts +++ b/tests/frontmatter.test.ts @@ -1,7 +1,7 @@ import { describe, it, mock } from "node:test"; import expect from "expect"; import { TFile } from "obsidian"; -import { getTerrainFromFile, setTerrainInFile, getIconOverrideFromFile, setIconOverrideInFile, getSubmapFromFile, setSubmapInFile } from "../src/frontmatter"; +import { getTerrainFromFile, setTerrainInFile, getIconOverrideFromFile, setIconOverrideInFile, getSubmapFromFile, setSubmapInFile, terrainFromFm, iconOverrideFromFm, gmIconsFromFm } from "../src/frontmatter"; /** Build a minimal mock App backed by an in-memory string. */ function makeApp(filePath: string, initialContent: string) { @@ -359,3 +359,47 @@ describe("updateSubmapReferences", () => { expect(getContent("world/hexes/map-a/1_1.md")).toContain("duckmage-submap: other-map"); }); }); + +// ── Pure frontmatter extractors (used by the hex-grid render loop, which +// fetches frontmatter once per hex and feeds it to all three) ────────────── + +describe("terrainFromFm", () => { + it("returns the terrain string", () => { + expect(terrainFromFm({ terrain: "forest" } as any)).toBe("forest"); + }); + it("returns null for missing/non-string/nullish frontmatter", () => { + expect(terrainFromFm({ icon: "x.png" } as any)).toBeNull(); + expect(terrainFromFm({ terrain: 3 } as any)).toBeNull(); + expect(terrainFromFm(null)).toBeNull(); + expect(terrainFromFm(undefined)).toBeNull(); + }); +}); + +describe("iconOverrideFromFm", () => { + it("returns the icon string", () => { + expect(iconOverrideFromFm({ icon: "castle.png" } as any)).toBe("castle.png"); + }); + it("returns null for missing/non-string/nullish frontmatter", () => { + expect(iconOverrideFromFm({ terrain: "forest" } as any)).toBeNull(); + expect(iconOverrideFromFm({ icon: true } as any)).toBeNull(); + expect(iconOverrideFromFm(null)).toBeNull(); + }); +}); + +describe("gmIconsFromFm", () => { + it("returns the gm-icons array, filtered to strings", () => { + expect(gmIconsFromFm({ "gm-icons": ["a.png", "b.png"] } as any)).toEqual(["a.png", "b.png"]); + expect(gmIconsFromFm({ "gm-icons": ["a.png", 2, null, "b.png"] } as any)).toEqual(["a.png", "b.png"]); + }); + it("falls back to legacy single gm-icon", () => { + expect(gmIconsFromFm({ "gm-icon": "legacy.png" } as any)).toEqual(["legacy.png"]); + }); + it("prefers gm-icons array over legacy gm-icon", () => { + expect(gmIconsFromFm({ "gm-icons": ["a.png"], "gm-icon": "legacy.png" } as any)).toEqual(["a.png"]); + }); + it("returns [] for missing/nullish frontmatter", () => { + expect(gmIconsFromFm({} as any)).toEqual([]); + expect(gmIconsFromFm(null)).toEqual([]); + expect(gmIconsFromFm(undefined)).toEqual([]); + }); +});