Two more glyph-members assumptions in the vertical-density axis (QMC 0.3.0)
Review follow-up on 43215c7. Both findings were right, and the second falsified a
comment I wrote in that very commit.
1. Region staff bands were still identified by glyph `members`. vertical_raw
measured content over all primitives but decided WHICH staff bands belong to a
region by glyph membership -- reintroducing the assumption the change exists to
shed. A staff band is allowed to own no glyphs: to_constrained emits one per
staff of the region regardless, and a percussion-clef staff (no bundled glyph,
so it engraves to a traced anchor stroke) with no notes owns only staff-line
strokes. Membership now comes from content presence in one of the region's
systems, which identifies the band exactly -- a staff band is per-(staff,
region), so its content can land nowhere else.
2. Only the first realizing system was measured, justified by a comment claiming
rigid system translation makes every realization agree. The inter-staff solve
had just falsified that: it sizes each system's gaps from that system's own
content. req:qmc:vertical now counts ONE UNIT PER REALIZATION, matching how
realized inter-system gaps were already counted. That is a contributing-unit
change, so unlike 43215c7's clarification the catalog moves: QMC 0.2.0 -> 0.3.0
(the P12-I12 precedent). Raw formula, anchor, orientation, thresholds unchanged.
Two new fixtures, because an unexercised fix is what I criticised last round:
percussion_placeholder_staff (a valid, invariant-clean score whose lower band owns
zero glyphs and six strokes) and two_staff_wrapping_pressure (one region, two
systems, staff-line gap 15.93 where it collides and 7.87 where it is slack). Both
mutation-verified: the members filter scores 4.8e-7, first-system-only scores
1.3e-7 -- each ~0 where the corrected axis reports real deviation.
What the per-realization count exposes is not comfortable, and is recorded rather
than smoothed over: two_staff_wrapping_pressure now scores 0.739. Its pressured
system solves to the declared gap exactly; its slack system sits at ~5 staff
spaces against a preferred 2.0. The axis is symmetric -- a gap wider than
preferred is sprawl exactly as a narrower one is crowding -- and this solve only
expands, never compresses. The deferral "compressing an OVER-wide fixed gap toward
preferred ... rarely wanted" is promoted to measurably wrong. Named, not fixed:
compression is a layout change (golden churn, ENGRAVER_VERSION move), not a
measurement one.
Adjacent finding, parked: Staff::default_clef is never consulted -- to_constrained
takes the active clef from the instance's clef_sequence and falls back to
Clef::default() (treble), so a staff declaring its clef only on Staff engraves as
treble. Verified (no layout-ir consumer reads the field). Filed in layout-ir
DECISIONS with the ConstrainedLayoutIR listing gap, pending a >=3-candidate batch.
Measurement-only: no layout change, ENGRAVER_VERSION stays 11, zero golden churn.
Gate green; QMC PDF rebuilds clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
43215c7d67
commit
5b016f33b7
|
|
@ -899,3 +899,44 @@ re-filed as a bug.
|
|||
while the lower staff descends. No primitive can name an `InterStaffGap` band
|
||||
today (`band_of` yields a staff band or the region's margin band), so this is
|
||||
unreachable rather than latent — building the machinery now would be speculative.
|
||||
|
||||
### Review follow-up: two more glyph-members assumptions (2026-07-09)
|
||||
|
||||
An adversarial review found the content-extent correction incomplete in two
|
||||
places. Both were right; the second falsified a comment written in the same
|
||||
commit that introduced it.
|
||||
|
||||
1. **Region staff bands were still identified by glyph `members`.** `vertical_raw`
|
||||
measured content over all primitives but decided *which* staff bands belong to
|
||||
a region by glyph membership — the very assumption the change exists to shed.
|
||||
A staff band is allowed to own no glyphs: `to_constrained` emits one per staff
|
||||
of the region regardless, and a percussion-clef staff (no bundled glyph, so it
|
||||
engraves to a traced anchor stroke) with no notes owns only its staff-line
|
||||
strokes. Region membership now comes from **content presence** in one of the
|
||||
region's systems, which identifies the band exactly, because a staff band is
|
||||
per-`(staff, region)` manifestation and its content can land nowhere else.
|
||||
Locked by `percussion_placeholder_staff` + `a_staff_band_owning_no_glyphs_
|
||||
still_contributes_its_gap`. Mutation-verified: the members filter scores
|
||||
`4.8e-7` where the fix reports real deviation.
|
||||
|
||||
2. **Only the first realizing system was measured.** The code took one system per
|
||||
gap band, justified by a comment claiming rigid system translation makes every
|
||||
realization agree. The inter-staff solve had *just* falsified that: it sizes
|
||||
each system's gaps from that system's own content. `req:qmc:vertical` now
|
||||
counts **one unit per realization** (QMC 0.2.0 → 0.3.0), matching how realized
|
||||
inter-system gaps were already counted. Locked by `two_staff_wrapping_pressure`
|
||||
(staff-line gap 15.93 in the pressured system, 7.87 in the slack one) +
|
||||
`inter_staff_gaps_are_measured_in_every_system_that_realizes_them`.
|
||||
Mutation-verified: first-system-only scores `1.3e-7`.
|
||||
|
||||
**What this exposes, and it is not comfortable.** `two_staff_wrapping_pressure`
|
||||
scores `vertical_density_penalty` **0.739**. Its pressured system solves to the
|
||||
declared gap exactly; its slack system sits at ~5 staff spaces of content gap
|
||||
against a preferred 2.0. The axis is symmetric — a gap wider than preferred is
|
||||
sprawl exactly as a narrower one is crowding — and this solve **only expands,
|
||||
never compresses**. The deferral recorded above ("compressing an OVER-wide fixed
|
||||
gap toward preferred… the fixed pitch is generous by default, so this is rarely
|
||||
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.
|
||||
|
|
|
|||
|
|
@ -2333,6 +2333,41 @@ 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.
|
||||
#[test]
|
||||
fn a_staff_band_owning_no_glyphs_still_contributes_its_gap() {
|
||||
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());
|
||||
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"
|
||||
);
|
||||
}
|
||||
}
|
||||
// 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}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn inter_staff_solve_separates_colliding_staves() {
|
||||
use epiphany_layout_ir::{to_constrained, to_logical};
|
||||
|
|
@ -2484,6 +2519,54 @@ 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};
|
||||
|
|
|
|||
|
|
@ -64,8 +64,8 @@ use epiphany_layout_ir::quality::{
|
|||
anchors, normalize, MetricThresholds, QUALITY_FLOOR_FRACTION, QUALITY_METRIC_KINDS,
|
||||
};
|
||||
use epiphany_layout_ir::{
|
||||
inter_staff_gap_id, ConstrainedLayoutIR, Curve, GlyphObjectId, QualityMetricVector,
|
||||
SolverWarning, SolverWarningKind, SpringSlotId, VerticalBand, VerticalBandId, VerticalBandKind,
|
||||
inter_staff_gap_id, ConstrainedLayoutIR, Curve, QualityMetricVector, SolverWarning,
|
||||
SolverWarningKind, SpringSlotId, VerticalBand, VerticalBandId, VerticalBandKind,
|
||||
};
|
||||
|
||||
use crate::casting::{CastLayout, PageGeometry};
|
||||
|
|
@ -385,26 +385,40 @@ fn vertical_raw(input: &ConstrainedLayoutIR, cast: &CastLayout, census: &SystemC
|
|||
}
|
||||
|
||||
for (region_index, region) in input.regions.iter().enumerate() {
|
||||
// The region's laid-out staff bands, top staff first, ordered within
|
||||
// the region's first system (systems translate rigidly, so within-
|
||||
// system y order is the region's staff order).
|
||||
let first_system = census.region.iter().position(|&r| r == region_index);
|
||||
let Some(first_system) = first_system else {
|
||||
// The region's systems, in page order. A staff band is `to_constrained`'s
|
||||
// per-*manifestation* band — one per (staff, region) — so its content can
|
||||
// only land in its own region's systems, and "has content in one of this
|
||||
// region's systems" identifies the region's staff bands exactly.
|
||||
//
|
||||
// Membership is NOT read from `VerticalBand::members`. That list holds
|
||||
// glyphs, and a staff band is allowed to have none: a staff whose clef is
|
||||
// unbundled engraves to an anchor *stroke*, and its staff lines are
|
||||
// strokes regardless. Filtering by glyph members would silently drop such
|
||||
// a staff from the axis — the same glyph-members assumption this metric
|
||||
// exists to shed.
|
||||
let region_systems: Vec<usize> = census
|
||||
.region
|
||||
.iter()
|
||||
.enumerate()
|
||||
.filter(|&(_, &r)| r == region_index)
|
||||
.map(|(system, _)| system)
|
||||
.collect();
|
||||
let Some(&first_system) = region_systems.first() else {
|
||||
continue;
|
||||
};
|
||||
let region_glyphs: BTreeSet<GlyphObjectId> = region.glyphs.iter().copied().collect();
|
||||
// Top staff first, ordered within the region's first system: a system
|
||||
// translates rigidly as a whole, so the staff ORDER is the same in each
|
||||
// (only the gaps between them are renegotiated per system).
|
||||
let mut staves: Vec<(f64, VerticalBandId)> = input
|
||||
.vertical_bands
|
||||
.iter()
|
||||
.filter(|band| matches!(band.kind, VerticalBandKind::Staff(_)))
|
||||
.filter(|band| band.members.iter().any(|id| region_glyphs.contains(id)))
|
||||
.filter_map(|band| {
|
||||
content
|
||||
.get(&(first_system, band.id))
|
||||
.map(|&(_, top)| (top, band.id))
|
||||
})
|
||||
.collect();
|
||||
// Top staff first.
|
||||
staves.sort_by(|a, b| b.0.total_cmp(&a.0));
|
||||
|
||||
// The region's declared inter-staff gap bands, by their derived ids
|
||||
|
|
@ -420,19 +434,21 @@ fn vertical_raw(input: &ConstrainedLayoutIR, cast: &CastLayout, census: &SystemC
|
|||
continue;
|
||||
}
|
||||
let (upper, lower) = (staves[gap - 1].1, staves[gap].1);
|
||||
// Realized iff the adjacent content shares a system; measure the
|
||||
// separation there (rigid system translation makes every common
|
||||
// system agree).
|
||||
let common = (0..census.region.len()).find(|&system| {
|
||||
content.contains_key(&(system, upper)) && content.contains_key(&(system, lower))
|
||||
});
|
||||
let Some(system) = common else {
|
||||
continue;
|
||||
};
|
||||
let upper_bottom = content[&(system, upper)].0;
|
||||
let lower_top = content[&(system, lower)].1;
|
||||
let realized = (upper_bottom - lower_top).max(0.0);
|
||||
per_unit.push((realized - preferred).abs() / preferred);
|
||||
// EVERY system realizing the pair contributes a unit, not just the
|
||||
// first. The inter-staff solve sizes each system's gaps from that
|
||||
// system's own content, so one band's realized height genuinely
|
||||
// differs across the systems of a region — a later system may carry
|
||||
// different pressure, or a bake bug touching one primitive class.
|
||||
// Measuring only the first would average away both.
|
||||
for &system in ®ion_systems {
|
||||
let (Some(&(upper_bottom, _)), Some(&(_, lower_top))) =
|
||||
(content.get(&(system, upper)), content.get(&(system, lower)))
|
||||
else {
|
||||
continue;
|
||||
};
|
||||
let realized = (upper_bottom - lower_top).max(0.0);
|
||||
per_unit.push((realized - preferred).abs() / preferred);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -695,6 +695,24 @@ them). Two fields the code carries are still absent from the listing:
|
|||
|
||||
Neither blocks an implementation the way a missing `strokes`/`curves` did: both
|
||||
are governed by requirement text elsewhere, so a conformant implementer is not
|
||||
left guessing. **Parked, not open.** The Pass-13 batch is closed and the house
|
||||
rule opens a pass at ≥3 candidates; this is one. It joins a future batch rather
|
||||
than reopening one on its own.
|
||||
left guessing.
|
||||
|
||||
## Parked: `Staff::default_clef` is never consulted (2026-07-09)
|
||||
|
||||
`to_constrained` takes a staff instance's active clef from its `clef_sequence`
|
||||
(via `staff_content`'s `PlacedClef` list) and, when that sequence is empty, falls
|
||||
back to `Clef::default()` — treble. It never reads `Staff::default_clef`. So a
|
||||
bass-clef staff that declares its clef *only* on the `Staff` engraves as treble;
|
||||
the field is decorative in the projection. No consumer in this crate reads it
|
||||
(verified: `default_clef` appears only in core's codec/generators and the
|
||||
fixtures).
|
||||
|
||||
Found while building `percussion_placeholder_staff`, which therefore has to
|
||||
declare its percussion clef as a `ClefChange` rather than on the staff. Whether
|
||||
the staff's default should seed the sequence, or the field should be removed, is
|
||||
a small design question — not a silent-corruption bug (nothing is lost, only
|
||||
ignored).
|
||||
|
||||
**Both of the above are parked, not open.** The Pass-13 batch is closed and the
|
||||
house rule opens a pass at ≥3 candidates; these are two. They join a future batch
|
||||
rather than reopening one.
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
//! The Quality Metric Catalog's normative constants (companion specification
|
||||
//! *Epiphany — Quality Metric Catalog*, v0.2.0): the per-axis normalization
|
||||
//! *Epiphany — Quality Metric Catalog*, v0.3.0): the per-axis normalization
|
||||
//! anchors, the per-tier metric threshold table, the profile→threshold-column
|
||||
//! mapping, and the `QualityFloorApproached` warning fraction. (The v0.2.0
|
||||
//! mapping, and the `QualityFloorApproached` warning fraction. (The v0.3.0
|
||||
//! `spacing_distortion` refinement scoped that axis's *measurement domain* to
|
||||
//! rhythmic columns; it touched no anchor or threshold, so this transcription
|
||||
//! of the numeric constants is unchanged.)
|
||||
|
|
|
|||
|
|
@ -333,6 +333,242 @@ pub fn three_staff_close_content(seed: u64) -> Score {
|
|||
score
|
||||
}
|
||||
|
||||
/// A TWELVE-measure, TWO-staff metric score that wraps into more than one
|
||||
/// system and carries its inter-staff pressure in the FIRST measure only: the
|
||||
/// top staff dips to C2 and the bottom staff climbs to C6 there, while every
|
||||
/// later measure is plain C4s on both staves.
|
||||
///
|
||||
/// So the two systems of the one region want genuinely different staff gaps —
|
||||
/// the first must open to clear the colliding ledgers, the second is already
|
||||
/// slack. The inter-staff solve sizes each system's gaps from that system's own
|
||||
/// content, so a quality metric that measured only the first system realizing a
|
||||
/// gap band would average the second one away. Invariant-clean.
|
||||
pub fn two_staff_wrapping_pressure(seed: u64) -> Score {
|
||||
let mut rng = SplitMix64::new(seed ^ 0x0004_57AF_F123);
|
||||
let replica =
|
||||
ReplicaId::from_entropy(rng.next_u64().to_le_bytes()).unwrap_or(ReplicaId(0x4ADF));
|
||||
let mut idc = IdentityContext::new(replica);
|
||||
|
||||
let top_staff: StaffId = idc.mint();
|
||||
let bottom_staff: StaffId = idc.mint();
|
||||
let instrument: InstrumentId = idc.mint();
|
||||
let region_id: RegionId = idc.mint();
|
||||
|
||||
const MEASURES: i64 = 12;
|
||||
const BEATS: i64 = 4;
|
||||
|
||||
// Measure 0 collides (top dives, bottom climbs); the rest sit inside the
|
||||
// staff, where the fixed staff pitch already leaves the gap slack.
|
||||
let octave = |staff_index: usize, index: i64| -> i8 {
|
||||
if index >= BEATS {
|
||||
return 4;
|
||||
}
|
||||
if staff_index == 0 {
|
||||
[4i8, 3, 2, 3][index as usize]
|
||||
} else {
|
||||
[4i8, 5, 6, 5][index as usize]
|
||||
}
|
||||
};
|
||||
|
||||
let mut arena = EventArena::new();
|
||||
let mut instances: Vec<StaffInstance> = Vec::new();
|
||||
for (staff_index, staff_id) in [top_staff, bottom_staff].into_iter().enumerate() {
|
||||
let instance_id: StaffInstanceId = idc.mint();
|
||||
let voice_id: VoiceId = idc.mint();
|
||||
let mut voice = Voice::user(voice_id);
|
||||
for index in 0..(MEASURES * BEATS) {
|
||||
let (eid, pid): (EventId, PitchId) = (idc.mint(), idc.mint());
|
||||
arena
|
||||
.insert(c_at(eid, voice_id, pid, index, octave(staff_index, index)))
|
||||
.unwrap();
|
||||
voice.events.push(eid);
|
||||
}
|
||||
let mut instance = StaffInstance::new(instance_id, staff_id);
|
||||
instance.voices.push(voice);
|
||||
for m in 0..MEASURES {
|
||||
instance.measures.push(Measure {
|
||||
id: idc.mint(),
|
||||
start: TimeAnchor::Region {
|
||||
id: region_id,
|
||||
edge: RegionEdge::Start,
|
||||
offset: AnchorOffset::Musical(MusicalDuration(
|
||||
RationalTime::new(m, 1).unwrap(),
|
||||
)),
|
||||
},
|
||||
time_signature: None,
|
||||
explicit_number: Some((m + 1) as u32),
|
||||
number_visibility: Default::default(),
|
||||
});
|
||||
}
|
||||
instances.push(instance);
|
||||
}
|
||||
|
||||
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(120_000_000),
|
||||
},
|
||||
},
|
||||
staff_extent: StaffExtent {
|
||||
staves: vec![top_staff, bottom_staff],
|
||||
},
|
||||
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("Keyboard"),
|
||||
)];
|
||||
score.staves = vec![staff(top_staff, "Right"), staff(bottom_staff, "Left")];
|
||||
score.events = arena;
|
||||
score.canvas = Canvas {
|
||||
regions: vec![region],
|
||||
..Default::default()
|
||||
};
|
||||
score
|
||||
}
|
||||
|
||||
/// A two-staff score whose LOWER staff engraves **no glyphs at all**: it is a
|
||||
/// percussion-clef placeholder — a staff instance with a `ClefChange` to
|
||||
/// `ClefShape::Percussion`, which has no bundled SMuFL glyph (it engraves to a
|
||||
/// traced anchor stroke), and no voices or measures. Its vertical band therefore
|
||||
/// owns five staff-line strokes plus that anchor, and **zero** members.
|
||||
///
|
||||
/// The upper staff carries twelve plain measures of C4, so it wraps and its gap
|
||||
/// to the placeholder is slack everywhere. Together: a valid score on which any
|
||||
/// consumer that identifies a region's staff bands by their glyph `members`
|
||||
/// silently loses the lower staff. Invariant-clean.
|
||||
pub fn percussion_placeholder_staff(seed: u64) -> Score {
|
||||
let mut rng = SplitMix64::new(seed ^ 0x0005_57AF_F123);
|
||||
let replica =
|
||||
ReplicaId::from_entropy(rng.next_u64().to_le_bytes()).unwrap_or(ReplicaId(0x5ADF));
|
||||
let mut idc = IdentityContext::new(replica);
|
||||
|
||||
let top_staff: StaffId = idc.mint();
|
||||
let drum_staff: StaffId = idc.mint();
|
||||
let instrument: InstrumentId = idc.mint();
|
||||
let region_id: RegionId = idc.mint();
|
||||
|
||||
const MEASURES: i64 = 12;
|
||||
const BEATS: i64 = 4;
|
||||
|
||||
let mut arena = EventArena::new();
|
||||
let top_instance: StaffInstanceId = idc.mint();
|
||||
let top_voice: VoiceId = idc.mint();
|
||||
let mut voice = Voice::user(top_voice);
|
||||
for index in 0..(MEASURES * BEATS) {
|
||||
let (eid, pid): (EventId, PitchId) = (idc.mint(), idc.mint());
|
||||
arena.insert(quarter(eid, top_voice, pid, index)).unwrap();
|
||||
voice.events.push(eid);
|
||||
}
|
||||
let mut top = StaffInstance::new(top_instance, top_staff);
|
||||
top.voices.push(voice);
|
||||
for m in 0..MEASURES {
|
||||
top.measures.push(Measure {
|
||||
id: idc.mint(),
|
||||
start: TimeAnchor::Region {
|
||||
id: region_id,
|
||||
edge: RegionEdge::Start,
|
||||
offset: AnchorOffset::Musical(MusicalDuration(RationalTime::new(m, 1).unwrap())),
|
||||
},
|
||||
time_signature: None,
|
||||
explicit_number: Some((m + 1) as u32),
|
||||
number_visibility: Default::default(),
|
||||
});
|
||||
}
|
||||
|
||||
// The placeholder: a percussion clef, no voices, no measures.
|
||||
let drum_instance: StaffInstanceId = idc.mint();
|
||||
let mut drums = StaffInstance::new(drum_instance, drum_staff);
|
||||
drums.clef_sequence.push(epiphany_core::ClefChange {
|
||||
anchor: TimeAnchor::WallClock {
|
||||
time: WallClockTime(0),
|
||||
},
|
||||
clef: epiphany_core::Clef {
|
||||
shape: epiphany_core::ClefShape::Percussion,
|
||||
line: 3,
|
||||
octave_shift: 0,
|
||||
},
|
||||
});
|
||||
|
||||
let region = epiphany_core::Region {
|
||||
id: region_id,
|
||||
time_model: RegionTimeModel::Metric(MetricTimeModel::default()),
|
||||
content: RegionContent::StaffBased(StaffBasedContent {
|
||||
staff_instances: vec![top, drums],
|
||||
..Default::default()
|
||||
}),
|
||||
time_extent: TimeExtent {
|
||||
start: TimeAnchor::WallClock {
|
||||
time: WallClockTime(0),
|
||||
},
|
||||
end: TimeAnchor::WallClock {
|
||||
time: WallClockTime(120_000_000),
|
||||
},
|
||||
},
|
||||
staff_extent: StaffExtent {
|
||||
staves: vec![top_staff, drum_staff],
|
||||
},
|
||||
local_tempo_map: None,
|
||||
permits_spanning_slurs: false,
|
||||
};
|
||||
|
||||
let staff = |id: StaffId, name: &str, clef: epiphany_core::Clef| Staff {
|
||||
id,
|
||||
name: String::from(name),
|
||||
abbreviation: None,
|
||||
instrument,
|
||||
default_staff_lines: StaffLineConfiguration::default(),
|
||||
group: None,
|
||||
default_clef: clef,
|
||||
};
|
||||
let mut score = Score::empty(idc.clone());
|
||||
score.identity = idc;
|
||||
score.instruments = vec![epiphany_core::Instrument::new(
|
||||
instrument,
|
||||
String::from("Ensemble"),
|
||||
)];
|
||||
score.staves = vec![
|
||||
staff(top_staff, "Melody", epiphany_core::Clef::treble()),
|
||||
staff(
|
||||
drum_staff,
|
||||
"Drums",
|
||||
epiphany_core::Clef {
|
||||
shape: epiphany_core::ClefShape::Percussion,
|
||||
line: 3,
|
||||
octave_shift: 0,
|
||||
},
|
||||
),
|
||||
];
|
||||
score.events = arena;
|
||||
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
|
||||
|
|
@ -613,6 +849,55 @@ mod tests {
|
|||
assert_eq!(s.cross_cutting.slurs.len(), 1, "a slur over the high notes");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn percussion_placeholder_staff_is_invariant_clean_and_glyphless_below() {
|
||||
use epiphany_layout_ir::{to_constrained, to_logical, VerticalBandKind};
|
||||
let s = percussion_placeholder_staff(1);
|
||||
let v = check_invariants(&s);
|
||||
assert!(v.is_empty(), "percussion fixture has violations: {v:?}");
|
||||
let c = to_constrained(&to_logical(&s));
|
||||
let glyphless: Vec<_> = c
|
||||
.vertical_bands
|
||||
.iter()
|
||||
.filter(|b| matches!(b.kind, VerticalBandKind::Staff(_)))
|
||||
.filter(|b| !c.glyphs.iter().any(|g| g.vertical_band == b.id))
|
||||
.collect();
|
||||
assert_eq!(
|
||||
glyphless.len(),
|
||||
1,
|
||||
"exactly one staff band owns no glyph (the percussion placeholder)"
|
||||
);
|
||||
assert!(
|
||||
glyphless[0].members.is_empty(),
|
||||
"and therefore no band members either"
|
||||
);
|
||||
assert!(
|
||||
c.strokes
|
||||
.iter()
|
||||
.filter(|st| st.vertical_band == glyphless[0].id)
|
||||
.count()
|
||||
>= 5,
|
||||
"but it does own its staff lines"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn two_staff_wrapping_pressure_is_invariant_clean_and_front_loaded() {
|
||||
let s = two_staff_wrapping_pressure(1);
|
||||
let v = check_invariants(&s);
|
||||
assert!(
|
||||
v.is_empty(),
|
||||
"wrapping two-staff fixture has violations: {v:?}"
|
||||
);
|
||||
assert_eq!(s.staves.len(), 2, "two staves");
|
||||
assert_eq!(s.events.len(), 96, "12 measures x 4 beats x 2 staves");
|
||||
assert_eq!(
|
||||
s.canvas.regions[0].staff_instances().len(),
|
||||
2,
|
||||
"two staff instances in ONE region -- so both staves share every system"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn three_staff_close_content_is_invariant_clean_and_asymmetric() {
|
||||
let s = three_staff_close_content(1);
|
||||
|
|
|
|||
|
|
@ -306,13 +306,13 @@ that was an implementation defect, not a definition defect.
|
|||
| "Content extent" was ambiguous | **clarify (editorial)** — `req:qmc:vertical` now spells out that content extent means every primitive the band owns, each attributed by its declared `vertical_band`, and not the band's glyph `members`. Before primitive band ownership this reading was arguably unimplementable, which is why the defect survived. The stale rationale (claiming the vertical spring solve is deferred) is refreshed, and the inter-system half of the axis is recorded as a genuine trade-off against `page_fill_efficiency`, not a defect | quality_metric_catalog §`vertical_density_penalty` rationale | — |
|
||||
| Solve read the gap from a constructor | **fix** — the inter-staff solve now targets the `preferred_height` of the `InterStaffGap` band `to_constrained` emitted for that staff pair, not `VerticalBand::inter_staff_gap`'s default. The band is now a height model: a region declaring a wider gap gets one, and the solve and the metric agree by construction rather than by both calling the same constructor | — (behavioural, within `req:layoutir:vertical-bands`) | `epiphany-engrave` (`casting.rs`) |
|
||||
|
||||
**Version movements.** None. Quality Metric Catalog stays **0.2.0** (formula,
|
||||
units, anchor, normalization unchanged — a clarification and a rationale refresh
|
||||
are not a definition change; contrast P12-I12, which redefined
|
||||
`spacing_distortion`'s unit and did move the version). Core spec unchanged.
|
||||
Operation Catalog 0.7.0, Binary Format 0.6.0 unchanged. **No layout change and no
|
||||
render-golden churn** — this is measurement-only, as the ENGRAVER_VERSION staying
|
||||
at 11 records.
|
||||
**Version movements.** Quality Metric Catalog **0.2.0 → 0.3.0** — see the review
|
||||
follow-up below. (The conformance fix above needed none on its own: formula,
|
||||
anchor, and normalization were unchanged, and a clarification is not a definition
|
||||
change. The follow-up's per-realization unit count *is* one, so the version moves
|
||||
with it.) Core spec unchanged. Operation Catalog 0.7.0, Binary Format 0.6.0
|
||||
unchanged. **No layout change and no render-golden churn** — measurement-only, as
|
||||
`ENGRAVER_VERSION` staying at 11 records.
|
||||
|
||||
**Deferred (documented, not open candidates).** Staff-less content placed
|
||||
*between* two staves would hold still while the lower staff descends away from
|
||||
|
|
@ -320,3 +320,39 @@ it. No primitive can name an `InterStaffGap` band today (`band_of` yields a staf
|
|||
band or the region's margin band), so this is unreachable rather than latent;
|
||||
building the machinery now would be speculative. Named in
|
||||
`epiphany-engrave/DECISIONS.md`.
|
||||
|
||||
### Review follow-up (2026-07-09) — two more glyph-members assumptions
|
||||
|
||||
An adversarial review of the fix above found the correction incomplete. Both
|
||||
findings were right, and the second falsified a comment I had written in the same
|
||||
commit.
|
||||
|
||||
| Item | Disposition | Spec locus | Consumer |
|
||||
|---|---|---|---|
|
||||
| Region staff bands identified by glyph `members` | **fix (conformance)** — `vertical_raw` measured content over all primitives but still decided *which* staff bands belong to a region by glyph membership, reintroducing the assumption the change exists to shed. A staff band is allowed to own no glyphs: `to_constrained` emits one per staff of the region regardless, and a percussion-clef staff (no bundled glyph → traced anchor stroke) with no notes owns only its staff-line strokes. Region membership now comes from content presence in one of the region's systems — a staff band is per-(staff, region), so this identifies it exactly. Locked by the new `percussion_placeholder_staff` fixture and `a_staff_band_owning_no_glyphs_still_contributes_its_gap` | — (conformance to `req:qmc:vertical`) | `epiphany-engrave` (`quality.rs::vertical_raw`) |
|
||||
| Only the first realizing system measured | **fix + catalog definition move (0.3.0)** — the code measured one system per gap band, justified by a comment claiming rigid system translation makes every realization agree. The inter-staff solve had *just* falsified that: it sizes each system's gaps from that system's own content. `req:qmc:vertical` now counts **one unit per realization**, matching how realized inter-system gaps were already counted. Locked by the new `two_staff_wrapping_pressure` fixture (staff-line gap 15.93 in the pressured system, 7.87 in the slack one) and `inter_staff_gaps_are_measured_in_every_system_that_realizes_them` | quality_metric_catalog §`vertical_density_penalty` (`req:qmc:vertical`), **0.2.0 → 0.3.0** | `epiphany-engrave` |
|
||||
|
||||
Both fixes are mutation-verified: reverting to glyph-member region identification
|
||||
scores `4.8e-7` on the percussion fixture, and reverting to first-system-only
|
||||
scores `1.3e-7` on the wrapping fixture — each ~0 where the corrected axis reports
|
||||
real deviation.
|
||||
|
||||
**What the per-realization count exposes.** `two_staff_wrapping_pressure` now
|
||||
scores `vertical_density_penalty` **0.739**: its pressured system solves to the
|
||||
declared gap exactly (≈0 deviation), while its slack system sits at ≈5 staff
|
||||
spaces of content gap against a preferred 2.0. The axis is symmetric — a gap
|
||||
wider than preferred is sprawl exactly as a narrower one is crowding — and the
|
||||
inter-staff solve **only expands, never compresses**. That deferral
|
||||
(`epiphany-engrave/DECISIONS.md`, "compressing an OVER-wide fixed gap toward
|
||||
preferred… rarely wanted") is hereby promoted from *rarely wanted* to
|
||||
*measurably wrong*: any un-pressured multi-staff system reports honest sprawl
|
||||
until the solve can pull staves together. Named, not fixed — compression is a
|
||||
layout change (golden churn, `ENGRAVER_VERSION` move), not a measurement one.
|
||||
|
||||
**Adjacent finding, not fixed here.** `Staff::default_clef` is never consulted by
|
||||
the projection: `to_constrained` takes the active clef from the staff instance's
|
||||
`clef_sequence` and falls back to `Clef::default()` (treble). A bass-clef staff
|
||||
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.
|
||||
|
|
|
|||
Binary file not shown.
|
|
@ -226,7 +226,7 @@
|
|||
{\Large\scshape\color{epiphanyslate}Quality Metric Catalog}\\[6pt]
|
||||
{\large\itshape\color{epiphanyslate}A companion to the Core Specification}\\[14pt]
|
||||
{\color{epiphanygold}\rule{3in}{0.8pt}}\\[24pt]
|
||||
{\normalsize\color{epiphanyink}Version 0.2.0 --- Phase 3 (the normative metric set: formal definitions, normalization, weights, tier thresholds, profile registry; \texttt{spacing\_distortion} scoped to rhythmic columns)}\\[4pt]
|
||||
{\normalsize\color{epiphanyink}Version 0.3.0 --- Phase 3 (the normative metric set: formal definitions, normalization, weights, tier thresholds, profile registry; \texttt{spacing\_distortion} scoped to rhythmic columns; \texttt{vertical\_density\_penalty} counted per realization)}\\[4pt]
|
||||
{\small\color{epiphanyslate}Normative for the metrics and thresholds it defines}
|
||||
\vfill
|
||||
\end{titlepage}
|
||||
|
|
@ -246,7 +246,7 @@ deliver ``per-metric normalization functions mapping raw measurements to
|
|||
metric thresholds, and the formal definition of each quality metric in the
|
||||
normative metric set.''
|
||||
|
||||
This companion (v0.2.0) delivers all four chartered items, plus two small
|
||||
This companion (v0.3.0) delivers all four chartered items, plus two small
|
||||
registries the core specification names but defers here:
|
||||
|
||||
\begin{itemize}
|
||||
|
|
@ -868,10 +868,15 @@ inter-system gaps realized far from the spacing the band model asked for.
|
|||
|
||||
\begin{requirement}
|
||||
\label{req:qmc:vertical}
|
||||
\textbf{Contributing units:} vertical bands of $C$ of kind
|
||||
\texttt{InterStaffGap} or \texttt{InterSystemGap} with preferred height
|
||||
$p > 0$ that are realized in $L$ (the adjacent content they separate was
|
||||
laid out).
|
||||
\textbf{Contributing units:} each \emph{realization} in $L$ of a vertical
|
||||
band of $C$ of kind \texttt{InterStaffGap} or \texttt{InterSystemGap} with
|
||||
preferred height $p > 0$ (a band is realized wherever the adjacent content
|
||||
it separates was laid out). A band realized in several systems contributes
|
||||
\textbf{one unit per system}, not one per band: a vertical solve sizes each
|
||||
system's gaps from that system's own content, so one band's realized height
|
||||
genuinely differs from system to system. This matches how the realized
|
||||
inter-system gaps are already counted --- one unit per adjacent system pair
|
||||
on a page.
|
||||
|
||||
\textbf{Raw measurement:} per unit, with $r \ge 0$ the realized vertical
|
||||
separation between the adjacent content extents the band separates
|
||||
|
|
@ -918,6 +923,18 @@ The inter-system half of the axis is a genuine trade-off, not a defect: the
|
|||
vertical justification pass deliberately stretches inter-system gaps past
|
||||
preferred to fill a non-final page, trading this axis against
|
||||
\texttt{page\_fill\_efficiency}. Both axes are reported; neither is wrong.
|
||||
|
||||
\textbf{Per-realization units} (0.3.0) followed from the same landing. While
|
||||
the reference pipeline preserved constrained $y$ verbatim, a band had exactly
|
||||
one realized height and the distinction was moot. Once the inter-staff solve
|
||||
began sizing each system's gaps from that system's own content, a band's
|
||||
realized height became per-system, and counting one unit per band would let a
|
||||
well-solved system average away a badly spaced one. The axis is deliberately
|
||||
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.
|
||||
\end{rationale}
|
||||
|
||||
\section{\texttt{system\_break\_penalty}}
|
||||
|
|
@ -1366,6 +1383,22 @@ than inventing a parallel one.
|
|||
irregularity. Normalization anchor, orientation, range, thresholds, and
|
||||
the eight other axes are unchanged; the optical-spacing open question
|
||||
(duration-proportional spacing) stays open. Batch item P12-I12. \\
|
||||
\midrule
|
||||
\today & \hyperref[sec:metrics:vertical]{\texttt{vertical\_density\_penalty}}
|
||||
& 0.3.0 --- Count \textbf{one contributing unit per realization} of an
|
||||
\texttt{InterStaffGap} band rather than one per band, matching how realized
|
||||
inter-system gaps were already counted. The inter-staff vertical solve sizes
|
||||
each system's gaps from that system's own content, so a band's realized
|
||||
height is per-system; one unit per band let a well-solved system average away
|
||||
a badly spaced one. Also \emph{clarifies} (no semantic change) that the
|
||||
``content extents'' the raw measurement compares are every primitive the
|
||||
adjacent band owns --- glyphs, strokes, and curves, each attributed by its
|
||||
declared \texttt{vertical\_band} --- and not the band's glyph
|
||||
\texttt{members}, a reading that was arguably unimplementable before
|
||||
primitive band ownership was ratified. Raw formula, normalization anchor,
|
||||
orientation, range, thresholds, and the eight other axes are unchanged. The
|
||||
reference implementation's non-conforming glyph-only measurement is fixed
|
||||
alongside. \\
|
||||
\bottomrule
|
||||
\end{longtable}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue