Vertical justification review follow-up: document + test the metric trade-off
An adversarial review confirmed the vertical-justification geometry is sound on all axes (sign, no-overflow, no-collision, content_bottom, forced/overfull, bake consistency, determinism, single-page invariance), and found one real gap: the pass stretches inter-system gaps, which vertical_density_penalty measures — so justified pages score higher on that axis, undocumented and untested. This is the same metric-vs-justification tension as the horizontal casting_off note: page_fill drops to ~0 (the win) while the deviation-from-preferred density metric charges for the stretch. A sparse justified page (large per-gap stretch) is charged more — a defensible signal, though the current linear penalty over-charges a moderate uniform stretch. The catalog refinement (score only excess stretch, or measure gap uniformity) is the deferred follow-up. Documented in DECISIONS and pinned by a new test (vertical_justification_trades_page_fill_for_inter_system_density: a multi-page solve fills non-final pages, page_fill < 0.1, vertical_density > 0), so the interaction is no longer silent. No code/geometry change. clippy 0, 53 engrave tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
69e0430062
commit
0db005db9a
|
|
@ -617,10 +617,24 @@ horizontal justification (independent axes). `ENGRAVER_VERSION` 8 → 9.
|
|||
convention wants — a single-page score is therefore unchanged (its only page is
|
||||
the last), so every existing single-page golden is byte-identical. A page with
|
||||
one system has no inter-system gap to grow; an already-full or overfull page has
|
||||
no positive slack. Drives `page_fill_efficiency` to ~0 on justified pages (a
|
||||
non-final page now fills the height). Locked by
|
||||
`vertical_justification_fills_non_final_pages` (a small custom `PageGeometry`
|
||||
forces the multi-page path; the non-final page fills, the last stays ragged).
|
||||
no positive slack. Locked by `vertical_justification_fills_non_final_pages` (a
|
||||
small custom `PageGeometry` forces the multi-page path; the non-final page fills,
|
||||
the last stays ragged).
|
||||
|
||||
**Quality-metric trade-off (honest, tested).** Vertical justification drives
|
||||
`page_fill_efficiency` to ~0 on justified pages (they fill the height — the
|
||||
point), but the same stretch grows their inter-system gaps beyond the band
|
||||
model's preferred height, which `vertical_density_penalty` measures directly
|
||||
(its inter-system-gap term, quality.rs `vertical_raw`). So the two axes TRADE:
|
||||
filling the page is paid for in inter-system density, and a *sparse* justified
|
||||
page (few systems, large per-gap stretch) is charged more — which is a defensible
|
||||
signal (a 2-systems-on-a-tall-page layout genuinely reads thin) but over-charges
|
||||
a *moderate*, uniform stretch that is good justification. Same family as the
|
||||
horizontal `casting_off` / justified-raggedness note; the catalog refinement
|
||||
(score only EXCESS stretch, or measure gap UNIFORMITY rather than deviation from
|
||||
preferred) is the deferred follow-up, needing catalog alignment. Pinned by
|
||||
`vertical_justification_trades_page_fill_for_inter_system_density` so the
|
||||
interaction is not silent (review finding).
|
||||
|
||||
**Still deferred (the rest of the vertical spring solve).** Inter-staff
|
||||
band-height renegotiation *within* a multi-staff system (today the constrained
|
||||
|
|
|
|||
|
|
@ -900,4 +900,48 @@ mod tests {
|
|||
let report = Engraver::default().solve(&ten_measure(), &SolverConfig::default());
|
||||
assert_eq!(report.metric_vector.vertical_density_penalty.0, 0.0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn vertical_justification_trades_page_fill_for_inter_system_density() {
|
||||
use crate::PageGeometry;
|
||||
use epiphany_layout_ir::{Margins, Size2D, StaffSpace};
|
||||
// On a MULTI-page layout, vertical justification spreads a non-final
|
||||
// page's systems to fill the height (`page_fill` → ~0), which stretches
|
||||
// its inter-system gaps beyond the band model's preferred height — a
|
||||
// cost the vertical-density axis measures. The two axes trade: filling
|
||||
// the page is paid for in inter-system density. A catalog refinement
|
||||
// that scores only EXCESS stretch (or measures gap uniformity) is a
|
||||
// deferred follow-up, alongside the casting_off / justified-raggedness
|
||||
// note. Pinned so the interaction is not silent (review finding).
|
||||
let geometry = PageGeometry {
|
||||
size: Size2D {
|
||||
width: StaffSpace(40.0),
|
||||
height: StaffSpace(30.0),
|
||||
},
|
||||
margins: Margins {
|
||||
top: StaffSpace(5.0),
|
||||
right: StaffSpace(5.0),
|
||||
bottom: StaffSpace(5.0),
|
||||
left: StaffSpace(5.0),
|
||||
},
|
||||
};
|
||||
let report =
|
||||
Engraver::with_geometry(geometry).solve(&ten_measure(), &SolverConfig::default());
|
||||
assert!(
|
||||
report.layout.pages.len() >= 2,
|
||||
"the small page forces multiple pages"
|
||||
);
|
||||
// Justification fills the non-final pages…
|
||||
assert!(
|
||||
report.metric_vector.page_fill_efficiency.0 < 0.1,
|
||||
"non-final pages fill: {}",
|
||||
report.metric_vector.page_fill_efficiency.0
|
||||
);
|
||||
// …at the measured cost of stretched inter-system gaps.
|
||||
assert!(
|
||||
report.metric_vector.vertical_density_penalty.0 > 0.0,
|
||||
"stretched inter-system gaps are measured: {}",
|
||||
report.metric_vector.vertical_density_penalty.0
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue