diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b96714e..ea9f92c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -130,6 +130,19 @@ jobs: - name: Test (GUI) run: cargo test -p epiphany-editor-gui + # Assertion-message artifact paths (`target/golden-failures//actual.png` + # etc.) name the ephemeral runner's filesystem — a local-reproduction aid, not + # a reviewable record on CI. This is the reviewable record: on a golden + # mismatch, the three per-test images (actual/expected/diff) upload so a + # reviewer can see the failure without reproducing it locally. + - name: Upload golden failure artifacts + if: failure() + uses: actions/upload-artifact@v4 + with: + name: golden-failures + path: target/golden-failures/ + if-no-files-found: ignore + conformance: name: conformance suite (testkit) runs-on: ubuntu-latest diff --git a/Cargo.lock b/Cargo.lock index 2bc63e0..4626ca2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1,6 +1,6 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. -version = 3 +version = 4 [[package]] name = "ab_glyph" @@ -1231,6 +1231,7 @@ dependencies = [ "epiphany-ops", "epiphany-render-svg", "epiphany-testkit", + "png 0.17.16", "resvg", ] diff --git a/crates/epiphany-editor-gui/Cargo.toml b/crates/epiphany-editor-gui/Cargo.toml index deedaa4..2d1566f 100644 --- a/crates/epiphany-editor-gui/Cargo.toml +++ b/crates/epiphany-editor-gui/Cargo.toml @@ -31,3 +31,16 @@ epiphany-testkit.workspace = true eframe = "0.29" # Rasterizes the rendered SVG to a pixmap for display (bundles usvg + tiny-skia). resvg = "0.45" + +[dev-dependencies] +# Used only by `goldens.rs`'s own comparator unit tests, to produce two +# byte-different PNG encodings of identical pixels (different filter/compression +# settings) and prove the golden comparator's decoded-pixel contract ignores +# that difference. A caret requirement on the version `tiny-skia` already +# resolves (Cargo.lock), so this is a dev-only edge that unifies with the +# existing dependency-tree node — today and after any future `tiny-skia` bump +# (an exact `=` pin would fork a duplicate node the day `tiny-skia` moves). +# The comparator's own encode/decode path uses +# `resvg::tiny_skia::Pixmap::{encode_png,decode_png}` exclusively; `png` is +# never used at runtime. +png = "0.17.16" diff --git a/crates/epiphany-editor-gui/DECISIONS.md b/crates/epiphany-editor-gui/DECISIONS.md new file mode 100644 index 0000000..00ad46e --- /dev/null +++ b/crates/epiphany-editor-gui/DECISIONS.md @@ -0,0 +1,155 @@ +# epiphany-editor-gui — Decisions + +This crate's first `DECISIONS.md`. Records the calls made standing up the T1a +visual golden harness (`spec/CONTRACT_EDITOR_T1A_GOLDENS.md`; plan +`spec/PLAN_EDITOR_APP.md` §Ruling C, granted as amended 2026-07-23) — pixel-level +golden baselines of the resvg-rasterized score, as ordinary `#[test]`s in this +crate, comparing decoded pixels rather than encoded PNG files. The harness lives +in `src/goldens.rs` (`#[cfg(test)]`-only; never ships in the built binary) and +`goldens/*.png`. + +## 1. The comparison contract: decoded pixels, never encoded PNG bytes + +A golden test decodes the committed baseline PNG and compares **dimensions +first, then raw RGBA bytes** exactly — never the encoded file bytes. Plan +§Ruling C (granted as amended 2026-07-23) is explicit about why: comparing +encoded bytes would also lock the PNG encoder's own behavior (compression +level, filter choice, chunk layout, …), so the goldens would churn on an +encoder change even when every rendered pixel is identical — exactly the +defect the amendment exists to prevent. `goldens.rs`'s +`reencoding_with_different_settings_still_passes` test makes this guarantee +executable rather than merely asserted: it encodes one pixmap twice with +deliberately different `png` encoder settings (filter type, compression +level) via the `png` crate directly (decision 7), confirms the two encoded +byte strings differ, and confirms the comparator accepts either as a baseline +for the same pixels. + +On a mismatch the comparator writes failure artifacts to +`target/golden-failures//` and names them in the panic message: a +dimension mismatch writes `actual.png` + `expected.png` (no `diff.png` — a +per-pixel map is not meaningful across differing dimensions, and the +comparator fails there *before* any pixel is compared); a pixel mismatch +writes all three, `diff.png` being a per-pixel highlight of exactly the +differing pixels. Decision 8 covers how that record survives past the +ephemeral CI runner. + +## 2. The bless mechanism and its policy + +`EPIPHANY_BLESS_GOLDENS=1` makes `assert_golden` write/overwrite the baseline +unconditionally instead of comparing (creating `goldens/` if needed). This is +a **reviewed decision**, stated at the function's definition: never a +mechanism for turning a red test green, only for accepting a new or +deliberately-changed raster after a human has looked at it. The three initial +baselines (`ten_measure_open.png`, `ten_measure_insert.png`, +`ten_measure_slurs_castoff.png`) are the first visual record of this editor's +output in the project's history — an unreviewed baseline is an unverified +claim wearing a checkmark (plan §Ruling C, user deep-dive point 2) — and were +**visually reviewed and approved by the user on 2026-07-23** before being +committed. Any future re-bless is the same kind of event: a diff is a finding, +reviewed before it is accepted, never a fix applied to make a failing test +pass. + +## 3. G3 reuses G1's baseline and deliberately bypasses the bless path + +There is no `ten_measure_undo.png`. G3 (post-undo) asserts the raster equals +**G1's own baseline file** byte-for-byte in decoded RGBA — undo must return +the pixels, not just the model — by calling `assert_golden_at` directly +against `baseline_path("ten_measure_open")`, never `assert_golden`. Routing G3 +through `assert_golden` would let `EPIPHANY_BLESS_GOLDENS=1` overwrite +`ten_measure_open.png`: an initial bless run performed before undo is known to +be correct would silently bless a broken undo's post-undo pixels as the new +"as opened" baseline, after which every future run would compare undo against +its own bug instead of against G1. Bypassing `assert_golden` removes that +failure mode entirely — this comparison always compares, and can never bless +itself green. (G3 replays G2's scripted insert on its own fresh session +rather than continuing G2's, so the two tests stay independent of each +other's mutations.) + +## 4. The casting-off fixture, and what its system-count assertion actually guards + +G4 uses `ten_measure_with_slurs(0)` (`fixtures.rs:777`) specifically for its +three slurs, one of which is forced across a system break — the cross-system +slur-split path (`casting.rs:2262`) — the layout path real documents take, +not exercised by any single-system fixture. + +An empirical finding changed the contract's original framing: at +`px_per_staff_space: 12.0`, **`ten_measure_single_staff(0)` itself already +casts off into two systems** (ten measures of quarter notes don't fit one +line at this scale) — casting-off is not unique to the slurred fixture. All +three baselines (G1/G2/G3's shared raster and G4's) are therefore +multi-system layouts. Consequently G4's `system_count > 1` assertion does not +guard "this is the slurred fixture, not the plain one" — a mutation +substituting `ten_measure_single_staff(0)` for the slurs fixture leaves the +system count at 2 either way, so that assertion still passes under the +mutation. What it guards is the more durable claim it was written for: +*casting-off itself has not stopped triggering* at this geometry — if it ever +did, this would fail as a named system-count value error rather than a +mystery pixel diff. The mutation instead dies on the golden pixel comparison +(dimensions differ: G4's baseline is taller, carrying the slur curves and the +wider slurred content), which is where the fixture-identity guarantee +actually lives. + +## 5. Known engraving gaps, locked knowingly + +The goldens lock the **current, real** output of the Minimal-tier engraver — +bugs and rough edges included, by design (plan §Ruling C: "a golden locks +whatever it sees"). Two are worth naming explicitly, both blessed by the user +with this record in hand on 2026-07-23: + +- **No clef restatement on second and subsequent systems.** A new system does + not redraw the governing clef at its start, unlike conventional engraving + practice. +- **The second system's spacing is noticeably denser than the first's.** The + casting-off balance between systems is not yet even. + +Both are engraving-track items (`epiphany-engrave`'s casting/spacing passes; +plan §3.7), not `epiphany-editor-gui` work — this crate only observes the +rendered score, it does not engrave it. When engraving improves, the fix will +change these three PNGs' pixels, and the golden harness will surface that as +a reviewable diff to bless deliberately. That is the harness doing its job, +not a defect of it. + +## 6. The baselines pin the raster stack + +Determinism basis: `GlyphMode::PathOutline` uses no fonts (inlined Bravura +outline paths), and `resvg`/`tiny-skia` are pure Rust with deterministic +rasterization; CI and dev are both Linux. Standing consequence: **the +baselines pin the raster stack**, so any `Cargo.lock` movement of +`resvg`, `tiny-skia`, or `png` is a golden-review event — the diff must be +inspected and deliberately re-blessed — never a silent re-bless folded into +an unrelated dependency bump. If cross-platform rasterization drift is ever +observed, the fallback is a bounded per-pixel tolerance, recorded as that +decision when it happens, not pre-engineered here. + +## 7. The `png` dev-dependency + +`tiny_skia::Pixmap::encode_png`/`decode_png` (used throughout the comparator +and the bless path) expose no encoder configuration — every call from a given +pixmap produces byte-identical output. Proving decision 1 executable (that +two *differently*-encoded PNGs of the same pixels both compare equal) needs +an encoder with configurable filter/compression settings, which only the +underlying `png` crate exposes directly. `png` is therefore a **dev-only** +dependency, used exclusively inside `goldens.rs`'s own test module — never in +the comparator or bless code paths, which stay on +`resvg::tiny_skia::Pixmap::{encode_png,decode_png}` exclusively, and never at +runtime. + +Declared as a caret requirement (`png = "0.17.16"`), not an exact `=` pin (a +W1-review amendment): `tiny-skia` 0.11.4 already resolves `png 0.17.16` in +`Cargo.lock`, so the caret requirement is a dev-only edge onto that same +dependency-tree node today — and stays unified with it after any future +`tiny-skia` bump that moves its own `png` requirement forward, rather than +forking a second `png` version into the tree (which an exact pin would force +the day `tiny-skia` moves). + +## 8. CI failure artifacts + +A comparator panic's assertion message names `actual.png`/`expected.png`/ +`diff.png` paths under `target/golden-failures//` — useful for local +reproduction, but those paths name the CI runner's own ephemeral filesystem, +gone the moment the job ends. The reviewable record is the `editor-gui` job's +one additive step (`.github/workflows/ci.yml`): an `if: failure()` +`actions/upload-artifact@v4` step uploading `target/golden-failures/` as the +`golden-failures` artifact (`if-no-files-found: ignore`, since it produces +nothing on a green run). This is the tranche's only CI change; nothing else +in `ci.yml` moves. diff --git a/crates/epiphany-editor-gui/goldens/ten_measure_insert.png b/crates/epiphany-editor-gui/goldens/ten_measure_insert.png new file mode 100644 index 0000000..58b67ba Binary files /dev/null and b/crates/epiphany-editor-gui/goldens/ten_measure_insert.png differ diff --git a/crates/epiphany-editor-gui/goldens/ten_measure_open.png b/crates/epiphany-editor-gui/goldens/ten_measure_open.png new file mode 100644 index 0000000..88abc92 Binary files /dev/null and b/crates/epiphany-editor-gui/goldens/ten_measure_open.png differ diff --git a/crates/epiphany-editor-gui/goldens/ten_measure_slurs_castoff.png b/crates/epiphany-editor-gui/goldens/ten_measure_slurs_castoff.png new file mode 100644 index 0000000..fa79bad Binary files /dev/null and b/crates/epiphany-editor-gui/goldens/ten_measure_slurs_castoff.png differ diff --git a/crates/epiphany-editor-gui/src/goldens.rs b/crates/epiphany-editor-gui/src/goldens.rs new file mode 100644 index 0000000..5dd421f --- /dev/null +++ b/crates/epiphany-editor-gui/src/goldens.rs @@ -0,0 +1,605 @@ +//! Pixel-level golden-image comparator and bless machinery for the score raster +//! this GUI displays (`spec/CONTRACT_EDITOR_T1A_GOLDENS.md`; plan +//! `spec/PLAN_EDITOR_APP.md` §Ruling C, granted 2026-07-23 as amended). +//! +//! **Comparison contract (Ruling C, amended):** a golden is compared as +//! *decoded* RGBA pixels — dimensions first, then raw bytes — never as encoded +//! PNG file bytes. Comparing encoded bytes would also lock the PNG encoder's own +//! behavior (compression level, filter choice, …) and churn on an encoder +//! change even when every pixel is identical; `reencoding_with_different_settings_still_passes` +//! below makes that guarantee executable, not just asserted. +//! +//! This module is declared `#[cfg(test)]`-only at its `mod goldens;` site in +//! `main.rs`, so none of it ships in the built binary. It carries two kinds of +//! test: the comparator's **own** unit tests (T1a W1), which never touch +//! `goldens/` — they point [`assert_golden_at`] at private temp locations, so +//! they hold even before any baseline exists — and the **four golden-state** +//! tests (T1a W2: fixture-as-opened, after a scripted insert, after undo, after +//! casting-off), which call [`assert_golden`] against the three committed +//! baselines under `goldens/` (G3 reuses G1's file rather than owning one; see +//! its test's doc comment). + +use std::fs; +use std::path::{Path, PathBuf}; + +use resvg::tiny_skia::Pixmap; + +/// An opaque highlight color painted over every differing pixel in a `diff.png`. +const DIFF_HIGHLIGHT: [u8; 4] = [255, 0, 0, 255]; + +/// The committed baseline path for a named golden: `{CARGO_MANIFEST_DIR}/goldens/{name}.png`. +fn baseline_path(name: &str) -> PathBuf { + PathBuf::from(concat!(env!("CARGO_MANIFEST_DIR"), "/goldens")).join(format!("{name}.png")) +} + +/// The failure-artifact directory for a named golden, resolved repo-root-relative +/// (`{CARGO_MANIFEST_DIR}/../../target/golden-failures/{name}/`, this crate being +/// two levels under the workspace root) so the path is the same regardless of the +/// working directory `cargo test` was invoked from — matching CI's +/// `target/golden-failures/**` upload path. +fn failure_dir_path(name: &str) -> PathBuf { + PathBuf::from(concat!( + env!("CARGO_MANIFEST_DIR"), + "/../../target/golden-failures" + )) + .join(name) +} + +/// Compares `pixmap` against the committed baseline `goldens/{name}.png`, +/// writing any failure artifacts under `target/golden-failures/{name}/` (see +/// [`baseline_path`] / [`failure_dir_path`]). +/// +/// **Bless policy:** setting `EPIPHANY_BLESS_GOLDENS=1` writes/overwrites the +/// baseline unconditionally instead of comparing, creating `goldens/` if +/// needed. This is a *reviewed decision* — the tranche's named user deep-dive +/// point on new baselines (plan §Ruling C) — **never** a mechanism for turning a +/// red test green. Do not set it to make a failing test pass; set it only after +/// visually inspecting the new PNG and deciding it is correct. +fn assert_golden(name: &str, pixmap: &Pixmap) { + let baseline = baseline_path(name); + if std::env::var("EPIPHANY_BLESS_GOLDENS").as_deref() == Ok("1") { + write_pixmap(&baseline, pixmap); + return; + } + assert_golden_at(&baseline, pixmap, &failure_dir_path(name)); +} + +/// The comparator's parameterized core: compares `pixmap` against the PNG +/// decoded from `baseline_path`, panicking with the mismatch description (which +/// names every artifact written) on failure. `assert_golden` is the thin +/// default-path wrapper above; tests call this directly with temp-directory +/// paths so they never depend on a committed baseline. +fn assert_golden_at(baseline_path: &Path, pixmap: &Pixmap, failure_dir: &Path) { + if let Err(message) = compare(baseline_path, pixmap, failure_dir) { + panic!("{message}"); + } +} + +/// Result-returning core of the comparison, so tests can observe a mismatch +/// without needing to catch a panic. Reads and decodes the baseline PNG, then: +/// +/// 1. compares dimensions — a mismatch fails here, before any pixel is looked +/// at, and writes `actual.png` + `expected.png` (no `diff.png`: a per-pixel +/// map is not meaningful across differing dimensions); +/// 2. compares decoded RGBA bytes exactly — a mismatch writes all three +/// artifacts (`actual.png`, `expected.png`, `diff.png`) and names them in the +/// returned message. +/// +/// A missing or undecodable baseline file is a harness/setup error, not a +/// reviewable visual diff, so it panics directly rather than returning `Err`. +fn compare(baseline_path: &Path, actual: &Pixmap, failure_dir: &Path) -> Result<(), String> { + let baseline_bytes = fs::read(baseline_path).unwrap_or_else(|err| { + panic!( + "no golden baseline at {} ({err}); run with EPIPHANY_BLESS_GOLDENS=1, after visually \ + reviewing the new image, to create it", + baseline_path.display() + ) + }); + let expected = Pixmap::decode_png(&baseline_bytes).unwrap_or_else(|err| { + panic!( + "golden baseline at {} is not a decodable PNG: {err}", + baseline_path.display() + ) + }); + + if actual.width() != expected.width() || actual.height() != expected.height() { + let actual_path = failure_dir.join("actual.png"); + let expected_path = failure_dir.join("expected.png"); + write_pixmap(&actual_path, actual); + write_pixmap(&expected_path, &expected); + return Err(format!( + "golden mismatch: dimensions differ (actual {}x{} vs expected {}x{}); no pixel \ + comparison performed. Artifacts: {}, {}", + actual.width(), + actual.height(), + expected.width(), + expected.height(), + actual_path.display(), + expected_path.display(), + )); + } + + // The comparison contract itself (Ruling C, amended): decoded pixels, never + // encoded file bytes. An encoder-settings change that leaves every pixel + // identical must never fail this comparison. + let pixels_equal = actual.data() == expected.data(); + if pixels_equal { + return Ok(()); + } + + let (diff, differing) = diff_pixmap(actual, &expected); + let actual_path = failure_dir.join("actual.png"); + let expected_path = failure_dir.join("expected.png"); + let diff_path = failure_dir.join("diff.png"); + write_pixmap(&actual_path, actual); + write_pixmap(&expected_path, &expected); + write_pixmap(&diff_path, &diff); + Err(format!( + "golden mismatch: {differing} of {} decoded pixels differ. Artifacts: {}, {}, {}", + u64::from(actual.width()) * u64::from(actual.height()), + actual_path.display(), + expected_path.display(), + diff_path.display(), + )) +} + +/// Builds a per-pixel highlight image: [`DIFF_HIGHLIGHT`] where `actual` and +/// `expected` disagree, `actual`'s own pixel where they agree (so the diff stays +/// legible against the surrounding score). Returns the image and the count of +/// differing pixels. Callers must have already established equal dimensions. +fn diff_pixmap(actual: &Pixmap, expected: &Pixmap) -> (Pixmap, usize) { + let mut diff = Pixmap::new(actual.width(), actual.height()) + .expect("dimensions already validated equal and nonzero"); + let mut differing = 0usize; + for ((out, a), b) in diff + .data_mut() + .chunks_exact_mut(4) + .zip(actual.data().chunks_exact(4)) + .zip(expected.data().chunks_exact(4)) + { + if a == b { + out.copy_from_slice(a); + } else { + differing += 1; + out.copy_from_slice(&DIFF_HIGHLIGHT); + } + } + (diff, differing) +} + +/// Writes `pixmap` to `path` as a PNG, creating parent directories as needed. +fn write_pixmap(path: &Path, pixmap: &Pixmap) { + if let Some(parent) = path.parent() { + fs::create_dir_all(parent) + .unwrap_or_else(|err| panic!("creating {}: {err}", parent.display())); + } + let bytes = pixmap + .encode_png() + .unwrap_or_else(|err| panic!("encoding {}: {err}", path.display())); + fs::write(path, bytes).unwrap_or_else(|err| panic!("writing {}: {err}", path.display())); +} + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::atomic::{AtomicU64, Ordering}; + use std::time::{SystemTime, UNIX_EPOCH}; + + use epiphany_core::{ + CmnNominal, MusicalDuration, MusicalPosition, RationalTime, TypedObjectId, + }; + use epiphany_editor_core::{EditorSession, GridResolution}; + use epiphany_engrave::Engraver; + use epiphany_layout_ir::{HitShape, Point}; + use epiphany_render_svg::{render, RenderOptions}; + use epiphany_testkit::fixtures; + + /// A private, per-call-unique scratch directory under the OS temp dir — W1 + /// has no committed baselines, so every test exercises the comparator + /// against its own throwaway files rather than `goldens/`. + fn unique_temp_dir(tag: &str) -> PathBuf { + static COUNTER: AtomicU64 = AtomicU64::new(0); + let n = COUNTER.fetch_add(1, Ordering::Relaxed); + let nanos = SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("system clock is after the epoch") + .as_nanos(); + std::env::temp_dir().join(format!("epiphany-goldens-test-{tag}-{nanos}-{n}")) + } + + /// A small pixmap with varied (non-uniform) content, alpha 255 throughout so + /// premultiply/demultiply round-trips through PNG encode/decode exactly — + /// deterministic pseudo-noise, not a flat fill, so PNG row filtering and + /// compression have something to actually differ over. + fn varied_pixmap(width: u32, height: u32) -> Pixmap { + let mut pm = Pixmap::new(width, height).expect("nonzero test dimensions"); + for y in 0..height { + for x in 0..width { + let idx = ((y * width + x) * 4) as usize; + let r = ((x.wrapping_mul(37)).wrapping_add(y.wrapping_mul(11)) % 256) as u8; + let g = ((x.wrapping_mul(59)).wrapping_add(y.wrapping_mul(3)) % 256) as u8; + let b = ((x.wrapping_mul(13)).wrapping_add(y.wrapping_mul(91)) % 256) as u8; + pm.data_mut()[idx..idx + 4].copy_from_slice(&[r, g, b, 255]); + } + } + pm + } + + #[test] + fn identical_pixmaps_pass() { + let pixmap = varied_pixmap(5, 4); + let dir = unique_temp_dir("identical"); + let baseline = dir.join("baseline.png"); + write_pixmap(&baseline, &pixmap); + let failure_dir = dir.join("failures"); + + assert_golden_at(&baseline, &pixmap, &failure_dir); + + assert!( + !failure_dir.exists(), + "a passing comparison must not write failure artifacts" + ); + } + + #[test] + fn altered_pixel_fails_and_writes_all_three_artifacts() { + let baseline_pixmap = varied_pixmap(5, 4); + let mut actual_pixmap = baseline_pixmap.clone(); + // Alter exactly one pixel (the second one) in the actual image. + actual_pixmap.data_mut()[4..8].copy_from_slice(&[9, 8, 7, 255]); + + let dir = unique_temp_dir("altered"); + let baseline = dir.join("baseline.png"); + write_pixmap(&baseline, &baseline_pixmap); + let failure_dir = dir.join("failures"); + + let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + assert_golden_at(&baseline, &actual_pixmap, &failure_dir); + })); + let payload = result.expect_err("a single-pixel diff must panic"); + let message = payload + .downcast_ref::() + .cloned() + .or_else(|| payload.downcast_ref::<&str>().map(|s| (*s).to_string())) + .expect("panic payload is a string message"); + + let actual_path = failure_dir.join("actual.png"); + let expected_path = failure_dir.join("expected.png"); + let diff_path = failure_dir.join("diff.png"); + assert!(actual_path.is_file(), "actual.png was written"); + assert!(expected_path.is_file(), "expected.png was written"); + assert!(diff_path.is_file(), "diff.png was written"); + assert!( + message.contains(&actual_path.display().to_string()), + "panic message names actual.png's path: {message}" + ); + assert!( + message.contains(&expected_path.display().to_string()), + "panic message names expected.png's path: {message}" + ); + assert!( + message.contains(&diff_path.display().to_string()), + "panic message names diff.png's path: {message}" + ); + } + + #[test] + fn reencoding_with_different_settings_still_passes() { + // Encode the *same* pixels twice, with deliberately different PNG + // encoder settings (filter type and compression level), via the `png` + // crate directly — `tiny_skia::Pixmap::encode_png` exposes no such + // knobs, so this is the only way to prove the comparator does not + // secretly depend on encoder behavior. Pinned to the exact `png` version + // already resolved via `tiny-skia` (Cargo.lock): a dev-only edge to an + // existing node, not a new dependency. + let pixmap = varied_pixmap(6, 5); + + let encode_with = |filter: png::FilterType, compression: png::Compression| -> Vec { + let mut bytes = Vec::new(); + let mut encoder = png::Encoder::new(&mut bytes, pixmap.width(), pixmap.height()); + encoder.set_color(png::ColorType::Rgba); + encoder.set_depth(png::BitDepth::Eight); + encoder.set_filter(filter); + encoder.set_compression(compression); + let mut writer = encoder.write_header().expect("valid PNG header"); + writer + .write_image_data(pixmap.data()) + .expect("valid image data"); + drop(writer); + bytes + }; + let bytes_a = encode_with(png::FilterType::NoFilter, png::Compression::Fast); + let bytes_b = encode_with(png::FilterType::Paeth, png::Compression::Best); + assert_ne!( + bytes_a, bytes_b, + "the two encodings must differ byte-wise, or this test is vacuous" + ); + + let dir = unique_temp_dir("reencode"); + let failure_dir = dir.join("failures"); + + let baseline_a = dir.join("a.png"); + fs::create_dir_all(&dir).expect("temp dir created"); + fs::write(&baseline_a, &bytes_a).expect("baseline a written"); + assert_golden_at(&baseline_a, &pixmap, &failure_dir); + + let baseline_b = dir.join("b.png"); + fs::write(&baseline_b, &bytes_b).expect("baseline b written"); + assert_golden_at(&baseline_b, &pixmap, &failure_dir); + + assert!( + !failure_dir.exists(), + "both differently-encoded baselines must decode to the same pixels and pass" + ); + } + + #[test] + fn mismatched_dimensions_fail_before_any_pixel_comparison() { + let baseline_pixmap = varied_pixmap(4, 3); + let differently_sized = varied_pixmap(5, 3); + + let dir = unique_temp_dir("dims"); + let baseline = dir.join("baseline.png"); + write_pixmap(&baseline, &baseline_pixmap); + let failure_dir = dir.join("failures"); + + let err = compare(&baseline, &differently_sized, &failure_dir) + .expect_err("differing dimensions must fail the comparison"); + + assert!( + err.contains("dimensions"), + "failure mentions dimensions: {err}" + ); + assert!( + !err.contains("decoded pixels"), + "failure must not describe a pixel diff: {err}" + ); + assert!( + !failure_dir.join("diff.png").exists(), + "no pixel diff is computable across differing dimensions, so none is written" + ); + } + + // ---- The four golden states (T1a W2) ------------------------------------ + + /// Renders `session`'s resolved layout at `px_per_staff_space` (the demo's + /// default is `12.0`; `RenderOptions`'s other fields default to + /// `GlyphMode::PathOutline`, no fonts) and rasterizes it through + /// `crate::rasterize_pixmap` — the exact pixmap `main.rs`'s + /// `EditorApp::rerender` displays (`main.rs:247`). Returns the SVG string + /// alongside the pixmap so callers can run the determinism double. + fn render_pixmap(session: &EditorSession, px_per_staff_space: f32) -> (String, Pixmap) { + let options = RenderOptions { + px_per_staff_space, + ..Default::default() + }; + let output = render(session.resolved(), &options); + let (pixmap, _logical) = + crate::rasterize_pixmap(&output.svg).expect("a rendered score's SVG rasterizes"); + (output.svg, pixmap) + } + + /// The click point G2 scripts (and G3 replays before undoing) — derived from + /// `session`'s own rendered geometry, never a magic screen constant. + /// + /// At this geometry, ten measures of quarter notes at `px_per_staff_space: + /// 12.0` don't fit one line, so `ten_measure_single_staff` itself casts off + /// into two systems (not only the slurred G4 fixture). That means the + /// score's *temporally* last note is not simply "the rightmost notehead + /// box": the first system happens to render wider than the second, so its + /// notes reach further right on the page even though they come first in + /// time. The last note is instead the rightmost `Pitch`-sourced notehead + /// **within the system with the lowest `bounding_box.origin.y`** — systems + /// stack top-to-bottom in this y-up world, so the lowest one is the last. + /// + /// The click point sits half a staff space past that notehead's right edge + /// (clearly past it, still read as the same system) and two staff spaces + /// above its vertical center — four diatonic steps (a fifth) above the + /// existing all-C4 content under treble clef, landing on a different, + /// mid-staff pitch rather than repeating the fixture's own notes. `staff_pitch_at` / + /// `default_grid_at` / `position_at` resolve this point to exact values, + /// asserted in `g2_ten_measure_insert_matches_baseline`. + fn scripted_insert_target(session: &EditorSession) -> Point { + let last_system = session + .resolved() + .pages + .iter() + .flat_map(|page| &page.systems) + .min_by(|a, b| { + a.bounding_box + .origin + .y + .0 + .total_cmp(&b.bounding_box.origin.y.0) + }) + .expect("casting-off produced at least one system"); + let sys_bottom = last_system.bounding_box.origin.y.0; + let sys_top = sys_bottom + last_system.bounding_box.size.height.0; + + let last_notehead = session + .hit_test() + .regions + .iter() + .filter_map(|r| match (&r.source, r.shape) { + (TypedObjectId::Pitch(_), HitShape::Box(b)) => Some(b), + _ => None, + }) + .filter(|b| { + let mid_y = (b.bottom.0 + b.top.0) / 2.0; + (sys_bottom..=sys_top).contains(&mid_y) + }) + .max_by(|a, b| a.right.0.total_cmp(&b.right.0)) + .expect("the last system renders at least one notehead"); + + Point::new( + last_notehead.right.0 + 0.5, + (last_notehead.bottom.0 + last_notehead.top.0) / 2.0 + 2.0, + ) + } + + /// **G1 — as opened.** `ten_measure_single_staff(0)`, exactly the demo's + /// open path (`main.rs:197`), locked against `goldens/ten_measure_open.png`. + /// This is also the file G3 (below) compares its post-undo raster against. + #[test] + fn g1_ten_measure_open_matches_baseline() { + let score = fixtures::ten_measure_single_staff(0); + let session = EditorSession::open(score, Box::new(Engraver::default())) + .expect("the ten-measure fixture renders under the real engraver"); + + let (svg1, pixmap1) = render_pixmap(&session, 12.0); + let (svg2, pixmap2) = render_pixmap(&session, 12.0); + assert_eq!(svg1, svg2, "G1 determinism double: SVG bytes must match"); + assert_eq!( + pixmap1.data(), + pixmap2.data(), + "G1 determinism double: rasterized pixels must match" + ); + + assert_golden("ten_measure_open", &pixmap1); + } + + /// **G2 — after a scripted pencil insert.** See [`scripted_insert_target`] + /// for the click-point derivation. `staff_pitch_at` / `default_grid_at` / + /// `position_at` are asserted to their exact values *before* the insert + /// runs, then the insert is applied and the result locked against + /// `goldens/ten_measure_insert.png`. + #[test] + fn g2_ten_measure_insert_matches_baseline() { + let score = fixtures::ten_measure_single_staff(0); + let mut session = EditorSession::open(score, Box::new(Engraver::default())) + .expect("the ten-measure fixture renders under the real engraver"); + + let target = scripted_insert_target(&session); + + let pitch = session + .staff_pitch_at(target) + .expect("the target point sits over the staff"); + assert_eq!( + (pitch.nominal, pitch.octave), + (CmnNominal::G, 4), + "the target resolves to G4 — a fifth above the fixture's all-C4 content" + ); + + let grid = session + .default_grid_at(target) + .expect("the target point sits over a metric region"); + assert_eq!( + grid, + GridResolution { + step: MusicalDuration(RationalTime::new(1, 4).expect("1/4 is valid")) + }, + "the 4/4 meter's default grid is a quarter-note step" + ); + + let placed = session + .position_at(target, &grid) + .expect("the target snaps to a musical position"); + assert_eq!( + placed.position, + MusicalPosition(RationalTime::new(10, 1).expect("10/1 is valid")), + "the insert lands exactly at whole-note 10 — immediately after the fixture's \ + last note ends (measure 10, beat 4 of 4), with no gap and nothing to overwrite" + ); + + let outcome = session + .insert_note_at(target, &grid) + .expect("the target is a clean, unoccupied insert slot"); + assert!( + outcome.graph_changed, + "the insert must change the score graph" + ); + + let (svg1, pixmap1) = render_pixmap(&session, 12.0); + let (svg2, pixmap2) = render_pixmap(&session, 12.0); + assert_eq!(svg1, svg2, "G2 determinism double: SVG bytes must match"); + assert_eq!( + pixmap1.data(), + pixmap2.data(), + "G2 determinism double: rasterized pixels must match" + ); + + assert_golden("ten_measure_insert", &pixmap1); + } + + /// **G3 — after undo of G2's insert. No third baseline:** this compares the + /// post-undo raster against **G1's own baseline file**, reached by calling + /// [`assert_golden_at`] directly rather than [`assert_golden`] — deliberately + /// bypassing the bless path. Going through `assert_golden` would let + /// `EPIPHANY_BLESS_GOLDENS=1` overwrite `ten_measure_open.png`: an initial + /// bless run performed before undo is known to be correct would silently + /// bless a broken undo's post-undo pixels as the new "as opened" baseline, + /// after which every future run would compare undo against its own bug + /// instead of against G1. Bypassing `assert_golden` makes that impossible — + /// this comparison always compares, never blesses. A fresh session (rather + /// than continuing G2's) keeps the two tests independent of each other's + /// mutations. + #[test] + fn g3_undo_matches_g1_baseline() { + let score = fixtures::ten_measure_single_staff(0); + let mut session = EditorSession::open(score, Box::new(Engraver::default())) + .expect("the ten-measure fixture renders under the real engraver"); + + let target = scripted_insert_target(&session); + let grid = session + .default_grid_at(target) + .expect("the target point sits over a metric region"); + session + .insert_note_at(target, &grid) + .expect("the target is a clean, unoccupied insert slot"); + + let outcome = session.undo().expect("there is one edit to undo"); + assert!(outcome.graph_changed, "undo must revert the inserted note"); + + let (svg1, pixmap1) = render_pixmap(&session, 12.0); + let (svg2, pixmap2) = render_pixmap(&session, 12.0); + assert_eq!(svg1, svg2, "G3 determinism double: SVG bytes must match"); + assert_eq!( + pixmap1.data(), + pixmap2.data(), + "G3 determinism double: rasterized pixels must match" + ); + + assert_golden_at( + &baseline_path("ten_measure_open"), + &pixmap1, + &failure_dir_path("ten_measure_undo"), + ); + } + + /// **G4 — casting-off.** `ten_measure_with_slurs(0)` (`fixtures.rs:777`): + /// the same ten-measure content as G1–G3 plus three slurs, which at this + /// geometry casts off into multiple systems **and** forces the second slur + /// (events 5..8) across the system break — the cross-system slur-split path + /// (`casting.rs:2262`). The system count is asserted first, as a named + /// value: if casting-off ever stops triggering here, this fails as a + /// reported system count, not a silent pixel diff easily misread as an + /// unrelated rendering regression. + #[test] + fn g4_ten_measure_slurs_castoff_matches_baseline() { + let score = fixtures::ten_measure_with_slurs(0); + let session = EditorSession::open(score, Box::new(Engraver::default())) + .expect("the slurred ten-measure fixture renders under the real engraver"); + + let system_count: usize = session + .resolved() + .pages + .iter() + .map(|page| page.systems.len()) + .sum(); + assert!( + system_count > 1, + "casting-off must produce more than one system, got {system_count}" + ); + + let (svg1, pixmap1) = render_pixmap(&session, 12.0); + let (svg2, pixmap2) = render_pixmap(&session, 12.0); + assert_eq!(svg1, svg2, "G4 determinism double: SVG bytes must match"); + assert_eq!( + pixmap1.data(), + pixmap2.data(), + "G4 determinism double: rasterized pixels must match" + ); + + assert_golden("ten_measure_slurs_castoff", &pixmap1); + } +} diff --git a/crates/epiphany-editor-gui/src/main.rs b/crates/epiphany-editor-gui/src/main.rs index 06c4345..750bfe8 100644 --- a/crates/epiphany-editor-gui/src/main.rs +++ b/crates/epiphany-editor-gui/src/main.rs @@ -150,14 +150,16 @@ fn payload_label(payload: &OperationPayload) -> &'static str { } } -/// Rasterizes a rendered SVG string to an `egui` image, returning the image and the +/// Rasterizes a rendered SVG string to a `tiny_skia` pixmap, returning it and the /// SVG's **logical** (sub-pixel, pre-`ceil`) size. The pixmap dimensions are the /// logical size rounded up to whole pixels; the logical size is what the image must /// be *displayed* at so the click plane maps back to the layout exactly (displaying /// the rounded-up pixmap size would stretch the mapping past the content). The score /// is drawn over an opaque white background so the pixmap's premultiplied alpha is -/// fully opaque, matching `from_rgba_unmultiplied`. -fn rasterize(svg: &str) -> Option<(egui::ColorImage, egui::Vec2)> { +/// fully opaque, matching `from_rgba_unmultiplied` (this crate's only consumer of the +/// pixmap's raw bytes, [`rasterize`], and the golden-image tests in `goldens.rs`, +/// which lock this exact pixmap — the surface the GUI displays). +fn rasterize_pixmap(svg: &str) -> Option<(resvg::tiny_skia::Pixmap, egui::Vec2)> { let tree = resvg::usvg::Tree::from_str(svg, &resvg::usvg::Options::default()).ok()?; let size = tree.size(); let logical = egui::vec2(size.width(), size.height()); @@ -170,8 +172,18 @@ fn rasterize(svg: &str) -> Option<(egui::ColorImage, egui::Vec2)> { resvg::tiny_skia::Transform::identity(), &mut pixmap.as_mut(), ); - let image = - egui::ColorImage::from_rgba_unmultiplied([width as usize, height as usize], pixmap.data()); + Some((pixmap, logical)) +} + +/// Rasterizes a rendered SVG string to an `egui` image (the GUI's display path): a +/// thin conversion over [`rasterize_pixmap`]'s pixmap, see its doc for the sizing and +/// background contract. +fn rasterize(svg: &str) -> Option<(egui::ColorImage, egui::Vec2)> { + let (pixmap, logical) = rasterize_pixmap(svg)?; + let image = egui::ColorImage::from_rgba_unmultiplied( + [pixmap.width() as usize, pixmap.height() as usize], + pixmap.data(), + ); Some((image, logical)) } @@ -527,6 +539,12 @@ impl eframe::App for EditorApp { } } +// The pixel-golden comparator, bless machinery, and the four golden-state tests +// (T1a); see `goldens.rs`'s module doc. Test-only: never compiled into the +// shipped binary. +#[cfg(test)] +mod goldens; + #[cfg(test)] mod tests { use super::*;