From dd33b34f08e66565f859ee47df38e29d138e302e Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Fri, 24 Jul 2026 14:47:31 -0400 Subject: [PATCH] Editor T4-pre W1: the resolved layout stops discarding its own partition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ResolvedLayoutIR carried a page/system tree and three flat primitive arrays with nothing joining them, so a canvas wanting to re-tessellate one damaged system had to infer ownership spatially — ambiguous exactly where it matters, at cross-system slurs and boundary-straddling strokes. The partition was never missing, though: casting-off computes it (system_of_slot, stroke_system, curve_system) and the fold into ResolvedLayoutIR dropped it. This publishes it instead of inferring it, so the answer is exact. ResolvedSystem gains PrimitiveIndices — u32 index lists into the layout's flat glyphs/strokes/curves — and ResolvedLayoutIR gains an unowned bucket of the same shape. No primitive is split, merged, reordered, or renumbered; the flat arrays are exactly what they were. The partition is total and disjoint: for each array, every system's list plus the unowned bucket covers 0..len exactly once, tested directly rather than assumed from construction. Unowned is a first-class bucket with a real producer. The stub solver resolves no per-system geometry — one default-rect system per region — so it publishes every primitive unowned rather than fabricating an attribution it never computed. Cross-system primitives need no special case: casting-off already splits a spanning stroke or curve into per-system segments carrying SYSTEM_CONTINUATION_SYNTHESIS provenance, and each segment is owned by the system it was split into, not by the source's. Ownership is excluded from the canonical encoding, for the reason vertical_band already is: it draws nothing, so two layouts differing only in it are the same rendered layout and hash alike. Encoding it would make the fingerprint more fragile than the rendering it fingerprints — a casting-off refactor that re-partitions without moving a pixel would become a byte-level break for something no renderer and no conformance claim can observe. Verified against main: all eight reference fixtures produce byte-identical canonical layouts, and all five GUI goldens are untouched. One derivation, structurally. The quality census consumed its own copy of the glyph-to-system rule, and so did vertical_raw's system_of_glyph closure; both now read the published glyph_system. That left CastLayout::system_of_slot with no consumer outside the casting pass, so it is removed from the published struct — a future consumer cannot grow a third copy of the rule, because the raw material is no longer there. The per-glyph answer travels; the derivation does not. Six mutations, each killed and coordinator-re-verified independently: dropping an index from a system's list, coercing unowned onto system 0, an off-by-one system index, publishing a constant attribution, attributing a continuation to its source's system, and encoding ownership into the canonical bytes. The fourth is the one that proves the single-derivation claim rather than asserting it — it kills three quality tests, two of which survived it before vertical_raw was migrated. Gate: 34 suites / 1341 tests / 0 failed, conformance 9/9 with golden-gate and 8/8 without, requirement labels 6/6 at 212/282/282, clippy 0, five goldens byte-identical. Co-Authored-By: Claude Opus 5 (1M context) --- crates/epiphany-editor-core/src/lib.rs | 4 + crates/epiphany-engrave/DECISIONS.md | 69 ++++++ crates/epiphany-engrave/src/casting.rs | 272 +++++++++++++++++++-- crates/epiphany-engrave/src/lib.rs | 11 +- crates/epiphany-engrave/src/quality.rs | 13 +- crates/epiphany-layout-ir/DECISIONS.md | 92 +++++++ crates/epiphany-layout-ir/src/lib.rs | 3 +- crates/epiphany-layout-ir/src/resolved.rs | 101 ++++++++ crates/epiphany-layout-ir/src/solver.rs | 85 ++++++- crates/epiphany-render-svg/src/svg.rs | 1 + crates/epiphany-testkit/src/layout_stub.rs | 5 + 11 files changed, 627 insertions(+), 29 deletions(-) diff --git a/crates/epiphany-editor-core/src/lib.rs b/crates/epiphany-editor-core/src/lib.rs index 9d1d890..9b5c679 100644 --- a/crates/epiphany-editor-core/src/lib.rs +++ b/crates/epiphany-editor-core/src/lib.rs @@ -4745,6 +4745,10 @@ mod tests { }, }], measures: Vec::new(), + // Mechanical field-add only (CONTRACT_EDITOR_T4PRE_IR.md W1): + // this hit-test fixture tests neither ownership nor consumes + // it — editor-core adopts the accessor at T4, not here. + primitives: Default::default(), }) .collect(); let strokes: Vec = origins diff --git a/crates/epiphany-engrave/DECISIONS.md b/crates/epiphany-engrave/DECISIONS.md index d5cadab..88d6e0b 100644 --- a/crates/epiphany-engrave/DECISIONS.md +++ b/crates/epiphany-engrave/DECISIONS.md @@ -1015,3 +1015,72 @@ 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. + +## W1: casting-off publishes the per-system partition it already computes (Editor T4-pre, `CONTRACT_EDITOR_T4PRE_IR.md`, 2026-07-24) + +`cast_off` already knew which system every glyph, stroke, and curve landed +in (`system_of_slot`/`stroke_system`/`curve_system`, private to `CastLayout`) +— it just never left the module. Three changes make it leave: + +1. **`glyph_system: Vec>` joins `CastLayout`**, computed once + in the same pass that bakes glyph positions (the glyphs-building `map` + that already reads `system_of_slot.get(&glyph.horizontal_slot)` for + `dx`/`dy` now also records that same lookup's result per glyph, via + `.unzip()`, instead of computing it twice). This is the single + glyph→system derivation pin 7 asks for: the quality census (`quality.rs`, + `census`) now reads `cast.glyph_system[index]` and no longer re-derives + the answer from `cast.system_of_slot` itself. +2. **The index partition** (`owned: Vec` plus one + `unowned: PrimitiveIndices`) is built from `glyph_system`/`stroke_system`/ + `curve_system` right before `resolved_systems` is constructed, and handed + to each `build_system` call (which now takes a `PrimitiveIndices` + parameter — pushing it to 8 arguments, `#[allow(clippy::too_many_arguments)]` + added alongside the crate's existing precedent on `walk_region`) and to + the returned `CastLayout.unowned`. `epiphany-layout-ir`'s solve-report fold + (`lib.rs`) copies `cast.unowned` straight into `ResolvedLayoutIR.unowned` + with no further computation; the `None`-cast branch (structurally invalid + input) gets `PrimitiveIndices::default()`, consistent with its empty + glyphs/strokes/curves/pages. +3. **No primitive changes shape.** The three flat arrays (`glyphs`, `strokes` + after continuation-append, `curves` after continuation-append) are exactly + what casting-off already produced; W1 publishes indices into them and + nothing else — including for the synthesized continuation segments a + system-spanning stroke or curve splits into, which were already being + pushed onto `stroke_system`/`curve_system` keyed by their *own* landing + system (`segments[k].0` inside the `Split` arm), not the source segment's. + W1 did not have to fix this; it only had to publish it faithfully, and the + mutation that deliberately breaks that faithfulness (attribute the + continuation to the source's system instead) kills a dedicated test + (`continuation_segments_are_owned_by_the_system_they_split_into`). + +**A second, un-migrated re-derivation was found while doing this packet, and +closed rather than named.** `quality.rs`'s `vertical_raw` (the inter-staff-gap +axis) had its own local closure, `system_of_glyph`, independently re-deriving +a glyph's system from the slot map — the exact second copy of the attribution +rule pin 7 exists to prevent, just not the one m4 tests for. The packet's +first pass migrated only the census (pin 7 names it specifically) and recorded +this as an open finding; coordinator review closed it instead, because a pin +whose stated goal is "two copies drift, so make that impossible" is not met by +migrating one of two copies. + +**And closing it made `CastLayout::system_of_slot` dead**, which is the real +lesson: with both consumers on `glyph_system`, nothing outside the casting +pass read the raw slot→system map any more. So the field is **removed** from +the published struct — the map stays a local inside `cast_off`, where the +stroke-fate walk still needs it. Pin 7 is now structural rather than +conventional: a future consumer *cannot* grow a third copy of the glyph→system +rule from `CastLayout`, because the raw material is no longer on it. The +per-glyph answer is published, the derivation is not. + +**Byte-neutral:** see the `epiphany-layout-ir` DECISIONS.md entry of the same +date for the full canonical-encoding exclusion rationale (two consequences: +fingerprint fragility, and what a future cross-implementation relayout test +may and may not compare) — `casting.rs`'s `encode_canonical` path +(`epiphany-layout-ir/src/resolved.rs`) is unchanged by this tranche. + +**Locked by** `primitive_ownership_partitions_every_flat_array_totally_and_ +disjointly`, `attribution_correctness_matches_the_real_per_system_counts`, +and `continuation_segments_are_owned_by_the_system_they_split_into` +(`casting.rs`), each against the real engraver on real fixtures — the real +per-system glyph/stroke counts are `[26, 25]` and `[51, 45]` for the +`ten_measure_single_staff` two-system split, not asserted as `> 0`. diff --git a/crates/epiphany-engrave/src/casting.rs b/crates/epiphany-engrave/src/casting.rs index 46b3055..7901e46 100644 --- a/crates/epiphany-engrave/src/casting.rs +++ b/crates/epiphany-engrave/src/casting.rs @@ -76,10 +76,10 @@ use epiphany_layout_ir::{ 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, + LayoutConstraint, LayoutObjectId, Margins, Point, PrimitiveIndices, Provenance, Rect, + ResolvedGlyph, ResolvedMeasure, ResolvedPage, ResolvedStaff, ResolvedSystem, Size2D, + SpringSlotId, StaffSpace, Stroke, SynthesisInstanceKey, SynthesisKind, SynthesisRegistryId, + VerticalBand, VerticalBandId, VerticalBandKind, }; use crate::owning_glyph; @@ -188,14 +188,17 @@ pub(crate) struct CastLayout { pub system_start_slots: BTreeSet, /// Slots at which a page begins: the first slot of each page's first system. pub page_start_slots: BTreeSet, - /// Which system (global index, page order) each realized slot landed in — - /// the casting pass's own assignment, which the quality-metric census - /// 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, + /// The system each baked glyph landed in, parallel to `glyphs` — derived + /// once, inside the casting pass, from the slot→system assignment that + /// pass computes for its own use. **This is the glyph→system attribution, + /// full stop** (W1 pin 7): the raw slot map is deliberately *not* + /// published, so no consumer can grow a second copy of the rule that then + /// drifts. `None`: the glyph's slot was claimed by no region, so it + /// belongs to no per-system aggregate. + pub glyph_system: Vec>, /// 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 + /// the slot map cannot answer for it; the casting pass records what it /// already knew. `None`: claimed by no region. pub stroke_system: Vec>, /// The system each baked curve landed in, parallel to `curves`. @@ -203,6 +206,11 @@ pub(crate) struct CastLayout { /// 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, + /// The primitives no system claims — `glyph_system`/`stroke_system`/ + /// `curve_system` entries of `None`, gathered into the same shape + /// [`ResolvedSystem::primitives`] uses (W1 pin 3: unowned is a first-class + /// bucket, never coerced onto a system). + pub unowned: PrimitiveIndices, } /// One realized spring slot in spaced (pre-casting) coordinates, with the @@ -987,13 +995,17 @@ pub(crate) fn cast_off( .copied() .unwrap_or(0.0) }; - let glyphs: Vec = spaced_glyphs + // Computed once, alongside the positioning it also drives (W1 pin 7): the + // quality-metric census consumes this published vector rather than + // re-deriving the same attribution from `system_of_slot` itself. + let (glyphs, glyph_system): (Vec, Vec>) = spaced_glyphs .iter() .zip(&input.glyphs) .enumerate() .map(|(gi, (spaced, glyph))| { - let (dx, dy) = match system_of_slot.get(&glyph.horizontal_slot) { - Some(&s) => { + let system = system_of_slot.get(&glyph.horizontal_slot).copied(); + let (dx, dy) = match system { + Some(s) => { // Slot-relative: every member of a slot translates by the // map at the slot's source, so intra-slot offsets survive. let sx = slot_source_x @@ -1007,12 +1019,13 @@ pub(crate) fn cast_off( } None => (0.0, 0.0), }; - ResolvedGlyph { + let resolved = ResolvedGlyph { position: Point::new(spaced.position.x.0 + dx, spaced.position.y.0 + dy), ..spaced.clone() - } + }; + (resolved, system) }) - .collect(); + .unzip(); // Per-system staff-line marks, for the resolved staff records below. let mut staff_marks: BTreeMap<(usize, StaffId), StaffAgg> = BTreeMap::new(); @@ -1164,6 +1177,34 @@ pub(crate) fn cast_off( curves.extend(curve_continuations); curve_system.extend(curve_continuation_system); + // ---- Per-system primitive ownership (W1) ------------------------------- + // The partition already exists in `glyph_system`/`stroke_system`/ + // `curve_system` above; this just stops discarding it. For each flat + // array, every index lands in exactly one system's list or in `unowned` + // (pin 4: a total, disjoint partition, tested in this module below). + let mut owned: Vec = (0..systems.len()) + .map(|_| PrimitiveIndices::default()) + .collect(); + let mut unowned = PrimitiveIndices::default(); + for (i, system) in glyph_system.iter().enumerate() { + match system { + Some(s) => owned[*s].glyphs.push(i as u32), + None => unowned.glyphs.push(i as u32), + } + } + for (i, system) in stroke_system.iter().enumerate() { + match system { + Some(s) => owned[*s].strokes.push(i as u32), + None => unowned.strokes.push(i as u32), + } + } + for (i, system) in curve_system.iter().enumerate() { + match system { + Some(s) => owned[*s].curves.push(i as u32), + None => unowned.curves.push(i as u32), + } + } + // ---- The resolved page tree --------------------------------------------- let resolved_systems: Vec = systems .iter() @@ -1177,6 +1218,7 @@ pub(crate) fn cast_off( &extents, &placements, &staff_marks, + owned[s].clone(), ) }) .collect(); @@ -1225,10 +1267,11 @@ pub(crate) fn cast_off( decisions, system_start_slots, page_start_slots, - system_of_slot, + glyph_system, stroke_system, curve_system, region_of_system: systems.iter().map(|plan| plan.region).collect(), + unowned, } } @@ -1855,6 +1898,7 @@ fn mark_staff( /// the pipeline does not know is left empty, never fabricated: a staff with no /// engraved lines yields no staff record, and the final-barline measure (whose /// start no column marks) yields no measure record. +#[allow(clippy::too_many_arguments)] fn build_system( system: usize, plan: &SystemPlan, @@ -1863,6 +1907,7 @@ fn build_system( extents: &[Extent], placements: &[Placement], staff_marks: &BTreeMap<(usize, StaffId), StaffAgg>, + primitives: PrimitiveIndices, ) -> ResolvedSystem { let region = &input.regions[plan.region]; let p = placements[system]; @@ -1952,6 +1997,7 @@ fn build_system( bounding_box, staves, measures, + primitives, } } @@ -2334,4 +2380,196 @@ mod tests { hi - lo ); } + + /// (m1) For each of the layout's three flat arrays, every system's owned + /// index list plus the layout's `unowned` bucket covers `0..len` exactly + /// once — pin 4's total, disjoint partition. The load-bearing invariant, + /// checked directly rather than assumed from construction. + fn assert_total_disjoint_partition(layout: &epiphany_layout_ir::ResolvedLayoutIR) { + let check = |label: &str, len: usize, owned: Vec<&Vec>, unowned: &[u32]| { + let mut seen = vec![0u8; len]; + for &i in owned.iter().flat_map(|v| v.iter()).chain(unowned.iter()) { + let idx = i as usize; + assert!(idx < len, "{label}: index {i} out of range (len {len})"); + seen[idx] += 1; + } + for (i, &count) in seen.iter().enumerate() { + assert_eq!( + count, 1, + "{label}: index {i} covered {count} times (want exactly 1)" + ); + } + }; + let glyph_lists: Vec<&Vec> = layout.systems().map(|s| &s.primitives.glyphs).collect(); + check( + "glyphs", + layout.glyphs.len(), + glyph_lists, + &layout.unowned.glyphs, + ); + let stroke_lists: Vec<&Vec> = + layout.systems().map(|s| &s.primitives.strokes).collect(); + check( + "strokes", + layout.strokes.len(), + stroke_lists, + &layout.unowned.strokes, + ); + let curve_lists: Vec<&Vec> = layout.systems().map(|s| &s.primitives.curves).collect(); + check( + "curves", + layout.curves.len(), + curve_lists, + &layout.unowned.curves, + ); + } + + #[test] + fn primitive_ownership_partitions_every_flat_array_totally_and_disjointly() { + use crate::Engraver; + use epiphany_layout_ir::{to_constrained, to_logical, ConstraintSolver, SolverConfig}; + + // The wrapping ten-measure fixture: real glyphs and strokes, no curves. + let wrapping = Engraver::default().solve( + &to_constrained(&to_logical( + &epiphany_testkit::fixtures::ten_measure_single_staff(0x000A_11CE), + )), + &SolverConfig::default(), + ); + assert_eq!( + wrapping.layout.pages[0].systems.len(), + 2, + "the fixture wraps into two systems" + ); + assert!(!wrapping.layout.glyphs.is_empty()); + assert!(!wrapping.layout.strokes.is_empty()); + assert_total_disjoint_partition(&wrapping.layout); + + // The slurred fixture: also exercises curves, including a + // system-spanning split (G4's own construction). + let slurred = Engraver::default().solve( + &to_constrained(&to_logical( + &epiphany_testkit::fixtures::ten_measure_with_slurs(0), + )), + &SolverConfig::default(), + ); + let slurred_systems: usize = slurred.layout.pages.iter().map(|p| p.systems.len()).sum(); + assert!( + slurred_systems > 1, + "casting-off wraps the slurred fixture too" + ); + assert!( + !slurred.layout.curves.is_empty(), + "the slur produces real curves" + ); + assert_total_disjoint_partition(&slurred.layout); + } + + #[test] + fn attribution_correctness_matches_the_real_per_system_counts() { + // (m3) The *actual* per-system glyph/stroke counts of the two-system + // fixture — real numbers, not `> 0` — value-asserted directly against + // what casting-off computed. + use crate::Engraver; + use epiphany_layout_ir::{to_constrained, to_logical, ConstraintSolver, SolverConfig}; + + let report = Engraver::default().solve( + &to_constrained(&to_logical( + &epiphany_testkit::fixtures::ten_measure_single_staff(0x000A_11CE), + )), + &SolverConfig::default(), + ); + let systems: Vec<_> = report.layout.systems().collect(); + assert_eq!(systems.len(), 2, "two systems"); + let glyph_counts: Vec = systems.iter().map(|s| s.primitives.glyphs.len()).collect(); + let stroke_counts: Vec = + systems.iter().map(|s| s.primitives.strokes.len()).collect(); + assert_eq!( + glyph_counts, + vec![26, 25], + "the six/four widow-rebalanced measure split's real per-system glyph counts" + ); + assert_eq!( + stroke_counts, + vec![51, 45], + "the six/four widow-rebalanced measure split's real per-system stroke counts" + ); + assert_eq!( + glyph_counts[0] + glyph_counts[1], + report.layout.glyphs.len() + ); + assert_eq!( + stroke_counts[0] + stroke_counts[1], + report.layout.strokes.len() + ); + assert!( + report.layout.unowned.glyphs.is_empty(), + "the whole score is inside the one region this fixture declares" + ); + } + + #[test] + fn continuation_segments_are_owned_by_the_system_they_split_into() { + // (m5) A slur crossing a system break: its synthesized continuation + // segment is owned by the system it was split INTO, not the source + // segment's system. + use crate::Engraver; + use epiphany_core::{Slur, SlurId, SlurKind, SpanStyle, TypedObjectId}; + use epiphany_layout_ir::{to_constrained, to_logical, ConstraintSolver, SolverConfig}; + + let mut score = epiphany_testkit::fixtures::ten_measure_single_staff(0x000A_11CE); + let events: Vec<_> = score.canvas.regions[0].staff_instances()[0].voices[0] + .events + .clone(); + let slur_id: SlurId = score.identity.mint(); + score.cross_cutting.slurs.push(Slur { + id: slur_id, + start_event: events[0], + end_event: events[events.len() - 1], + kind: SlurKind::Legato, + curvature_override: None, + style: SpanStyle::default(), + }); + let report = Engraver::default().solve( + &to_constrained(&to_logical(&score)), + &SolverConfig::default(), + ); + assert_eq!(report.layout.pages[0].systems.len(), 2, "two systems"); + + let original_index = report + .layout + .curves + .iter() + .position(|c| { + c.provenance.source == TypedObjectId::Slur(slur_id) + && c.provenance.synthesis.is_none() + }) + .expect("one segment keeps the slur's exact provenance"); + let continuation_index = report + .layout + .curves + .iter() + .position(|c| { + c.provenance.source == TypedObjectId::Slur(slur_id) + && c.provenance.synthesis.is_some() + }) + .expect("the break-spanning slur splits and synthesizes a continuation"); + + let owner_of = |index: usize| -> Option { + report + .layout + .systems() + .position(|s| s.primitives.curves.contains(&(index as u32))) + }; + let owner_first = owner_of(original_index).expect("the original segment is owned"); + let owner_continuation = owner_of(continuation_index).expect("the continuation is owned"); + assert_eq!( + owner_first, 0, + "the original segment starts in the first system" + ); + assert_eq!( + owner_continuation, 1, + "the continuation is owned by the system it was split INTO, not the source's" + ); + } } diff --git a/crates/epiphany-engrave/src/lib.rs b/crates/epiphany-engrave/src/lib.rs index 71fff8e..3738490 100644 --- a/crates/epiphany-engrave/src/lib.rs +++ b/crates/epiphany-engrave/src/lib.rs @@ -82,9 +82,9 @@ use epiphany_core::TypedObjectId; use epiphany_layout_ir::{ all_available, profile_thresholds, Axis, BravuraCatalog, ConstrainedLayoutIR, ConstraintId, ConstraintSolver, ConstraintStrength, Curve, GlyphCatalog, GlyphObject, GlyphObjectId, - InvalidationSet, LayoutConstraint, Point, QualityMetricVector, Rect, ResolvedGlyph, - ResolvedLayoutIR, SolveReport, SolveStatus, SolverBudgetUsed, SolverConfig, SolverState, - SolverTier, SolverVersion, SolverWarning, SolverWarningKind, SpringSlotId, Stroke, + InvalidationSet, LayoutConstraint, Point, PrimitiveIndices, QualityMetricVector, Rect, + ResolvedGlyph, ResolvedLayoutIR, SolveReport, SolveStatus, SolverBudgetUsed, SolverConfig, + SolverState, SolverTier, SolverVersion, SolverWarning, SolverWarningKind, SpringSlotId, Stroke, }; pub use casting::{PageGeometry, INTER_PAGE_GAP, SYSTEM_CONTINUATION_SYNTHESIS}; @@ -380,7 +380,7 @@ impl Engraver { // glyph/stroke positions baked, the engraver's break decisions appended // to the pipeline's (Chapter 7 §"ResolvedLayoutIR": decisions "including // any the solver itself made"). - let (glyphs, strokes, curves, pages, engraving_decisions) = match cast { + let (glyphs, strokes, curves, pages, engraving_decisions, unowned) = match cast { Some(cast) => { let mut decisions = input.engraving_decisions.clone(); decisions.extend(cast.decisions); @@ -390,6 +390,7 @@ impl Engraver { cast.curves, cast.pages, decisions, + cast.unowned, ) } None => ( @@ -398,6 +399,7 @@ impl Engraver { Vec::new(), Vec::new(), input.engraving_decisions.clone(), + PrimitiveIndices::default(), ), }; @@ -412,6 +414,7 @@ impl Engraver { curves, engraving_decisions, catalog: input.catalog.clone(), + unowned, }, unsatisfied_constraints, warnings, diff --git a/crates/epiphany-engrave/src/quality.rs b/crates/epiphany-engrave/src/quality.rs index 4291339..a53fb0d 100644 --- a/crates/epiphany-engrave/src/quality.rs +++ b/crates/epiphany-engrave/src/quality.rs @@ -229,7 +229,9 @@ fn census(input: &ConstrainedLayoutIR, cast: &CastLayout) -> SystemCensus { // convention the spacing and casting passes use for a slot's reference x. let mut columns: Vec> = vec![BTreeMap::new(); count]; for (index, glyph) in input.glyphs.iter().enumerate() { - let Some(&system) = cast.system_of_slot.get(&glyph.horizontal_slot) else { + // W1 pin 7: the published attribution, not a second derivation from + // `system_of_slot` — one glyph→system rule, computed once in casting. + let Some(system) = cast.glyph_system[index] else { // A slot no region claimed: positioned by no system, so its glyphs // join no per-system aggregate (catalog §"The Measurement Domain"). continue; @@ -369,11 +371,10 @@ fn vertical_units( // 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 { - cast.system_of_slot - .get(&input.glyphs[index].horizontal_slot) - .copied() - }; + // W1 pin 7, same as the census: the published attribution, not a second + // derivation. `glyph_system` is parallel to `input.glyphs`, exactly as the + // `stroke_system`/`curve_system` reads below are to their arrays. + let system_of_glyph = |index: usize| -> Option { cast.glyph_system[index] }; let mut content: BTreeMap<(usize, VerticalBandId), (f64, f64)> = BTreeMap::new(); { let mut add = |system: usize, band: VerticalBandId, lo: f64, hi: f64| { diff --git a/crates/epiphany-layout-ir/DECISIONS.md b/crates/epiphany-layout-ir/DECISIONS.md index 375fb4d..ea04c3c 100644 --- a/crates/epiphany-layout-ir/DECISIONS.md +++ b/crates/epiphany-layout-ir/DECISIONS.md @@ -930,3 +930,95 @@ table as a non-normative note) and no new `\label{req:...}` (counts stay 212/282/282). Both Chapter 9 usages (`GlyphCatalog::smufl_version()`, `GlyphCatalogIdentity.smufl_version`) gain a cross-reference sentence back to this single definition. + +## W1: per-system primitive ownership — a published partition, not a new computation (Editor T4-pre, `CONTRACT_EDITOR_T4PRE_IR.md`, 2026-07-24) + +`ResolvedSystem` gains one field, `primitives: PrimitiveIndices` (index lists +— `u32`, one per flat array — into the layout's `glyphs`/`strokes`/`curves`); +`ResolvedLayoutIR` gains one field of the same shape, `unowned`, for a +primitive no system claims. Nothing is split, merged, reordered, or +renumbered to populate either: casting-off (`epiphany-engrave`) already +computed this partition (`system_of_slot`/`stroke_system`/`curve_system` on +its private `CastLayout`) and discarded it at the fold into `ResolvedLayoutIR`; +this tranche stops discarding it. `ResolvedLayoutIR` also gains a `systems()` +accessor — every system across every page, in page order — which the +partition's own tests use; `epiphany-editor-core` keeps its hand-rolled +equivalent (`lib.rs`, `containing_system`) until it adopts the accessor at +T4, not before (Ruling A's stated prerequisite boundary: this tranche is IR +work, not editor-crate consumption). + +**Verified caution for T4b, recorded here because the aliasing is easy to miss +and the bug it invites is specific.** A system's `provenance.stable_id` is +**not** always distinct from its page's or its region's: a region's first +system reuses the region's own provenance verbatim (`casting.rs`, `build_system`), +and page 1 reuses the first region's provenance too (`casting.rs`, the page-tree +construction) — so on the real engraver's path, a system's `stable_id` can equal +a page's and a region's for the *first* system of the *first* region. On the +**stub solver's path the aliasing is total, not just first-system**: every stub +system reuses its region's provenance verbatim (`solver.rs`), because the stub +resolves no per-system geometry and does not synthesize per-system identity +either. Uniqueness *among systems* is structural regardless — synthesized ids +are domain-tagged hashes over `(source, kind discriminant, namespaced instance +key)`, and `KEY_NS_SYSTEM`/`KEY_NS_PAGE` occupy distinct namespaces — which is +all W1 needs (position is primary; `stable_id` is available identity, not the +addressing scheme). But **any future cross-kind map keyed on a raw +`LayoutObjectId`/`SystemId` `u128`** (a T4b incremental-relayout cache +keying systems, pages, and regions into one structure, say) **must +disambiguate by kind** — collapsing them into one untyped key space will +silently conflate a system with its page or region on exactly the inputs +above, and the collision will not show up on any fixture that gives every +region more than one system. + +**Byte-neutral by construction, and deliberately not revisited here.** +`encode_canonical` is unchanged; the module's canonical-serialization note +(`ResolvedLayoutIR::canonical_bytes`) now names `primitives`/`unowned` +alongside `vertical_band` in its stated exclusions, same reasoning: the +partition draws nothing a renderer paints, so two layouts differing only in +it render identically and hash alike. Two consequences follow, stated +outright so a later reader does not "fix" this by wiring the fields into the +encoder: + +1. Encoding ownership would make the fingerprint **more fragile than the + rendering it fingerprints** — a casting-off refactor that re-partitions + systems without moving a single pixel (a different but equally valid + greedy-vs-optimal break search, say) would become a byte-level break and + force a schema major for a change no renderer and no conformance claim can + observe. +2. Two conformant implementations may legitimately partition a layout + differently (the spec pins no partitioning algorithm, only that resolved + primitives exist). Pinning ownership on the wire would manufacture a false + disagreement between two implementations that render pixel-identical + output. **Any future cross-implementation test of incremental relayout + therefore compares final bytes, never intermediate partitions** — this is + the test-design consequence of (1), not a restatement of it. + +If a future normative requirement wants per-system ownership pinned on the +wire regardless (a determinism claim about the partitioning algorithm itself, +not just its output), that is a spec-side schema-major decision — not a call +this packet's byte-neutrality budget was scoped to make. + +**Stub solver:** publishes every primitive unowned rather than fabricating an +attribution — it resolves no per-system geometry (every region becomes one +degenerate default-rect system), so any per-system claim would be a lie about +what that path computed. Locked by +`the_stub_solver_publishes_every_primitive_unowned` (`solver.rs`), which +value-asserts the real per-system-zero / all-unowned shape against a +non-trivial generated score (`epiphany_core::generators::valid_score_rich`: +11 glyphs, 38 strokes, 3 regions, plus a hand-added curve exercising all +three arrays) rather than a synthetic one-glyph fixture. + +**Locked by** (`epiphany-engrave`'s `casting.rs` test module and this crate's +`solver.rs`/`resolved.rs`): the partition's totality/disjointness on a real +multi-system engrave (`ten_measure_single_staff`, `ten_measure_with_slurs`); +the stub-solver's all-unowned publication; the real per-system glyph/stroke +counts of the two-system fixture (`[26, 25]` glyphs, `[51, 45]` strokes — +value-asserted, not `> 0`); a system-spanning slur's synthesized continuation +owned by the system it splits *into*, not the source segment's; and +byte-identical `canonical_bytes()` before/after perturbing only the ownership +fields, while `PartialEq` still sees the difference. Six mutations verified +by hand (drop an index from a system's list; coerce the stub's unowned bucket +onto system 0; off-by-one the system index; publish a wrong `glyph_system` +while positioning stays correct; attribute a continuation to its source +segment's system instead of the system it splits into; encode the ownership +lists in `encode_canonical`) — each reverted by hand after observing the +real failure, never `git checkout`. diff --git a/crates/epiphany-layout-ir/src/lib.rs b/crates/epiphany-layout-ir/src/lib.rs index 2fa7438..31ef1c0 100644 --- a/crates/epiphany-layout-ir/src/lib.rs +++ b/crates/epiphany-layout-ir/src/lib.rs @@ -139,7 +139,8 @@ pub use render::{ RenderTarget, }; pub use resolved::{ - ResolvedGlyph, ResolvedLayoutIR, ResolvedMeasure, ResolvedPage, ResolvedStaff, ResolvedSystem, + PrimitiveIndices, ResolvedGlyph, ResolvedLayoutIR, ResolvedMeasure, ResolvedPage, + ResolvedStaff, ResolvedSystem, }; pub use roundtrip::{laid_out_object_ids, round_trip, round_trip_with, RoundTripReport}; pub use solver::{ diff --git a/crates/epiphany-layout-ir/src/resolved.rs b/crates/epiphany-layout-ir/src/resolved.rs index 69bd306..25aed35 100644 --- a/crates/epiphany-layout-ir/src/resolved.rs +++ b/crates/epiphany-layout-ir/src/resolved.rs @@ -65,6 +65,37 @@ pub struct ResolvedSystem { pub bounding_box: Rect, pub staves: Vec, pub measures: Vec, + /// Which of the layout's flat `glyphs`/`strokes`/`curves` this system + /// owns — index lists, not copies (see [`PrimitiveIndices`]). A primitive + /// no system claims is not omitted; it is in [`ResolvedLayoutIR::unowned`] + /// instead, so the partition stays total (Chapter 7's flat arrays are + /// otherwise untouched by this field's existence — no primitive is split, + /// merged, reordered, or renumbered to populate it). + pub primitives: PrimitiveIndices, +} + +/// Index lists into a [`ResolvedLayoutIR`]'s flat `glyphs`/`strokes`/`curves` +/// arrays — never the primitives themselves. `u32`: matches the flat arrays' +/// own canonical count-prefix width and comfortably exceeds any real layout +/// (no resolved layout nears 4 billion primitives). +/// +/// One of these lives on every [`ResolvedSystem`] (what it owns) and one on +/// [`ResolvedLayoutIR`] itself (`unowned`: claimed by no system — e.g. every +/// primitive under the stub solver, which resolves no per-system geometry +/// and so publishes everything unowned rather than fabricating an +/// attribution). For each of the three flat arrays, the union of every +/// system's list plus the unowned bucket is exactly `0..len`, each index +/// appearing exactly once — a tested invariant (`epiphany-engrave`'s casting +/// module), not merely a convention. +/// +/// **Not part of the canonical encoding** (see the exclusion note on +/// [`ResolvedLayoutIR::canonical_bytes`]): this partition draws nothing, so +/// two layouts differing only in it render identically and hash alike. +#[derive(Clone, PartialEq, Eq, Debug, Default)] +pub struct PrimitiveIndices { + pub glyphs: Vec, + pub strokes: Vec, + pub curves: Vec, } #[derive(Clone, PartialEq, Debug)] @@ -97,9 +128,22 @@ pub struct ResolvedLayoutIR { /// The catalog identity under which this layout was produced — required for /// any byte-equal conformance claim (Chapter 7 §7.3.2). pub catalog: GlyphCatalogIdentity, + /// The primitives no system claims — the other half of the total, + /// disjoint partition [`ResolvedSystem::primitives`] describes. See + /// [`PrimitiveIndices`]. + pub unowned: PrimitiveIndices, } impl ResolvedLayoutIR { + /// Every system across every page, in page order (a page's systems in + /// their own stored order, pages visited in `pages` order) — top to + /// bottom, reading order. `epiphany-editor-core` hand-rolls this same + /// flatten today (`editor-core/src/lib.rs`); it adopts this accessor at + /// T4, not before. + pub fn systems(&self) -> impl Iterator { + self.pages.iter().flat_map(|page| page.systems.iter()) + } + /// The canonical serialized output (Appendix D §"Quantized Layout /// Coordinates"): the layout's *rendering fingerprint*, with glyph positions /// quantized to the `1/1024` grid. Equivalent to @@ -113,6 +157,13 @@ impl ResolvedLayoutIR { /// carry `vertical_band` through but do not encode it. Band ownership tells a /// vertical solver which staff owns a primitive; it draws nothing, so two /// layouts differing only in it are the same rendered layout and hash alike. + /// **Per-system primitive ownership is excluded for the identical reason**: + /// [`ResolvedSystem::primitives`] and [`ResolvedLayoutIR::unowned`] name + /// which system (or no system) a primitive belongs to, and draw nothing — + /// two layouts differing only in that partition are the same rendered + /// layout and hash alike. Pinning it on the wire (if a future normative + /// requirement wants that) is a spec-side schema-major decision, not one + /// this type makes silently; see `DECISIONS.md`. /// /// Two solves whose internal f32 computations agree to better than `1/2048` /// staff space at every coordinate produce identical bytes; two layouts that @@ -410,6 +461,7 @@ mod tests { curves: vec![], engraving_decisions: decisions, catalog: GlyphCatalogIdentity::default(), + unowned: PrimitiveIndices::default(), } } @@ -541,4 +593,53 @@ mod tests { let bad = ir(vec![glyph(1, f32::NAN)], vec![]); let _ = bad.canonical_bytes(); } + + #[test] + fn primitive_ownership_is_excluded_from_canonical_bytes() { + // (m6) Perturbing ONLY the ownership lists — a system's `primitives` + // and the layout's `unowned` bucket — must not change + // `canonical_bytes()`, even though `PartialEq` sees the difference: + // the same exclusion `vertical_band` gets (Chapter 7's ownership + // partition draws nothing). + let system = ResolvedSystem { + provenance: Provenance::projected(TypedObjectId::Event(EventId::from_raw(9)), vec![]), + bounding_box: Rect::default(), + staves: vec![], + measures: vec![], + primitives: PrimitiveIndices::default(), + }; + let page = ResolvedPage { + provenance: Provenance::projected(TypedObjectId::Event(EventId::from_raw(10)), vec![]), + number: 1, + size: Size2D::default(), + margins: Margins::default(), + systems: vec![system], + free_objects: vec![], + }; + let mut base = ir(vec![glyph(1, 1.0)], vec![]); + base.pages = vec![page]; + let base_bytes = base.canonical_bytes(); + + let mut perturbed = base.clone(); + perturbed.pages[0].systems[0].primitives = PrimitiveIndices { + glyphs: vec![0], + strokes: vec![], + curves: vec![], + }; + perturbed.unowned = PrimitiveIndices { + glyphs: vec![], + strokes: vec![7], + curves: vec![3, 4], + }; + + assert_ne!( + base, perturbed, + "PartialEq must see the ownership difference" + ); + assert_eq!( + base_bytes, + perturbed.canonical_bytes(), + "canonical bytes must not see it" + ); + } } diff --git a/crates/epiphany-layout-ir/src/solver.rs b/crates/epiphany-layout-ir/src/solver.rs index 0f43ca5..950b671 100644 --- a/crates/epiphany-layout-ir/src/solver.rs +++ b/crates/epiphany-layout-ir/src/solver.rs @@ -29,7 +29,9 @@ use epiphany_core::TypedObjectId; use crate::constrained::{ConstrainedLayoutIR, GlyphObjectId}; use crate::glyph::{all_available, BravuraCatalog, GlyphCatalog}; -use crate::resolved::{ResolvedGlyph, ResolvedLayoutIR, ResolvedPage, ResolvedSystem}; +use crate::resolved::{ + PrimitiveIndices, ResolvedGlyph, ResolvedLayoutIR, ResolvedPage, ResolvedSystem, +}; use crate::spatial::{Margins, Rect, Size2D}; use crate::vertical_band::VerticalBandId; @@ -487,6 +489,15 @@ impl StubSolver { Vec::new() }; let resolved_glyphs = glyphs.len(); + // The stub resolves no per-system geometry (every region becomes one + // degenerate default-rect system, per below), so it has no honest + // per-system attribution to publish: every primitive is unowned + // rather than a fabricated claim (W1 pin 3). + let unowned = PrimitiveIndices { + glyphs: (0..glyphs.len() as u32).collect(), + strokes: (0..strokes.len() as u32).collect(), + curves: (0..curves.len() as u32).collect(), + }; let pages = input .regions .first() @@ -503,6 +514,7 @@ impl StubSolver { bounding_box: Rect::default(), staves: Vec::new(), measures: Vec::new(), + primitives: PrimitiveIndices::default(), }) .collect(), free_objects: Vec::new(), @@ -523,6 +535,7 @@ impl StubSolver { curves, engraving_decisions: input.engraving_decisions.clone(), catalog: input.catalog.clone(), + unowned, }, unsatisfied_constraints: Vec::new(), warnings, @@ -728,6 +741,76 @@ mod tests { assert!(report.warnings.is_empty()); } + #[test] + fn the_stub_solver_publishes_every_primitive_unowned() { + // (m2) The stub resolves no per-system geometry (every region becomes + // one degenerate default-rect `ResolvedSystem`, `resolve` above), so + // it must not coerce an unattributed primitive onto system 0 — real, + // non-trivial counts, via the real `to_logical`/`to_constrained` + // pipeline over a rich generated score (RS-2's own construction). + use crate::{to_constrained, to_logical}; + use epiphany_core::generators::valid_score_rich; + + let score = valid_score_rich(0xF302); + let mut input = to_constrained(&to_logical(&score)); + assert_eq!( + input.regions.len(), + 3, + "the rich fixture's real region count" + ); + // `valid_score_rich` carries no slur, so hand-add a curve (mirrors + // `strokes_survive_the_solve_and_enter_the_canonical_bytes` below) to + // exercise all three flat arrays, not just glyphs and strokes. + input.curves.push(crate::Curve { + provenance: input.glyphs[0].provenance.clone(), + p0: crate::Point::new(0.0, 0.0), + p1: crate::Point::new(1.0, 1.0), + p2: crate::Point::new(2.0, 1.0), + p3: crate::Point::new(3.0, 0.0), + thickness: crate::StaffSpace(0.1), + layer: 0, + style: crate::GlyphStyle::default(), + line: epiphany_core::LineStyle::Solid, + vertical_band: input.glyphs[0].vertical_band, + }); + + let report = StubSolver.solve(&input, &SolverConfig::default()); + let layout = &report.layout; + // Real counts (not `> 0`): the rich fixture's own glyph/stroke tally. + assert_eq!(layout.glyphs.len(), input.glyphs.len()); + assert_eq!(layout.strokes.len(), input.strokes.len()); + assert_eq!( + layout.glyphs.len(), + 11, + "the rich fixture's real glyph count" + ); + assert_eq!( + layout.strokes.len(), + 38, + "the rich fixture's real stroke count" + ); + assert_eq!(layout.curves.len(), 1); + + // Every system's own bucket is empty — the stub attributes nothing. + let system_count = layout.systems().count(); + assert_eq!(system_count, input.regions.len()); + for system in layout.systems() { + assert!(system.primitives.glyphs.is_empty()); + assert!(system.primitives.strokes.is_empty()); + assert!(system.primitives.curves.is_empty()); + } + // Everything unowned: the exact index range, in order. + assert_eq!( + layout.unowned.glyphs, + (0..layout.glyphs.len() as u32).collect::>() + ); + assert_eq!( + layout.unowned.strokes, + (0..layout.strokes.len() as u32).collect::>() + ); + assert_eq!(layout.unowned.curves, vec![0]); + } + #[test] fn strokes_survive_the_solve_and_enter_the_canonical_bytes() { let mut input = constrained(vec![glyph("noteheadBlack")]); diff --git a/crates/epiphany-render-svg/src/svg.rs b/crates/epiphany-render-svg/src/svg.rs index 7f8c74f..23d9844 100644 --- a/crates/epiphany-render-svg/src/svg.rs +++ b/crates/epiphany-render-svg/src/svg.rs @@ -1054,6 +1054,7 @@ mod tests { curves: vec![], engraving_decisions: vec![], catalog: Default::default(), + unowned: Default::default(), }; let out = render(&layout, &RenderOptions::default()); assert!(out.is_well_formed()); diff --git a/crates/epiphany-testkit/src/layout_stub.rs b/crates/epiphany-testkit/src/layout_stub.rs index 4c5d50b..55c7e84 100644 --- a/crates/epiphany-testkit/src/layout_stub.rs +++ b/crates/epiphany-testkit/src/layout_stub.rs @@ -467,6 +467,11 @@ pub fn gen_resolved_system(rng: &mut Rng) -> ResolvedSystem { measures: (0..rng.range_usize(0, 2)) .map(|_| gen_resolved_measure(rng)) .collect(), + // Not tied to any real flat array here (this generator builds an + // isolated `ResolvedSystem`, not a whole `ResolvedLayoutIR`), so a + // random index list would be equally fake; left empty rather than + // fabricated. + primitives: Default::default(), } }