diff --git a/crates/epiphany-layout-ir/DECISIONS.md b/crates/epiphany-layout-ir/DECISIONS.md index 78c8dcd..4cee811 100644 --- a/crates/epiphany-layout-ir/DECISIONS.md +++ b/crates/epiphany-layout-ir/DECISIONS.md @@ -778,7 +778,7 @@ always-up + fixed-octave fails it. and `stemDownNW` at −0.168, so a stem should meet the head slightly off its centre; we attach at the centre. Cosmetic at this tier. -## Parked: the notehead stem anchors are unusable as written (2026-07-09) +## RESOLVED (P13-I3): the notehead stem anchors are deleted (2026-07-09) `BRAVURA_METRICS`' `NOTEHEAD_ANCHORS` declares `stemUpNW` at x = 0 and `stemDownSE` at x = 1180 (i.e. 1.152 staff spaces). Two problems, found while @@ -797,8 +797,28 @@ moves the `GlyphCatalogIdentity` every conformance claim declares. Hence parked rather than fixed in passing. The stem work sidesteps them by reading the head's bounding box, whose right edge (1.1807) *is* the correct attachment. -**Three parked candidates now stand** (this, `Staff::default_clef`, and the -`ConstrainedLayoutIR` listing gap). The house rule opens a batch pass at ≥3. +**Resolved as P13-I3: deleted, and the extractor taught to emit them.** They were +hand-derived where every neighbouring number in the table is machine-extracted +from the SHA-pinned font — the same mistake as inferring band ownership downstream +instead of reading it from the source that had it. Shipping data we cannot stand +behind, into a hash every conformance claim declares, is worse than shipping none. +`extract_bravura_outlines.py --anchors` now emits anchors from the pinned +`bravura_metadata.json`; the table regains them at the next regeneration, +generated rather than remembered. That metadata's SHA-256 is deliberately left +unpinned and `verify()` refuses against an unpinned source, printing the digest to +paste — the script cannot regenerate anchors until an operator with the font pins +it in a reviewable commit. + +`GlyphCatalogIdentity` moves once, now, while no conformance claim declares the +old one. Verified empirically before deciding: changing the anchors breaks nothing +in-tree (30/30 targets, zero golden churn, no pinned literal hash). + +**And the test that guarded them proved nothing.** `anchors_participate_in_the_hash` +compared `noteheadBlack` (anchored) with `noteheadWhole` (not) and asserted they +hash differently — but their `advance` and `bbox` differ too, so it passed with +the anchors ignored entirely. It now varies the anchors while holding every other +field fixed (presence, a coordinate, a name), against synthetic metrics through a +factored-out `metrics_hash_of`. ## Slur placement: side, endpoints, and clearance (2026-07-09) diff --git a/crates/epiphany-layout-ir/src/glyph.rs b/crates/epiphany-layout-ir/src/glyph.rs index 0f4b75a..20709a9 100644 --- a/crates/epiphany-layout-ir/src/glyph.rs +++ b/crates/epiphany-layout-ir/src/glyph.rs @@ -181,18 +181,6 @@ impl GlyphMetric { } } -const STEM_UP_NW: GlyphAnchor = GlyphAnchor { - name: Cow::Borrowed("stemUpNW"), - x: 0, - y: 0, -}; -const STEM_DOWN_SE: GlyphAnchor = GlyphAnchor { - name: Cow::Borrowed("stemDownSE"), - x: 1180, - y: 0, -}; -const NOTEHEAD_ANCHORS: &[GlyphAnchor] = &[STEM_UP_NW, STEM_DOWN_SE]; - /// A representative in-tree slice of Bravura's SMuFL metrics /// (`(name, advance, [left, bottom, right, top])`, `1/1024`-staff-space units), /// extracted from the SHA-pinned `bravura-1.392` font by `epiphany-render-svg`'s @@ -203,17 +191,19 @@ const NOTEHEAD_ANCHORS: &[GlyphAnchor] = &[STEM_UP_NW, STEM_DOWN_SE]; /// a containing box keeps a no-collision result honest on paper (a `render-svg` test /// proves the containment). Every glyph the v0 pipeline names is in this table; the /// stub solver checks that, so a missing entry surfaces as -/// [`crate::SolveStatus::InternalError`]. The named anchors are SMuFL -/// engraving-default approximations (font metadata, not glyf bounds — so not part of -/// the outline extraction). +/// [`crate::SolveStatus::InternalError`]. +/// +/// **No glyph carries anchors.** They live in the font's SMuFL metadata +/// (`bravura_metadata.json`), not its glyf bounds, so the outline extraction never +/// covered them and the two the table once held were written by hand — and were +/// wrong under any reading (P13-I3). `extract_bravura_outlines.py --anchors` now +/// emits them from the pinned metadata; the table regains them at the next +/// regeneration, generated rather than remembered. Nothing consumes anchors today: +/// a stem takes its attachment from the notehead's bounding box, whose edges are +/// the anchor x's the font declares. pub const BRAVURA_METRICS: &[GlyphMetric] = &[ - GlyphMetric::anchored( - "noteheadBlack", - 1208, - [0, -512, 1209, 512], - NOTEHEAD_ANCHORS, - ), - GlyphMetric::anchored("noteheadHalf", 1208, [0, -512, 1209, 512], NOTEHEAD_ANCHORS), + GlyphMetric::new("noteheadBlack", 1208, [0, -512, 1209, 512]), + GlyphMetric::new("noteheadHalf", 1208, [0, -512, 1209, 512]), GlyphMetric::new("noteheadWhole", 1729, [0, -512, 1729, 512]), GlyphMetric::new("noteheadDoubleWhole", 2454, [0, -635, 2454, 635]), GlyphMetric::new("gClef", 2748, [0, -2696, 2749, 4498]), @@ -361,10 +351,26 @@ impl GlyphCatalog for BravuraCatalog { /// metrics — every glyph delivered to a solve MUST name available metrics. pub fn metrics_hash_for<'a>(names: impl IntoIterator) -> [u8; 32] { let names: BTreeSet<&str> = names.into_iter().collect(); + let entries: Vec<(&str, &GlyphMetric)> = names + .into_iter() + .map(|name| { + ( + name, + metrics(name).expect("every consulted glyph must name bundled metrics"), + ) + }) + .collect(); + metrics_hash_of(&entries) +} + +/// Hashes `(name, metrics)` pairs already in canonical name order — the body of +/// [`metrics_hash_for`], factored out so a test can hash metrics that are not in +/// the bundled table (the only way to prove a *field* participates in the hash is +/// to vary it while holding every other field fixed). +fn metrics_hash_of(entries: &[(&str, &GlyphMetric)]) -> [u8; 32] { let mut p = Preimage::new(DomainTag::FONT_METRICS); - p.push_u64_le(names.len() as u64); - for name in names { - let m = metrics(name).expect("every consulted glyph must name bundled metrics"); + p.push_u64_le(entries.len() as u64); + for (name, m) in entries { p.push_u64_le(name.len() as u64); p.push_bytes(name.as_bytes()); p.push_u64_le(m.advance as u64); @@ -424,16 +430,66 @@ mod tests { assert_eq!(a, b); } + /// A glyph's anchors are part of its catalog identity: two catalogs whose + /// glyphs differ only in an anchor are different inputs, and a byte-equal + /// conformance claim across them is not expected. + /// + /// This must be shown by varying the anchors while holding every other field + /// fixed. The previous version of this test compared `noteheadBlack` (which + /// carried anchors) with `noteheadWhole` (which did not) and asserted they + /// hash differently — but their `advance` and `bbox` differ too, so it would + /// have passed with the anchors ignored entirely. It proved nothing. #[test] fn anchors_participate_in_the_hash() { - // noteheadBlack carries stem anchors; noteheadWhole does not. Even with - // equal bbox/advance they must hash differently. - assert_ne!( - metrics_hash_for(["noteheadBlack"]), - metrics_hash_for(["noteheadWhole"]) - ); - assert!(!metrics("noteheadBlack").unwrap().anchors.is_empty()); - assert!(metrics("noteheadWhole").unwrap().anchors.is_empty()); + let bare = GlyphMetric::new("g", 1024, [0, 0, 1024, 1024]); + let anchored = GlyphMetric { + anchors: Cow::Owned(vec![GlyphAnchor { + name: Cow::Owned("stemUpSE".to_owned()), + x: 1208, + y: 172, + }]), + ..bare.clone() + }; + let moved = GlyphMetric { + anchors: Cow::Owned(vec![GlyphAnchor { + name: Cow::Owned("stemUpSE".to_owned()), + x: 1209, + y: 172, + }]), + ..bare.clone() + }; + let renamed = GlyphMetric { + anchors: Cow::Owned(vec![GlyphAnchor { + name: Cow::Owned("stemDownNW".to_owned()), + x: 1208, + y: 172, + }]), + ..bare.clone() + }; + let hash = |m: &GlyphMetric| metrics_hash_of(&[("g", m)]); + assert_ne!(hash(&bare), hash(&anchored), "presence changes the hash"); + assert_ne!(hash(&anchored), hash(&moved), "a coordinate does too"); + assert_ne!(hash(&anchored), hash(&renamed), "and so does a name"); + } + + /// The bundled table carries no anchors. They were hand-written where every + /// other number in it is machine-extracted from the SHA-pinned font, and they + /// were wrong under any reading: they named `stemUpNW`/`stemDownSE` — corners + /// a normal notehead's stems do not attach to, and a pair Bravura's + /// `noteheadBlack` does not define — with an x of 1180, which reads as 1.18 + /// staff spaces written in thousandths rather than the table's 1/1024 units + /// (1.18 sp = 1208). Nothing consumed them. `extract_bravura_outlines.py` + /// now emits anchors from `bravura_metadata.json`; the table regains them at + /// the next regeneration, generated rather than remembered (P13-I3). + #[test] + fn the_bundled_table_carries_no_hand_written_anchors() { + for metric in BRAVURA_METRICS { + assert!( + metric.anchors.is_empty(), + "{} carries hand-written anchors", + metric.name + ); + } } #[test] diff --git a/crates/epiphany-render-svg/tools/extract_bravura_outlines.py b/crates/epiphany-render-svg/tools/extract_bravura_outlines.py index 7ea1b75..923ee6d 100644 --- a/crates/epiphany-render-svg/tools/extract_bravura_outlines.py +++ b/crates/epiphany-render-svg/tools/extract_bravura_outlines.py @@ -27,8 +27,17 @@ FONT_REF = "301087ca0b0d30b65d81bc3e718ff64b613e2a9a" NAMES_REF = "31a327a29640c313b12076739987bd7f25bdddde" # w3c/smufl gh-pages FONT_URL = f"https://raw.githubusercontent.com/steinbergmedia/bravura/{FONT_REF}/redist/otf/Bravura.otf" NAMES_URL = f"https://raw.githubusercontent.com/w3c/smufl/{NAMES_REF}/metadata/glyphnames.json" +METADATA_URL = f"https://raw.githubusercontent.com/steinbergmedia/bravura/{FONT_REF}/redist/bravura_metadata.json" FONT_SHA256 = "dca2d90c88437a701b1c2e71fa54e76f9fa41d7deee935d74dc871ea66ecfdd2" NAMES_SHA256 = "1d05352599a20983d1c901635dc75d76f063c0987a7bee65f145325fc3e0d29f" +# UNPINNED. `bravura_metadata.json` (the source of glyph anchors) was never +# fetched by this script, so its digest was never recorded. Run the script once +# with the font available: `verify` prints the actual digest and refuses to +# proceed. Paste it here, in the same reviewable-commit spirit as FONT_SHA256. +# Until then `--anchors` refuses rather than regenerating against an unverified +# source, and `BRAVURA_METRICS` carries no anchors at all — which is correct: the +# ones it used to carry were hand-written, and wrong (see P13-I3). +METADATA_SHA256 = "" # Exactly the glyph set the v0 layout pipeline can name (layout-ir BRAVURA_METRICS). NAMES = ["noteheadBlack","noteheadHalf","noteheadWhole","noteheadDoubleWhole", @@ -42,6 +51,10 @@ NAMES = ["noteheadBlack","noteheadHalf","noteheadWhole","noteheadDoubleWhole", def verify(data, expected, what): actual = hashlib.sha256(data).hexdigest() + if not expected: + sys.exit(f"{what} has no pinned SHA-256; refusing to regenerate against an " + f"unverified source.\n actual {actual}\n" + "paste that into this script's checksum constant to pin it deliberately") if actual != expected: sys.exit(f"{what} SHA-256 mismatch:\n expected {expected}\n actual {actual}\n" "the pinned source changed; refusing to regenerate against an unverified font " @@ -51,7 +64,8 @@ def verify(data, expected, what): def load(): from fontTools.ttLib import TTFont import io - if "--local" in sys.argv: + local = "--local" in sys.argv + if local: font_bytes = open("Bravura.otf", "rb").read() names_bytes = open("glyphnames.json", "rb").read() else: @@ -61,7 +75,29 @@ def load(): verify(names_bytes, NAMES_SHA256, "glyphnames.json") font = TTFont(io.BytesIO(font_bytes)) names = json.loads(names_bytes) - return font, names, font_bytes + + # Glyph anchors live in the font's SMuFL metadata, not the font itself. + meta = None + if "--anchors" in sys.argv: + meta_bytes = (open("bravura_metadata.json", "rb").read() if local + else urllib.request.urlopen(METADATA_URL).read()) + verify(meta_bytes, METADATA_SHA256, "bravura_metadata.json") + meta = json.loads(meta_bytes).get("glyphsWithAnchors", {}) + return font, names, font_bytes, meta + + +def anchor_rows(meta, name): + """The glyph's SMuFL anchors as `GlyphAnchor` literals, in canonical name + order. SMuFL anchor coordinates are in staff spaces; `BRAVURA_METRICS` is in + 1/1024 staff space. These are *points*, not bounds, so they round to nearest + — unlike a bbox, which rounds outward so the metric box contains the ink.""" + if not meta: + return [] + out = [] + for anchor, (x, y) in sorted(meta.get(name, {}).items()): + out.append(f'GlyphAnchor {{ name: Cow::Borrowed("{anchor}"), ' + f'x: {round(x * 1024)}, y: {round(y * 1024)} }}') + return out # The subset is a Modified Version under the OFL, so its primary user-facing name @@ -152,7 +188,7 @@ def main(): from fontTools.pens.svgPathPen import SVGPathPen from fontTools.pens.transformPen import TransformPen from fontTools.pens.boundsPen import BoundsPen - font, glyphnames, font_bytes = load() + font, glyphnames, font_bytes, anchor_meta = load() upm = font["head"].unitsPerEm sp = upm / 4.0 # font units per staff space (SMuFL em = 4 staff spaces) scale = 1.0 / sp @@ -186,7 +222,13 @@ def main(): print("// --- BRAVURA_METRICS rows (advance, [l,b,r,t] in 1/1024 staff space) ---", file=sys.stderr) for name, adv1024, bbox1024 in sorted(metrics): - print(f' GlyphMetric::new("{name}", {adv1024}, {bbox1024}),', file=sys.stderr) + anchors = anchor_rows(anchor_meta, name) + if anchors: + joined = ", ".join(anchors) + print(f' GlyphMetric::anchored("{name}", {adv1024}, {bbox1024}, ' + f'&[{joined}]),', file=sys.stderr) + else: + print(f' GlyphMetric::new("{name}", {adv1024}, {bbox1024}),', file=sys.stderr) o = [] o.append("//! GENERATED by `tools/extract_bravura_outlines.py` — do not edit by hand.") o.append("//!") diff --git a/spec/PASS13_CANDIDATES.md b/spec/PASS13_CANDIDATES.md index a2a1bd3..7bbbeda 100644 --- a/spec/PASS13_CANDIDATES.md +++ b/spec/PASS13_CANDIDATES.md @@ -19,17 +19,24 @@ execute-then-fix regressions. | P13-D2 | Cue-cascade recursion re-anchors against the triggering event before its tombstone lands in `objects`: a structure anchored on {X, cue-of-X} can record `Reanchored{to: X}` then `CascadeDeleted` in one effect (contradictory repair trail; plausible by code trace, unexecuted) | `crates/epiphany-ops/DECISIONS.md` (Schema major 2, Phase D) | **resolved** (Pass 13: `delete_event` tombstones before the graph delete, matching `cascade_cue`/undo; repro executed then fixed) | | P13-D3 | `CreateCrossCutting` validates only event endpoints (`CrossCuttingValue::endpoints()`), so a SPANNER anchored to a missing region/measure mints dangling past `anchor_target_exists`; and non-event referent tombstones (`DeleteRegion` under a region-anchored spanner/repeat) re-anchor nothing — "every referenced endpoint is live" is events-only as implemented | `crates/epiphany-ops/DECISIONS.md` (Phase D follow-up) | **resolved** (Pass 13: mint fixed via `anchor_object_refs`; non-event referent re-anchoring ratified events-only, user "fix the mint only") | -## Batch 2 — OPEN (2026-07-09) +## Batch 2 — CLOSED (2026-07-09) Three candidates accumulated while the Standard-tier solver track closed and the notation-quality pass (stems, slurs) landed. All three were **parked** as they were found, each in `crates/epiphany-layout-ir/DECISIONS.md`, and reaching three -reopens the pass per the house rule. None is a live incorrectness in shipped -output; each is a place where the code, the spec, and the data disagree about +reopened the pass per the house rule. None was a live incorrectness in shipped +output; each was a place where the code, the spec, and the data disagreed about what is true. +**All three resolved**, worked down in order. Two grew when examined: P13-I1 was +filed as two elided fields and was three, the third (`diagnostics`) named nowhere +in core_spec; P13-I2's fix had to reach `editor-core` as well as the projection, +or a click on a bass staff would have resolved its pitch as treble. Zero golden +churn across all three. No open Pass-13 candidates remain; a future ≥3-candidate +batch reopens the pass. + | Id | One-line statement | Filed in | Status | |---|---|---|---| | P13-I1 | Chapter 7's `ConstrainedLayoutIR` listing elides **three** fields the code carries: `break_origins: Vec` (named by `req:layoutir:break-origin-attribution`, its own shape unlisted), `catalog: GlyphCatalogIdentity` (its type specified, the field unlisted), and `diagnostics: Vec` — which appears **nowhere** in core_spec, though it is how the projection's honesty rule manifests: an unspellable pitch or an unbundled glyph is placed as a fallback *and recorded*, never silently guessed | `crates/epiphany-layout-ir/DECISIONS.md` ("the ConstrainedLayoutIR listing is still abridged") | **resolved** (Pass 13: listing gains all three fields; `BreakOrigin` and `LayoutDiagnostic` shapes added; new `req:layoutir:coverage-diagnostics` ratifies as-implemented that an unengravable object is recorded AND still placed — never guessed, never dropped) | | P13-I2 | `Staff::default_clef` is never consulted: `to_constrained` takes the active clef from the staff instance's `clef_sequence` and falls back to `Clef::default()` (treble), so a bass-clef staff that declares its clef only on the `Staff` engraves as treble. The field is decorative in the projection — is it the fallback, or should it not exist? | `crates/epiphany-layout-ir/DECISIONS.md` ("`Staff::default_clef` is never consulted") | **resolved** (Pass 13: it IS the fallback. `StaffContent` carries it, `active_clef_or` resolves against it, and `editor-core`'s hit-test reads the same function — else a click on a bass staff would resolve its pitch as treble. Removal was rejected: the field is named for its purpose, is encoded on the wire, and dropping it is schema-major) | -| P13-I3 | `BRAVURA_METRICS`' `NOTEHEAD_ANCHORS` are hand-written, unconsumed, and doubly suspect: they name `stemUpNW`/`stemDownSE` — the corners a normal notehead's stems do *not* attach to, and a pair Bravura's `noteheadBlack` does not define — and their x of `1180` reads like 1.18 staff spaces written in thousandths rather than the table's `1/1024` units (1.18 sp = 1208). They enter only `metrics_hash`, so any correction moves the `GlyphCatalogIdentity` every conformance claim declares. The font is not vendored, so the values cannot be verified in-tree | `crates/epiphany-layout-ir/DECISIONS.md` ("the notehead stem anchors are unusable as written") | **open** | +| P13-I3 | `BRAVURA_METRICS`' `NOTEHEAD_ANCHORS` are hand-written, unconsumed, and doubly suspect: they name `stemUpNW`/`stemDownSE` — the corners a normal notehead's stems do *not* attach to, and a pair Bravura's `noteheadBlack` does not define — and their x of `1180` reads like 1.18 staff spaces written in thousandths rather than the table's `1/1024` units (1.18 sp = 1208). They enter only `metrics_hash`, so any correction moves the `GlyphCatalogIdentity` every conformance claim declares. The font is not vendored, so the values cannot be verified in-tree | `crates/epiphany-layout-ir/DECISIONS.md` ("the notehead stem anchors are unusable as written") | **resolved** (Pass 13: deleted. Unverifiable in-tree, unconsumed, and hand-derived where every neighbouring number is machine-extracted. `extract_bravura_outlines.py --anchors` now emits them from the pinned `bravura_metadata.json`, so the table regains them generated rather than remembered; the metadata's SHA-256 is deliberately unpinned and the script refuses until an operator with the font pins it. `GlyphCatalogIdentity` moves once, now, while no conformance claim declares the old one; user "delete them + teach the extractor") |