fix(composer): center divider strokes and clamp gauge radii in preview canvas

Two outstanding CodeRabbit outside-diff findings on this PR:

- The divider branch applied its 0.5px centering offset after scaling
  (`ay * s + 0.5`) instead of before it (`(ay + 0.5) * s`), so at SCALE>1
  the stroke bled into the preceding LED row/column instead of straddling
  its own.
- A small imported gauge with a wide lineWidth (e.g. width=1, height=1,
  lineWidth=3) produced a negative radius, which ctx.ellipse() throws
  IndexSizeError on, aborting render() for every element still to be
  drawn. Radii are now clamped to zero.

Verified both against current code (neither was fixed by later commits
on this branch) and mutation-checked: reverting either fix fails the new
test.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Claude
2026-09-11 20:33:06 +00:00
parent be7a7b7baf
commit f12d11334a
2 changed files with 40 additions and 6 deletions
+30
View File
@@ -185,3 +185,33 @@ def test_align_clears_the_anchor_and_moves_line_endpoints():
"element resolves to the wrong edge"
assert "el.x0" in body and "el.y0" in body, \
"_alignElement no longer moves line endpoints"
def test_divider_stroke_is_centered_in_led_pixels():
"""A 0.5 canvas-pixel offset (correct only at SCALE=1) was applied after
scaling instead of before it, so at SCALE>1 the stroke bled into the
preceding LED row/column instead of straddling its own."""
body = _function_source(CANVAS, "_drawElement")
divider_branch = body[body.index("case 'divider'"):]
divider_branch = divider_branch[:divider_branch.index("case 'pips'")]
assert "ay * s + 0.5" not in divider_branch, \
"divider still offsets by 0.5 canvas pixels after scaling"
assert "ax * s + 0.5" not in divider_branch, \
"divider still offsets by 0.5 canvas pixels after scaling"
assert "(ay + 0.5) * s" in divider_branch and "(ax + 0.5) * s" in divider_branch, \
"divider stroke is not centered within its LED pixel"
def test_gauge_radii_are_clamped_to_zero():
"""An imported design can carry a small gauge with a wide lineWidth --
width=1, height=1, lineWidth=3 sends a negative radius into
ctx.ellipse(), which throws IndexSizeError and aborts render() for every
element still to be drawn, not just the gauge."""
body = _function_source(CANVAS, "_drawElement")
gauge_branch = body[body.index("case 'gauge'"):]
assert re.search(r"Math\.max\(0,\s*rx\s*-\s*lwPx\s*/\s*2\)", gauge_branch), \
"gauge x-radius is not clamped to zero"
assert re.search(r"Math\.max\(0,\s*ry\s*-\s*lwPx\s*/\s*2\)", gauge_branch), \
"gauge y-radius is not clamped to zero"
assert "rx - lwPx / 2" not in re.sub(r"Math\.max\(0,\s*rx\s*-\s*lwPx\s*/\s*2\)", "", gauge_branch), \
"an unclamped gauge radius is still passed to ctx.ellipse()"
@@ -486,11 +486,11 @@ window.ComposerCanvas = (() => {
ctx.lineWidth = s;
ctx.beginPath();
if (isH) {
ctx.moveTo(0, ay * s + 0.5);
ctx.lineTo(_canvas.width, ay * s + 0.5);
ctx.moveTo(0, (ay + 0.5) * s);
ctx.lineTo(_canvas.width, (ay + 0.5) * s);
} else {
ctx.moveTo(ax * s + 0.5, 0);
ctx.lineTo(ax * s + 0.5, _canvas.height);
ctx.moveTo((ax + 0.5) * s, 0);
ctx.lineTo((ax + 0.5) * s, _canvas.height);
}
ctx.stroke();
break;
@@ -560,13 +560,17 @@ window.ComposerCanvas = (() => {
? Math.max(0, Math.min(100, parseFloat(pvGauge) || 0)) / 100
: Math.max(0, Math.min(100, el.previewPct ?? 65)) / 100;
const fillSweep = totalSweep * pct;
// A small imported gauge with a wide lineWidth would otherwise send
// a negative radius into ctx.ellipse(), which throws IndexSizeError
// and aborts the whole render() call.
const arx = Math.max(0, rx - lwPx / 2), ary = Math.max(0, ry - lwPx / 2);
// biome-ignore lint/correctness/useQwikValidLexicalScope: not Qwik -- this is an Alpine.js component, and the rule is about Qwik's $() serialization boundary, which does not exist here.
const toRad = deg => (deg - 90) * Math.PI / 180; // canvas 0=top, PIL 0=right → offset -90
// Track arc
if (el.hasTrack !== false) {
ctx.beginPath();
ctx.ellipse(cx, cy, rx - lwPx / 2, ry - lwPx / 2, 0, toRad(startDeg), toRad(startDeg + totalSweep), false);
ctx.ellipse(cx, cy, arx, ary, 0, toRad(startDeg), toRad(startDeg + totalSweep), false);
ctx.strokeStyle = `rgb(${el.trackR ?? 40},${el.trackG ?? 40},${el.trackB ?? 40})`;
ctx.lineWidth = lwPx;
ctx.stroke();
@@ -574,7 +578,7 @@ window.ComposerCanvas = (() => {
// Fill arc
if (pct > 0) {
ctx.beginPath();
ctx.ellipse(cx, cy, rx - lwPx / 2, ry - lwPx / 2, 0, toRad(startDeg), toRad(startDeg + fillSweep), false);
ctx.ellipse(cx, cy, arx, ary, 0, toRad(startDeg), toRad(startDeg + fillSweep), false);
ctx.strokeStyle = `rgb(${el.r},${el.g},${el.b})`;
ctx.lineWidth = lwPx;
ctx.stroke();