The inter-staff gap band becomes a height model; fix a non-conforming metric
Two items the inter-staff solve deferred turned out to be one thing -- and it was
not the "metric-vs-solver tension" I filed it as. The catalog was right; the
engraver was non-conforming.
req:qmc:vertical has always defined the realized inter-staff gap as the separation
"between the adjacent CONTENT EXTENTS the band separates". vertical_raw measured
the separation between the two bands' glyph `members`, because until primitive
band ownership (efaebb9) a band listed no strokes or curves to own. A staff's
outermost ink is usually not a glyph. On two_staff_close_content the solve cleared
the declared 2.0 gap exactly, while the glyph-ink gap was 5.06 -- so the axis
reported |5.06-2|/2 = 1.53, saturated to 1.0, and fired a Standard-tier floor
warning on a correct layout. The metric was charging the solver for the ledger and
slur ink it had made room for. Axis now reads 2.7e-7; the warning is gone.
Two design calls worth naming:
- The geometry is read back from the BAKED output, not from the solve's own
staff_ext. Reading back solver intent would make the axis circular and blind
to exactly the bug class that bit twice this week; now a shift the bake fails
to apply to some primitive class surfaces as a real deviation. CastLayout
gained stroke_system/curve_system for it -- a stroke carries no spring slot,
so system_of_slot cannot answer for it.
- The solve now targets the preferred_height of the InterStaffGap band
to_constrained emitted for that staff pair, not VerticalBand::inter_staff_gap's
default. That is what makes the band a height model rather than a constant:
solve and metric agree by construction, not by both calling one constructor.
NO version move. Formula, contributing units, anchor, and normalization are
unchanged -- only a wrong measurement was. This is the P12-I11 precedent
(engrave-side resolution), not P12-I12 (which redefined spacing_distortion's unit
and did move the catalog). The catalog gains a clarification of what "content
extent" means, since before band ownership that reading was arguably
unimplementable, which is why the defect survived. Its stale rationale (still
claiming the vertical spring solve is deferred) is refreshed, and the axis's
inter-system half is recorded as a genuine trade-off against page_fill_efficiency
rather than a defect, so it is not re-filed as a bug.
Measurement-only: no layout change, ENGRAVER_VERSION stays 11, zero golden churn.
Mutation-verified: reverting vertical_raw to glyph-only measurement scores 1 and
fails the new assertion. Gate green, QMC PDF rebuilds clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
ab411f4cae
commit
43215c7d67
|
|
@ -772,15 +772,13 @@ a real band.
|
|||
|
||||
**Known gaps (reviewed, not bugs today).**
|
||||
|
||||
- `vertical_density_penalty` **saturates at 1.0** on the two-staff fixture. The
|
||||
solve targets *content* extents; the metric scores the realized gap against the
|
||||
band model's *preferred* height (2 staff spaces). The separation the colliding
|
||||
ledgers and slur require dwarfs that, so the axis pins and its floor diagnostic
|
||||
fires. This is the same metric-vs-solver tension as `casting_off_quality` under
|
||||
justification and inter-system density under vertical justification: a catalog
|
||||
refinement scoring only the EXCESS beyond the content-required minimum is the
|
||||
deferred follow-up. Asserted, not silently tolerated, in
|
||||
`inter_staff_solve_separates_colliding_staves`.
|
||||
- ~~`vertical_density_penalty` **saturates at 1.0** on the two-staff fixture.~~
|
||||
**RESOLVED** (see "The gap band is a height model" below). It was not the
|
||||
metric-vs-solver tension I first filed it as, and needed no catalog refinement:
|
||||
the engraver was simply **non-conforming**. The catalog always defined the
|
||||
realized gap over the *content extents* the band separates; the implementation
|
||||
measured the band's glyph `members`, because until primitive band ownership was
|
||||
ratified a band listed no strokes or curves to own.
|
||||
- **Staff-less content takes no staff shift** — margin-band glyphs, and a volta
|
||||
bracket whose repeat spans several staves. It stays put while the staves below
|
||||
it descend, which is right for content that sits above the top staff (the top
|
||||
|
|
@ -791,10 +789,10 @@ a real band.
|
|||
above. What remains undecided is genuinely staff-less content placed *between*
|
||||
two staves: it holds still while the lower staff descends away from it. That
|
||||
wants a height model for the inter-staff gap band, not an attribution rule.
|
||||
- The preferred gap is read from `VerticalBand::inter_staff_gap(VerticalBandId(0))`
|
||||
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 preferred gap is read from `VerticalBand::inter_staff_gap(VerticalBandId(0))`
|
||||
rather than from the region's *declared* inter-staff band.~~ **RESOLVED**: the
|
||||
solve now reads the `InterStaffGap` band `to_constrained` emitted for that
|
||||
staff pair, the same band the metric scores against.
|
||||
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
|
||||
|
|
@ -842,3 +840,62 @@ 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.
|
||||
|
||||
## The gap band is a height model (Push 3 residue, 2026-07-09)
|
||||
|
||||
Two items deferred by the inter-staff solve — a saturated
|
||||
`vertical_density_penalty` and a preferred gap read from a constructor rather
|
||||
than the region's declared band — turned out to be one thing, and neither was
|
||||
the "metric-vs-solver tension" I filed them as.
|
||||
|
||||
**The solve reads the declared band.** Per system, per adjacent staff pair, the
|
||||
gap it targets is the `preferred_height` of the `InterStaffGap` band
|
||||
`to_constrained` emitted for that pair (`inter_staff_gap_id(region, g)`, gap `g`
|
||||
separating the region's staves `g-1` and `g`), not `VerticalBand::inter_staff_gap`'s
|
||||
default. That is what makes the band a *height model* rather than a constant: a
|
||||
region declaring a wider gap gets one, and the metric — which scores against the
|
||||
same declared band — agrees with the solve by construction rather than because
|
||||
both call the same constructor. Every staff of a region carries content in every
|
||||
system of that region (its staff lines are per-staff strokes, split into each
|
||||
system), so the staves present in a system are the region's full staff order and
|
||||
the window index is the gap index.
|
||||
|
||||
**The metric was non-conforming, and the catalog was right.** `req:qmc:vertical`
|
||||
has always defined the realized gap as the separation "between the adjacent
|
||||
**content extents** the band separates". `vertical_raw` measured the separation
|
||||
between the two bands' glyph `members` — because until
|
||||
`req:layoutir:primitive-band-ownership` landed, a band listed no strokes or curves
|
||||
to own. A staff's outermost ink is usually not a glyph: on
|
||||
`two_staff_close_content` the solve cleared the declared 2.0 gap exactly (content
|
||||
gap 2.0), while the glyph-ink gap was **5.06**, giving `|5.06 - 2|/2 = 1.53`,
|
||||
clamped to a saturated **1.0** — and a Standard-tier floor warning on a layout
|
||||
that was correct. The metric was charging the solver for the ledger and slur ink
|
||||
it had made room for.
|
||||
|
||||
`vertical_raw` now measures each staff band's full content — glyphs, strokes, and
|
||||
curves, each attributed by its declared `vertical_band` — and the axis reads
|
||||
`2.7e-7` on that fixture. Two consequences worth stating:
|
||||
|
||||
- **No catalog version move.** The formula, contributing units, anchor, and
|
||||
normalization are unchanged; only a wrong measurement was. This is the P12-I11
|
||||
precedent (engrave-side resolution), not P12-I12 (a definition defect). The
|
||||
catalog gains a *clarification* of what "content extent" means, plus a
|
||||
rationale refresh — the v0.1 rationale still claimed the vertical spring solve
|
||||
was deferred.
|
||||
- **The geometry is read back from the BAKED output**, not from the solve's own
|
||||
`staff_ext`. Reading back the solver's intent would make the axis circular and
|
||||
blind to exactly the bug class that bit twice: a shift the bake fails to apply
|
||||
to some primitive class now surfaces as a real deviation. `CastLayout` gained
|
||||
`stroke_system` / `curve_system` for this (a stroke carries no spring slot, so
|
||||
`system_of_slot` cannot answer for it).
|
||||
|
||||
**Still a real trade-off, not a defect:** the axis's *inter-system* half. Vertical
|
||||
justification deliberately stretches inter-system gaps past preferred to fill a
|
||||
non-final page, trading this axis against `page_fill_efficiency`. Both are
|
||||
reported; neither is wrong. Recorded in the catalog rationale so it is not
|
||||
re-filed as a bug.
|
||||
|
||||
**Still open:** staff-less content placed *between* two staves would hold still
|
||||
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.
|
||||
|
|
|
|||
|
|
@ -73,12 +73,13 @@ use std::collections::{BTreeMap, BTreeSet};
|
|||
|
||||
use epiphany_core::{StaffId, TypedObjectId};
|
||||
use epiphany_layout_ir::{
|
||||
continuation_instance_key, is_barline_glyph, is_rigid_width_stroke, synthesized_layout_id,
|
||||
BreakClass, BreakKind, ConstrainedLayoutIR, Curve, DecisionSource, EngravingDecision,
|
||||
EngravingDecisionKind, EngravingOverrideId, GlyphObject, GlyphObjectId, LayoutConstraint,
|
||||
LayoutObjectId, Margins, Point, Provenance, Rect, ResolvedGlyph, ResolvedMeasure, ResolvedPage,
|
||||
ResolvedStaff, ResolvedSystem, Size2D, SpringSlotId, StaffSpace, Stroke, SynthesisInstanceKey,
|
||||
SynthesisKind, SynthesisRegistryId, VerticalBand, VerticalBandId, VerticalBandKind,
|
||||
continuation_instance_key, inter_staff_gap_id, is_barline_glyph, is_rigid_width_stroke,
|
||||
synthesized_layout_id, BreakClass, BreakKind, ConstrainedLayoutIR, Curve, DecisionSource,
|
||||
EngravingDecision, EngravingDecisionKind, EngravingOverrideId, GlyphObject, GlyphObjectId,
|
||||
LayoutConstraint, LayoutObjectId, Margins, Point, Provenance, Rect, ResolvedGlyph,
|
||||
ResolvedMeasure, ResolvedPage, ResolvedStaff, ResolvedSystem, Size2D, SpringSlotId, StaffSpace,
|
||||
Stroke, SynthesisInstanceKey, SynthesisKind, SynthesisRegistryId, VerticalBand, VerticalBandId,
|
||||
VerticalBandKind,
|
||||
};
|
||||
|
||||
use crate::owning_glyph;
|
||||
|
|
@ -192,6 +193,13 @@ pub(crate) struct CastLayout {
|
|||
/// ranges over (a slot absent here was claimed by no region and its glyphs
|
||||
/// belong to no per-system aggregate).
|
||||
pub system_of_slot: BTreeMap<SpringSlotId, usize>,
|
||||
/// The system each baked stroke landed in, parallel to `strokes` (including
|
||||
/// the appended continuation segments). A stroke carries no spring slot, so
|
||||
/// `system_of_slot` cannot answer for it; the casting pass records what it
|
||||
/// already knew. `None`: claimed by no region.
|
||||
pub stroke_system: Vec<Option<usize>>,
|
||||
/// The system each baked curve landed in, parallel to `curves`.
|
||||
pub curve_system: Vec<Option<usize>>,
|
||||
/// The region each system slices, indexed by global system index (the
|
||||
/// per-region grouping the casting-off quality metrics aggregate by).
|
||||
pub region_of_system: Vec<usize>,
|
||||
|
|
@ -763,14 +771,39 @@ 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 band
|
||||
// model's preferred inter-staff gap. `staff_shift[(system, staff)]` is the
|
||||
// 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.
|
||||
let preferred_gap = VerticalBand::inter_staff_gap(VerticalBandId(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.
|
||||
//
|
||||
// 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 —
|
||||
// its staff lines are per-staff strokes, split into each system — so the
|
||||
// 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 mut staff_shift: BTreeMap<(usize, StaffId), f32> = BTreeMap::new();
|
||||
for s in 0..systems.len() {
|
||||
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 id = inter_staff_gap_id(region_layout_id, gap_index);
|
||||
input
|
||||
.vertical_bands
|
||||
.iter()
|
||||
.find(|band| band.id == id)
|
||||
.map(|band| band.preferred_height.0)
|
||||
.unwrap_or(default_gap)
|
||||
};
|
||||
let mut staves: Vec<(StaffId, (f32, f32))> = staff_ext
|
||||
.iter()
|
||||
.filter(|((sys, _), _)| *sys == s)
|
||||
|
|
@ -787,13 +820,13 @@ pub(crate) fn cast_off(
|
|||
key(b.0, b.1).total_cmp(&key(a.0, a.1)).then(a.0.cmp(&b.0))
|
||||
});
|
||||
let mut shift = 0.0_f32;
|
||||
for w in staves.windows(2) {
|
||||
for (g, w) in staves.windows(2).enumerate() {
|
||||
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 - gap).max(0.0);
|
||||
shift += (preferred_gap(g + 1) - gap).max(0.0);
|
||||
staff_shift.insert((s, lower), shift);
|
||||
}
|
||||
if staves.len() == 1 {
|
||||
|
|
@ -967,6 +1000,11 @@ pub(crate) fn cast_off(
|
|||
let mut staff_marks: BTreeMap<(usize, StaffId), StaffAgg> = BTreeMap::new();
|
||||
let mut strokes: Vec<Stroke> = Vec::with_capacity(spaced_strokes.len());
|
||||
let mut continuations: Vec<Stroke> = Vec::new();
|
||||
// The system each baked stroke landed in, parallel to `strokes` (a quality
|
||||
// metric measures a system's realized per-staff content extents, and a
|
||||
// stroke carries no spring slot to look one up with).
|
||||
let mut stroke_system: Vec<Option<usize>> = Vec::with_capacity(spaced_strokes.len());
|
||||
let mut continuation_system: Vec<Option<usize>> = Vec::new();
|
||||
for (si, ((source, spaced), fate)) in input
|
||||
.strokes
|
||||
.iter()
|
||||
|
|
@ -990,6 +1028,7 @@ pub(crate) fn cast_off(
|
|||
mark_staff(&mut staff_marks, *s, staff, &stroke);
|
||||
}
|
||||
strokes.push(stroke);
|
||||
stroke_system.push(*sys);
|
||||
}
|
||||
StrokeFate::Split(segments) => {
|
||||
for (k, (s, from, to)) in segments.iter().enumerate() {
|
||||
|
|
@ -1022,14 +1061,17 @@ pub(crate) fn cast_off(
|
|||
}
|
||||
if k == 0 {
|
||||
strokes.push(stroke);
|
||||
stroke_system.push(Some(*s));
|
||||
} else {
|
||||
continuations.push(stroke);
|
||||
continuation_system.push(Some(*s));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
strokes.extend(continuations);
|
||||
stroke_system.extend(continuation_system);
|
||||
|
||||
// Curves: a curve that fits in one system is translated whole by that
|
||||
// system's placement (or left in the spaced frame if no region claimed it).
|
||||
|
|
@ -1040,6 +1082,8 @@ pub(crate) fn cast_off(
|
|||
// split stroke's are.
|
||||
let mut curves: Vec<Curve> = Vec::with_capacity(spaced_curves.len());
|
||||
let mut curve_continuations: Vec<Curve> = Vec::new();
|
||||
let mut curve_system: Vec<Option<usize>> = Vec::with_capacity(spaced_curves.len());
|
||||
let mut curve_continuation_system: Vec<Option<usize>> = Vec::new();
|
||||
for (ci, (curve, fate)) in spaced_curves.iter().zip(&curve_fates).enumerate() {
|
||||
let curve_staff = curve_staff_of[ci];
|
||||
// A slur has no intra-slot structure, so its control points map straight
|
||||
|
|
@ -1060,6 +1104,7 @@ pub(crate) fn cast_off(
|
|||
p3,
|
||||
..curve.clone()
|
||||
});
|
||||
curve_system.push(*system);
|
||||
}
|
||||
CurveFate::Split(segments) => {
|
||||
for (k, (s, cp)) in segments.iter().enumerate() {
|
||||
|
|
@ -1089,14 +1134,17 @@ pub(crate) fn cast_off(
|
|||
};
|
||||
if k == 0 {
|
||||
curves.push(segment);
|
||||
curve_system.push(Some(*s));
|
||||
} else {
|
||||
curve_continuations.push(segment);
|
||||
curve_continuation_system.push(Some(*s));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
curves.extend(curve_continuations);
|
||||
curve_system.extend(curve_continuation_system);
|
||||
|
||||
// ---- The resolved page tree ---------------------------------------------
|
||||
let resolved_systems: Vec<ResolvedSystem> = systems
|
||||
|
|
@ -1160,6 +1208,8 @@ pub(crate) fn cast_off(
|
|||
system_start_slots,
|
||||
page_start_slots,
|
||||
system_of_slot,
|
||||
stroke_system,
|
||||
curve_system,
|
||||
region_of_system: systems.iter().map(|plan| plan.region).collect(),
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2362,15 +2362,27 @@ mod tests {
|
|||
report.metric_vector.collision_penalty.0, 0.0,
|
||||
"the separated staves collide nowhere"
|
||||
);
|
||||
// The solve targets CONTENT extents, while `vertical_density_penalty`
|
||||
// measures the realized gap against the band model's *preferred* height
|
||||
// (2 staff spaces). The separation this fixture requires is far larger,
|
||||
// so the deviation-from-preferred axis saturates at 1.0 and its floor
|
||||
// diagnostic fires. Scoring only the EXCESS beyond the content-required
|
||||
// minimum is the deferred catalog refinement (see DECISIONS.md).
|
||||
assert_eq!(
|
||||
report.metric_vector.vertical_density_penalty.0, 1.0,
|
||||
"the required separation saturates the deviation-from-preferred axis"
|
||||
// A correctly separated system scores ~0 on the deviation-from-preferred
|
||||
// axis: the solve expands each gap until the staves' CONTENT clears the
|
||||
// declared `InterStaffGap` band's preferred height, and the metric reads
|
||||
// that same declared band against the same content. It used to saturate
|
||||
// at 1.0 here -- the metric measured the gap between the staves' *glyph*
|
||||
// ink while the solve targeted their full content, so the ledger and slur
|
||||
// ink the solve had correctly accounted for was rescored as crowding, and
|
||||
// a Standard-tier floor warning fired on a correct layout.
|
||||
let density = report.metric_vector.vertical_density_penalty.0;
|
||||
assert!(
|
||||
density < 1e-4,
|
||||
"a correctly separated system deviates from its declared gap by ~0: {density}"
|
||||
);
|
||||
assert!(
|
||||
!report.warnings.iter().any(|w| matches!(
|
||||
w.kind,
|
||||
epiphany_layout_ir::SolverWarningKind::QualityFloorApproached {
|
||||
metric: epiphany_layout_ir::QualityMetricKind::VerticalDensity
|
||||
}
|
||||
)),
|
||||
"and earns no vertical-density floor warning"
|
||||
);
|
||||
// A no-pressure single-staff score has no inter-staff pair, so it is
|
||||
// untouched — its goldens stay byte-stable (see the render-svg suite).
|
||||
|
|
|
|||
|
|
@ -64,9 +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, GlyphObject, GlyphObjectId,
|
||||
QualityMetricVector, SolverWarning, SolverWarningKind, SpringSlotId, VerticalBand,
|
||||
VerticalBandId, VerticalBandKind,
|
||||
inter_staff_gap_id, ConstrainedLayoutIR, Curve, GlyphObjectId, QualityMetricVector,
|
||||
SolverWarning, SolverWarningKind, SpringSlotId, VerticalBand, VerticalBandId, VerticalBandKind,
|
||||
};
|
||||
|
||||
use crate::casting::{CastLayout, PageGeometry};
|
||||
|
|
@ -330,17 +329,61 @@ fn vertical_raw(input: &ConstrainedLayoutIR, cast: &CastLayout, census: &SystemC
|
|||
let mut per_unit: Vec<f64> = Vec::new();
|
||||
|
||||
// --- InterStaffGap bands declared by the constrained input -------------
|
||||
let index_of: BTreeMap<GlyphObjectId, usize> = input
|
||||
.glyphs
|
||||
.iter()
|
||||
.enumerate()
|
||||
.map(|(index, glyph)| (GlyphObject::id(glyph), index))
|
||||
.collect();
|
||||
// The realized gap between two staves is measured over their FULL content —
|
||||
// every glyph, stroke, and curve the band owns — not over the band's glyph
|
||||
// `members` alone. A staff's ink is mostly not glyphs: ledger lines, stems,
|
||||
// and slurs routinely reach further from the staff than any notehead, and
|
||||
// the inter-staff solve separates staves until their *content* clears the
|
||||
// declared gap. Measuring glyphs only scored a correctly separated system
|
||||
// against a gap it never targeted, reporting the ledger and slur ink the
|
||||
// solve had already accounted for as crowding.
|
||||
//
|
||||
// Ownership comes from each primitive's declared `vertical_band`
|
||||
// (`req:layoutir:primitive-band-ownership`); the geometry is read back from
|
||||
// the BAKED output, so a shift the bake failed to apply to some primitive
|
||||
// class surfaces here as a real deviation rather than hiding behind the
|
||||
// solver's own intent.
|
||||
let system_of_glyph = |index: usize| -> Option<usize> {
|
||||
cast.system_of_slot
|
||||
.get(&input.glyphs[index].horizontal_slot)
|
||||
.copied()
|
||||
};
|
||||
let mut content: BTreeMap<(usize, VerticalBandId), (f64, f64)> = BTreeMap::new();
|
||||
{
|
||||
let mut add = |system: usize, band: VerticalBandId, lo: f64, hi: f64| {
|
||||
let entry = content
|
||||
.entry((system, band))
|
||||
.or_insert((f64::INFINITY, f64::NEG_INFINITY));
|
||||
entry.0 = entry.0.min(lo);
|
||||
entry.1 = entry.1.max(hi);
|
||||
};
|
||||
for (index, glyph) in input.glyphs.iter().enumerate() {
|
||||
if let Some(system) = system_of_glyph(index) {
|
||||
let ink = ink_box(cast, input, index);
|
||||
add(system, glyph.vertical_band, ink[1], ink[3]);
|
||||
}
|
||||
}
|
||||
for (index, stroke) in cast.strokes.iter().enumerate() {
|
||||
let Some(system) = cast.stroke_system[index] else {
|
||||
continue;
|
||||
};
|
||||
let half = f64::from(stroke.thickness.0.max(0.0)) * 0.5;
|
||||
let lo = f64::from(stroke.from.y.0.min(stroke.to.y.0)) - half;
|
||||
let hi = f64::from(stroke.from.y.0.max(stroke.to.y.0)) + half;
|
||||
add(system, stroke.vertical_band, lo, hi);
|
||||
}
|
||||
for (index, curve) in cast.curves.iter().enumerate() {
|
||||
let Some(system) = cast.curve_system[index] else {
|
||||
continue;
|
||||
};
|
||||
let half = f64::from(curve.thickness.0.max(0.0)) * 0.5;
|
||||
for point in curve.control_points() {
|
||||
let y = f64::from(point.y.0);
|
||||
add(system, curve.vertical_band, y - half, y + half);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
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-
|
||||
|
|
@ -350,28 +393,17 @@ fn vertical_raw(input: &ConstrainedLayoutIR, cast: &CastLayout, census: &SystemC
|
|||
continue;
|
||||
};
|
||||
let region_glyphs: BTreeSet<GlyphObjectId> = region.glyphs.iter().copied().collect();
|
||||
let mut staves: Vec<(f64, Vec<usize>)> = Vec::new();
|
||||
for band in &input.vertical_bands {
|
||||
if !matches!(band.kind, VerticalBandKind::Staff(_)) {
|
||||
continue;
|
||||
}
|
||||
if !band.members.iter().any(|id| region_glyphs.contains(id)) {
|
||||
continue;
|
||||
}
|
||||
let members: Vec<usize> = band
|
||||
.members
|
||||
.iter()
|
||||
.filter_map(|id| index_of.get(id).copied())
|
||||
.collect();
|
||||
let top_in_first = members
|
||||
.iter()
|
||||
.filter(|&&index| system_of_glyph(index) == Some(first_system))
|
||||
.map(|&index| ink_box(cast, input, index)[3])
|
||||
.fold(f64::NEG_INFINITY, f64::max);
|
||||
if top_in_first.is_finite() {
|
||||
staves.push((top_in_first, members));
|
||||
}
|
||||
}
|
||||
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));
|
||||
|
||||
|
|
@ -387,33 +419,18 @@ fn vertical_raw(input: &ConstrainedLayoutIR, cast: &CastLayout, census: &SystemC
|
|||
if preferred <= 0.0 || staves.len() <= gap {
|
||||
continue;
|
||||
}
|
||||
let upper = &staves[gap - 1].1;
|
||||
let lower = &staves[gap].1;
|
||||
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: BTreeSet<usize> = upper
|
||||
.iter()
|
||||
.filter_map(|&index| system_of_glyph(index))
|
||||
.filter(|system| {
|
||||
lower
|
||||
.iter()
|
||||
.any(|&index| system_of_glyph(index) == Some(*system))
|
||||
})
|
||||
.collect();
|
||||
let Some(&system) = common.iter().next() else {
|
||||
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 = upper
|
||||
.iter()
|
||||
.filter(|&&index| system_of_glyph(index) == Some(system))
|
||||
.map(|&index| ink_box(cast, input, index)[1])
|
||||
.fold(f64::INFINITY, f64::min);
|
||||
let lower_top = lower
|
||||
.iter()
|
||||
.filter(|&&index| system_of_glyph(index) == Some(system))
|
||||
.map(|&index| ink_box(cast, input, index)[3])
|
||||
.fold(f64::NEG_INFINITY, f64::max);
|
||||
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);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -280,3 +280,43 @@ inter-staff gap band — the missing piece behind both staff-less content placed
|
|||
extents while the metric scores the realized gap against the band's *preferred*
|
||||
height. Both are named in `epiphany-engrave/DECISIONS.md`; neither is a Pass-13
|
||||
ambiguity.
|
||||
|
||||
## Push-3 tranche (2026-07-09) — the inter-staff gap band as a height model
|
||||
|
||||
Residue of the Standard-tier inter-staff solve, closed. **No spec requirement
|
||||
changes and no version moves**: the Quality Metric Catalog was right and the
|
||||
engraver was non-conforming. Delivered alongside `efaebb9`/`ced4b72` (primitive
|
||||
band ownership), which is what made the fix expressible.
|
||||
|
||||
`req:qmc:vertical` has always defined the realized inter-staff gap as the
|
||||
separation "between the adjacent **content extents** the band separates".
|
||||
`engrave::quality::vertical_raw` measured the separation between the two bands'
|
||||
glyph `members` — because until `req:layoutir:primitive-band-ownership` landed, a
|
||||
band listed no strokes or curves to own. A staff's outermost ink is usually not a
|
||||
glyph. On `two_staff_close_content` the solve cleared the declared 2.0 gap exactly
|
||||
while the glyph-ink gap was 5.06, so the axis reported `|5.06 − 2|/2 = 1.53` →
|
||||
saturated **1.0**, and a Standard-tier `QualityFloorApproached` warning fired on a
|
||||
correct layout. Per §Conformance ("a solver that reports a `QualityMetricVector`
|
||||
computed by any function other than the ones defined here is non-conforming"),
|
||||
that was an implementation defect, not a definition defect.
|
||||
|
||||
| Item | Disposition | Spec locus | Consumer |
|
||||
|---|---|---|---|
|
||||
| `vertical_density_penalty` measured over glyph members | **fix (conformance), no catalog change** — `vertical_raw` now measures each staff band's full content (glyphs, strokes, curves), attributed by declared `vertical_band`, read back from the BAKED output rather than the solve's own extents (so a shift the bake drops surfaces as a real deviation instead of hiding behind solver intent). Axis on the pressure fixture: 1.0 → 2.7e-7; the floor warning is gone. Formula, contributing units, anchor, normalization all unchanged | quality_metric_catalog §`vertical_density_penalty` (`req:qmc:vertical`) — **clarification only** | `epiphany-engrave` (`quality.rs::vertical_raw`, `casting.rs::CastLayout::{stroke_system,curve_system}`) |
|
||||
| "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.
|
||||
|
||||
**Deferred (documented, not open candidates).** Staff-less content placed
|
||||
*between* two staves would hold still while the lower staff descends away from
|
||||
it. 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. Named in
|
||||
`epiphany-engrave/DECISIONS.md`.
|
||||
|
|
|
|||
Binary file not shown.
|
|
@ -881,6 +881,15 @@ separation between the adjacent content extents the band separates
|
|||
\]
|
||||
the axis raw value is the arithmetic mean over contributing units.
|
||||
|
||||
\textbf{Content extent} means \emph{every} primitive the adjacent band
|
||||
owns --- glyphs, strokes, and curves alike, each attributed by its declared
|
||||
\texttt{vertical\_band} (core spec
|
||||
\sectionsc{ConstrainedLayoutIR}, primitive band ownership) --- and not the
|
||||
band's glyph \texttt{members} alone. A staff's outermost ink is usually not
|
||||
a glyph: ledger lines, stems, and slurs reach past every notehead. A solver
|
||||
separates staves until their content clears the gap, so scoring it against
|
||||
its noteheads alone would charge it for the very ink it made room for.
|
||||
|
||||
\textbf{Normalization:} $R_{\mathrm{worst}} = 1.0$;
|
||||
$n = \min(1, \mathit{raw})$.
|
||||
\end{requirement}
|
||||
|
|
@ -889,11 +898,26 @@ $n = \min(1, \mathit{raw})$.
|
|||
A gap off by its own preferred size --- staves twice as far apart as asked,
|
||||
or fully collapsed --- is unambiguous vertical failure; proportional
|
||||
deviation makes one anchor serve both tight inter-staff gaps and wide
|
||||
inter-system gaps. The v0.1 reference pipeline preserves constrained $y$
|
||||
verbatim (the vertical spring solve is deferred), so realized gaps equal
|
||||
preferred gaps wherever bands are realized and the axis reports its
|
||||
honest near-zero; the definition is what makes a future vertical solve
|
||||
measurable.
|
||||
inter-system gaps.
|
||||
|
||||
The v0.1 reference pipeline preserved constrained $y$ verbatim, so realized
|
||||
gaps equalled preferred gaps wherever bands were realized and the axis
|
||||
reported its honest near-zero. The vertical spring solve has since landed
|
||||
(inter-staff gap renegotiation and inter-system vertical justification), and
|
||||
the axis now measures what it was defined to measure. The
|
||||
\textbf{content extent} clarification above is the lesson from that landing:
|
||||
the reference implementation read ``content extents'' as the band's glyph
|
||||
\texttt{members}, because until primitive band ownership was ratified a band
|
||||
listed no strokes or curves to own. A correctly separated two-staff system
|
||||
then scored a saturated $1.0$ and tripped the \textsc{Standard} floor
|
||||
warning --- the metric charging the solver for the ledger and slur ink it had
|
||||
correctly cleared. The formula, units, anchor, and normalization are
|
||||
unchanged; only a non-conforming measurement was.
|
||||
|
||||
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.
|
||||
\end{rationale}
|
||||
|
||||
\section{\texttt{system\_break\_penalty}}
|
||||
|
|
|
|||
Loading…
Reference in New Issue