diff --git a/crates/epiphany-engrave/DECISIONS.md b/crates/epiphany-engrave/DECISIONS.md index 35758a4..e7a8ab4 100644 --- a/crates/epiphany-engrave/DECISIONS.md +++ b/crates/epiphany-engrave/DECISIONS.md @@ -772,10 +772,7 @@ under a staff space of margin). rather than from the region's *declared* inter-staff band. Harmless while both come from the same constructor; it would silently diverge from the metric the day per-region gaps become customizable. -- The cumulative shift **cascades** correctly for 3+ staves (staff *i* carries the - sum of every gap correction above it), but is unexercised: `valid_score_rich`'s - three staves are three separate single-staff regions, so each lands in its own - system. +The 3+-staff cascade was the last of these to be closed; see below. **The solve.** Per system, per staff, the real content y-extent is collected (glyphs, strokes, curves — ledgers and slurs included, not just noteheads). The @@ -800,3 +797,25 @@ render golden (the visible before/after: slice 1 tight, slice 2 separated). only expands, never pulls staves together — the fixed pitch is generous by default, so this is rarely wanted); per-staff spring *stretch* to fill spare system height (the inter-system justification carries the fill for now). + +**The cascade, and why it grows faster than the raw corrections.** A pair's gap +is measured against the upper staff's **already shifted** bottom, so staff *i*'s +shift is the sum of every correction above it plus its own. A consequence worth +stating because it is counter-intuitive: an *increment* `shift(i+1) - shift(i)` +generally EXCEEDS the lower pair's own raw correction, because the upper staff's +descent has itself eaten into that pair's gap and must be undone. (The first +version of the cascade test asserted the opposite — that a gently-pressed lower +pair's increment would be the small one — and failed against a correct solve.) +Locked by `inter_staff_shifts_cascade_down_three_staves` over the new +`three_staff_close_content` fixture: three staves in ONE region (so all three +land in one system), with **asymmetric** pressure — the upper pair collides hard +(C1 against C7), the lower pair only gently. Sizing each pair independently — the +plausible wrong implementation — measures the lower pair against the middle +staff's ORIGINAL position and hands the bottom staff only its small raw +correction, dragging it back up through the middle staff. **Verified by +mutation:** with the cascade removed the bottom staff's shift collapses from +34.68 to 4.56 (against the middle staff's 15.06) and both the shift ordering and +the staff-line-gap assertions fail — while `two_staff_close_content` still +passes, which is exactly why the three-staff fixture was needed. The fixture also +pins curve attribution against a THREE-band choice (the slur must still find the +bottom staff, not merely the nearer of two) and carries its own render golden. diff --git a/crates/epiphany-engrave/src/lib.rs b/crates/epiphany-engrave/src/lib.rs index 1802b81..907a528 100644 --- a/crates/epiphany-engrave/src/lib.rs +++ b/crates/epiphany-engrave/src/lib.rs @@ -2383,6 +2383,91 @@ mod tests { ); } + /// Three staves in one system, with deliberately ASYMMETRIC pressure: the + /// upper pair collides hard (C1 against C7), the lower pair only gently. A + /// staff's shift must accumulate the corrections of every pair above it, + /// because each pair's gap is measured against the upper staff's *already + /// shifted* position. So the bottom staff's shift is large — it must clear + /// the middle staff where the middle staff now sits, not where it started. + /// + /// That is exactly what a solve sizing each pair independently gets wrong: + /// it would measure the lower pair against the middle staff's ORIGINAL + /// position, hand the bottom staff only its own small correction, and leave + /// it above the middle staff's new position — closing their staff-line gap + /// to well under the fixed pitch. Verified by mutation: with the cascade + /// removed this test fails on both `s2 > s1` and the staff-line gap. + #[test] + fn inter_staff_shifts_cascade_down_three_staves() { + use epiphany_layout_ir::{to_constrained, to_logical}; + let report = Engraver::default().solve( + &to_constrained(&to_logical( + &epiphany_testkit::fixtures::three_staff_close_content(1), + )), + &SolverConfig::default(), + ); + let sys = &report.layout.pages[0].systems[0]; + assert_eq!(sys.staves.len(), 3, "three staves in ONE system"); + + // Staff records are top-first. Recover each staff's solved shift: under + // fixed stacking staff `i` sits `i * SYSTEM_STAFF_PITCH` below the top, + // so whatever separation exceeds that is the shift the solve added. + let top_y = sys.staves[0].bounding_box.origin.y.0; + let shift = |i: usize| (top_y - sys.staves[i].bounding_box.origin.y.0) - 12.0 * i as f32; + let (s0, s1, s2) = (shift(0), shift(1), shift(2)); + assert!(s0.abs() < 1e-3, "the top staff anchors the system: {s0}"); + assert!(s1 > 0.0, "the hard-colliding upper pair separates: {s1}"); + // The cascade. The bottom staff strictly outruns the middle staff: its + // correction is measured against the middle staff's SHIFTED bottom, so + // it inherits that descent and adds its own clearance on top. Sizing the + // lower pair independently yields only its own small raw correction — + // less than `s1` for this fixture, which the strict inequality rejects. + assert!( + s2 > s1, + "the bottom staff inherits the shift above it: s1={s1} s2={s2}" + ); + + // Every adjacent pair ends up clear of the fixed pitch, and nothing + // collides — the invariant a broken cascade destroys for the lower pair. + for i in 0..2 { + let (upper, lower) = (&sys.staves[i].bounding_box, &sys.staves[i + 1].bounding_box); + let gap = upper.origin.y.0 - (lower.origin.y.0 + lower.size.height.0); + assert!( + gap > 12.0, + "staves {i}/{} separate: staff-line gap {gap}", + i + 1 + ); + } + assert_eq!( + report.metric_vector.collision_penalty.0, 0.0, + "the cascaded staves collide nowhere" + ); + + // The slur belongs to the BOTTOM staff, the one carrying the largest + // cascaded shift — so curve attribution has to survive a THREE-band + // choice, not just pick the nearer of two. Misattributing it to the + // middle staff would leave it ~`s2 - s1` above its own notes. + assert_eq!(report.layout.curves.len(), 1, "the fixture draws one slur"); + let p0y = report.layout.curves[0].p0.y.0; + let band_gap = |b: &epiphany_layout_ir::Rect| { + let (lo, hi) = (b.origin.y.0, b.origin.y.0 + b.size.height.0); + if p0y < lo { + lo - p0y + } else if p0y > hi { + p0y - hi + } else { + 0.0 + } + }; + let gaps: Vec = (0..3) + .map(|i| band_gap(&sys.staves[i].bounding_box)) + .collect(); + assert!( + gaps[2] < gaps[1] && gaps[2] < gaps[0], + "the slur rides its OWN (bottom) staff: {gaps:?}" + ); + assert!(gaps[2] < 2.0, "and sits close against it: {}", gaps[2]); + } + #[test] fn vertical_justification_fills_non_final_pages() { use epiphany_layout_ir::{Margins, Size2D, StaffSpace}; diff --git a/crates/epiphany-render-svg/tests/acceptance.rs b/crates/epiphany-render-svg/tests/acceptance.rs index 8b387b3..fedcf6b 100644 --- a/crates/epiphany-render-svg/tests/acceptance.rs +++ b/crates/epiphany-render-svg/tests/acceptance.rs @@ -49,6 +49,10 @@ fn fixtures() -> Vec<(&'static str, Score)> { "two_staff_close_content", epiphany_testkit::fixtures::two_staff_close_content(0x000A_11CE), ), + ( + "three_staff_close_content", + epiphany_testkit::fixtures::three_staff_close_content(0x000A_11CE), + ), ] } diff --git a/crates/epiphany-render-svg/tests/golden/three_staff_close_content.engrave.snapshot.txt b/crates/epiphany-render-svg/tests/golden/three_staff_close_content.engrave.snapshot.txt new file mode 100644 index 0000000..f064552 --- /dev/null +++ b/crates/epiphany-render-svg/tests/golden/three_staff_close_content.engrave.snapshot.txt @@ -0,0 +1,15 @@ +fixture=three_staff_close_content solver=engrave +glyph_count=18 +path_count=18 +fallback_rect_count=0 +stroke_count=77 +curve_count=1 +provenance_count=96 +layer_count=1 +hard_constraint_count=27 +xml_well_formed=true +view_box=[5.5 -75.204575 17.379084 69.703995] +class_counts: + barline=3 + clef=3 + notehead=12 diff --git a/crates/epiphany-render-svg/tests/golden/three_staff_close_content.engrave.svg b/crates/epiphany-render-svg/tests/golden/three_staff_close_content.engrave.svg new file mode 100644 index 0000000..e38201c --- /dev/null +++ b/crates/epiphany-render-svg/tests/golden/three_staff_close_content.engrave.svg @@ -0,0 +1,104 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/crates/epiphany-render-svg/tests/golden/three_staff_close_content.stub.snapshot.txt b/crates/epiphany-render-svg/tests/golden/three_staff_close_content.stub.snapshot.txt new file mode 100644 index 0000000..ac4183e --- /dev/null +++ b/crates/epiphany-render-svg/tests/golden/three_staff_close_content.stub.snapshot.txt @@ -0,0 +1,15 @@ +fixture=three_staff_close_content solver=stub +glyph_count=18 +path_count=18 +fallback_rect_count=0 +stroke_count=77 +curve_count=1 +provenance_count=96 +layer_count=1 +hard_constraint_count=27 +xml_well_formed=true +view_box=[-3.065 -27.632 16.876999 35.024002] +class_counts: + barline=3 + clef=3 + notehead=12 diff --git a/crates/epiphany-render-svg/tests/golden/three_staff_close_content.stub.svg b/crates/epiphany-render-svg/tests/golden/three_staff_close_content.stub.svg new file mode 100644 index 0000000..6f9f516 --- /dev/null +++ b/crates/epiphany-render-svg/tests/golden/three_staff_close_content.stub.svg @@ -0,0 +1,104 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/crates/epiphany-testkit/src/fixtures.rs b/crates/epiphany-testkit/src/fixtures.rs index f05858e..89feb36 100644 --- a/crates/epiphany-testkit/src/fixtures.rs +++ b/crates/epiphany-testkit/src/fixtures.rs @@ -211,6 +211,128 @@ pub fn two_staff_close_content(seed: u64) -> Score { score } +/// A one-measure, THREE-staff metric score with **asymmetric** inter-staff +/// pressure, built to exercise the vertical solve's cumulative shift cascade. +/// +/// The upper pair collides hard (staff 1 plunges to C1 while staff 2 towers to +/// C7); the lower pair collides gently (staff 2's C3 against staff 3's C6 and +/// the slur arcing over it). So the first correction is several times the +/// second — which is the point. A solve that computed each pair's correction +/// *independently* would shift staff 3 by only the small amount while staff 2 +/// took the large one, pulling staff 3 back **through** staff 2 and closing +/// their staff-line gap to well under the fixed pitch. Only a solve whose shift +/// accumulates down the stack keeps every pair separated. Invariant-clean. +pub fn three_staff_close_content(seed: u64) -> Score { + let mut rng = SplitMix64::new(seed ^ 0x0003_57AF_F123); + let replica = + ReplicaId::from_entropy(rng.next_u64().to_le_bytes()).unwrap_or(ReplicaId(0x3ADF)); + let mut idc = IdentityContext::new(replica); + + let staves: Vec = (0..3).map(|_| idc.mint()).collect(); + let instrument: InstrumentId = idc.mint(); + let region_id: RegionId = idc.mint(); + + let measure = |id: MeasureId| Measure { + id, + start: TimeAnchor::Region { + id: region_id, + edge: RegionEdge::Start, + offset: AnchorOffset::Zero, + }, + time_signature: None, + explicit_number: Some(1), + number_visibility: Default::default(), + }; + + // Staff 1 dips to low ledgers; staff 2 spans C7 down to C3 (so it presses + // upward on staff 1 and downward on staff 3); staff 3 climbs to C6. + let octaves: [[i8; 4]; 3] = [[4, 3, 2, 1], [7, 3, 7, 3], [6, 5, 6, 5]]; + let mut arena = EventArena::new(); + let mut instances: Vec = Vec::new(); + let mut low_events: Vec = Vec::new(); + for (staff_index, staff_id) in staves.iter().enumerate() { + let instance_id: StaffInstanceId = idc.mint(); + let voice_id: VoiceId = idc.mint(); + let mut voice = Voice::user(voice_id); + for (i, oct) in octaves[staff_index].into_iter().enumerate() { + let (eid, pid): (EventId, PitchId) = (idc.mint(), idc.mint()); + arena + .insert(c_at(eid, voice_id, pid, i as i64, oct)) + .unwrap(); + voice.events.push(eid); + if staff_index == 2 { + low_events.push(eid); + } + } + let mut instance = StaffInstance::new(instance_id, *staff_id); + instance.voices.push(voice); + instance.measures.push(measure(idc.mint())); + instances.push(instance); + } + + // A slur over the bottom staff's high notes, arcing further up — the extra + // upward extent that turns the lower pair's near-miss into real pressure. + let mut cross_cutting = CrossCuttingRegistry::default(); + cross_cutting.slurs.push(Slur { + id: idc.mint::(), + start_event: low_events[0], + end_event: low_events[2], + kind: SlurKind::Legato, + curvature_override: None, + style: SpanStyle::default(), + }); + + let region = epiphany_core::Region { + id: region_id, + time_model: RegionTimeModel::Metric(MetricTimeModel::default()), + content: RegionContent::StaffBased(StaffBasedContent { + staff_instances: instances, + ..Default::default() + }), + time_extent: TimeExtent { + start: TimeAnchor::WallClock { + time: WallClockTime(0), + }, + end: TimeAnchor::WallClock { + time: WallClockTime(10_000_000), + }, + }, + staff_extent: StaffExtent { + staves: staves.clone(), + }, + local_tempo_map: None, + permits_spanning_slurs: false, + }; + + let staff = |id: StaffId, name: &str| Staff { + id, + name: String::from(name), + abbreviation: None, + instrument, + default_staff_lines: StaffLineConfiguration::default(), + group: None, + default_clef: epiphany_core::Clef::treble(), + }; + let mut score = Score::empty(idc.clone()); + score.identity = idc; + score.instruments = vec![epiphany_core::Instrument::new( + instrument, + String::from("Organ"), + )]; + score.staves = vec![ + staff(staves[0], "Upper"), + staff(staves[1], "Middle"), + staff(staves[2], "Lower"), + ]; + score.events = arena; + score.cross_cutting = cross_cutting; + score.canvas = Canvas { + regions: vec![region], + ..Default::default() + }; + score +} + /// A 10-measure, single-staff, single-voice metric score with 40 quarter notes /// (four per measure), plus a tie, a spanner, a marker, and a chord symbol. The /// QUICKSTART layout hand-off case. Invariant-clean (the returned graph passes @@ -491,6 +613,21 @@ mod tests { assert_eq!(s.cross_cutting.slurs.len(), 1, "a slur over the high notes"); } + #[test] + fn three_staff_close_content_is_invariant_clean_and_asymmetric() { + let s = three_staff_close_content(1); + let v = check_invariants(&s); + assert!(v.is_empty(), "three-staff fixture has violations: {v:?}"); + assert_eq!(s.staves.len(), 3, "three staves"); + assert_eq!( + s.canvas.regions[0].staff_instances().len(), + 3, + "three staff instances in ONE region — so all three land in one system" + ); + assert_eq!(s.events.len(), 12, "four notes per staff"); + assert_eq!(s.cross_cutting.slurs.len(), 1, "a slur over the low staff"); + } + #[test] fn repeat_fixture_is_invariant_clean_and_carries_three_repeats() { let s = ten_measure_with_repeats(1);