Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .agents/skills/test-musicxml/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,17 @@ Use this skill when adding or updating a `vexml` MusicXML rendering test case, e
5. Update the implementation in `src/` to fill the rendering gap.
- Prefer a minimal, root-cause fix.
- Keep changes consistent with the existing renderer and test patterns.
- **Collisions must go through the collision pipeline.** When a fix involves moving an
element so it clears another (an above-stave annotation over a notehead/tie, a chord
diagram next to another, two annotations stacking) — or detecting that content is clipped
("no-man's land") — route it through `CollisionDetector` (`src/collision.ts`):
register the fixed thing as an obstacle (`add`/`addRect`), resolve the movable thing
(`liftClear` to lift above-stave text, `pushRightOf` to space diagrams), then draw at the
resolved position and register it. Do **not** add new bespoke magic-offset clearance
logic. Not everything is movable — noteheads, stems, and ties are obstacles, not nudge
targets. Fixed structural placement (page margins, part spacing, bracket offsets,
widget-internal geometry) is exempt — it is deterministic, not clash resolution. See
`docs/collision-audit.md` for what is and isn't in scope.

6. Run the test command again:

Expand Down
1 change: 1 addition & 0 deletions cli/fix.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export async function fix({ check }: { check: boolean }) {
'.',
]);
await run('bunx', ['tsc', '--noEmit']);
console.log('Compilation successful.');
// Validate all .musicxml fixtures against the MusicXML XSD.
await run('./xmllint/validate.sh', []);
}
70 changes: 70 additions & 0 deletions docs/collision-audit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
# Nudge / collision audit

> **Caveat — not a maintained list.** This is a point-in-time snapshot taken while the
> collision pipeline (`src/geometry.ts`, `src/quadtree.ts`, `src/collision.ts`) was introduced.
> Line numbers and entries **will drift** as the code changes — treat it as a map, not a
> source of truth. Re-audit (`rg`-style sweep for offset/clearance code) if you need current
> data. It exists so the historical "where did we nudge things?" knowledge isn't lost.

The renderer (`src/draw.ts`) draws to a `<canvas>` in immediate mode. Historically, every
time two elements could clash, a one-off fix nudged one of them with its own magic constant —
a whack-a-mole. The pipeline replaces the recurring class of those with one mechanism. This
doc records what existed, split by whether the pipeline absorbs it.

## How the pipeline works (the one true path)

Compute a movable element's natural `Rect` → ask the `CollisionDetector` to resolve it against
everything already placed (`liftClear` for above-stave text, `pushRightOf` for diagrams) →
draw at the resolved position → register the placed `Rect` so later elements avoid it. Fixed
elements (noteheads, stems, ties) are registered as obstacles but never moved. The detector
also reports out-of-bounds content (`escaping`) — the "no-man's land" where renders get
clipped. A `QuadTree` (with an out-of-bounds overflow bucket) is the spatial index.

**Rule for future work:** any new "move this so it clears that" (or "is this being clipped?")
goes through `CollisionDetector`. Do not add new bespoke magic-offset clearance code.

## A. Recurring runtime clash resolution — the whack-a-mole

| Site | What moves | Clears | Status |
|---|---|---|---|
| `draw.ts::noteTop` | (obstacle calc) notehead ∪ stem tip ∪ above-articulation | — | feeds `noteRect` |
| `draw.ts::drawHarmony` | chord symbol lifts | notehead, stem-down tie apex, other text | **migrated** → `liftClear` |
| `draw.ts::drawWords` | words text lifts | notehead, tie, other text | **migrated** → `liftClear` |
| `draw.ts` chord-diagram block | fret box pushes right | adjacent diagram across a barline | **migrated** → `pushRightOf` |
| `draw.ts::drawTempo` | metronome mark lifts (`shiftY`) | high first note | **deferred** (scaled mark + reserved headroom; Phase 2) |
| chord diagram vertical | fixed `CHORD_DIAGRAM_GAP`, does **not** clear notes | (latent clip bug) | **deferred** (Phase 3) |

The migration was behavior-preserving: the full screenshot suite was byte-identical except a
1–2px improvement on `harmony_grace` (the symbol now also clears the grace note's stem tip).
The new capabilities the pipeline enables but no fixture yet exercises: a wide symbol clearing
a *neighbouring* tall note under its text, and stacking multiple annotations in one column.

## B. Out-of-bounds / clip hazards ("no-man's land") — covered by `escaping`

| Site | Hazard |
|---|---|
| `draw.ts` `pageTop`/`pageBottom` growth | crop grown ad hoc so content isn't clipped |
| `constants.ts::LEDGER_HEADROOM`, `draw.ts` `topSlack` | scratch slack; "bump if an extreme tessitura ever clips" |
| `draw.ts` system-overflow two-pass (`topOverflow`) | reserves vertical space so stacked systems don't collide |

`CollisionDetector.escaping(scratchViewport)` now flags vertical clips per system and warns
(see `warnEscapes` in `draw.ts`). The existing `pageTop`/`pageBottom` crop logic is unchanged —
`escaping` is the generic home for clip handling going forward, the two-pass overflow could
fold into it later.

## C. Fixed structural placement — left alone (NOT collisions)

These are deterministic engraving placement, not runtime clash resolution. Routing them through
the detector would add indirection for no benefit. Documented so future work doesn't "migrate"
them by mistake:

- Page margins (`PAGE_MARGIN_*`), `INTER/INTRA_PART_SPACING`, `BASE_VOICE_WIDTH`.
- Lead-glyph width estimates (`LEAD_BARLINE/CLEF/KEY/TIME`) — generous reservations so notes
never collide with clef/key/time glyphs.
- `LABEL_GAP` / part-label centering (`+1.5`), `BRACKET_X_SHIFT` (bracket sits just outside the
system line, `draw.ts` bracket blocks).
- Tab note-area centering, tab grace alignment, `GRACE_SPACING` pre-reservation (`layout.ts`).
- Chord-diagram-internal geometry (`barShiftX`, `bridgeWidth`, title/dot/barre positions in
`chord-diagram.ts`).
- Tab fret / harmonic-bracket centering (`notes.ts` `boldFret`/harmonic layout).
- Analytic slur control-point lift (`spanners.ts` `cpYFor`), wrapped tie/slur split.
56 changes: 49 additions & 7 deletions site/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,23 @@ export default function App() {
const width =
config.layout?.type === 'standard' ? (config.layout.width ?? 900) : 900;
const notationFont = config.fonts?.notation?.family ?? 'Bravura';
const reset = (
key: 'noteSpacing' | 'softmaxFactor' | 'systemSpacing' | 'maxSystemFill',
) => setConfig(({ [key]: _, ...rest }) => rest);
const resetKeys = [
'noteSpacing',
'softmaxFactor',
'systemSpacing',
'maxSystemFill',
] as const;
const reset = (key: (typeof resetKeys)[number]) =>
setConfig(({ [key]: _, ...rest }) => rest);
const canReset = resetKeys.some((k) => config[k] !== undefined);
const resetAll = () =>
setConfig((c) => {
const next = { ...c };
for (const k of resetKeys) {
delete next[k];
}
return next;
});

// `config` stays live so the sliders/reset respond instantly; `renderConfig` lags
// behind it by the debounce so dragging a slider re-renders once it settles, not on
Expand All @@ -109,13 +123,20 @@ export default function App() {
skipConfigDebounce.current = false;
return;
}
// If the last render was fast, apply config changes immediately; only debounce
// once renders get slow enough to lag the sliders.
if (renderMs != null && renderMs <= 50) {
setRenderConfig(config);
setDebouncing(false);
return;
}
setDebouncing(true);
const t = setTimeout(() => {
setRenderConfig(config);
setDebouncing(false);
}, 500);
return () => clearTimeout(t);
}, [config]);
}, [config, renderMs]);

useEffect(() => {
const canvas = canvasRef.current;
Expand Down Expand Up @@ -216,6 +237,13 @@ export default function App() {
setDebouncing(false);
return;
}
// If the last render was fast, just render on every keystroke; only debounce
// once renders get slow enough to lag typing.
if (renderMs != null && renderMs <= 50) {
setDebouncing(false);
setInput(value);
return;
}
// Spinner shows while we wait out the typing, then render the settled text.
setDebouncing(true);
debounceRef.current = setTimeout(() => {
Expand Down Expand Up @@ -369,9 +397,23 @@ export default function App() {
</button>

<div className="flex flex-col gap-4 rounded-lg border border-zinc-200 bg-zinc-50 p-3">
<span className="text-xs font-semibold uppercase tracking-wide text-zinc-500">
Config
</span>
<div className="flex items-center justify-between">
<span className="text-xs font-semibold uppercase tracking-wide text-zinc-500">
Config
</span>
<button
type="button"
onClick={resetAll}
disabled={!canReset}
className="text-xs font-medium text-zinc-400 hover:text-zinc-600 disabled:cursor-default disabled:text-zinc-300 disabled:hover:text-zinc-300"
>
Reset all
</button>
</div>
<p className="text-xs text-zinc-400">
With only a single system, some controls (e.g. system spacing
and max system fill) won't have a visible effect.
</p>
<label
htmlFor="darkMode"
className="flex items-center gap-2 text-xs font-medium text-zinc-500"
Expand Down
84 changes: 84 additions & 0 deletions src/collision.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import { expect, test } from 'bun:test';
import { CollisionDetector } from './collision';
import { Rect } from './geometry';

function detector(): CollisionDetector {
return new CollisionDetector(new Rect(-1000, -1000, 4000, 4000));
}

test('query reports the kind and minimum-translation of each hit', () => {
const d = detector();
d.addRect(new Rect(5, 0, 10, 10), 'note');
const hits = d.query(new Rect(0, 0, 10, 10));
expect(hits).toHaveLength(1);
expect(hits[0]?.other.kind).toBe('note');
// Overlap is 5 wide, 10 tall → separate along x, leftward (candidate is left of note).
expect(hits[0]?.mtv.axis).toBe('x');
expect(hits[0]?.mtv.dx).toBe(-5);
});

test('liftClear raises a rect to sit `gap` above the highest obstacle in its column', () => {
const d = detector();
d.addRect(new Rect(5, 90, 10, 30), 'note'); // top at y=90
d.addRect(new Rect(5, 70, 10, 5), 'tie'); // higher: top at y=70 (kind-agnostic)
const natural = new Rect(0, 85, 20, 15); // bottom at y=100, overlaps both
const placed = d.liftClear(natural, 8);
expect(placed.bottom).toBe(62); // 70 (highest top) - 8 gap
expect(placed.x).toBe(0); // x unchanged — text only lifts vertically
});

test('liftClear never lowers a rect that is already clear', () => {
const d = detector();
d.addRect(new Rect(5, 200, 10, 30), 'note'); // well below the annotation
const natural = new Rect(0, 85, 20, 15); // bottom at y=100
expect(d.liftClear(natural, 8)).toBe(natural); // returned unchanged
});

test('liftClear stacks successive annotations above one another', () => {
const d = detector();
d.addRect(new Rect(5, 90, 10, 30), 'note');
const a = d.liftClear(new Rect(0, 85, 20, 15), 8); // bottom 82
d.add({ rect: a, kind: 'annotation' });
const b = d.liftClear(new Rect(0, 85, 20, 15), 8); // must clear `a`, not the note
expect(b.bottom).toBe(a.y - 8); // a.y is a's top
});

test('liftClear with a kind filter ignores other kinds (text clears notes, not diagrams)', () => {
const d = detector();
d.addRect(new Rect(5, 90, 10, 30), 'note'); // top at y=90
d.addRect(new Rect(0, 70, 20, 5), 'diagram'); // higher, but a diagram
const natural = new Rect(0, 85, 20, 15); // bottom at y=100
// Clearing only note/tie/annotation: lifts to clear the note (90), not the diagram (70).
const placed = d.liftClear(natural, 8, ['note', 'tie', 'annotation']);
expect(placed.bottom).toBe(82); // 90 - 8, the diagram was ignored
});

test('pushRightOf reproduces the chord-diagram running-cursor (gap enforced even when close)', () => {
const d = detector();
d.addRect(new Rect(0, 0, 88, 84), 'diagram'); // right edge at x=88
// Overlapping neighbour → pushed to 88 + 6.
expect(d.pushRightOf(new Rect(50, 0, 88, 84), 'diagram', 6).x).toBe(94);
// Close but not overlapping (90 > 88) → gap still enforced to 94.
expect(d.pushRightOf(new Rect(90, 0, 88, 84), 'diagram', 6).x).toBe(94);
// Far enough away → left where it is.
expect(d.pushRightOf(new Rect(200, 0, 88, 84), 'diagram', 6).x).toBe(200);
});

test('pushRightOf ignores diagrams in a different vertical band', () => {
const d = detector();
d.addRect(new Rect(0, 0, 88, 84), 'diagram');
// Same x-range but a different stave (y far below) → no horizontal push.
expect(d.pushRightOf(new Rect(50, 500, 88, 84), 'diagram', 6).x).toBe(50);
});

test('escaping flags rects that cross the viewport edges', () => {
const d = detector();
d.addRect(new Rect(10, 10, 5, 5), 'note'); // inside
d.addRect(new Rect(10, -20, 5, 5), 'annotation'); // off the top
d.addRect(new Rect(95, 10, 20, 5), 'diagram'); // off the right
const escaped = d.escaping(new Rect(0, 0, 100, 100));
const byKind = new Map(escaped.map((e) => [e.item.kind, e.edges]));
expect(byKind.size).toBe(2);
expect(byKind.get('annotation')).toEqual(['top']);
expect(byKind.get('diagram')).toEqual(['right']);
});
Loading
Loading