diff --git a/test/test_composer_js_contracts.py b/test/test_composer_js_contracts.py new file mode 100644 index 00000000..7e612ee6 --- /dev/null +++ b/test/test_composer_js_contracts.py @@ -0,0 +1,137 @@ +"""Structural checks on the composer's JavaScript. + +There is no JS test runner in this repo, but three defects here are structural +enough to assert on the parse tree, and each was a real bug: + +- Stroke widths inside _drawElement were left in canvas pixels while the + geometry around them scaled by SCALE, so at SCALE>1 every outline rendered + thinner than one LED pixel and the preview stopped matching the panel. +- The `line` branch drew raw el.x0/el.y0/el.x1/el.y1, ignoring the anchor that + every other element type honours, so setting xAnchor moved everything except + lines -- and getBoundingBox had the same omission, leaving the hit box behind. +- Four methods mutated exactly what _snapshot() serialises (metadata, + currentPreset) without calling it. _snapshot is the only caller of + _debouncedAutosave, so those changes were lost on reload and could not be + undone. +""" +import re +from pathlib import Path + +import pytest + +tree_sitter = pytest.importorskip("tree_sitter") +tree_sitter_javascript = pytest.importorskip("tree_sitter_javascript") + +JS_DIR = Path(__file__).resolve().parent.parent / "web_interface/static/v3/js/composer" +CANVAS = JS_DIR / "composer-canvas.js" +APP = JS_DIR / "composer-app.js" + + +def _function_source(path: Path, name: str) -> str: + """Return the source of a top-level function declaration by name.""" + src = path.read_bytes() + lang = tree_sitter.Language(tree_sitter_javascript.language()) + tree = tree_sitter.Parser(lang).parse(src) + found = [] + + def walk(node): + if node.type == "function_declaration": + ident = node.child_by_field_name("name") + if ident is not None and src[ident.start_byte:ident.end_byte].decode() == name: + found.append(src[node.start_byte:node.end_byte].decode()) + for c in node.children: + walk(c) + + walk(tree.root_node) + assert found, f"{name} not found in {path.name}" + return found[0] + + +def _method_source(path: Path, name: str) -> str: + """Return the source of a top-level object method by name.""" + src = path.read_bytes() + lang = tree_sitter.Language(tree_sitter_javascript.language()) + tree = tree_sitter.Parser(lang).parse(src) + found = [] + + def walk(node): + if node.type == "method_definition": + ident = node.child_by_field_name("name") + if ident is not None and src[ident.start_byte:ident.end_byte].decode() == name: + found.append(src[node.start_byte:node.end_byte].decode()) + for c in node.children: + walk(c) + + walk(tree.root_node) + assert found, f"{name} not found in {path.name}" + return found[0] + + +def test_both_files_parse(): + lang = tree_sitter.Language(tree_sitter_javascript.language()) + parser = tree_sitter.Parser(lang) + for path in (CANVAS, APP): + tree = parser.parse(path.read_bytes()) + errors = [] + + def walk(node): + if node.type == "ERROR" or node.is_missing: + errors.append(node.start_point[0] + 1) + for c in node.children: + walk(c) + + walk(tree.root_node) + assert not errors, f"{path.name} has parse errors at lines {errors}" + + +def test_element_strokes_scale_with_scale(): + """No bare `ctx.lineWidth = 1` inside _drawElement. + + Selection handles and the grid are drawn in canvas pixels deliberately and + live in other functions, so this is scoped to the element drawing routine. + """ + body = _function_source(CANVAS, "_drawElement") + offenders = re.findall(r"ctx\.lineWidth\s*=\s*1\s*;", body) + assert not offenders, f"{len(offenders)} unscaled stroke width(s) in _drawElement" + + +def test_line_branch_applies_the_anchor_offset(): + text = CANVAS.read_text() + line_branch = text[text.index("case 'line': {"):] + line_branch = line_branch[:line_branch.index("case 'divider'")] + assert "ax - el.x0" in line_branch and "ay - el.y0" in line_branch, \ + "line drawing ignores xAnchor/yAnchor" + assert "el.x0 * s" not in line_branch, "line still drawn from unanchored endpoints" + + +def test_line_bounding_box_applies_the_anchor_offset(): + text = CANVAS.read_text() + i = text.index("case 'line':", text.index("getBoundingBox")) + box = text[i:i + 400] + assert "ax - el.x0" in box, "line bounding box ignores the anchor" + + +@pytest.mark.parametrize("method", [ + "onBgColorChange", # mutates metadata.bgColor + "setCustomSize", # mutates currentPreset / MATRIX_W / MATRIX_H + "changePreset", # mutates currentPreset / MATRIX_W / MATRIX_H + "applyPresetLabel", # same, for sizes not in DISPLAY_PRESETS + "onColorChange", # the one that was already fixed — keeps it fixed +]) +def test_state_mutations_take_a_snapshot(method): + body = _method_source(APP, method) + assert "_snapshot()" in body, \ + f"{method} changes snapshotted state without calling _snapshot()" + assert "isDirty = true" in body, f"{method} does not mark the design dirty" + + +@pytest.mark.parametrize("method", ["changePreset", "applyPresetLabel"]) +def test_restore_path_stays_snapshot_free(method): + """_applyState and loadTemplate call these with {silent: true} while + restoring; snapshotting there would push restore steps onto the undo stack + and re-autosave the state just loaded.""" + body = _method_source(APP, method) + assert "opts.silent" in body, f"{method} lost its silent guard" + snap = body.index("_snapshot()") + guard = body.index("!opts.silent") + assert guard < snap, f"{method} snapshots outside the !opts.silent guard" diff --git a/web_interface/static/v3/js/composer/composer-app.js b/web_interface/static/v3/js/composer/composer-app.js index 92b9c114..ff49ea41 100644 --- a/web_interface/static/v3/js/composer/composer-app.js +++ b/web_interface/static/v3/js/composer/composer-app.js @@ -480,7 +480,11 @@ function composerApp() { this.currentPreset = `${w}×${h}`; this.SCALE = w <= 64 ? 6 : w <= 128 ? 4 : 2; this._applyScale(); - if (!opts.silent) this.render(); + if (!opts.silent) { + this.isDirty = true; + this._snapshot(); + this.render(); + } }, changePreset(presetLabel, opts = {}) { @@ -496,7 +500,11 @@ function composerApp() { canvas.style.width = (this.MATRIX_W * this.SCALE) + 'px'; canvas.style.height = (this.MATRIX_H * this.SCALE) + 'px'; } - if (!opts.silent) this.render(); + if (!opts.silent) { + this.isDirty = true; + this._snapshot(); + this.render(); + } }, // ── Rendering ───────────────────────────────────────────────────── @@ -626,6 +634,10 @@ function composerApp() { this.currentPreset = `${w}×${h}`; this.SCALE = w <= 64 ? 6 : w <= 128 ? 4 : 2; this._applyScale(); + // currentPreset is part of the snapshot; a custom size set here was + // otherwise dropped on reload and could not be undone. + this.isDirty = true; + this._snapshot(); this._setStatus(`Canvas set to ${w}×${h}`, 'info'); }, zoomFit() { @@ -645,6 +657,11 @@ function composerApp() { g: parseInt(hex.slice(3, 5), 16), b: parseInt(hex.slice(5, 7), 16), }; + // _snapshot serialises metadata and is the only caller of + // _debouncedAutosave, so without this the background colour was lost on + // reload and could not be undone. Same defect as onColorChange. + this.isDirty = true; + this._snapshot(); this.render(); }, diff --git a/web_interface/static/v3/js/composer/composer-canvas.js b/web_interface/static/v3/js/composer/composer-canvas.js index 5019b81b..b07c26b8 100644 --- a/web_interface/static/v3/js/composer/composer-canvas.js +++ b/web_interface/static/v3/js/composer/composer-canvas.js @@ -208,12 +208,16 @@ window.ComposerCanvas = (() => { return { x: ax, y: ay, w: el.width, h: el.height }; case 'pixel': return { x: ax, y: ay, w: 1, h: 1 }; - case 'line': + case 'line': { + // Same anchor offset the draw path applies, or the hit box sits where + // the line used to be. + const dx = ax - el.x0, dy = ay - el.y0; return { - x: Math.min(el.x0, el.x1), y: Math.min(el.y0, el.y1), + x: Math.min(el.x0, el.x1) + dx, y: Math.min(el.y0, el.y1) + dy, w: Math.max(1, Math.abs(el.x1 - el.x0)), h: Math.max(1, Math.abs(el.y1 - el.y0)), }; + } case 'divider': return el.orientation === 'horizontal' ? { x: 0, y: ay, w: matrixW, h: 1 } @@ -384,7 +388,9 @@ window.ComposerCanvas = (() => { } if (el.hasOutline) { ctx.strokeStyle = `rgb(${el.outR},${el.outG},${el.outB})`; - ctx.lineWidth = 1; + // 1 LED pixel, not 1 canvas pixel: at SCALE>1 an unscaled stroke + // renders thinner than the geometry it outlines. + ctx.lineWidth = s; ctx.strokeRect(rx, ry, rw, rh); } break; @@ -393,8 +399,11 @@ window.ComposerCanvas = (() => { case 'ellipse': { const cx = (ax + el.width / 2) * s; const cy = (ay + el.height / 2) * s; - const rx = (el.width / 2) * s; - const ry = (el.height / 2) * s; + const lwPx = s; // 1 LED pixel + // Inset by half the stroke, which straddles the path, so the outline + // stays within the element's bounds. + const rx = Math.max(0, (el.width / 2) * s - (el.hasOutline ? lwPx / 2 : 0)); + const ry = Math.max(0, (el.height / 2) * s - (el.hasOutline ? lwPx / 2 : 0)); ctx.beginPath(); ctx.ellipse(cx, cy, rx, ry, 0, 0, Math.PI * 2); if (el.hasFill) { @@ -403,7 +412,7 @@ window.ComposerCanvas = (() => { } if (el.hasOutline) { ctx.strokeStyle = `rgb(${el.outR},${el.outG},${el.outB})`; - ctx.lineWidth = 1; + ctx.lineWidth = lwPx; ctx.stroke(); } break; @@ -412,15 +421,16 @@ window.ComposerCanvas = (() => { case 'arc': { const cx = (ax + el.width / 2) * s; const cy = (ay + el.height / 2) * s; - const rx = (el.width / 2) * s; - const ry = (el.height / 2) * s; + const lwPx = Math.max(1, el.lineWidth || 2) * s; + const rx = Math.max(0, (el.width / 2) * s - lwPx / 2); + const ry = Math.max(0, (el.height / 2) * s - lwPx / 2); // PIL: 0°=right, clockwise. Canvas: same with anticlockwise=false const startRad = (el.startAngle ?? 0) * Math.PI / 180; const endRad = (el.endAngle ?? 270) * Math.PI / 180; ctx.beginPath(); ctx.ellipse(cx, cy, rx, ry, 0, startRad, endRad, false); ctx.strokeStyle = `rgb(${el.r},${el.g},${el.b})`; - ctx.lineWidth = Math.max(1, el.lineWidth || 2); + ctx.lineWidth = lwPx; ctx.stroke(); break; } @@ -443,18 +453,21 @@ window.ComposerCanvas = (() => { } if (el.hasOutline) { ctx.strokeStyle = `rgb(${el.outR},${el.outG},${el.outB})`; - ctx.lineWidth = 1; + ctx.lineWidth = s; ctx.stroke(); } break; } case 'line': { + // ax/ay resolve from el.x0 for a line, so this is the anchor offset. + // Without it a line stayed put while every other type moved. + const dx = ax - el.x0, dy = ay - el.y0; ctx.strokeStyle = `rgb(${el.r},${el.g},${el.b})`; - ctx.lineWidth = Math.max(1, el.lineWidth || 1); + ctx.lineWidth = Math.max(1, el.lineWidth || 1) * s; ctx.beginPath(); - ctx.moveTo(el.x0 * s, el.y0 * s); - ctx.lineTo(el.x1 * s, el.y1 * s); + ctx.moveTo((el.x0 + dx) * s, (el.y0 + dy) * s); + ctx.lineTo((el.x1 + dx) * s, (el.y1 + dy) * s); ctx.stroke(); break; } @@ -462,7 +475,7 @@ window.ComposerCanvas = (() => { case 'divider': { const isH = (el.orientation || 'horizontal') === 'horizontal'; ctx.strokeStyle = `rgb(${el.r},${el.g},${el.b})`; - ctx.lineWidth = 1; + ctx.lineWidth = s; ctx.beginPath(); if (isH) { ctx.moveTo(0, ay * s + 0.5); @@ -614,7 +627,7 @@ window.ComposerCanvas = (() => { } if (el.hasOutline) { ctx.strokeStyle = `rgb(${el.outR ?? 100},${el.outG ?? 100},${el.outB ?? 100})`; - ctx.lineWidth = 1; + ctx.lineWidth = s; ctx.strokeRect(rx, ry, bw * s, bh * s); } break;