Skip to content

fix(cli): connector dash gate honours pathLength - #3954

Open
miga-heygen wants to merge 2 commits into
fix/connector-detached-dash-hiddenfrom
fix/connector-dash-gate-pathlength
Open

miga-heygen wants to merge 2 commits into
fix/connector-detached-dash-hiddenfrom
fix/connector-dash-gate-pathlength

Conversation

@miga-heygen

@miga-heygen miga-heygen commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #3912 (stacks on fix/connector-detached-dash-hidden; merge that first).

Summary

  • shaftDashHidden now honours the SVG pathLength attribute: the dash window is judged in the author's units (path.pathLength.baseVal, a plain number that is 0 when unset) instead of always against getTotalLength(). dashLength / dashArrayLengths remain the single owners of parsing and units.
  • The total > period shortcut is gone so the ≤10% rule applies uniformly — a window that spans a whole period without enclosing a whole dash (2 97 on a 100-long path at offset 0.5 paints 3%) is hidden like any other sub-10% stroke; patterns that do enclose a whole dash are still caught by the in-loop check.
  • A present pathLength that resolves to 0 (pathLength="0", or an unparseable value such as abc) zeroes Chromium's dash scale factor and the stroke paints solid, so the gate returns painted instead of falling back to the real length and judging the window inside a gap (which suppressed a real connector_detached). A negative pathLength is ignored by the browser and keeps the existing fallback.
  • The dash-offset wrap adds the period only to negative remainders; ((offset % period) + period) % period is off by one ulp for fractional offsets (0.9 % 2 → 0.8999…), which flipped the exact 10% boundary in pathLength units.

Why

<path pathLength="1"> with stroke-dasharray: 1; stroke-dashoffset: 1 is the canonical CSS draw-on idiom and renders zero ink, but the gate compared the dashes (author units) against the real user-unit length, so the period looked tiny next to the path and connector_detached still fired on a shaft that had not started drawing.

Test plan

  • layout-audit.browser.test.ts dash table: +7 rows — pathLength=1 with 1/1 (hidden), 1/0.9 (10% boundary, hidden), 1/0.5 (painted), 0.1 0.9 round dot (painted); 100 at offset -150 (painted; only the wrap gets it right); 10% / 10% (hidden; percentages resolve against the 1740×830 viewport, not as bare numbers); 2 97 at 0.5 (hidden, no full-period shortcut); pathLength=0 with 100/100 (painted: a zero pathLength renders solid). installConnectorGeometry mirrors the DOM's pathLength for happy-dom, parsing it as an SVG number (anything else, including Infinity and hex, reads as 0).
  • Each new row was mutation-checked: ignoring pathLength, dropping the negative wrap, parsing % as bare numbers, restoring the full-period shortcut, restoring the double-modulo wrap, or dropping the zero-pathLength guard — every mutation kills at least one row.
  • Suite: 143 pass under vitest run src/commands/layout-audit.browser.test.ts (node runner, sibling packages built first as in CI).
  • Real Chromium (rasterised, 6px stroke on a 360-unit path): pathLength=1 1/1 → 0 inked px; 1/0.5 → 1080 (50%); 0.1 0.9 round → 248; 1/0.9 → 216 (exactly 10%); 2 97 at 0.5 with pathLength=100 → 60 (2.8%); solid control 2160; pathLength=0 and pathLength=abc with 100/100 → 100% ink (solid), where the previous gate reported hidden. path.pathLength is an SVGAnimatedNumber whose baseVal is the number itself (1 when set, 0 when unset).
  • tsc --noEmit -p packages/cli, oxlint, oxfmt --check clean on the changed files.

🤖 Generated with Claude Code

miga-heygen and others added 2 commits September 15, 2026 02:31
`shaftDashHidden` compared the stroke's dash window against
`getTotalLength()`, but a `pathLength` attribute rescales every distance
along the path, so the CSS draw-on idiom (`<path pathLength="1">` with
`stroke-dasharray: 1; stroke-dashoffset: 1`) — which renders zero ink —
still fired `connector_detached`. The window is now judged in the author's
units: `pathLength` when set (`SVGAnimatedNumber.baseVal`, 0 when unset),
the real length otherwise. `dashLength`/`dashArrayLengths` stay the only
owners of parsing and units.

Two adjacent changes to the gate's arithmetic:

- The `total > period` shortcut is removed so the 10% rule applies
  uniformly. A window that spans a whole period without enclosing a whole
  dash (`2 97` on a 100-long path at offset 0.5 paints 3%) is hidden, like
  any other stroke painting under 10%; any pattern that does enclose a
  whole dash is still caught by the in-loop check.
- The dash-offset wrap adds the period only to negative remainders. The
  previous `((offset % period) + period) % period` is off by one ulp for
  fractional offsets (`0.9 % 2` → 0.8999…), which flipped the exact 10%
  boundary once dashes are expressed in `pathLength` units.

Tests: rows for `pathLength="1"` (`1/1` hidden, `1/0.9` on the boundary
hidden, `1/0.5` painted, `0.1 0.9` round dot painted), a negative offset
that only the wrap gets right (`100` at -150), a percentage row (`10%`
resolves against the viewport, not as a bare number), and the `2 97`
full-period row. `installConnectorGeometry` mirrors the DOM's `pathLength`
for happy-dom, which has no SVGGeometryElement.

Co-Authored-By: Miguel Ángel <miguel.sierra@heygen.com>
A present `pathLength` that resolves to 0 (`"0"` or an unparseable value)
zeroes the browser's dash scale factor, so the stroke paints solid. The gate
saw `baseVal` 0, fell back to the real length, judged the window inside a gap
and dropped the shaft, suppressing a real `connector_detached` finding.

Return painted for that case. Add a `pathLength=0` row to the dash table and
make the happy-dom `pathLength` mirror parse like the DOM (finite SVG number
only; `Infinity`, hex, `1.` read as 0).

Co-Authored-By: Miguel Ángel <miguel.sierra@heygen.com>
@miga-heygen

Copy link
Copy Markdown
Contributor Author

Addressed in e61ec8d.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant