From 28f210e16f692bc36a56b0cedeba433564be3193 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Wed, 8 Jul 2026 16:07:13 -0400 Subject: [PATCH] E2 follow-up: surface slur style deferral, conservative curve hit capsule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three source-audit findings on 81b7f42, fixed: 1. Slur kind and line style were dropped at projection and every slur drew solid with no signal. SlurContent now carries `kind` (SlurKind) and `line` (LineStyle) through the projection, and a non-Solid authored line style emits a `LayoutDiagnosticKind::SlurLineStyleNotRendered` — the curve still draws (solid, ink + provenance kept), but the ignored dash/dotted intent is surfaced, not papered over. The Minimal tier still draws one canonical arc per kind (kind-aware rendering is higher-tier); the kind is now preserved for it. The slur fixture's editorial slur is authored Dashed to exercise it. 2. Curve hit-testing flattened to a fixed 16 chords and tested capsules at exactly half_width, so a thin/high-curvature slur's true ink between samples could miss a click. contains/intersects_rect now inflate the capsule by a proven flattening-error bound (h²/8·max‖B''‖ = (3/4N²)·max second-difference) — provably conservative, so a click on the drawn arc never misses. The AABB (control hull ± half_width) already bounds the true ink, unchanged. 3. The engrave crate-level doc still said "no drawn slur geometry exists yet"; updated to slur_shape 0.0 by construction (Minimal draws the ideal arc), beam_slope still vacuous. 931 tests, docs clean, conformance 8/8. Zero golden churn (dashed slur draws solid; layout diagnostics don't reach the render output). Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/epiphany-engrave/src/lib.rs | 6 +- crates/epiphany-layout-ir/DECISIONS.md | 14 +++- crates/epiphany-layout-ir/src/constrained.rs | 50 +++++++++++- crates/epiphany-layout-ir/src/hittest.rs | 86 ++++++++++++++++++-- crates/epiphany-layout-ir/src/logical.rs | 15 +++- crates/epiphany-testkit/src/fixtures.rs | 11 ++- 6 files changed, 162 insertions(+), 20 deletions(-) diff --git a/crates/epiphany-engrave/src/lib.rs b/crates/epiphany-engrave/src/lib.rs index f9ee73e..9b6c0e6 100644 --- a/crates/epiphany-engrave/src/lib.rs +++ b/crates/epiphany-engrave/src/lib.rs @@ -51,8 +51,10 @@ //! the private `quality` module computes all nine normative axes per the //! ratified *Quality Metric Catalog* companion (collision census, spacing //! regularity, break/page/casting-off distribution, vertical gap deviation; -//! slur/beam shape are vacuous-`0.0` because no drawn slur/beam geometry exists -//! yet), normalized through the catalog's pinned anchors +//! `slur_shape` is `0.0` **by construction** now that slurs draw — the Minimal +//! tier emits the ideal arc, so a drawn slur has zero shape deviation — while +//! `beam_slope` stays vacuous-`0.0` because no drawn beam geometry exists yet), +//! normalized through the catalog's pinned anchors //! ([`epiphany_layout_ir::quality`]), with //! [`SolverWarningKind::QualityFloorApproached`] diagnostics against the //! threshold column the config's profile selects. The all-worst diff --git a/crates/epiphany-layout-ir/DECISIONS.md b/crates/epiphany-layout-ir/DECISIONS.md index 8affd71..6518689 100644 --- a/crates/epiphany-layout-ir/DECISIONS.md +++ b/crates/epiphany-layout-ir/DECISIONS.md @@ -581,9 +581,17 @@ out of scope), so these are E2 decisions for the Phase-F ratification pass: - **curvature_override honored structurally.** `direction` (Above/Below; default Auto = above) flips the arc; `height` (a `SpaceUnit`) sets the apex, else a span-proportional default clamped to `[SLUR_MIN_HEIGHT, - SLUR_MAX_HEIGHT]`. `style.line` (dashed/dotted) is a Push-3 refinement — the - Minimal tier draws every slur solid (the curve carries an `ink` style like a - stroke); nothing authored is lost, only its dash rendering deferred. + SLUR_MAX_HEIGHT]`. +- **Kind and line style: carried, and the deferral is surfaced (review fix).** + `SlurContent` carries `kind` (`SlurKind`) and `line` (`LineStyle`) through the + projection — nothing is dropped — but the Minimal tier draws one canonical + solid arc for *every* kind (a phrase mark's longer curve, an editorial + slur's distinct line are kind-aware higher-tier work). `style.line` (dashed/ + dotted) is likewise a Push-3 refinement; rather than silently rendering an + authored dashed slur solid, a non-`Solid` line style emits a + `LayoutDiagnosticKind::SlurLineStyleNotRendered` — the curve still draws + (solid, ink and provenance preserved), but the ignored intent is surfaced, + not papered over (the crate's non-overreach discipline). - **Honest non-drawing.** No curve is drawn — the traced anchor keeps provenance, the same discipline as an unresolvable repeat boundary — when: an endpoint is unresolved (dangling event, or a column in another region); diff --git a/crates/epiphany-layout-ir/src/constrained.rs b/crates/epiphany-layout-ir/src/constrained.rs index c629ad9..03b4fde 100644 --- a/crates/epiphany-layout-ir/src/constrained.rs +++ b/crates/epiphany-layout-ir/src/constrained.rs @@ -13,8 +13,8 @@ use std::cmp::Ordering; use std::collections::{BTreeMap, BTreeSet}; use epiphany_core::{ - Clef, EventId, KeySignature, MeasureId, MeasurePosition, MusicalDuration, NoteValue, PitchId, - PitchSpelling, RepeatStructureId, SpellingNominal, StaffId, TimeAnchor, TypedObjectId, + Clef, EventId, KeySignature, LineStyle, MeasureId, MeasurePosition, MusicalDuration, NoteValue, + PitchId, PitchSpelling, RepeatStructureId, SpellingNominal, StaffId, TimeAnchor, TypedObjectId, WallClockTime, }; use epiphany_determinism::{DomainTag, Preimage}; @@ -176,6 +176,11 @@ pub enum LayoutDiagnosticKind { /// sixteenth-or-shorter rest); the object is carried as a traced anchor /// rather than drawn at a guessed shape. UnbundledGlyph(GlyphReference), + /// A slur carries an authored non-`Solid` line style (dashed/dotted) that + /// the Minimal tier does not render — the curve is drawn solid, and the + /// unrendered style is surfaced here rather than silently dropped (its dash + /// pattern is a higher-tier refinement). + SlurLineStyleNotRendered, } #[derive(Copy, Clone, PartialEq, Eq, Debug, Default)] @@ -1471,7 +1476,20 @@ pub fn try_to_constrained( _ => None, }; match curve { - Some(curve) => emit.curve(curve), + Some(curve) => { + // A drawn slur whose authored line style is not solid + // renders solid at this tier; surface the gap so the + // dash/dotted intent is not silently lost. + if let Some(LayoutContent::Slur(slur)) = content { + if slur.line != LineStyle::Solid { + emit.diag( + provenance.source, + LayoutDiagnosticKind::SlurLineStyleNotRendered, + ); + } + } + emit.curve(curve); + } None => emit.stroke(anchor(provenance, Point::new(default_x, yo))), } } @@ -4417,6 +4435,32 @@ mod tests { crate::roundtrip::round_trip(&score); } + #[test] + fn an_authored_dashed_slur_draws_solid_but_surfaces_a_diagnostic() { + use epiphany_core::{LineStyle, SpanStyle}; + let (mut score, _) = repeat_ready_score(45); + let events = region_a_events(&score); + let id: SlurId = score.identity.mint(); + let mut dashed = slur(id, events[0], events[2], None); + dashed.style = SpanStyle { + line: LineStyle::Dashed, + thickness: None, + }; + score.cross_cutting.slurs.push(dashed); + let constrained = to_constrained(&to_logical(&score)); + + // The curve still draws (solid) — ink and provenance preserved. + assert!(slur_curve_of(&constrained, id).is_some()); + // …and the unrendered dash style is surfaced, not silently dropped. + assert!( + constrained.diagnostics.iter().any(|d| { + d.source == TypedObjectId::Slur(id) + && d.kind == LayoutDiagnosticKind::SlurLineStyleNotRendered + }), + "a dashed slur surfaces a SlurLineStyleNotRendered diagnostic" + ); + } + #[test] fn out_of_range_authored_slur_dimensions_fall_back_to_defaults() { let (mut score, _) = repeat_ready_score(44); diff --git a/crates/epiphany-layout-ir/src/hittest.rs b/crates/epiphany-layout-ir/src/hittest.rs index 463857d..15832b4 100644 --- a/crates/epiphany-layout-ir/src/hittest.rs +++ b/crates/epiphany-layout-ir/src/hittest.rs @@ -57,8 +57,10 @@ pub enum HitShape { }, /// A curve (slur, …): its four cubic-bézier control points and a half-width. /// Its geometry tests flatten the cubic into [`CURVE_FLATTEN_SEGMENTS`] - /// straight capsule segments (a polyline), so a click near the drawn arc - /// selects it — one region per curve, unlike per-segment hit fragments. + /// straight capsule segments (a polyline), inflated by a conservative + /// flattening-error bound so a click on the true drawn arc — which can bow + /// outside the sampled chords for a thin, high-curvature slur — never + /// misses. One region per curve, unlike per-segment hit fragments. Curve { p0: Point, p1: Point, @@ -94,6 +96,26 @@ fn flatten_cubic(p0: Point, p1: Point, p2: Point, p3: Point) -> Vec { .collect() } +/// A **conservative** upper bound on the maximum distance between the true cubic +/// and its [`flatten_cubic`] polyline. The chord error over a parameter +/// interval of length `h` is at most `h²/8 · max‖B''‖`; a cubic's second +/// derivative is linear, so `max‖B''‖ = 6 · max(‖p0−2p1+p2‖, ‖p1−2p2+p3‖)` +/// (attained at an endpoint), and `h = 1/N`. The [`HitShape::Curve`] tests +/// inflate the capsule half-width by this bound so a click on the *drawn* arc — +/// which can bow outside the flattened chords for a thin, high-curvature slur — +/// never misses (the true curve lies within this distance of the polyline). +fn flatten_error_bound(p0: Point, p1: Point, p2: Point, p3: Point) -> f32 { + let second_diff = |a: Point, b: Point, c: Point| { + let dx = a.x.0 - 2.0 * b.x.0 + c.x.0; + let dy = a.y.0 - 2.0 * b.y.0 + c.y.0; + (dx * dx + dy * dy).sqrt() + }; + let d = second_diff(p0, p1, p2).max(second_diff(p1, p2, p3)); + let n = CURVE_FLATTEN_SEGMENTS as f32; + // h²/8 · 6·d = (3/(4N²))·d. + (3.0 * d) / (4.0 * n * n) +} + impl HitShape { /// Whether a world `point` lies within this shape — the click-selection test. /// A box is closed (edges included); a segment is within `half_width` of the @@ -112,9 +134,14 @@ impl HitShape { p2, p3, half_width, - } => flatten_cubic(*p0, *p1, *p2, *p3) - .windows(2) - .any(|seg| distance_point_segment(point, seg[0], seg[1]) <= *half_width), + } => { + // Inflate by the flattening-error bound so ink that bows outside + // the sampled chords is still hit (see `flatten_error_bound`). + let reach = *half_width + flatten_error_bound(*p0, *p1, *p2, *p3); + flatten_cubic(*p0, *p1, *p2, *p3) + .windows(2) + .any(|seg| distance_point_segment(point, seg[0], seg[1]) <= reach) + } } } @@ -141,9 +168,12 @@ impl HitShape { p2, p3, half_width, - } => flatten_cubic(*p0, *p1, *p2, *p3) - .windows(2) - .any(|seg| segment_intersects_rect(seg[0], seg[1], *half_width, &rect)), + } => { + let reach = *half_width + flatten_error_bound(*p0, *p1, *p2, *p3); + flatten_cubic(*p0, *p1, *p2, *p3) + .windows(2) + .any(|seg| segment_intersects_rect(seg[0], seg[1], reach, &rect)) + } } } @@ -555,6 +585,46 @@ mod tests { assert!(s.intersects_rect(BoundingBox::new(1.5, 1.3, 2.5, 1.7))); } + #[test] + fn a_thin_high_curvature_curve_is_hit_on_its_true_arc_between_samples() { + // A near-degenerate-thin curve (half_width 0.001) with strong curvature: + // a point ON the true cubic strictly between two flatten samples bows + // outside the chord capsule at that half-width, but the error-bound + // inflation still hits it. + let (p0, p1, p2, p3) = ( + Point::new(0.0, 0.0), + Point::new(0.0, 6.0), + Point::new(6.0, 6.0), + Point::new(6.0, 0.0), + ); + let s = HitShape::Curve { + p0, + p1, + p2, + p3, + half_width: 0.001, + }; + // A true-cubic point at a parameter offset from the 1/16 grid (t = 1/32 + // falls between samples 0 and 1). + let on_arc = cubic_point(p0, p1, p2, p3, 1.0 / 32.0); + assert!( + s.contains(on_arc), + "a point on the drawn arc between flatten samples must be hit" + ); + // The error bound is positive for a genuinely curved shape and zero for + // collinear control points (a straight "curve"). + assert!(flatten_error_bound(p0, p1, p2, p3) > 0.0); + assert_eq!( + flatten_error_bound( + Point::new(0.0, 0.0), + Point::new(1.0, 0.0), + Point::new(2.0, 0.0), + Point::new(3.0, 0.0), + ), + 0.0 + ); + } + #[test] fn a_curve_becomes_one_hit_region_tracing_its_source() { use epiphany_core::{SlurId, TypedObjectId}; diff --git a/crates/epiphany-layout-ir/src/logical.rs b/crates/epiphany-layout-ir/src/logical.rs index b0f6091..697d647 100644 --- a/crates/epiphany-layout-ir/src/logical.rs +++ b/crates/epiphany-layout-ir/src/logical.rs @@ -18,9 +18,9 @@ use std::collections::{BTreeMap, BTreeSet}; use epiphany_core::prepass::{derive_annotations, DerivedAnnotations, PrePassProfile}; use epiphany_core::{ AleatoricAnchoringDiscipline, AnchorOffset, AnnotationAnchor, CanonicalValue, Clef, - CoordinateDiscipline, Event, EventId, EventPosition, KeySignature, MeasurePosition, + CoordinateDiscipline, Event, EventId, EventPosition, KeySignature, LineStyle, MeasurePosition, MusicalDuration, MusicalPosition, NotatedComponent, PitchId, PitchSpelling, Region, RegionEdge, - RegionId, RegionTimeModel, Score, SpaceUnit, StaffId, StaffPosition, TimeAnchor, + RegionId, RegionTimeModel, Score, SlurKind, SpaceUnit, StaffId, StaffPosition, TimeAnchor, TimeSignatureDisplay, TupletId, TupletRatio, TypedObjectId, WallClockTime, }; use epiphany_determinism::{DomainTag, Preimage}; @@ -220,6 +220,15 @@ pub struct SlurContent { /// Authored line thickness (`style.thickness`); `None` = the engraver's /// default. pub thickness: Option, + /// The slur's kind (Chapter 5 `SlurKind`). Carried through the projection — + /// the Minimal tier draws one canonical arc for every kind, but the kind is + /// preserved for a kind-aware higher tier (a phrase mark's longer curve, an + /// editorial slur's distinct line). + pub kind: SlurKind, + /// The authored line style (`style.line`). The Minimal tier draws every + /// slur solid; a non-`Solid` style is surfaced as a diagnostic rather than + /// silently rendered solid (its dash pattern is a higher-tier refinement). + pub line: LineStyle, } /// A slur endpoint on the region's spacing axis. @@ -1087,6 +1096,8 @@ fn slur_content(score: &Score, slur: &epiphany_core::Slur) -> LayoutContent { direction, height: slur.curvature_override.as_ref().and_then(|o| o.height), thickness: slur.style.thickness, + kind: slur.kind, + line: slur.style.line, }) } diff --git a/crates/epiphany-testkit/src/fixtures.rs b/crates/epiphany-testkit/src/fixtures.rs index 4711b1b..f554c89 100644 --- a/crates/epiphany-testkit/src/fixtures.rs +++ b/crates/epiphany-testkit/src/fixtures.rs @@ -12,7 +12,7 @@ use epiphany_core::{ AcousticPitch, AcousticRealization, AnchorOffset, Canvas, ChordSymbol, CmnNominal, CrossCuttingRegistry, CurvatureOverride, CurveDirection, Event, EventArena, EventDuration, - EventPosition, IdentifiedPitch, IdentityContext, Marker, Measure, MeasurePosition, + EventPosition, IdentifiedPitch, IdentityContext, LineStyle, Marker, Measure, MeasurePosition, MetricTimeModel, MusicalDuration, MusicalPosition, Pitch, PitchSpaceId, PitchSpacePosition, RationalTime, RegionContent, RegionEdge, RegionTimeModel, RepeatKind, RepeatStructure, ScalePosition, Score, Slur, SlurKind, SpaceUnit, SpanStyle, Spanner, Staff, StaffBasedContent, @@ -296,7 +296,14 @@ pub fn ten_measure_with_slurs(seed: u64) -> Score { end_event: events[12], kind: SlurKind::Editorial, curvature_override: None, - style: SpanStyle::default(), + // An authored dashed line — the Minimal tier draws it solid and + // surfaces a `SlurLineStyleNotRendered` layout diagnostic (its dash + // pattern is a higher-tier refinement), so the fixture exercises that + // honest-deferral path. + style: SpanStyle { + line: LineStyle::Dashed, + thickness: None, + }, }); score }