From 6db9c8dcedf6e72325065b4e8a47533c230e0c38 Mon Sep 17 00:00:00 2001 From: byrongamatos Date: Thu, 9 Jul 2026 12:49:24 +0200 Subject: [PATCH] refactor(editor): extract canvas geometry to src/geometry.js (R2, step 5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The time<->x and string<->y mappings every draw and hit-test path goes through move out of src/main.js (20,799 -> 20,764): timeToX/xToTime, laneToY/yToLane, strToY/yToStr, the lane metrics they read (WAVEFORM_H, LANE_H, BEAT_H, LABEL_W, ANCHOR_LANE_H, HS_LANE_H) and the pure scroll-bound arithmetic. Reads S and the lane model; no DOM. Graph stays acyclic: geometry -> lanes -> state. NO RENAMES, unlike step 4's LC container. The three lane metrics are reassigned on every resize, but their only writer — the lane-sizing arithmetic inside resizeCanvas() — moves with them, as setLaneMetrics(canvasHeightPx). ES import bindings are LIVE and read-only, so main.js's ~100 read sites keep reading the current value verbatim and none of them can write it. LC needed a container only because its writers (draw(), onMouseMove()) had to stay in main.js. main.js's whole diff is the deletions, the import block, and replacing three assignments in resizeCanvas with one setLaneMetrics(h) call. Tests: scroll_bounds — the last @pure-block slicer among the coordinate helpers — imports the real module. New tests/geometry.test.mjs pins the mappings (inversion, out-of-range clamping, gutter offset, lane height dividing by the CURRENT string count) and the live-binding contract: setLaneMetrics updates what importers see, an importer assigning throws TypeError, and laneToY tracks a resize instead of capturing boot-time metrics. Verified: node --test 84/84, pytest 248/248. Served from local uvicorn on core@main (R0): src/geometry.js 200 as text/javascript. Four headless Chromium harnesses green — draw path, state round-trip, hit test, and a NEW resize harness written for this change: shrinking the viewport 663px -> 283px moves the fret-label baseline -276px (so draw() sees the new metrics), a click at the new coordinates still selects the note (so hit-testing agrees with draw), and no phantom hit remains at the stale position. A read site that captured a metric at module-eval time would fail the first of those. Codex preflight: clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 18 +++ src/geometry.js | 82 +++++++++++ src/main.js | 78 +++-------- tests/geometry.test.mjs | 130 ++++++++++++++++++ ..._bounds.test.js => scroll_bounds.test.mjs} | 22 +-- 5 files changed, 258 insertions(+), 72 deletions(-) create mode 100644 src/geometry.js create mode 100644 tests/geometry.test.mjs rename tests/{scroll_bounds.test.js => scroll_bounds.test.mjs} (75%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b23cbcd..992e4560 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,24 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **ES-module migration, step 5 — canvas geometry (R2).** `src/geometry.js` + (`main.js` 20,799 → 20,764): the time⇄x and string⇄y mappings every draw and + hit-test path goes through (`timeToX`/`xToTime`, `laneToY`/`yToLane`, + `strToY`/`yToStr`), the lane metrics they read (`WAVEFORM_H`, `LANE_H`, + `BEAT_H`, `LABEL_W`, `ANCHOR_LANE_H`, `HS_LANE_H`), and the scroll-bound + arithmetic. Reads `S` and the lane model; no DOM. + **No renames, unlike step 4's `LC`.** The three lane metrics are reassigned on + every resize, but their only writer — the lane-sizing arithmetic inside + `resizeCanvas()` — moves with them as `setLaneMetrics(canvasHeightPx)`. ES import + bindings are *live* and read-only, so `main.js`'s ~100 read sites keep reading + the current value verbatim and none of them can write it. A container was needed + for `LC` only because its writers (`draw()`, `onMouseMove()`) must stay behind. + `tests/scroll_bounds.test.mjs` — the last `@pure`-block slicer among the + coordinate helpers — now imports the real module. New `tests/geometry.test.mjs` + pins the mappings (inversion, clamping, gutter offset) and the live-binding + contract itself: `setLaneMetrics` updates what importers see, importers get a + `TypeError` if they assign, and `laneToY` tracks a resize rather than capturing + boot-time metrics. - **ES-module migration, step 4 — the string/lane model (R2).** `src/lanes.js` (21,036 → 20,800 lines in `main.js`): how many strings the active arrangement has (`_stringCountFor`, `lanes`, `_seedExtendedStringsFromTuning`), diff --git a/src/geometry.js b/src/geometry.js new file mode 100644 index 00000000..e5966068 --- /dev/null +++ b/src/geometry.js @@ -0,0 +1,82 @@ +/* Slopsmith Arrangement Editor — canvas geometry. + * + * The time⇄x and string⇄y mappings every draw and hit-test path goes through, + * the lane metrics they read, and the scroll-bound arithmetic. Reads `S` and + * the lane model; no DOM. + * + * The lane metrics are `export let`, not consts: `resizeCanvas()` re-derives + * them from the canvas height on every resize. ES import bindings are LIVE and + * read-only, so every importer sees the current value and none of them can + * write it — the writer is `setLaneMetrics()` here. That is why these three + * need no container (unlike lanes.js's `LC`, whose writers must stay in + * main.js inside draw() and onMouseMove()). + */ + +import { S } from './state.js'; +import { laneToStr, lanes, strToLane } from './lanes.js'; + +export const LABEL_W = 52; + +// ─── Anchor-lane constants (PR3d) ────────────────────────────────── +// Anchor lane lives below the beat bar so its time axis stays +// aligned with notes and tones. 18px gives enough room for a fret +// label plus a width-strip visualization. +export const ANCHOR_LANE_H = 18; +export const HS_LANE_H = 20; // E2: handshape (chord-shape / arpeggio) span lane + +// Lane metrics, re-derived from the canvas height on resize. See the header: +// importers read these as live bindings; only setLaneMetrics() may write them. +export let WAVEFORM_H = 70; +export let LANE_H = 44; +export let BEAT_H = 24; + +// Size the lanes to fill `canvasHeightPx`. The beat bar and waveform take a +// fixed fraction (with floors so a short canvas stays legible); the anchor and +// handshape strips are reserved; the note lanes divide what's left, never +// dropping below 30px. +export function setLaneMetrics(canvasHeightPx) { + const h = canvasHeightPx; + const minBeat = 20, minWave = 50; + BEAT_H = Math.max(minBeat, Math.floor(h * 0.05)); + WAVEFORM_H = Math.max(minWave, Math.floor(h * 0.12)); + LANE_H = Math.max(30, Math.floor((h - WAVEFORM_H - BEAT_H - ANCHOR_LANE_H - HS_LANE_H) / lanes())); +} + +// ── time ⇄ x ──────────────────────────────────────────────────────── +export function timeToX(t) { return LABEL_W + (t - S.scrollX) * S.zoom; } +export function xToTime(x) { return (x - LABEL_W) / S.zoom + S.scrollX; } + +export const EDITOR_SCROLL_TAIL_SECONDS = 2; + +export function _editorViewportDurationPure(canvasWidthPx, labelWidthPx, zoomPxPerSecond) { + const w = Number(canvasWidthPx); + const label = Number(labelWidthPx); + const zoom = Number(zoomPxPerSecond); + if (!Number.isFinite(w) || !Number.isFinite(label) || !Number.isFinite(zoom) || zoom <= 0) return 0; + return Math.max(0, (w - label) / zoom); +} + +export function _editorMaxScrollXPure(durationSeconds, viewportDurationSeconds, tailSeconds) { + const duration = Number(durationSeconds); + const view = Number(viewportDurationSeconds); + const tail = Number(tailSeconds); + if (!Number.isFinite(duration) || duration <= 0) return 0; + const v = Math.max(0, Number.isFinite(view) ? view : 0); + // The song already fits on screen → pin to the start (no scroll, no tail). + // The tail only extends the range once the content itself runs past the + // viewport, so a short/zoomed-out song can't hide its beginning. + if (duration <= v) return 0; + return Math.max(0, duration + Math.max(0, Number.isFinite(tail) ? tail : 0) - v); +} + +export function _editorClampScrollXPure(scrollX, durationSeconds, viewportDurationSeconds, tailSeconds) { + const raw = Number(scrollX); + const safe = Number.isFinite(raw) ? raw : 0; + return Math.max(0, Math.min(safe, _editorMaxScrollXPure(durationSeconds, viewportDurationSeconds, tailSeconds))); +} + +// ── string ⇄ lane ⇄ y ─────────────────────────────────────────────── +export function laneToY(l) { return WAVEFORM_H + l * LANE_H; } +export function yToLane(y) { return Math.floor((y - WAVEFORM_H) / LANE_H); } +export function strToY(s) { return laneToY(strToLane(s)); } +export function yToStr(y) { const l = Math.max(0, Math.min(lanes() - 1, yToLane(y))); return laneToStr(l); } diff --git a/src/main.js b/src/main.js index 03a49fe7..a205db5c 100644 --- a/src/main.js +++ b/src/main.js @@ -26,6 +26,23 @@ import { lanes, strToLane, } from './lanes.js'; +import { + ANCHOR_LANE_H, + BEAT_H, + EDITOR_SCROLL_TAIL_SECONDS, + HS_LANE_H, + LABEL_W, + LANE_H, + WAVEFORM_H, + _editorClampScrollXPure, + _editorViewportDurationPure, + laneToY, + setLaneMetrics, + strToY, + timeToX, + xToTime, + yToStr, +} from './geometry.js'; (function () { 'use strict'; @@ -34,10 +51,6 @@ import { // Constants // ════════════════════════════════════════════════════════════════════ -let WAVEFORM_H = 70; -let LANE_H = 44; -let BEAT_H = 24; -const LABEL_W = 52; const MIN_NOTE_W = 18; const NOTE_PAD = 3; const DPR = window.devicePixelRatio || 1; @@ -67,12 +80,6 @@ const TONE_LANE_H = 16; const _TONE_SLOT_DEFAULTS = ['Clean', 'Drive', 'Lead', 'Crunch', 'Effect']; const _TONE_SLOT_COLORS = ['#7dd3fc', '#f87171', '#fbbf24', '#a78bfa', '#34d399']; -// ─── Anchor-lane constants (PR3d) ────────────────────────────────── -// Anchor lane lives below the beat bar so its time axis stays -// aligned with notes and tones. 18px gives enough room for a fret -// label plus a width-strip visualization. -const ANCHOR_LANE_H = 18; -const HS_LANE_H = 20; // E2: handshape (chord-shape / arpeggio) span lane let canvas, ctx; let rafId = null; @@ -80,39 +87,6 @@ let rafId = null; // ════════════════════════════════════════════════════════════════════ // Coordinate mapping // ════════════════════════════════════════════════════════════════════ -function timeToX(t) { return LABEL_W + (t - S.scrollX) * S.zoom; } -function xToTime(x) { return (x - LABEL_W) / S.zoom + S.scrollX; } - -const EDITOR_SCROLL_TAIL_SECONDS = 2; - -/* @pure:scroll-bounds:start */ -function _editorViewportDurationPure(canvasWidthPx, labelWidthPx, zoomPxPerSecond) { - const w = Number(canvasWidthPx); - const label = Number(labelWidthPx); - const zoom = Number(zoomPxPerSecond); - if (!Number.isFinite(w) || !Number.isFinite(label) || !Number.isFinite(zoom) || zoom <= 0) return 0; - return Math.max(0, (w - label) / zoom); -} - -function _editorMaxScrollXPure(durationSeconds, viewportDurationSeconds, tailSeconds) { - const duration = Number(durationSeconds); - const view = Number(viewportDurationSeconds); - const tail = Number(tailSeconds); - if (!Number.isFinite(duration) || duration <= 0) return 0; - const v = Math.max(0, Number.isFinite(view) ? view : 0); - // The song already fits on screen → pin to the start (no scroll, no tail). - // The tail only extends the range once the content itself runs past the - // viewport, so a short/zoomed-out song can't hide its beginning. - if (duration <= v) return 0; - return Math.max(0, duration + Math.max(0, Number.isFinite(tail) ? tail : 0) - v); -} - -function _editorClampScrollXPure(scrollX, durationSeconds, viewportDurationSeconds, tailSeconds) { - const raw = Number(scrollX); - const safe = Number.isFinite(raw) ? raw : 0; - return Math.max(0, Math.min(safe, _editorMaxScrollXPure(durationSeconds, viewportDurationSeconds, tailSeconds))); -} -/* @pure:scroll-bounds:end */ function _editorViewportDuration() { const w = canvas ? canvas.width / DPR : 800; @@ -263,10 +237,6 @@ function _downbeatTimes() { function _barSpanForTimes(t0, t1) { return _barSpanForTimesPure(_downbeatTimes(), S.duration || Math.max(t0, t1), t0, t1); } -function laneToY(l) { return WAVEFORM_H + l * LANE_H; } -function yToLane(y) { return Math.floor((y - WAVEFORM_H) / LANE_H); } -function strToY(s) { return laneToY(strToLane(s)); } -function yToStr(y) { const l = Math.max(0, Math.min(lanes() - 1, yToLane(y))); return laneToStr(l); } function canvasH() { return _beatBarTopY() + BEAT_H; } @@ -9954,16 +9924,10 @@ function resizeCanvas() { const h = wrap.clientHeight; if (w <= 0 || h <= 0) return; - // Dynamically size lanes to fill available height - const minBeat = 20, minWave = 50; - BEAT_H = Math.max(minBeat, Math.floor(h * 0.05)); - WAVEFORM_H = Math.max(minWave, Math.floor(h * 0.12)); - // Reserve `ANCHOR_LANE_H` for the anchor strip below the beat bar - // so the lanes still fill the remaining vertical space. - // Reserve the handshape lane (HS_LANE_H) alongside the anchor lane so the - // note lanes still fill the remaining height; the max(30,…) floor keeps - // short canvases from starving them. - LANE_H = Math.max(30, Math.floor((h - WAVEFORM_H - BEAT_H - ANCHOR_LANE_H - HS_LANE_H) / lanes())); + // Dynamically size lanes to fill available height. The metrics live in + // geometry.js as live `export let` bindings — everything reads them, only + // setLaneMetrics writes them. + setLaneMetrics(h); canvas.width = w * DPR; canvas.height = h * DPR; diff --git a/tests/geometry.test.mjs b/tests/geometry.test.mjs new file mode 100644 index 00000000..1193ca20 --- /dev/null +++ b/tests/geometry.test.mjs @@ -0,0 +1,130 @@ +/* + * Tests for canvas geometry (src/geometry.js), driven through the real `S` + * and the real lane model. + * + * The load-bearing contract here is the LIVE BINDING: `WAVEFORM_H` / `LANE_H` / + * `BEAT_H` are exported `let`s that only `setLaneMetrics()` writes. Every + * importer must see the updated value without a re-import, and no importer may + * assign one. If that ever stopped holding, the ~100 read sites in main.js + * would silently draw at the boot-time lane heights after every resize. + * + * Run: node tests/geometry.test.mjs + */ +import assert from 'node:assert'; +import * as geo from '../src/geometry.js'; +import { S } from '../src/state.js'; +import { LC, lanes } from '../src/lanes.js'; +import { + EDITOR_SCROLL_TAIL_SECONDS, LABEL_W, laneToY, strToY, timeToX, xToTime, + yToLane, yToStr, +} from '../src/geometry.js'; + +let pass = 0, fail = 0; +function t(name, fn) { + try { fn(); pass++; console.log(' ok ' + name); } + catch (e) { fail++; console.error(' FAIL ' + name + ': ' + e.message); } +} + +function setArr(arr) { + S.arrangements = arr ? [arr] : []; + S.currentArr = 0; + LC.active = false; + LC.labels = null; +} +const guitar = () => ({ name: 'Lead', tuning: new Array(6).fill(0), notes: [], chords: [] }); + +// ── time ⇄ x ──────────────────────────────────────────────────────── + +t('timeToX offsets by the label gutter and scales by zoom', () => { + S.scrollX = 0; S.zoom = 100; + assert.strictEqual(timeToX(0), LABEL_W, 't=0 sits at the gutter edge'); + assert.strictEqual(timeToX(2), LABEL_W + 200); +}); + +t('xToTime inverts timeToX, including while scrolled', () => { + for (const [scrollX, zoom] of [[0, 100], [3.5, 120], [61.25, 37]]) { + S.scrollX = scrollX; S.zoom = zoom; + for (const t0 of [0, 1.5, 42, 1234.75]) { + assert.ok(Math.abs(xToTime(timeToX(t0)) - t0) < 1e-9, + `roundtrip t=${t0} @ scroll=${scrollX} zoom=${zoom}`); + } + } +}); + +// ── string ⇄ lane ⇄ y ─────────────────────────────────────────────── + +t('laneToY and yToLane invert each other at lane tops', () => { + for (let l = 0; l < 6; l++) assert.strictEqual(yToLane(laneToY(l)), l); +}); + +t('strToY / yToStr round-trip inside each lane band', () => { + setArr(guitar()); + for (let s = 0; s < 6; s++) { + assert.strictEqual(yToStr(strToY(s) + 1), s, `string ${s} mid-band`); + } + assert.ok(strToY(0) > strToY(5), 'low E draws below high e'); +}); + +t('yToStr clamps out-of-range y rather than returning a phantom string', () => { + setArr(guitar()); + assert.strictEqual(yToStr(-10_000), 5, 'above the first lane → highest string'); + assert.strictEqual(yToStr(10_000), 0, 'below the last lane → lowest string'); +}); + +// ── lane metrics: the live-binding contract ────────────────────────── + +t('setLaneMetrics is the only writer, and importers see the new values', () => { + setArr(guitar()); + const before = geo.WAVEFORM_H; + geo.setLaneMetrics(1000); + assert.notStrictEqual(geo.WAVEFORM_H, before, 'the exported binding updated'); + assert.strictEqual(geo.BEAT_H, 50, '5% of 1000'); + assert.strictEqual(geo.WAVEFORM_H, 120, '12% of 1000'); + // lanes fill what's left after the beat bar, waveform, anchor + handshape strips + const expected = Math.floor((1000 - 120 - 50 - geo.ANCHOR_LANE_H - geo.HS_LANE_H) / lanes()); + assert.strictEqual(geo.LANE_H, expected); +}); + +t('an importer cannot assign a lane metric (read-only binding)', () => { + assert.throws(() => { geo.WAVEFORM_H = 1; }, TypeError); +}); + +t('geometry reads the NEW metrics after a resize (no stale capture)', () => { + setArr(guitar()); + geo.setLaneMetrics(1000); + const tall = laneToY(1); + geo.setLaneMetrics(400); + const short = laneToY(1); + assert.notStrictEqual(tall, short, 'laneToY tracks the resized metrics'); + assert.strictEqual(short, geo.WAVEFORM_H + geo.LANE_H); +}); + +t('short canvases hit the floors instead of collapsing', () => { + setArr(guitar()); + geo.setLaneMetrics(10); + assert.strictEqual(geo.BEAT_H, 20, 'beat bar floor'); + assert.strictEqual(geo.WAVEFORM_H, 50, 'waveform floor'); + assert.strictEqual(geo.LANE_H, 30, 'lane floor — never a negative height'); +}); + +t('lane height divides by the CURRENT string count', () => { + setArr(guitar()); + geo.setLaneMetrics(1000); + const six = geo.LANE_H; + setArr({ name: 'Bass', tuning: [0, 0, 0, 0], notes: [], chords: [] }); + geo.setLaneMetrics(1000); + assert.ok(geo.LANE_H > six, '4 bass lanes are taller than 6 guitar lanes'); +}); + +// ── scroll bounds ──────────────────────────────────────────────────── + +t('the scroll tail is exported alongside the clamp that consumes it', () => { + assert.strictEqual(EDITOR_SCROLL_TAIL_SECONDS, 2); + const view = geo._editorViewportDurationPure(1000, LABEL_W, 120); + assert.strictEqual( + geo._editorClampScrollXPure(1e9, 300, view, EDITOR_SCROLL_TAIL_SECONDS), + geo._editorMaxScrollXPure(300, view, EDITOR_SCROLL_TAIL_SECONDS)); +}); + +console.log(`\n${pass} passed, ${fail} failed`); +process.exit(fail ? 1 : 0); diff --git a/tests/scroll_bounds.test.js b/tests/scroll_bounds.test.mjs similarity index 75% rename from tests/scroll_bounds.test.js rename to tests/scroll_bounds.test.mjs index 7e121a54..7b0d9134 100644 --- a/tests/scroll_bounds.test.js +++ b/tests/scroll_bounds.test.mjs @@ -1,18 +1,10 @@ -const fs = require('fs'); -const path = require('path'); -const assert = require('assert'); - -const src = fs.readFileSync(path.join(__dirname, '..', 'src', 'main.js'), 'utf8'); -const m = src.match(/\/\* @pure:scroll-bounds:start \*\/[\s\S]*?\/\* @pure:scroll-bounds:end \*\//); -if (!m) { - console.error('FAIL: @pure:scroll-bounds block not found in src/main.js'); - process.exit(1); -} - -const api = new Function( - 'console', - '"use strict";' + m[0] + '\nreturn { _editorViewportDurationPure, _editorMaxScrollXPure, _editorClampScrollXPure };' -)(console); +/* + * Scroll-bound arithmetic (src/geometry.js), driven by real imports. + * + * Run: node tests/scroll_bounds.test.mjs + */ +import assert from 'node:assert'; +import * as api from '../src/geometry.js'; function t(name, fn) { try {