From 1a386c272ddd6fd231699e1377bf0e0b3c2421f5 Mon Sep 17 00:00:00 2001 From: Byron Gamatos Date: Sun, 12 Jul 2026 12:31:56 +0200 Subject: [PATCH] refactor(highway): carve the PURE geometry primitives into highway-geometry.js (R3c) (#915) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 6 functions, 53 lines. highway.js 4,158 -> 4,105. NOT ONE CALL SITE CHANGES. project, roundRect, bnvNormalizedPoints, teachingFingerLabel, teachingDegreeLabel, chordHarmonyLabels — the shared primitives every drawing function leans on. ━━━ PURITY IS THE WHOLE POINT OF THIS SLICE ━━━ Every one of these is a pure function of its arguments. None touches hwState. None closes over the canvas context — roundRect() already took `ctx` explicitly, and the rest need nothing but numbers. project() reads only the module-level constants from #914. That matters because createHighway() is a FACTORY: a plugin can build a second highway for its own panel, so anything holding per-instance state must be PASSED hwState rather than importing it, or two panels silently share one clock and palette. These six hold no state at all, so they move VERBATIM — the module boundary is invisible to every caller. The asserts are mechanical and in the extractor: it REFUSES to move a function whose body mentions hwState, or that references `ctx` without taking it as a parameter. Purity is checked, not assumed. ━━━ WHAT IS DELIBERATELY LEFT BEHIND ━━━ The four primitives that DO need hwState — fretX, fillTextReadable, _noteState, _paintGemGlow — stay in the factory for now. They need an explicit hwState parameter threaded through 53 call sites, which is a real behavioural change and belongs in its own commit rather than smuggled in beside a provably-identical move. Separating the provable from the risky is the whole discipline of this epic. TESTS. Three harnesses brace-match these functions out of the source and run them in a sandbox; they now read static/js/highway-geometry.js. `export function x` still contains `function x`, so the extractor needed no change — only the path. VERIFIED. A/B against origin/main: 15 probes IDENTICAL, zero page errors. PERF GATE PASSES AT 1.91ms against its 12ms budget — and this is the one that could plausibly have cost something: project() runs for every visible note on every frame and is now a CROSS-MODULE call. It costs nothing measurable. That is the answer #910 was built to give. node 1045, pytest 2416, ESLint 0, Codex 0. Co-authored-by: Claude Opus 4.8 (1M context) --- static/highway.js | 69 +++------------------ static/js/highway-geometry.js | 79 +++++++++++++++++++++++++ tests/js/highway_bend_curve.test.js | 4 +- tests/js/highway_chord_harmony.test.js | 4 +- tests/js/highway_teaching_marks.test.js | 6 +- 5 files changed, 97 insertions(+), 65 deletions(-) create mode 100644 static/js/highway-geometry.js diff --git a/static/highway.js b/static/highway.js index 9d9405c..1da18d9 100644 --- a/static/highway.js +++ b/static/highway.js @@ -33,6 +33,14 @@ import { _PAUSED_FRAME_INTERVAL_MS, _SHIMMER_LUT_SIZE, } from './js/highway-constants.js'; +import { + bnvNormalizedPoints, + chordHarmonyLabels, + project, + roundRect, + teachingDegreeLabel, + teachingFingerLabel, +} from './js/highway-geometry.js'; function createHighway() { // R3c: per-instance mutable state in one object, so extracted renderer/ws @@ -362,19 +370,6 @@ function createHighway() { return _toHex(c.r + (255 - c.r) * t, c.g + (255 - c.g) * t, c.b + (255 - c.b) * t); } - // ── Projection ─────────────────────────────────────────────────────── - function project(tOffset) { - if (tOffset > VISIBLE_SECONDS || tOffset < -0.05) return null; - if (tOffset < 0) return { y: 0.82 + Math.abs(tOffset) * 0.3, scale: 1.0 }; - - const z = tOffset * (Z_MAX / VISIBLE_SECONDS); - const denom = z + Z_CAM; - if (denom < 0.01) return null; - const scale = Z_CAM / denom; - const y = 0.82 + (0.08 - 0.82) * (1.0 - scale); - return { y, scale }; - } - // ── Anchor / Fret mapping ──────────────────────────────────────────── // Zoom approach: fret 0 at the left edge, fret N at the right (entire canvas mirrored when lefty). // The "zoom level" determines how many frets are visible. @@ -437,50 +432,16 @@ function createHighway() { /** Map a bend curve [{t, v}] (§6.2.1) to [{x, v}] with x normalized to * 0..1 across the curve's time span (0 when the span is degenerate). * Pure — drives the 2D bend-shape glyph. */ - function bnvNormalizedPoints(bnv, sus) { - if (!Array.isArray(bnv) || bnv.length === 0) return []; - // Map each point's time over the NOTE's span [0, sus] so it sits at its - // real fraction of the note (a bend that completes before the note ends - // draws short of the glyph's right edge). Fall back to the curve's own - // t-range only when the note has no usable sustain. - if (Number.isFinite(sus) && sus > 0) { - return bnv.map(p => ({ x: Math.min(Math.max(p.t / sus, 0), 1), v: p.v })); - } - const t0 = bnv[0].t; - const span = bnv[bnv.length - 1].t - t0; - return bnv.map(p => ({ x: span > 0 ? (p.t - t0) / span : 0, v: p.v })); - } - /** Teaching mark (§6.2.2): fret-hand-finger label for a note's `fg`. * '' when unset/out of range; 0 → 'T' (thumb), 1..4 → '1'..'4'. Pure. */ - function teachingFingerLabel(fg) { - if (!Number.isInteger(fg) || fg < 0 || fg > 4) return ''; - return fg === 0 ? 'T' : String(fg); - } - /** Teaching mark (§6.2.2): scale-degree label for a note's `sd` (chromatic * 0..11 above the active key tonic). '' when unset/out of range. Pure. */ - function teachingDegreeLabel(sd) { - if (!Number.isInteger(sd) || sd < 0 || sd > 11) return ''; - return String(sd); - } - /** Harmony annotations (§6.3.1 / §6.6): display labels for a chord's * harmonic function (the instance `fn.rn` Roman numeral) and its template * `voicing`, `caged` shape, and `guideTones`. Returns '' for each when * absent or malformed; `caged`/`guideTones` come back pre-formatted * ("CAGED: E" / "gt 4,10"). Pure; node-tested and shared by both highways. * Display/teaching only — MUST NEVER feed a grader (honesty rule). */ - function chordHarmonyLabels(fn, voicing, caged, guideTones) { - const rn = (fn && typeof fn.rn === 'string') ? fn.rn.trim() : ''; - const vc = (typeof voicing === 'string') ? voicing.trim() : ''; - const cg = (typeof caged === 'string' && /^[CAGED]$/.test(caged.trim())) - ? 'CAGED: ' + caged.trim() : ''; - const gt = Array.isArray(guideTones) - ? guideTones.filter(n => Number.isInteger(n) && n >= 0 && n <= 11) : []; - return { rn, voicing: vc, caged: cg, guideTones: gt.length ? 'gt ' + gt.join(',') : '' }; - } - /** Teaching mark (§6.2.2): bucket drawn notes by their strum-group key `ch`. * Returns the groups (in first-seen order) for each ch value >= 0 that has * at least two members — a lone note is not a strum gesture. Pure; drives @@ -2586,20 +2547,6 @@ function createHighway() { } } - function roundRect(ctx, x, y, w, h, r) { - ctx.beginPath(); - ctx.moveTo(x + r, y); - ctx.lineTo(x + w - r, y); - ctx.quadraticCurveTo(x + w, y, x + w, y + r); - ctx.lineTo(x + w, y + h - r); - ctx.quadraticCurveTo(x + w, y + h, x + w - r, y + h); - ctx.lineTo(x + r, y + h); - ctx.quadraticCurveTo(x, y + h, x, y + h - r); - ctx.lineTo(x, y + r); - ctx.quadraticCurveTo(x, y, x + r, y); - ctx.closePath(); - } - function bsearch(arr, time) { let lo = 0, hi = arr.length; while (lo < hi) { diff --git a/static/js/highway-geometry.js b/static/js/highway-geometry.js new file mode 100644 index 0000000..cf1ea34 --- /dev/null +++ b/static/js/highway-geometry.js @@ -0,0 +1,79 @@ +// highway.js's PURE geometry + label primitives. +// +// Every function here is a pure function of its arguments. None of them touches hwState, and +// none closes over the canvas context — roundRect() already took `ctx` explicitly, and the +// rest need nothing but numbers. project() reads only the module-level constants from +// ./highway-constants.js. +// +// THAT PURITY IS WHY THIS SLICE IS SAFE, and why it is the one to do first. createHighway() is +// a FACTORY — a plugin can build a second highway for its own panel — so anything holding +// per-instance state (hwState) must be passed it as an argument rather than importing it, or +// two panels silently share one clock and palette. These six hold no state at all, so they +// move VERBATIM: not one call site changes. +// +// The primitives that DO need hwState (fretX, fillTextReadable, _noteState, _paintGemGlow) +// are deliberately left behind. They need an explicit hwState parameter threaded through 53 +// call sites, which is a real change and belongs in its own commit, not smuggled in beside a +// provably-identical move. +import { VISIBLE_SECONDS, Z_CAM, Z_MAX } from './highway-constants.js'; + +// ── Projection ─────────────────────────────────────────────────────── +export function project(tOffset) { + if (tOffset > VISIBLE_SECONDS || tOffset < -0.05) return null; + if (tOffset < 0) return { y: 0.82 + Math.abs(tOffset) * 0.3, scale: 1.0 }; + + const z = tOffset * (Z_MAX / VISIBLE_SECONDS); + const denom = z + Z_CAM; + if (denom < 0.01) return null; + const scale = Z_CAM / denom; + const y = 0.82 + (0.08 - 0.82) * (1.0 - scale); + return { y, scale }; +} + +export function bnvNormalizedPoints(bnv, sus) { + if (!Array.isArray(bnv) || bnv.length === 0) return []; + // Map each point's time over the NOTE's span [0, sus] so it sits at its + // real fraction of the note (a bend that completes before the note ends + // draws short of the glyph's right edge). Fall back to the curve's own + // t-range only when the note has no usable sustain. + if (Number.isFinite(sus) && sus > 0) { + return bnv.map(p => ({ x: Math.min(Math.max(p.t / sus, 0), 1), v: p.v })); + } + const t0 = bnv[0].t; + const span = bnv[bnv.length - 1].t - t0; + return bnv.map(p => ({ x: span > 0 ? (p.t - t0) / span : 0, v: p.v })); +} + +export function teachingFingerLabel(fg) { + if (!Number.isInteger(fg) || fg < 0 || fg > 4) return ''; + return fg === 0 ? 'T' : String(fg); +} + +export function teachingDegreeLabel(sd) { + if (!Number.isInteger(sd) || sd < 0 || sd > 11) return ''; + return String(sd); +} + +export function chordHarmonyLabels(fn, voicing, caged, guideTones) { + const rn = (fn && typeof fn.rn === 'string') ? fn.rn.trim() : ''; + const vc = (typeof voicing === 'string') ? voicing.trim() : ''; + const cg = (typeof caged === 'string' && /^[CAGED]$/.test(caged.trim())) + ? 'CAGED: ' + caged.trim() : ''; + const gt = Array.isArray(guideTones) + ? guideTones.filter(n => Number.isInteger(n) && n >= 0 && n <= 11) : []; + return { rn, voicing: vc, caged: cg, guideTones: gt.length ? 'gt ' + gt.join(',') : '' }; +} + +export function roundRect(ctx, x, y, w, h, r) { + ctx.beginPath(); + ctx.moveTo(x + r, y); + ctx.lineTo(x + w - r, y); + ctx.quadraticCurveTo(x + w, y, x + w, y + r); + ctx.lineTo(x + w, y + h - r); + ctx.quadraticCurveTo(x + w, y + h, x + w - r, y + h); + ctx.lineTo(x + r, y + h); + ctx.quadraticCurveTo(x, y + h, x, y + h - r); + ctx.lineTo(x, y + r); + ctx.quadraticCurveTo(x, y, x + r, y); + ctx.closePath(); +} diff --git a/tests/js/highway_bend_curve.test.js b/tests/js/highway_bend_curve.test.js index 5b051e9..4441a66 100644 --- a/tests/js/highway_bend_curve.test.js +++ b/tests/js/highway_bend_curve.test.js @@ -25,7 +25,9 @@ function loadFn(file, name) { return new Function('"use strict";' + extractFn(src, name) + `\nreturn ${name};`)(); } -const bnvNormalizedPoints = loadFn('static/highway.js', 'bnvNormalizedPoints'); +// R3c: the PURE geometry/label primitives were carved out of highway.js into +// static/js/highway-geometry.js. Same bodies, byte-for-byte — only the file moved. +const bnvNormalizedPoints = loadFn('static/js/highway-geometry.js', 'bnvNormalizedPoints'); const bnvSampleAt = loadFn('plugins/highway_3d/screen.js', 'bnvSampleAt'); // ── bnvNormalizedPoints (2D) ───────────────────────────────────────────────── diff --git a/tests/js/highway_chord_harmony.test.js b/tests/js/highway_chord_harmony.test.js index 80f1048..85b5580 100644 --- a/tests/js/highway_chord_harmony.test.js +++ b/tests/js/highway_chord_harmony.test.js @@ -25,7 +25,9 @@ function loadFn(file, name) { return new Function('"use strict";' + extractFn(src, name) + `\nreturn ${name};`)(); } -const labels2D = loadFn('static/highway.js', 'chordHarmonyLabels'); +// R3c: the PURE geometry/label primitives were carved out of highway.js into +// static/js/highway-geometry.js. Same bodies, byte-for-byte — only the file moved. +const labels2D = loadFn('static/js/highway-geometry.js', 'chordHarmonyLabels'); const labels3D = loadFn('plugins/highway_3d/screen.js', 'chordHarmonyLabels'); for (const [name, fn] of [['2D', labels2D], ['3D', labels3D]]) { diff --git a/tests/js/highway_teaching_marks.test.js b/tests/js/highway_teaching_marks.test.js index 6e729ef..aab4d12 100644 --- a/tests/js/highway_teaching_marks.test.js +++ b/tests/js/highway_teaching_marks.test.js @@ -26,8 +26,10 @@ function loadFn(file, name) { return new Function('"use strict";' + extractFn(src, name) + `\nreturn ${name};`)(); } -const fingerLabel2D = loadFn('static/highway.js', 'teachingFingerLabel'); -const degreeLabel2D = loadFn('static/highway.js', 'teachingDegreeLabel'); +// R3c: the PURE geometry/label primitives were carved out of highway.js into +// static/js/highway-geometry.js. Same bodies, byte-for-byte — only the file moved. +const fingerLabel2D = loadFn('static/js/highway-geometry.js', 'teachingFingerLabel'); +const degreeLabel2D = loadFn('static/js/highway-geometry.js', 'teachingDegreeLabel'); const fingerLabel3D = loadFn('plugins/highway_3d/screen.js', 'teachingFingerLabel'); const degreeLabel3D = loadFn('plugins/highway_3d/screen.js', 'teachingDegreeLabel'); const strumGroupBuckets = loadFn('static/highway.js', 'strumGroupBuckets');