diff --git a/crates/epiphany-engrave/DECISIONS.md b/crates/epiphany-engrave/DECISIONS.md index 951882a..d5cadab 100644 --- a/crates/epiphany-engrave/DECISIONS.md +++ b/crates/epiphany-engrave/DECISIONS.md @@ -817,10 +817,12 @@ is untouched. Locked by `inter_staff_solve_separates_colliding_staves` (the two-staff pressure fixture's staff-line gap opens past the fixed pitch while a single-staff score keeps one staff per system) and the `two_staff_close_content` render golden (the visible before/after: slice 1 tight, slice 2 separated). -**Deferred:** compressing an OVER-wide fixed gap toward preferred (the solve +~~**Deferred:** compressing an OVER-wide fixed gap toward preferred (the solve 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). +default, so this is rarely wanted).~~ **DONE** — see "Two-sided renegotiation" +below; "rarely wanted" was wrong, and the metric said so. +**Still deferred:** 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 @@ -943,3 +945,73 @@ wanted") is therefore promoted from *rarely wanted* to **measurably wrong**: any un-pressured multi-staff system now reports honest sprawl until the solve can pull staves together. Named here rather than fixed in the same breath — compression is a layout change (golden churn, `ENGRAVER_VERSION` move), not a measurement one. + +## Two-sided renegotiation, and the cascade defect it uncovered (2026-07-09) + +`ENGRAVER_VERSION` 11 → 12. The inter-staff solve now **closes a slack pair as +well as opening a crowded one**, realizing the `InterStaffGap` band's declared +height exactly. The constrained stage's fixed `SYSTEM_STAFF_PITCH` is demoted +from a floor to an initial arrangement the solve fully renegotiates. + +**Why:** the expand-only solve made `vertical_density_penalty` report honest +sprawl on every relaxed multi-staff system (0.739 on `two_staff_wrapping_pressure`). +The axis is symmetric — a gap wider than preferred is sprawl exactly as a +narrower one is crowding — so either the layout or the number was wrong. It was +both, in different ways. + +**What the band's height MEANS.** It is an **ink clearance**: the separation +between the two staves' outermost content (ledgers, stems, slurs), which is the +unit `req:qmc:vertical` measures. The old `preferred = 2.0` was a placeholder +reconciled with nothing — neither the 8.0 staff-box gap the fixed pitch of 12 +produces, nor the ~6.4 ink clearance that stacking leaves for plain content. +Realizing 2.0 would have crushed a relaxed system to a staff pitch of ~7.6. +`preferred` is now **5.0** (a staff height plus a space) and `min` **2.0** — plain +ledgered content settles near a pitch of 10.6, close to convention, while +ledgered or slurred content pushes the staves apart on its own. The user chose +this value; `4.0` ("one staff height") was rejected once its true consequence — +pitch 9.57, not the 11.04 an arithmetic slip of mine had projected — was measured +rather than inferred. + +**The cascade was wrong, and had been since v11.** Making the solve two-sided +made `three_staff_close_content` report `vertical_density_penalty` **1.0**: its +lower pair realized 21.06 against a declared 4.0. The recurrence subtracted the +upper staff's shift from the measured gap and then added it back through the +accumulator: + +```text +gap = (upper_lo - shift_upper) - lower_hi // WRONG +shift += target - gap // ⇒ realized = target + shift_upper +``` + +Both staves move, so the correct relation is +`shift_lower = shift_upper + target - (upper_lo - lower_hi)` — the **unshifted** +gap. Every pair below the first was over-separated by exactly the shift above it. +It was invisible on two-staff fixtures (`shift_upper = 0`) and invisible to +`inter_staff_shifts_cascade_down_three_staves`, which asserted only `s2 > s1` — +true under both the correct and the double-counting recurrence. + +**What caught it was the metric.** Measuring the realized clearance back from the +BAKED output, rather than reading the solver's own `staff_ext`, is what made an +independent check possible; had the axis read back solver intent it would have +reported 0 and the defect would have shipped again. That design choice was made +one commit earlier for exactly this reason, and it paid immediately. + +**Locking it.** Once the solve realizes each declared clearance exactly, every +inter-staff unit is `0` on a healthy solve — the axis becomes a *solver +self-check*, and its mean can no longer distinguish "measured every realization" +from "measured one". So `vertical_raw` is split into `vertical_units`, and the +regressions assert the **unit set**: `a_glyphless_staff_band_still_contributes_an_ +inter_staff_unit` (2 units, one per system) and `each_system_realizing_a_gap_band_ +contributes_its_own_unit` (2 units for the wrapping fixture, 2 for the three-staff +cascade), each unit ≈ 0. Four mutations verified: the double-counting recurrence, +expand-only, the glyph-`members` band filter, and first-system-only measurement +each fail a named test. + +**Churn:** the two multi-staff engrave goldens only. `two_staff_close_content` +grew by exactly 3.0 (the target change, no cascade); `three_staff_close_content` +*shrank* by 9.06 — the same +3 per pair, less the 17.06 of over-separation the +cascade defect was adding. Single-staff and every stub golden are byte-stable. + +**Still open:** genuinely staff-less content placed *between* two staves. No +primitive can name an `InterStaffGap` band today, so it remains unreachable +rather than latent. diff --git a/crates/epiphany-engrave/src/casting.rs b/crates/epiphany-engrave/src/casting.rs index 79621ae..e2b048d 100644 --- a/crates/epiphany-engrave/src/casting.rs +++ b/crates/epiphany-engrave/src/casting.rs @@ -771,17 +771,25 @@ pub(crate) fn cast_off( // Solve each system's inter-staff gaps: order the staves top-to-bottom by // their reference y (staff line, else content mid), keep that order fixed, - // and shift each staff down until its gap to the one above meets the gap - // band's own preferred height. `staff_shift[(system, staff)]` is the - // downward shift (subtracted from y); the top staff's is 0. + // and shift each staff so its INK CLEARANCE to the one above realizes the + // gap band's declared height. `staff_shift[(system, staff)]` is the downward + // shift (subtracted from y); the top staff's is 0. // - // The gap the solve targets is the one the REGION DECLARED, read from the - // `InterStaffGap` band `to_constrained` emitted for that pair, not from the - // band constructor's default. That is what makes the band a height model - // rather than a constant: a region that declares a wider gap gets one, and - // `vertical_density_penalty` — which scores the realized gap against this - // same declared band — agrees with the solve by construction rather than by - // both happening to call the same constructor. + // The renegotiation is TWO-SIDED. A pair whose content collides is pushed + // apart; a pair the constrained stage left slack is pulled together. The + // fixed `SYSTEM_STAFF_PITCH` that stage stacks by is therefore an initial + // arrangement, not a floor: the band model is the height model, and the solve + // realizes it. (Expanding only was the earlier behaviour, and it was + // measurably wrong — `vertical_density_penalty` scored honest sprawl on every + // relaxed multi-staff system, because a gap wider than preferred is sprawl + // exactly as a narrower one is crowding.) + // + // The target is the gap band's `preferred_height`, held at or above its + // `min_height` — the hardest squeeze permitted. Validation already brackets + // preferred by min and max, so the clamp is belt-and-braces rather than a + // second policy. The band is the one the REGION DECLARED, not the + // constructor's default, so the solve and `vertical_density_penalty` — which + // scores the realized clearance against that same band — read one number. // // Gap `g` separates the region's staves `g-1` and `g` (see `to_constrained`). // Every staff of a region carries content in every system of that region — @@ -789,20 +797,18 @@ pub(crate) fn cast_off( // staves present here are the region's full staff order and the window index // is the gap index. A band that somehow does not exist falls back to the // constructor's default rather than silently skipping the pair. - let default_gap = VerticalBand::inter_staff_gap(VerticalBandId(0)) - .preferred_height - .0; + let fallback = VerticalBand::inter_staff_gap(VerticalBandId(0)); let mut staff_shift: BTreeMap<(usize, StaffId), f32> = BTreeMap::new(); for (s, plan) in systems.iter().enumerate() { let region_layout_id = input.regions[plan.region].provenance.stable_id; - let preferred_gap = |gap_index: usize| -> f32 { + let target_gap = |gap_index: usize| -> f32 { let id = inter_staff_gap_id(region_layout_id, gap_index); - input + let band = input .vertical_bands .iter() .find(|band| band.id == id) - .map(|band| band.preferred_height.0) - .unwrap_or(default_gap) + .unwrap_or(&fallback); + band.preferred_height.0.max(band.min_height.0) }; let mut staves: Vec<(StaffId, (f32, f32))> = staff_ext .iter() @@ -824,9 +830,21 @@ pub(crate) fn cast_off( let (upper, (upper_lo, _)) = w[0]; let (lower, (_, lower_hi)) = w[1]; staff_shift.insert((s, upper), shift); - // Realized gap after the upper's shift: (upper_lo - shift) − lower_hi. - let gap = (upper_lo - shift) - lower_hi; - shift += (preferred_gap(g + 1) - gap).max(0.0); + // Both staves move, so solve the recurrence rather than guessing it. + // With `shift` the upper staff's cumulative shift, the realized + // clearance is `(upper_lo - shift_upper) - (lower_hi - shift_lower)`, + // and setting that equal to the target gives + // + // shift_lower = shift_upper + target - (upper_lo - lower_hi) + // + // — the UNSHIFTED gap. Subtracting `shift_upper` from the gap here + // and adding it back through `shift +=` would count it twice, which + // over-separated every pair below the first by exactly the shift + // above it (invisible on two staves, where that shift is 0). The + // correction is signed: positive opens a crowded pair, negative + // closes a slack one, and it accumulates down the stack. + let gap = upper_lo - lower_hi; + shift += target_gap(g + 1) - gap; staff_shift.insert((s, lower), shift); } if staves.len() == 1 { diff --git a/crates/epiphany-engrave/src/lib.rs b/crates/epiphany-engrave/src/lib.rs index 14da586..9656ea7 100644 --- a/crates/epiphany-engrave/src/lib.rs +++ b/crates/epiphany-engrave/src/lib.rs @@ -204,8 +204,16 @@ pub struct Engraver { /// gaps between a system's staves are renegotiated so tightly ledgered or /// slurred adjacent staves separate to the band model's preferred gap; a /// multi-staff score whose staves press together shifts them apart, while a -/// single-staff score — with no inter-staff pair — is unchanged). -pub const ENGRAVER_VERSION: SolverVersion = SolverVersion(11); +/// single-staff score — with no inter-staff pair — is unchanged), and to `12` +/// when that solve became **two-sided** (it now closes a slack pair as well as +/// opening a crowded one, realizing the `InterStaffGap` band's declared ink +/// clearance exactly; the constrained stage's fixed `SYSTEM_STAFF_PITCH` is an +/// initial arrangement, not a floor. Every multi-staff score's staff distances +/// move — plain content settles near a pitch of 10.6 staff spaces — while a +/// single-staff score is again unchanged. The same version repairs a cascade +/// defect: a pair below the first was over-separated by exactly the shift above +/// it, so 3+-staff scores tighten further). +pub const ENGRAVER_VERSION: SolverVersion = SolverVersion(12); impl Engraver { /// An engraver casting off against the given page geometry. @@ -2333,39 +2341,67 @@ mod tests { ); } - /// A staff band that owns no GLYPHS is still a staff of its region. The - /// percussion placeholder's clef has no bundled glyph (it engraves to a - /// traced anchor stroke) and it carries no notes, so its band's `members` - /// list — which holds glyphs only — is empty, while the band does own its - /// five staff-line strokes. Identifying a region's staff bands by their - /// glyph members would drop it from the axis entirely, reintroducing the - /// very glyph-members assumption the content-extent measurement sheds. + /// A staff band that owns no GLYPHS is still a staff of its region: the + /// percussion placeholder's clef has no bundled glyph (a traced anchor + /// stroke) and it carries no notes, so its band's glyph `members` list is + /// empty while the band owns its five staff-line strokes. The solve must + /// still separate it. (That the *metric* also counts its gap is locked by + /// `quality::tests::a_glyphless_staff_band_still_contributes_an_inter_staff_unit`.) #[test] - fn a_staff_band_owning_no_glyphs_still_contributes_its_gap() { + fn a_staff_band_owning_no_glyphs_is_still_laid_out_as_a_staff() { use epiphany_layout_ir::{to_constrained, to_logical}; - let input = to_constrained(&to_logical( - &epiphany_testkit::fixtures::percussion_placeholder_staff(1), - )); - let report = Engraver::default().solve(&input, &SolverConfig::default()); + let report = Engraver::default().solve( + &to_constrained(&to_logical( + &epiphany_testkit::fixtures::percussion_placeholder_staff(1), + )), + &SolverConfig::default(), + ); assert_eq!(report.status, SolveStatus::Solved); for page in &report.layout.pages { for sys in &page.systems { - assert_eq!( - sys.staves.len(), - 2, - "the glyph-less placeholder is still laid out as a staff" - ); + assert_eq!(sys.staves.len(), 2, "the placeholder is a staff"); } } - // Its gap to the melody staff is slack (the solve expands, never - // compresses), so the band contributes a large deviation. Dropping the - // band would leave the region with one staff, no inter-staff unit, and - // an axis of exactly 0. - let density = report.metric_vector.vertical_density_penalty.0; - assert!( - density > 0.5, - "the glyph-less band's gap reaches the axis: {density}" + assert_eq!(report.metric_vector.collision_penalty.0, 0.0); + } + + /// The solve sizes each system's gaps from that system's own content, so one + /// region's systems end up at genuinely different staff distances: the system + /// carrying the colliding first measure opens wide, the slack one after it is + /// pulled CLOSER than the constrained stage's fixed pitch. Two-sided + /// renegotiation is what makes the second half of that sentence true. + #[test] + fn each_system_solves_its_own_staff_distance() { + use epiphany_layout_ir::{to_constrained, to_logical}; + let report = Engraver::default().solve( + &to_constrained(&to_logical( + &epiphany_testkit::fixtures::two_staff_wrapping_pressure(1), + )), + &SolverConfig::default(), ); + let systems: Vec<_> = report + .layout + .pages + .iter() + .flat_map(|page| &page.systems) + .collect(); + assert_eq!(systems.len(), 2, "the region wraps into two systems"); + let pitch = |sys: &epiphany_layout_ir::ResolvedSystem| { + assert_eq!(sys.staves.len(), 2, "both staves ride every system"); + sys.staves[0].bounding_box.origin.y.0 - sys.staves[1].bounding_box.origin.y.0 + }; + let (first, second) = (pitch(systems[0]), pitch(systems[1])); + assert!( + first > second + 8.0, + "the pressured system opens far wider: {first} vs {second}" + ); + // The constrained stage stacks at SYSTEM_STAFF_PITCH = 12; the slack + // system is COMPRESSED below it, which an expand-only solve cannot do. + assert!( + second < 12.0, + "the slack system is pulled tighter than the fixed pitch: {second}" + ); + assert_eq!(report.metric_vector.collision_penalty.0, 0.0); } #[test] @@ -2436,17 +2472,25 @@ 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. + /// staff's shift accumulates the corrections of every pair above it — the + /// bottom staff must clear the middle staff where the middle staff now sits, + /// not where it started — so `shift_lower = shift_upper + target - gap_raw`, + /// with `gap_raw` the pair's UNSHIFTED clearance. /// - /// 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. + /// A solve sizing each pair independently gets this wrong the obvious way: + /// it hands the bottom staff only its own small correction and leaves it + /// above the middle staff's new position. Verified by mutation: with the + /// cascade removed this test fails on `s2 > s1`. + /// + /// It is wrong the *subtle* way too, by subtracting `shift_upper` from the + /// measured gap and then adding it back through the accumulator — which + /// over-separates every pair below the first by exactly the shift above it. + /// That defect shipped in `ENGRAVER_VERSION` 11 and this test could not see + /// it (`s2 > s1` holds either way; two-staff fixtures have `shift_upper = 0`). + /// It is caught by `quality::tests::each_system_realizing_a_gap_band_ + /// contributes_its_own_unit`, which asserts every pair realizes its declared + /// clearance — the over-separated lower pair reported 21.06 against a + /// declared 4.0. #[test] fn inter_staff_shifts_cascade_down_three_staves() { use epiphany_layout_ir::{to_constrained, to_logical}; @@ -2519,54 +2563,6 @@ mod tests { assert!(gaps[2] < 2.0, "and sits close against it: {}", gaps[2]); } - /// A gap band realized in several systems contributes a unit PER SYSTEM. - /// The inter-staff solve sizes each system's gaps from that system's own - /// content, so one band's realized height genuinely differs across a - /// region's systems — here 15.93 staff spaces in the system carrying the - /// colliding first measure, 7.87 in the slack one after it. Measuring only - /// the first system realizing the band (as this metric once did, on the - /// since-falsified premise that rigid system translation makes every - /// realization agree) would report the pressured system's near-perfect gap - /// and discard the slack one entirely. - #[test] - fn inter_staff_gaps_are_measured_in_every_system_that_realizes_them() { - use epiphany_layout_ir::{to_constrained, to_logical}; - let report = Engraver::default().solve( - &to_constrained(&to_logical( - &epiphany_testkit::fixtures::two_staff_wrapping_pressure(1), - )), - &SolverConfig::default(), - ); - let systems: Vec<_> = report - .layout - .pages - .iter() - .flat_map(|page| &page.systems) - .collect(); - assert_eq!(systems.len(), 2, "the region wraps into two systems"); - let gap = |sys: &epiphany_layout_ir::ResolvedSystem| { - assert_eq!(sys.staves.len(), 2, "both staves ride every system"); - let (top, bottom) = (&sys.staves[0].bounding_box, &sys.staves[1].bounding_box); - top.origin.y.0 - (bottom.origin.y.0 + bottom.size.height.0) - }; - let (first, second) = (gap(systems[0]), gap(systems[1])); - assert!( - first > second + 4.0, - "the pressured system opens much further than the slack one: {first} vs {second}" - ); - - // The slack system's gap sits well past the band's preferred height (the - // solve expands but never compresses), so it contributes a large - // deviation. The pressured system was solved to preferred and contributes - // ~0. A first-system-only measurement would therefore report ~0 overall; - // counting both realizations reports the sprawl honestly. - let density = report.metric_vector.vertical_density_penalty.0; - assert!( - density > 0.5, - "the slack system's sprawl reaches the axis: {density}" - ); - } - #[test] fn vertical_justification_fills_non_final_pages() { use epiphany_layout_ir::{Margins, Size2D, StaffSpace}; diff --git a/crates/epiphany-engrave/src/quality.rs b/crates/epiphany-engrave/src/quality.rs index fbaa7cd..4291339 100644 --- a/crates/epiphany-engrave/src/quality.rs +++ b/crates/epiphany-engrave/src/quality.rs @@ -329,6 +329,29 @@ fn spacing_raw(census: &SystemCensus) -> f64 { /// relative deviation `|r − p| / p` over the realized inter-staff and /// inter-system gaps (see the module docs for the unit reconstruction). fn vertical_raw(input: &ConstrainedLayoutIR, cast: &CastLayout, census: &SystemCensus) -> f64 { + let units = vertical_units(input, cast, census); + mean_or_zero(&[units.inter_staff, units.inter_system].concat()) +} + +/// The axis's contributing units, split by kind (catalog §`vertical_density_penalty`: +/// "each *realization* … contributes one unit per system"). +/// +/// Split out so tests can assert the unit **set**, not merely its mean. Once the +/// inter-staff solve realizes each gap band's declared clearance exactly, every +/// inter-staff unit is `0` on a healthy solve — the axis is a self-check that +/// fires only on a solver or bake defect. Its mean therefore cannot distinguish +/// "measured every realization" from "measured one", which is precisely the +/// defect this split lets a regression catch. +struct VerticalUnits { + inter_staff: Vec, + inter_system: Vec, +} + +fn vertical_units( + input: &ConstrainedLayoutIR, + cast: &CastLayout, + census: &SystemCensus, +) -> VerticalUnits { let mut per_unit: Vec = Vec::new(); // --- InterStaffGap bands declared by the constrained input ------------- @@ -455,6 +478,8 @@ fn vertical_raw(input: &ConstrainedLayoutIR, cast: &CastLayout, census: &SystemC } } + let inter_staff = std::mem::take(&mut per_unit); + // --- Realized inter-system gaps (consecutive systems on a page) -------- let preferred = f64::from( VerticalBand::inter_system_gap(VerticalBandId(0)) @@ -473,7 +498,10 @@ fn vertical_raw(input: &ConstrainedLayoutIR, cast: &CastLayout, census: &SystemC } } - mean_or_zero(&per_unit) + VerticalUnits { + inter_staff, + inter_system: per_unit, + } } /// `system_break_penalty` (catalog §`system_break_penalty`): mean @@ -644,6 +672,7 @@ pub(crate) fn floor_warnings( #[cfg(test)] mod tests { + use super::{census, vertical_units, VerticalUnits}; use crate::Engraver; use epiphany_layout_ir::{ to_constrained, to_logical, ConstrainedLayoutIR, ConstraintSolver, QualityMetricKind, @@ -982,4 +1011,61 @@ mod tests { report.metric_vector.vertical_density_penalty.0 ); } + + /// Runs the real pipeline far enough to hand `vertical_units` a `CastLayout` + /// — the same spacing + casting-off `Engraver::resolve` performs. + fn units(score: &epiphany_core::Score) -> VerticalUnits { + let input = to_constrained(&to_logical(score)); + let engraver = Engraver::default(); + let remap = crate::HorizontalRemap::build(&input); + let (glyphs, strokes, curves) = ( + remap.glyphs(&input), + remap.strokes(&input), + remap.curves(&input), + ); + let geometry = engraver.geometry(); + let cast = crate::casting::cast_off(&input, &glyphs, &strokes, &curves, &geometry); + let census = census(&input, &cast); + vertical_units(&input, &cast, &census) + } + + /// A staff band owning no glyphs is still a staff of its region, so its gap + /// contributes. The percussion placeholder's clef has no bundled glyph and it + /// carries no notes, leaving its band's `members` (a glyph list) empty while + /// the band owns its staff-line strokes. Identifying a region's staff bands by + /// `members` drops it, and the axis loses the unit entirely. + #[test] + fn a_glyphless_staff_band_still_contributes_an_inter_staff_unit() { + let u = units(&epiphany_testkit::fixtures::percussion_placeholder_staff(1)); + assert_eq!( + u.inter_staff.len(), + 2, + "one unit per system realizing the pair, not zero" + ); + for value in &u.inter_staff { + assert!( + *value < 1e-4, + "and the solve realizes the declared gap: {value}" + ); + } + } + + /// A gap band realized in several systems contributes one unit PER SYSTEM. + /// The inter-staff solve sizes each system's gaps from that system's own + /// content, so the realizations are independent measurements; the wrapping + /// fixture's two systems sit at very different staff distances. + #[test] + fn each_system_realizing_a_gap_band_contributes_its_own_unit() { + let u = units(&epiphany_testkit::fixtures::two_staff_wrapping_pressure(1)); + assert_eq!(u.inter_staff.len(), 2, "two systems, two realizations"); + for value in &u.inter_staff { + assert!(*value < 1e-4, "each solved to its declared gap: {value}"); + } + // Three staves in one system: two gap bands, one realization each. + let u = units(&epiphany_testkit::fixtures::three_staff_close_content(1)); + assert_eq!(u.inter_staff.len(), 2, "two gap bands, one system"); + for value in &u.inter_staff { + assert!(*value < 1e-4, "the cascade realizes both gaps: {value}"); + } + } } diff --git a/crates/epiphany-layout-ir/src/vertical_band.rs b/crates/epiphany-layout-ir/src/vertical_band.rs index 90c6697..07637e6 100644 --- a/crates/epiphany-layout-ir/src/vertical_band.rs +++ b/crates/epiphany-layout-ir/src/vertical_band.rs @@ -112,12 +112,32 @@ impl VerticalBand { /// An inter-staff gap band: the empty (member-less) spacing region between /// two staves of a system (Chapter 7 §"Vertical Bands": `InterStaffGap`). Its /// height is a spring the solver resolves; it carries no glyphs. + /// + /// **Its height is an INK CLEARANCE**, not a distance between staff lines: + /// the vertical separation between the two staves' outermost *content* — + /// ledger lines, stems, slurs, everything. That is the unit the Quality + /// Metric Catalog's `vertical_density_penalty` measures against + /// (`req:qmc:vertical`, "the adjacent content extents the band separates"), + /// so the solve and the metric read one number. + /// + /// `preferred` is a staff height plus a space: two staves whose ink is that + /// far apart read as separate systems of lines without wasting the page. + /// Plain ledgered content then lands at a staff *pitch* of about 10.6 staff + /// spaces — near conventional two-staff spacing — while ledgered or slurred + /// content pushes the staves further apart on its own. `min` is the hardest + /// squeeze a compressing solve may apply. + /// + /// (The earlier `preferred = 2.0` was a placeholder reconciled with nothing: + /// it is neither the 8.0 staff-box gap the constrained stage's fixed + /// `SYSTEM_STAFF_PITCH` produces, nor the ~6.4 ink clearance that stacking + /// leaves for plain content. Realizing it would have crushed a relaxed + /// system to a pitch of ~7.6.) pub fn inter_staff_gap(id: VerticalBandId) -> Self { VerticalBand { id, kind: VerticalBandKind::InterStaffGap, - min_height: StaffSpace(1.0), - preferred_height: StaffSpace(2.0), + min_height: StaffSpace(2.0), + preferred_height: StaffSpace(5.0), max_height: None, stretch_factor: 1.0, compress_factor: 1.0, 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 index f064552..033883c 100644 --- 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 @@ -8,7 +8,7 @@ provenance_count=96 layer_count=1 hard_constraint_count=27 xml_well_formed=true -view_box=[5.5 -75.204575 17.379084 69.703995] +view_box=[5.5 -66.14458 17.379084 60.643997] class_counts: barline=3 clef=3 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 index e38201c..73888e3 100644 --- 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 @@ -1,5 +1,5 @@ - + @@ -9,16 +9,16 @@ - - - - - - - - - - + + + + + + + + + + @@ -48,57 +48,57 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - - - - - - - - - - + + + + + + + + + + - - + + diff --git a/crates/epiphany-render-svg/tests/golden/two_staff_close_content.engrave.snapshot.txt b/crates/epiphany-render-svg/tests/golden/two_staff_close_content.engrave.snapshot.txt index 9fa0611..77b17c8 100644 --- a/crates/epiphany-render-svg/tests/golden/two_staff_close_content.engrave.snapshot.txt +++ b/crates/epiphany-render-svg/tests/golden/two_staff_close_content.engrave.snapshot.txt @@ -8,7 +8,7 @@ provenance_count=54 layer_count=1 hard_constraint_count=18 xml_well_formed=true -view_box=[5.5 -36.58458 17.379084 31.084002] +view_box=[5.5 -39.584576 17.379084 34.084] class_counts: barline=2 clef=2 diff --git a/crates/epiphany-render-svg/tests/golden/two_staff_close_content.engrave.svg b/crates/epiphany-render-svg/tests/golden/two_staff_close_content.engrave.svg index bdf19b8..c6a915c 100644 --- a/crates/epiphany-render-svg/tests/golden/two_staff_close_content.engrave.svg +++ b/crates/epiphany-render-svg/tests/golden/two_staff_close_content.engrave.svg @@ -1,5 +1,5 @@ - + @@ -9,11 +9,11 @@ - - - - - + + + + + @@ -36,27 +36,27 @@ - - - - - - - - - + + + + + + + + + - - - - - + + + + + - + diff --git a/spec/PASS12_RATIFICATION_LOG.md b/spec/PASS12_RATIFICATION_LOG.md index be1e9b6..24b70a6 100644 --- a/spec/PASS12_RATIFICATION_LOG.md +++ b/spec/PASS12_RATIFICATION_LOG.md @@ -356,3 +356,45 @@ that declares its clef only via `Staff::default_clef` engraves as treble. Found while building the percussion fixture (which therefore had to declare its clef as a `ClefChange`). Filed in `epiphany-layout-ir/DECISIONS.md`; parked with the `ConstrainedLayoutIR` listing gap pending a ≥3-candidate Pass-13 batch. + +## Push-3 tranche (2026-07-09) — two-sided inter-staff renegotiation + +Acting on the sprawl the per-realization axis exposed. `ENGRAVER_VERSION` 11 → +12. **No normative change and no version move:** the Quality Metric Catalog's +formula, contributing units, anchors and thresholds are untouched; only its +non-normative rationale is refreshed (the reference solver no longer merely +expands, and the axis's inter-staff half is now documented as a solver +self-check). Core spec unchanged. Operation Catalog 0.7.0, Binary Format 0.6.0, +Quality Metric Catalog 0.3.0 all unchanged. + +| Item | Disposition | Spec locus | Consumer | +|---|---|---|---| +| Expand-only inter-staff solve | **fix (layout)** — the solve now closes a slack pair as well as opening a crowded one, realizing the `InterStaffGap` band's declared height exactly. `SYSTEM_STAFF_PITCH` is demoted from a floor to an initial arrangement. This is what `vertical_density_penalty` was reporting: an un-pressured multi-staff system sat at 0.739, honest sprawl against the declared gap | — (behavioural; `req:layoutir:vertical-bands`) | `epiphany-engrave` (`casting.rs`) | +| The gap band's height had no agreed meaning | **pin (code default, not spec)** — it is an **ink clearance**: the separation between the two staves' outermost content, which is exactly the unit `req:qmc:vertical` measures. `preferred` 2.0 → **5.0**, `min` 1.0 → **2.0**. The old 2.0 was a placeholder reconciled with nothing — neither the 8.0 staff-box gap the fixed pitch of 12 produces nor the ~6.4 ink clearance it leaves for plain content; realizing it would have crushed a relaxed system to a pitch of ~7.6. At 5.0 plain ledgered content settles near a pitch of 10.6 | `VerticalBand::inter_staff_gap` doc | `epiphany-layout-ir` | +| **Cascade defect, latent since v11** | **fix (correctness)** — the recurrence subtracted the upper staff's shift from the measured gap and added it back through the accumulator, so every pair below the first was over-separated by exactly the shift above it. Both staves move: `shift_lower = shift_upper + target − (upper_lo − lower_hi)`, the *unshifted* gap. Invisible on two-staff fixtures (`shift_upper = 0`) and invisible to the cascade regression, which asserted only `s2 > s1` — true under both recurrences. `three_staff_close_content`'s lower pair realized **21.06** against a declared 4.0 | — | `epiphany-engrave` (`casting.rs`) | + +**How the defect was caught, and why that vindicates a design choice.** The +metric measures the realized clearance back from the **baked output**, not from +the solve's own extents. Reading back solver intent would have reported 0 and let +the over-separation ship a second time. That choice was made one commit earlier +for exactly this reason and paid immediately. The catalog rationale now +recommends it to any conforming implementation. + +**Consequence for testing.** Once the solve realizes each declared clearance +exactly, every inter-staff unit is 0 on a healthy solve — the axis is a solver +self-check, and its *mean* can no longer distinguish "measured every realization" +from "measured one". The regressions therefore assert the **unit set**, not the +mean. Four mutations verified: the double-counting recurrence, expand-only, the +glyph-`members` band filter, and first-system-only measurement each fail a named +test. + +**Churn.** The two multi-staff engrave goldens only. `two_staff_close_content` +grew by exactly 3.0 (the target change, no cascade); `three_staff_close_content` +*shrank* by 9.06 — the same +3 per pair, less the 17.06 of over-separation the +cascade defect was adding. Single-staff and every stub golden are byte-stable. + +**Process note.** The value 5.0 was the user's call. `4.0` ("one staff height") +was chosen first and then withdrawn once its true consequence — a staff pitch of +9.57, not the 11.04 an arithmetic slip had projected — was **measured rather than +inferred**. The slip: deriving plain-content ink clearance from an aggregate +metric by assuming it had two contributing units when it had three. diff --git a/spec/quality_metric_catalog.pdf b/spec/quality_metric_catalog.pdf index d1965b2..d4e98ee 100644 Binary files a/spec/quality_metric_catalog.pdf and b/spec/quality_metric_catalog.pdf differ diff --git a/spec/quality_metric_catalog.tex b/spec/quality_metric_catalog.tex index 048316b..dde04ef 100644 --- a/spec/quality_metric_catalog.tex +++ b/spec/quality_metric_catalog.tex @@ -934,7 +934,17 @@ symmetric: a gap wider than preferred is sprawl exactly as a narrower one is crowding. A solver that only ever \emph{expands} a fixed stacking --- never compressing an over-wide gap back toward preferred --- will therefore report honest sprawl on its slack systems. That is the axis working, not -mis-measuring. +mis-measuring, and it is how the reference implementation's expand-only solve +was found: it now renegotiates in both directions. + +Where a solver realizes each band's declared height exactly, the inter-staff +half of this axis reads its honest zero and serves as a \emph{self-check} on the +solve rather than a judgement on the score. A conforming implementation should +therefore measure the realized separation back from the geometry it produced, +not from the target it aimed at: the two agree only if the solve and the bake +both did what they claimed. The reference implementation reads it back from the +baked output, and that is how a defect in its multi-staff cascade --- one that +over-separated every pair below the first --- was caught. \end{rationale} \section{\texttt{system\_break\_penalty}}