rick/LEDMatrix
1
0
Fork 0
mirror of https://github.com/ChuckBuilds/LEDMatrix.git synced 2026-08-23 11:28:14 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(composer): scale strokes, anchor lines, snapshot state changes

Browse Source 
Three review findings in the composer's JavaScript, all confirmed against the
code.

Stroke widths did not scale with SCALE
--------------------------------------
_drawElement scales all geometry by `s`, but left ctx.lineWidth in canvas
pixels, so at SCALE>1 every outline rendered thinner than one LED pixel and
the preview stopped matching the panel it is previewing. Fixed for rectangle,
ellipse, arc, rounded_rectangle, line, divider and progress_bar. Ellipse and
arc also inset their radii by half the scaled width -- a stroke straddles its
path, so without the inset the outline spills outside the element's bounds.
The gauge branch already did this; the rest now match it.

Selection handles and the grid stay in canvas pixels deliberately: they are
editor chrome, not LED geometry, and live in other functions.

`line` ignored anchors
----------------------
_drawElement resolves ax/ay for every element, but the line branch drew raw
el.x0/el.y0/el.x1/el.y1. Setting xAnchor or yAnchor moved every other element
type and left lines where they were. getBoundingBox had the same omission, so
even once a line moved its hit box would not have. Both now translate by
(ax - el.x0, ay - el.y0); ax resolves from el.x0 for a line, so that is
exactly the anchor offset.

Four state mutations skipped _snapshot
--------------------------------------
_snapshot serialises metadata and currentPreset and is the only caller of
_debouncedAutosave. onBgColorChange, setCustomSize, changePreset and
applyPresetLabel each changed exactly those values without calling it, so the
background colour and the canvas size were lost on reload and could not be
undone. Same defect already fixed in onColorChange.

The review named three; applyPresetLabel has it too -- it is the branch that
handles sizes absent from DISPLAY_PRESETS.

Snapshotting is on the user-driven path only. _applyState and loadTemplate
drive these with {silent: true} while restoring, and snapshotting there would
push restore steps onto the undo stack and re-autosave the state just loaded.

Tests
-----
No JS runner here, so test_composer_js_contracts.py asserts on the parse tree
via tree-sitter: both files parse, no bare `ctx.lineWidth = 1` inside
_drawElement, the line branch and its bounding box carry the anchor offset,
each of the five mutations snapshots, and the two preset paths keep their
!opts.silent guard ahead of the snapshot.

9 of its 11 checks fail against the previous JS. Full suite 3978 passed, the
one failure being test_install_lowmem (pre-existing, awaiting #492).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
This commit is contained in:
ChuckBuilds
2026-08-22 13:52:48 -04:00
co-authored by Claude Opus 5
parent 1a0864e5d4
commit acc55ef119
3 changed files with 184 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files
web_interface/static/v3/js/composer/composer-canvas.js
+28 -15
View File
@@ -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;