Skip to content

fix: stop the arrow shaft overlapping the arrowhead#62

Merged
dancj merged 5 commits into
stagingfrom
fix/shaft-head-overlap
Jul 7, 2026
Merged

fix: stop the arrow shaft overlapping the arrowhead#62
dancj merged 5 commits into
stagingfrom
fix/shaft-head-overlap

Conversation

@dancj

@dancj dancj commented Jul 6, 2026

Copy link
Copy Markdown
Owner

Summary

Arrows with heads used to draw the shaft all the way into the arrowhead's tip, so the incoming line visibly crossed the head's V (plus rough.js wobble overshooting past it). The shaft now stops cleanly at the head's base — the tip still touches the target edge, and the junction reads as one drawn stroke.

The two routes get there differently, because they fail differently:

  • Curve route: endpoints are pulled back along their arrival/departure tangents by the head's base depth (headSize * cos(π/7), the same spread constant arrowHeadPath uses). Obstacle-avoidance detection runs against the inset endpoints, so routing and rendering agree on the actual shaft. Tiny arrows scale both insets down so the endpoints can never cross.
  • Elbow route: insetting endpoints would move the corners themselves (shifting the mid-rail and, for a short final leg, inverting it past the corner entirely), so the elbow is built from the true endpoints and only its first/last leg is trimmed, clamped to that leg's own length.

Arrowheads stay anchored at the true socket points, and the draw-on animation ordering is unchanged — heads still draw after the (now slightly shorter) line.

Plan: docs/plans/2026-07-06-001-fix-shaft-head-overlap-plan.md. 19 new tests (geometry insets/trims/clamps and render-level assertions per head mode and route).

Validation

vitest run — 169/169 green (15 files), tsc --noEmit and eslint clean, prettier check clean. Render behavior asserted via deterministic seeded rough.js d strings under jsdom; no live browser pass was run this session (dev-server use is disabled by user preference).

Residual Review Findings

From the multi-agent code review (correctness, testing, maintainability, project-standards, adversarial + validators; verdict: ready with fixes — formatting fix applied on-branch):

  • P2 test/scroll-arrow.test.tshead: 'start' alone is never exercised at the render level (only 'end'/'both'); filed as test: head:'start' alone is never exercised at the render level #61
  • P3 advisory src/geometry.ts:249 — elbow + head: 'both' on an arrow smaller than both trims collapses the shaft to a zero-length path (heads still render at their sockets); accepted degradation for arrows smaller than their own arrowheads
  • P3 advisory src/geometry.ts:419 — the chord-length clamp also throttles insets that run perpendicular to the chord (same-side-socket U-loops with short chords), partially re-admitting the old overlap in that narrow configuration

Related

Related: #59, #60


Compound Engineering
Claude Code

dancj added 5 commits July 6, 2026 16:52
insetEndpoints pulls curve endpoints back along their tangents by an
arrowhead's base depth (headBaseInset = size * cos(spread)), scaling both
insets down on tiny chords so endpoints never cross. buildElbowPath gains
per-leg trims computed after the corners, so the mid-rail stays put and a
trim can never invert a short final leg. Groundwork for #59.
When a head is drawn, the curve route feeds inset endpoints to routing and
path build (so avoid detection matches the rendered shaft) and the elbow
route trims its first/last leg. Heads stay anchored at the true socket
points, so tips still touch the target edge. Draw-on ordering unchanged.
Math.min for the clamp scale, hoisted hasStartHead/hasEndHead so the
shaft inset and head drawing can't desync, and one shared pathDs test
helper. Behavior unchanged; suite stays green.
@dancj
dancj merged commit ab92960 into staging Jul 7, 2026
2 checks passed
@dancj
dancj deleted the fix/shaft-head-overlap branch July 7, 2026 03:02
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