From 85d8af612fb6a38e31ac40e18229fa5201582a80 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Fri, 24 Jul 2026 12:55:05 -0400 Subject: [PATCH] Editor T3: the note-entry caret [W1+W2] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The seam (editor-core): a session-local caret — (voice, position, entry duration), never in the op log, undo leaves it advanced, a deleted voice clears it — with enter_nominal/enter_pitch/enter_rest funneling to one insertion core that reuses the pencil's make-room verbatim and advances only on success. Octave inference is nearest-in-staff-steps to the preceding note with a downward tie-break — proven unreachable for real letter pairs (candidate octaves sit an odd 7 steps apart), so the comparator is tested directly. midi_note_to_pitch (sharps-spelled v1 policy) is the testable core of MIDI entry; x_at_position mirrors invert_x for the GUI's caret line, extrapolation-tested on non-uniform anchors after the uniform fixture proved blind to the wrong-segment mutation. The GUI (editor-gui): N-toggled entry mode, mutually exclusive with pencil via a pure toggle_exclusive; mode-gated letters A-G, R for rests, arrow-key caret movement, the duration palette doing double duty; the caret drawn full staff height at x_at_position from the staff's own rendered strokes. G5 locks the entry loop: C-D-E-F entered at the caret, each advancing position value-asserted as an exact rational before any pixel, baseline user-reviewed. Twelve mutations killed across the pair, coordinator re-verified. Co-Authored-By: Claude Fable 5 --- crates/epiphany-editor-core/DECISIONS.md | 165 ++++ crates/epiphany-editor-core/src/lib.rs | 790 +++++++++++++++++- crates/epiphany-editor-gui/DECISIONS.md | 138 +++ .../goldens/ten_measure_caret_entry.png | Bin 0 -> 57379 bytes crates/epiphany-editor-gui/src/goldens.rs | 96 +++ crates/epiphany-editor-gui/src/main.rs | 364 +++++++- 6 files changed, 1528 insertions(+), 25 deletions(-) create mode 100644 crates/epiphany-editor-gui/goldens/ten_measure_caret_entry.png diff --git a/crates/epiphany-editor-core/DECISIONS.md b/crates/epiphany-editor-core/DECISIONS.md index 8f585ec..4cf5a37 100644 --- a/crates/epiphany-editor-core/DECISIONS.md +++ b/crates/epiphany-editor-core/DECISIONS.md @@ -404,3 +404,168 @@ count with a slur aboard (descriptor + 2 inserts + 1 respell + 1 instead of the transaction dispatch" mutation the test's own doc comment already described for the pre-slur version — the mutation still kills it (0 descriptors instead of 1), confirmed and reverted. + +## The note-entry caret (2026-07-24, T3-W1) + +Dispatched under `spec/CONTRACT_EDITOR_T3_CARET.md` §W1. Adds +`EditorSession`'s `caret: Option` (`Caret { voice, position, +entry_duration }`, all `pub` fields) plus `caret()`, `clear_caret()`, +`set_caret_at(point)`, `set_entry_duration(duration)`, `advance()`, +`retreat()`, `enter_nominal(nominal)`, `enter_pitch(pitch)`, `enter_rest()`, +the pure `midi_note_to_pitch(u8) -> Pitch`, and `x_at_position(region, +within, position)`. Session-local, never in the op log; undo does not move +it. `EditorError::NoCaret` is a new variant (see below). + +**`NoCaret`, not an overload of `NoSelection`.** The contract asked to +check whether `NoSelection`'s shape generalizes. It doesn't cleanly: a +caret and a selection are independent session-local cursors (one can be +set while the other is empty, e.g. mid-caret-entry with nothing selected), +and `NoSelection`'s doc ("An intent needed a selection but none is set") +would read as a lie for a caret-only intent. `NoCaret` is its own variant, +same shape, one line in `Display`. + +**One insertion core, reusing the pencil's machinery verbatim.** +`enter_nominal`/`enter_pitch`/`enter_rest` all funnel to a private +`enter_at_caret(pitch: Option)`: builds `[start, start+entry_duration)` +from the caret, calls the *same* `make_room`/`make_room_ops`/`Minter` the +pencil (`insert_note_at`) uses, appends one `InsertEvent`, dispatches +through the existing `apply`/`apply_transaction` N=1 idiom, then advances +the caret by the entry duration **only after** the apply/transaction +succeeds (the `?` on the apply result runs before the advance, so a +refused edit changes nothing including the caret). `enter_rest` passes +`pitch: None` (an empty pitch list mints a `Rest` — `note_event`'s existing +behavior, unchanged). + +**Caret re-resolution rides the same seam as selection re-resolution.** +`reresolve_caret` (clears the caret iff `caret.voice` is absent from +`self.score.voices()`) is called from the same three sites +`reresolve_selection` already is — `commit`, `undo`, `redo` — right after +`self.install(materialized)`. Unlike the selection set's survivor/fallback +rule, a point cursor has no fallback: vanish just clears it. + +**The vanish test uses a real op, not a synthetic swap.** The contract's +own fallback ("if no current op can remove a voice, ... test the clear via +... a synthetic score swap") turned out not to be needed: +`epiphany-ops::DeleteVoiceOp` exists (Chapter 6 §6.10, "tombstone an empty +voice … precondition no-op if it still has live events") and cleanly +deletes an empty voice. `caret_clears_when_its_voice_is_deleted` hand-adds +a fresh empty `Voice` to a fixture's staff instance (`score.identity.mint()` ++ `Voice::user(id)`, pushed via `RegionContent::staff_based_mut()`), points +the caret at it directly (`session.caret = Some(...)`, legal from the same- +crate `tests` submodule), applies `DeleteVoice`, and asserts the caret is +`None` afterward. + +**Octave inference: implemented, table-tested, and its tie-break is +*provably unreachable* through the table.** `infer_octave(reference: +Option<(CmnNominal, i8)>, nominal) -> i8` computes the nearest candidate +octave in diatonic staff steps (`diff = ref_index - nominal_val = 7q + r`, +`0 <= r < 7`; down candidate at distance `r`, up candidate at distance +`7 - r`; `nearer_is_down(r, 7 - r)` picks). **Finding, verified by direct +computation, not assumed:** because the candidate octaves for a fixed +`nominal` are exactly 7 diatonic steps apart and 7 is odd, `r` and `7 - r` +can never be equal for an integer `r` (equal would need `r = 3.5`) — so +**no (reference, nominal) pair drawn from the seven CMN letters can ever +produce a genuine tie**. The contract's own suggested case, "ref G4 enter +D" (a perfect fifth either direction), was flagged by the contract itself +as needing verification ("verify this IS the equidistant case in staff +steps and if not, construct the true equidistant pair") — checked directly: +down is 3 staff steps (G4→F4→E4→D4), up is 4 (G4→A4→B4→C5→D5); **not** a +tie in staff steps (it's only a tie in semitones — a P5 both ways — which +is not the metric the contract pins: "nearest ... in diatonic staff +steps"). No true equidistant *letter* pair exists to substitute for it. +Consequence for testing: the downward tie-break (`nearer_is_down`, +`down_distance <= up_distance`) is implemented as specified and documented, +but `octave_inference_table`'s four contract rows cannot kill a flipped +comparison (verified directly — see the mutation report). `nearer_is_down` +is therefore unit-tested on its own, directly, with a synthetic tied input +(`nearer_is_down(3, 3)`) that no real call site can ever produce; this +proves the *written comparison* is correct without pretending the table +exercises it. + +**Reference-pitch policy, two silent decisions the contract left open,** +both in `reference_pitch_before(voice, position)`: (1) **chord reference** +— when the nearest preceding note is a chord, its **first** pitch (as +authored) is the reference; the contract names no chord tie-break. (2) +**non-CMN reference** — a preceding note whose pitch is not a `Cmn` scale +position (a JI/serial score) is treated the same as "no reference" +(falls back to octave 4), since there is no staff-step notion to measure +from. Both are flagged here rather than silently baked in. + +**Naturals-only / sharp-spelled MIDI, as pinned.** `enter_nominal` mints +via `cmn_pitch(nominal, octave)` (alteration always 0 — accidentals are a +follow-up transpose gesture, unchanged). `midi_note_to_pitch` is new and +needed an alteration-bearing pitch constructor `cmn_pitch` doesn't have; +rather than duplicate `cmn_pitch`'s body, it now delegates to a new +`chromatic_pitch(nominal, alteration, octave)`, and `midi_note_to_pitch` +is `chromatic_pitch`'s only non-zero-alteration caller. The 12-entry table +is the ordinary MIDI-to-SPN convention (`60 = C4`, `69 = A4`, every black +key sharp) with octave `note/12 - 1` (integer division; correct for the +full `u8` range including the low notes below MIDI 12, where it goes +negative into `i8`). + +**`x_at_position`'s signature mirrors `position_anchors`, not +`position_at`'s point-based one.** The contract left the shape open +("region-or-point-context"). `position_at` starts from a `Point` because a +*click* is a point; the caret has no click, only a `(region-bearing voice, +position)`, and — under cast-off geometry — knowing *which system* a bare +musical position falls on requires either a point to test against +(`containing_system`) or scanning every system's anchor coverage, which is +GUI-side work this packet doesn't own. So `x_at_position(region: RegionId, +within: Option<&Rect>, position: &MusicalPosition) -> Option` takes +exactly `position_anchors`'s own two scoping parameters — `within = None` +reads the whole region as one flat run (the stub solver, and every test +here); a GUI drawing into one system supplies that system's box the same +way `position_at` derives one via `containing_system`. The engine, +`forward_x`, is the literal mirror of `invert_x` with the roles of `x` and +musical time swapped (interpolate the bracketing segment; extrapolate the +nearest end segment's slope outside the anchored span). + +**`retreat`'s clamped subtraction.** There is no `MusicalPosition - +MusicalDuration` in Chapter 3's type algebra (only `Duration - Duration` +and `Position - Position -> Duration`) because an unclamped result could +go negative, which is not a valid position. `retreat_position` reimplements +it directly over the raw `RationalTime`, clamping at `MusicalPosition::origin()`. + +**`set_entry_duration`'s check order** mirrors `set_selection_duration`'s +existing precedent: duration positivity (`InvalidDuration`) is checked +*before* caret existence (`NoCaret`) — a malformed argument is rejected +independent of session state. + +**Tests and mutations** (all substituted, observed failing, then reversed +— never `git checkout`): t1 `caret_advances_across_a_would_be_barline` +(mutation: skip the advance in `enter_at_caret` → `left: MusicalPosition +(3/4)` vs `right: MusicalPosition(1/1)`, dies). t2 +`octave_inference_table` + `octave_tie_break_prefers_downward` + +`enter_nominal_infers_octave_from_the_nearest_preceding_note` (mutation: +flip `nearer_is_down`'s `<=` to `<` → kills `octave_tie_break_prefers_downward` +only, confirming by direct observation that `octave_inference_table` is +insensitive to it — the tie-break's real unreachability, not a testing +gap). t3 `enter_pitch_reproduces_insert_note_at_s_overwrite` — twin +sessions from the same seed, one via `insert_note_at`, one via the caret at +the same voice/position/duration/pitch, asserted **byte-identical** +(`assert_eq!(session_a.score(), session_b.score())`) rather than just +field-by-field, since both sessions mint fresh ids deterministically from +identical starting state (mutation: skip `make_room_ops`, insert directly +→ **not** a reducer refusal — `apply` returns `Ok`, but the reducer's own +overlap precondition silently no-ops the bare `InsertEvent` against the +already-occupied slot, so `session_b`'s score is simply unchanged from +before the edit while `session_a`'s carries the overwrite; the assertion +catches the value mismatch, confirmed by inspecting both failing `Score` +dumps). t4 `midi_note_to_pitch_table` (mutation: `note/12` instead of +`note/12 - 1` → `A0` becomes `A1`, dies). t5 +`x_at_position_round_trips_with_position_at` + +`forward_x_extrapolates_from_the_last_segment_not_the_first` — the +"wrong segment" mutation (`n - 2` → `0` in `forward_x`'s past-the-last- +anchor branch) does **not** kill the round-trip test: `valid_score`'s +onsets are uniformly time-spaced and the stub renders them uniformly in +`x`, so every segment shares one slope and segment 0 vs. the true last +segment are indistinguishable there — an honest finding, not swept under +the round-trip test's apparent coverage. A second, direct test calls +`forward_x` with hand-built, deliberately non-uniform anchors (slopes 40 +and 8 x-per-whole-note); the mutation there gives 30 instead of the +correct 14, confirmed killed, then reverted. t6 +`undo_restores_the_score_but_leaves_the_caret_advanced` (mutation: undo +also retreats the caret by one entry duration → `1/1` vs `5/4`, dies) and +`caret_clears_when_its_voice_is_deleted` (mutation: empty out +`reresolve_caret`'s body → caret stays `Some` after `DeleteVoice` +succeeds, dies). diff --git a/crates/epiphany-editor-core/src/lib.rs b/crates/epiphany-editor-core/src/lib.rs index 4060584..9d1d890 100644 --- a/crates/epiphany-editor-core/src/lib.rs +++ b/crates/epiphany-editor-core/src/lib.rs @@ -280,6 +280,23 @@ pub struct GridPosition { pub position: MusicalPosition, } +/// The note-entry caret: **session-local** editing-cursor state, never recorded +/// in the op log (contract: `spec/CONTRACT_EDITOR_T3_CARET.md` §W1 — undo does +/// not move it; `DECISIONS.md`). `(voice, position, entry_duration)`: the voice +/// and musical position [`EditorSession::enter_nominal`] / +/// [`EditorSession::enter_pitch`] / [`EditorSession::enter_rest`] insert at next, +/// and the written duration they insert with (also the step +/// [`EditorSession::advance`] / [`EditorSession::retreat`] move by). +#[derive(Clone, PartialEq, Eq, Debug)] +pub struct Caret { + /// The voice the caret inserts into. + pub voice: VoiceId, + /// The musical position the caret inserts at. + pub position: MusicalPosition, + /// The written duration the caret inserts with (and steps by). + pub entry_duration: MusicalDuration, +} + /// What an [`EditorSession::apply`] did. #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub struct EditOutcome { @@ -382,6 +399,16 @@ pub enum EditorError { /// The kind of object the intent expected. expected: &'static str, }, + /// An intent needed the note-entry **caret** ([`EditorSession::enter_nominal`], + /// [`EditorSession::enter_pitch`], [`EditorSession::enter_rest`], + /// [`EditorSession::advance`], [`EditorSession::retreat`], + /// [`EditorSession::set_entry_duration`]) but none is set + /// ([`EditorSession::set_caret_at`] places one). A caret and a selection are + /// independent session-local cursors (contract: + /// `spec/CONTRACT_EDITOR_T3_CARET.md` §W1), so this is its own variant rather + /// than an overload of [`NoSelection`](EditorError::NoSelection) — flagged as + /// an underdetermined call in `DECISIONS.md`. + NoCaret, /// An insert-after would land on a musical position already occupied by another /// event in the same voice (the reducer would silently no-op it). The edit is /// refused; inserting into a packed voice needs an explicit make-room policy. @@ -480,6 +507,7 @@ impl fmt::Display for EditorError { EditorError::WrongSelection { expected } => { write!(f, "the selection is not a {expected}") } + EditorError::NoCaret => f.write_str("no caret is set"), EditorError::InsertSlotOccupied => { f.write_str("the position after the selection is already occupied in its voice") } @@ -565,6 +593,12 @@ pub struct EditorSession { // vertical half of click-to-insert reads this to spell the clicked height. start_clefs: BTreeMap<(RegionId, StaffId), Clef>, selection: SelectionSet, + // The note-entry caret (contract: `spec/CONTRACT_EDITOR_T3_CARET.md` §W1) — + // session-local, never in the op log. `reresolve_caret` (called alongside + // `reresolve_selection` after every apply/undo/redo) clears it when its voice + // no longer exists; nothing else moves it except `advance`/`retreat` and a + // successful entry (which advances it by the entry duration). + caret: Option, // Operation-minting identity. A real client supplies its own replica/author. // Minted operations form this replica's monotonic local history; the next op's // counter is `authored.len()` (never reused across undo), so a failed apply consumes @@ -632,6 +666,7 @@ impl EditorSession { map, start_clefs, selection: SelectionSet::default(), + caret: None, replica: ReplicaId(1), author: AuthorId(0), applied: Vec::new(), @@ -1027,6 +1062,93 @@ impl EditorSession { Some(self.region_default_grid(region)) } + /// The current note-entry caret, if any (contract: + /// `spec/CONTRACT_EDITOR_T3_CARET.md` §W1). `None` until [`Self::set_caret_at`] + /// places one, or after [`Self::clear_caret`] or a voice-vanish re-resolution + /// (checked after every apply/undo/redo — see `DECISIONS.md`). + pub fn caret(&self) -> Option { + self.caret.clone() + } + + /// Clears the caret (no note-entry cursor). + pub fn clear_caret(&mut self) { + self.caret = None; + } + + /// Places the caret at a world `point` — the same staff/region resolution + /// [`Self::insert_note_at`] uses (its private `nearest_manifestation`), taking the + /// staff instance's **primary voice** (or its first, mirroring + /// [`Self::insert_note_at`]'s own voice choice), the musical position snapped + /// via [`Self::default_grid_at`] + [`Self::position_at`], and the entry + /// duration initialized from that grid's step. Errors with + /// [`EditorError::NoInsertTarget`] under the same conditions + /// [`Self::insert_note_at`] does (off any staff, a non-metric region, or too + /// few rendered events to place a position). + pub fn set_caret_at(&mut self, point: Point) -> Result { + let (region, voice) = { + let (region, si, _origin) = self + .nearest_manifestation(point) + .ok_or(EditorError::NoInsertTarget)?; + let voice = si + .voices + .iter() + .find(|v| v.is_primary) + .or_else(|| si.voices.first()) + .ok_or(EditorError::NoInsertTarget)?; + (region, voice.id) + }; + let grid = self + .default_grid_at(point) + .ok_or(EditorError::NoInsertTarget)?; + let placed = self + .position_at(point, &grid) + .ok_or(EditorError::NoInsertTarget)?; + if placed.region != region { + return Err(EditorError::NoInsertTarget); + } + let caret = Caret { + voice, + position: placed.position, + entry_duration: grid.step, + }; + self.caret = Some(caret.clone()); + Ok(caret) + } + + /// Sets the caret's entry duration — the written value + /// [`Self::enter_nominal`]/[`Self::enter_pitch`]/[`Self::enter_rest`] insert, + /// and the step [`Self::advance`]/[`Self::retreat`] move by. The duration is + /// checked first, independent of caret state (the same order + /// [`Self::set_selection_duration`] uses): [`EditorError::InvalidDuration`] + /// for a non-positive duration, else [`EditorError::NoCaret`] if no caret is + /// set. + pub fn set_entry_duration(&mut self, duration: MusicalDuration) -> Result<(), EditorError> { + if !duration.is_positive() { + return Err(EditorError::InvalidDuration); + } + let caret = self.caret.as_mut().ok_or(EditorError::NoCaret)?; + caret.entry_duration = duration; + Ok(()) + } + + /// Moves the caret forward by its entry duration. No insertion. Positions are + /// global rationals, so this crosses barlines with no special casing. Errors + /// with [`EditorError::NoCaret`] if no caret is set. + pub fn advance(&mut self) -> Result { + let caret = self.caret.as_mut().ok_or(EditorError::NoCaret)?; + caret.position = caret.position.clone() + caret.entry_duration.clone(); + Ok(caret.clone()) + } + + /// Moves the caret backward by its entry duration, clamped at position zero + /// (the region origin) — never negative. No insertion. Errors with + /// [`EditorError::NoCaret`] if no caret is set. + pub fn retreat(&mut self) -> Result { + let caret = self.caret.as_mut().ok_or(EditorError::NoCaret)?; + caret.position = retreat_position(&caret.position, &caret.entry_duration); + Ok(caret.clone()) + } + /// The meter-derived default [`GridResolution`] for `region`: the beat (`1/ /// denominator`) of its governing time signature — the first one a region measure /// references, else the score's first — and a quarter note when none is determinable @@ -1296,6 +1418,7 @@ impl EditorSession { self.redo_stack.clear(); self.install(materialized); let selection_preserved = self.reresolve_selection(); + self.reresolve_caret(); Ok(EditOutcome { graph_changed, selection_preserved, @@ -1324,6 +1447,7 @@ impl EditorSession { self.undo_units.pop(); self.install(materialized); let selection_preserved = self.reresolve_selection(); + self.reresolve_caret(); Some(EditOutcome { graph_changed, selection_preserved, @@ -1345,6 +1469,7 @@ impl EditorSession { self.undo_units.push(unit.len()); self.install(materialized); let selection_preserved = self.reresolve_selection(); + self.reresolve_caret(); Some(EditOutcome { graph_changed, selection_preserved, @@ -2110,6 +2235,118 @@ impl EditorSession { } } + /// The world `x` a musical `position` in `region` renders at — the forward + /// companion of [`Self::position_at`], inverting the same piecewise-linear + /// `(onset, x)` anchors (its private `position_anchors`) it does, extrapolating + /// past the first/last anchor exactly as [`Self::position_at`]'s own inverse + /// (the private `invert_x`) does. `within` scopes the anchors to one cast-off system, + /// exactly mirroring `position_anchors`'s own parameter (`None` reads + /// the whole region as one flat monotonic run — the common case without cast + /// geometry, e.g. the stub solver); a caller drawing into a specific system + /// supplies its box the same way [`Self::position_at`] derives one from a + /// click point via its private `containing_system`. `None` when `region` is not + /// metric, or fewer than two of its events render inside `within` to fix a + /// scale — the same two-anchor floor [`Self::position_at`] enforces. `pub` + /// so a GUI (W2) can draw the caret at its exact musical position. + pub fn x_at_position( + &self, + region: RegionId, + within: Option<&Rect>, + position: &MusicalPosition, + ) -> Option { + if !self.region_is_metric(region) { + return None; + } + let anchors = self.position_anchors(region, within); + if anchors.len() < 2 { + return None; + } + Some(forward_x(&anchors, position)) + } + + /// Enters a **natural** `nominal` (no accidental — alteration is a follow-up + /// gesture via the existing transpose intents, MuseScore's model) at the + /// caret. The octave is inferred as the one nearest, in diatonic staff steps, + /// to the caret voice's reference pitch — the nearest preceding note strictly + /// before the caret's position (its private `reference_pitch_before`), or octave 4 + /// with no reference (see the private `infer_octave` for the rule and its downward + /// tie-break). Funnels to this session's shared insertion core. + pub fn enter_nominal(&mut self, nominal: CmnNominal) -> Result { + let caret = self.caret.clone().ok_or(EditorError::NoCaret)?; + let reference = self.reference_pitch_before(caret.voice, &caret.position); + let octave = infer_octave(reference, nominal); + self.enter_at_caret(Some(cmn_pitch(nominal, octave))) + } + + /// Enters an explicit `pitch` at the caret — no octave inference; the caller + /// names the exact pitch (e.g. via [`midi_note_to_pitch`], the step-time MIDI + /// path `spec/PLAN_EDITOR_APP.md`'s T3 entry names as the primary input + /// method). Funnels to this session's shared insertion core. + pub fn enter_pitch(&mut self, pitch: Pitch) -> Result { + self.enter_at_caret(Some(pitch)) + } + + /// Enters a rest at the caret. Funnels to this session's shared insertion core. + pub fn enter_rest(&mut self) -> Result { + self.enter_at_caret(None) + } + + /// The shared insertion core [`Self::enter_nominal`]/[`Self::enter_pitch`]/ + /// [`Self::enter_rest`] funnel to: a fresh single-note (`pitch = Some`) or + /// rest (`pitch = None`) event at the caret's `(voice, position)`, its + /// written duration the caret's `entry_duration`, **make-room overwrite + /// exactly as [`Self::insert_note_at`]** — the same `make_room`/ + /// `make_room_ops`/[`Minter`] machinery, one atomic apply/transaction (a bare + /// insert needs no transaction; a make-room insert commits as one) — then + /// **advances the caret by the entry duration, only on success**: a refused + /// edit changes nothing, including where the caret sits. + /// + /// Errors with [`EditorError::NoCaret`] if no caret is set, or the pencil's + /// own make-room refusals (see [`Self::insert_note_at`]). + fn enter_at_caret(&mut self, pitch: Option) -> Result { + let caret = self.caret.clone().ok_or(EditorError::NoCaret)?; + let staff_instance = self + .metric_staff_instance_of_voice(caret.voice) + .ok_or(EditorError::NoInsertTarget)?; + let start = caret.position.clone(); + let duration = caret.entry_duration.clone(); + let end = start.clone() + duration.clone(); + let room = self.make_room(caret.voice, &start, &end, None)?; + + let mut minter = self.minter(); + let mut ops = self.make_room_ops(room, staff_instance, &mut minter); + let pitches = match pitch { + Some(p) => vec![IdentifiedPitch { + id: minter.pitch(), + pitch: p, + }], + None => Vec::new(), + }; + let new_event = note_event( + minter.event(), + caret.voice, + start, + duration.clone(), + pitches, + ); + ops.push(OperationKind::InsertEvent(InsertEventOp { + staff_instance, + event: new_event, + })); + + // A bare insert needs no transaction; a make-room insert is atomic. + let outcome = if ops.len() == 1 { + self.apply(ops.into_iter().next().expect("one op")) + } else { + self.apply_transaction("enter note", Some(TransactionCategory::NoteEntry), ops) + }?; + + if let Some(c) = self.caret.as_mut() { + c.position = c.position.clone() + duration; + } + Ok(outcome) + } + /// Copies the current selection to the clipboard fragment projection /// (Ruling E, `spec/PLAN_EDITOR_APP.md` §Ruling E): per-event **values** /// (pitches, durations, an authored spelling override if the source @@ -3056,6 +3293,70 @@ impl EditorSession { fn reresolve_selection(&mut self) -> bool { self.selection.reresolve(&self.map) } + + /// Re-resolves the caret against the current score (contract: + /// `spec/CONTRACT_EDITOR_T3_CARET.md` §W1 — "if its voice ceases to exist, + /// the caret clears"; called alongside [`Self::reresolve_selection`] after + /// every apply/undo/redo). Unlike the selection set's survivor rule, a point + /// cursor has no fallback: a vanished voice simply clears the caret. Nothing + /// else here relocates the caret — undo does not move it (the contract's + /// own pin), and this method only ever clears, never repositions. + fn reresolve_caret(&mut self) { + if let Some(caret) = &self.caret { + if self.score.voices().all(|(_, _, v)| v.id != caret.voice) { + self.caret = None; + } + } + } + + /// The reference pitch [`Self::enter_nominal`]'s octave inference measures + /// from: the nearest preceding **note** (a rest does not count — it has no + /// pitch) in `voice`, strictly before `position` — the closest earlier + /// onset — as its CMN nominal/octave (the alteration is dropped: only the + /// staff position matters for the nearest-in-steps rule). `None` with no + /// earlier note, or when the nearest one is not a CMN pitch (there is then + /// no staff-step notion to measure from) — both fall back to + /// [`infer_octave`]'s default octave 4. + /// + /// A chord's **first** pitch is the reference; the contract names no + /// chord tie-break, so this is a documented, arbitrary pick (`DECISIONS.md`). + fn reference_pitch_before( + &self, + voice: VoiceId, + position: &MusicalPosition, + ) -> Option<(CmnNominal, i8)> { + let mut best: Option<(MusicalPosition, Pitch)> = None; + for event in self.score.events.iter() { + if event.voice() != voice { + continue; + } + let Event::Pitched(pe) = event else { + continue; + }; + let EventPosition::Musical(at) = &pe.position else { + continue; + }; + if at >= position { + continue; + } + let Some(first) = pe.pitches.first() else { + continue; + }; + let better = match &best { + None => true, + Some((best_at, _)) => at > best_at, + }; + if better { + best = Some((at.clone(), first.pitch.clone())); + } + } + best.and_then(|(_, pitch)| match pitch.scale_position.position { + PitchSpacePosition::Cmn { + nominal, octave, .. + } => Some((nominal, octave)), + _ => None, + }) + } } /// The pitch one or more diatonic **staff steps** from `pitch`: the CMN nominal is @@ -3082,6 +3383,46 @@ fn staff_step(pitch: &Pitch, steps: i32) -> Option { Some(moved) } +/// The octave [`EditorSession::enter_nominal`] infers for `nominal`: the one +/// that puts it nearest, in diatonic **staff steps**, to `reference` (its +/// nominal + octave), or octave 4 with no reference. Equidistant ties resolve +/// **downward** (contract-pinned: `spec/CONTRACT_EDITOR_T3_CARET.md` §W1) via +/// [`nearer_is_down`]. +/// +/// **No (reference, nominal) pair drawn from the seven CMN letters can ever +/// tie**, provably: the candidate octaves for a fixed `nominal` are exactly 7 +/// diatonic steps apart, so the two flanking distances from `reference` are `r` +/// and `7 - r` for some integer `r` in `0..7`; these are equal only at `r = +/// 3.5`, unreachable for an integer. `nearer_is_down` is still exercised +/// directly, with a synthetic tied input, to prove the written comparison +/// itself rather than leave it dead code no real table row can reach +/// (`DECISIONS.md` — this is a documented finding, not an assumption). +fn infer_octave(reference: Option<(CmnNominal, i8)>, nominal: CmnNominal) -> i8 { + let Some((ref_nominal, ref_octave)) = reference else { + return 4; + }; + let ref_index = ref_octave as i64 * 7 + ref_nominal as i64; + let nominal_val = nominal as i64; + // `diff = ref_index - nominal_val = 7*q + r` (`0 <= r < 7`, `div_euclid`/ + // `rem_euclid`): the down candidate is octave `q` (distance `r`, at or below + // the reference), the up candidate is octave `q + 1` (distance `7 - r`). + let diff = ref_index - nominal_val; + let r = diff.rem_euclid(7); + let q = diff.div_euclid(7); + let octave = if nearer_is_down(r, 7 - r) { q } else { q + 1 }; + octave.clamp(i8::MIN as i64, i8::MAX as i64) as i8 +} + +/// The tie-break [`infer_octave`] applies when a candidate a diatonic-step +/// distance of `down_distance` **below** the reference ties with one +/// `up_distance` **above** it: downward wins — `<=`, not `<`, so an equal- +/// distance tie resolves down while a strictly nearer "up" candidate still +/// wins on its own merits. See [`infer_octave`]'s doc for why no real +/// (reference, nominal) pair ever calls this with equal arguments. +fn nearer_is_down(down_distance: i64, up_distance: i64) -> bool { + down_distance <= up_distance +} + /// A 5-line staff spans four staff spaces above its bottom line. const STAFF_SPAN: f32 = 4.0; @@ -3157,6 +3498,40 @@ fn invert_x(anchors: &[(MusicalPosition, f32)], x: f32) -> f64 { p0 + (x as f64 - x0) / span * (p1 - p0) } +/// Inverts a musical `position` to a world `x` through `(onset, x)` anchors in +/// ascending order (`>= 2`, leftmost first) — [`EditorSession::x_at_position`]'s +/// engine, and the exact mirror of [`invert_x`] with the roles of `x` and musical +/// time swapped: within the anchored span it interpolates the bracketing segment; +/// outside it, it extrapolates the nearest end segment's slope. `f64` for the same +/// reason `invert_x` is: this is geometry, not exact musical time. +fn forward_x(anchors: &[(MusicalPosition, f32)], position: &MusicalPosition) -> f32 { + let n = anchors.len(); + debug_assert!(n >= 2, "forward_x needs at least two anchors for a scale"); + let onset = |i: usize| anchors[i].0 .0.to_f64(); + let x = |i: usize| anchors[i].1 as f64; + let target = position.0.to_f64(); + // The segment to (inter/extra)polate on: the one bracketing `target`, clamped + // to the first/last segment when `target` is before / after every anchor. + let seg = if target <= onset(0) { + 0 + } else if target >= onset(n - 1) { + n - 2 + } else { + (0..n - 1) + .find(|&i| target >= onset(i) && target <= onset(i + 1)) + .unwrap_or(n - 2) + }; + let (p0, p1) = (onset(seg), onset(seg + 1)); + let (x0, x1) = (x(seg), x(seg + 1)); + let span = p1 - p0; + let result = if span.abs() < f64::EPSILON { + x0 + } else { + x0 + (target - p0) / span * (x1 - x0) + }; + result as f32 +} + /// Snaps a raw musical position to the nearest multiple of `step` from the origin, /// clamped to be non-negative (no position precedes the region start). The multiple /// is taken in `f64`, then the position is rebuilt by exact rational arithmetic @@ -3171,6 +3546,22 @@ fn snap_to_grid(raw: f64, step: &MusicalDuration) -> MusicalPosition { MusicalPosition(step.0.mul(&RationalTime::from_int(k))) } +/// [`EditorSession::retreat`]'s clamped subtraction: `position` moved back by +/// `step`, clamped to the region origin (never negative). There is no +/// `MusicalPosition - MusicalDuration` in the type-level algebra of Chapter 3 +/// (`crate::time`'s module doc: "position + position is not defined" — and +/// symmetrically, an unclamped position minus a duration could go negative, +/// which is not a valid position either), so this reimplements the subtraction +/// directly over the raw [`RationalTime`], clamping the result. +fn retreat_position(position: &MusicalPosition, step: &MusicalDuration) -> MusicalPosition { + let raw = position.0.sub(&step.0); + if raw.is_negative() { + MusicalPosition::origin() + } else { + MusicalPosition(raw) + } +} + /// The diatonic staff index of a CMN pitch — `octave * 7 + nominal`, so two pitches /// compare by staff position and a step is `± 1`. `None` for a non-CMN position. fn diatonic_index(pitch: &Pitch) -> Option { @@ -3220,14 +3611,23 @@ fn note_stepped(top: &Pitch, steps: i32) -> Option { /// A natural CMN pitch at `nominal`/`octave`, sounding at its written position /// ([`AcousticRealization::Implicit`]) — the value a click-to-insert mints for the -/// height under the cursor (a caller respells if an accidental is wanted). +/// height under the cursor (a caller respells if an accidental is wanted), and +/// [`EditorSession::enter_nominal`]'s own insertion value. fn cmn_pitch(nominal: CmnNominal, octave: i8) -> Pitch { + chromatic_pitch(nominal, 0, octave) +} + +/// A CMN pitch at `nominal`/`alteration`/`octave`, sounding at its written +/// position ([`AcousticRealization::Implicit`]) — the alteration generalization +/// of [`cmn_pitch`] (naturals only); [`midi_note_to_pitch`] is its only +/// non-zero-alteration caller today. +fn chromatic_pitch(nominal: CmnNominal, alteration: i8, octave: i8) -> Pitch { Pitch { scale_position: ScalePosition { space: PitchSpaceId::new("cmn-12"), position: PitchSpacePosition::Cmn { nominal, - alteration: 0, + alteration, octave, }, }, @@ -3238,6 +3638,34 @@ fn cmn_pitch(nominal: CmnNominal, octave: i8) -> Pitch { } } +/// The v1 MIDI-to-pitch policy [`EditorSession::enter_pitch`] takes note-on +/// events through (contract: `spec/CONTRACT_EDITOR_T3_CARET.md` §W1 — the +/// step-time MIDI path `spec/PLAN_EDITOR_APP.md`'s T3 names as the primary entry +/// method, wired app-side onto this pure seam): scientific-pitch octave +/// numbering (`60 = C4`, `69 = A4`) and every black key spelled **sharp**, never +/// flat — a later spelling prepass may refine the enharmonic choice; this is +/// deliberately the raw, un-key-aware mapping. +pub fn midi_note_to_pitch(note: u8) -> Pitch { + const CHROMATIC: [(CmnNominal, i8); 12] = [ + (CmnNominal::C, 0), + (CmnNominal::C, 1), + (CmnNominal::D, 0), + (CmnNominal::D, 1), + (CmnNominal::E, 0), + (CmnNominal::F, 0), + (CmnNominal::F, 1), + (CmnNominal::G, 0), + (CmnNominal::G, 1), + (CmnNominal::A, 0), + (CmnNominal::A, 1), + (CmnNominal::B, 0), + ]; + let note = note as i32; + let octave = (note / 12 - 1) as i8; + let (nominal, alteration) = CHROMATIC[(note % 12) as usize]; + chromatic_pitch(nominal, alteration, octave) +} + /// The musical duration spanning `from..to` (`to - from`), exact over rational time. fn span_between(from: &MusicalPosition, to: &MusicalPosition) -> MusicalDuration { MusicalDuration(to.0.sub(&from.0)) @@ -5033,6 +5461,364 @@ mod tests { MusicalDuration(RationalTime::new(numerator, denominator).unwrap()) } + // --- T3-W1: the note-entry caret (`spec/CONTRACT_EDITOR_T3_CARET.md` §W1). --- + + /// t1: the caret's advance arithmetic crosses a would-be barline with no + /// special casing — positions are global rationals. Entering a quarter at + /// 3/4 must land the caret at exactly whole-note 1 (`RationalTime` 1/1). + /// The literal fixture has no declared `TimeSignature`/measures (this is + /// deliberate, not an oversight — `DECISIONS.md`: the caret's positions are + /// meter-agnostic, so the test needs no real barline structure to make the + /// point that none is special-cased); it asserts the raw arithmetic + /// directly against a hand-placed caret. + #[test] + fn caret_advances_across_a_would_be_barline() { + let mut session = open_plain(0x5EED); + let region = a_clean_metric_region(&session); + let voice = primary_voice(&session, region); + session.caret = Some(Caret { + voice, + position: MusicalPosition(RationalTime::new(3, 4).unwrap()), + entry_duration: dur(1, 4), + }); + session.enter_rest().expect("enters at the caret"); + let caret = session.caret().expect("the caret is still set"); + assert_eq!( + caret.position, + MusicalPosition(RationalTime::new(1, 1).unwrap()), + "3/4 + 1/4 lands exactly on whole-note 1" + ); + } + + /// t2 (table): the minimum octave-inference rows the contract pins, + /// including the "fifth below vs. fifth above" case — which this proves is + /// **not** actually equidistant in diatonic staff steps (down wins 3 steps + /// to 4), matching `infer_octave`'s own doc that no real letter pair ever + /// ties. + #[test] + fn octave_inference_table() { + assert_eq!( + infer_octave(Some((CmnNominal::C, 4)), CmnNominal::D), + 4, + "ref C4 enter D -> D4 (up a step, distance 1, beats down distance 6)" + ); + assert_eq!( + infer_octave(Some((CmnNominal::C, 4)), CmnNominal::B), + 3, + "ref C4 enter B -> B3 (down a step, distance 1, beats up distance 6)" + ); + assert_eq!( + infer_octave(Some((CmnNominal::G, 4)), CmnNominal::D), + 4, + "ref G4 enter D -> D4: down 3 steps beats up 4 -- NOT a tie in staff \ + steps (only a tie in semitones, a P5 either way); the downward \ + tie-break plays no role here" + ); + assert_eq!( + infer_octave(None, CmnNominal::A), + 4, + "no reference -> the octave-4 default" + ); + } + + /// t2 (tie-break): `infer_octave`'s doc proves no (reference, nominal) pair + /// over the seven CMN letters can produce equal `down_distance`/ + /// `up_distance` arguments (the candidate octaves are 7 apart, and 7 is + /// odd), so the downward tie-break is unreachable through the octave + /// table alone. This exercises the actual comparison directly, with a + /// synthetic tied input, so the policy is genuinely tested rather than + /// merely asserted in a doc comment. + #[test] + fn octave_tie_break_prefers_downward() { + assert!( + nearer_is_down(3, 3), + "an equal-distance tie resolves downward" + ); + assert!(nearer_is_down(2, 3), "strictly nearer down still wins"); + assert!( + !nearer_is_down(4, 3), + "strictly nearer up wins on its own merits" + ); + } + + /// t2 (end-to-end): `reference_pitch_before` finds the fixture's own last + /// note (whatever seed-dependent nominal/octave it happens to carry) and + /// `enter_nominal` inserts at the octave `infer_octave` computes from it -- + /// proving the two pieces are wired together correctly, not just each + /// tested in isolation. + #[test] + fn enter_nominal_infers_octave_from_the_nearest_preceding_note() { + let mut session = open_plain(0x5EED); + let region = a_clean_metric_region(&session); + let voice = primary_voice(&session, region); + let events = voice_events(&session, voice); + let (last_id, last_pos, last_dur) = events.last().unwrap().clone(); + + let mut buf: Vec<&IdentifiedPitch> = Vec::new(); + session + .score() + .events + .get(last_id) + .unwrap() + .collect_identified_pitches(&mut buf); + let PitchSpacePosition::Cmn { + nominal: ref_nominal, + octave: ref_octave, + .. + } = buf.first().unwrap().pitch.scale_position.position + else { + panic!("the fixture's notes are CMN pitches"); + }; + + let insert_pos = MusicalPosition(last_pos.0.add(&last_dur.0)); + session.caret = Some(Caret { + voice, + position: insert_pos.clone(), + entry_duration: dur(1, 4), + }); + let entered = CmnNominal::D; + let expected_octave = infer_octave(Some((ref_nominal, ref_octave)), entered); + session.enter_nominal(entered).expect("enters at the caret"); + + let after = session + .score() + .events + .iter() + .find(|e| { + e.voice() == voice + && matches!(e.position(), EventPosition::Musical(p) if *p == insert_pos) + }) + .expect("the new note was inserted at the caret's pre-advance position"); + let mut pbuf: Vec<&IdentifiedPitch> = Vec::new(); + after.collect_identified_pitches(&mut pbuf); + let PitchSpacePosition::Cmn { + nominal: got_nominal, + octave: got_octave, + .. + } = pbuf.first().unwrap().pitch.scale_position.position + else { + panic!("enter_nominal inserts a CMN pitch"); + }; + assert_eq!(got_nominal, entered); + assert_eq!(got_octave, expected_octave); + } + + /// t3: entering over an occupied beat via the caret reproduces + /// `insert_note_at`'s own make-room overwrite **exactly** -- compared on + /// twin sessions opened from the same seed (so their fresh ids mint + /// identically), one driven by the pencil, one by the caret at the same + /// voice/position/duration and the same pitch the pencil's click would + /// have resolved. The two resulting scores are asserted byte-for-byte + /// equal, the strongest form of "matches what `insert_note_at` produces". + #[test] + fn enter_pitch_reproduces_insert_note_at_s_overwrite() { + let seed = 0x5EED; + let mut session_a = open_plain(seed); + let region = a_clean_metric_region(&session_a); + let voice = primary_voice(&session_a, region); + let before = voice_events(&session_a, voice); + let (_, _, origin_y) = region_staff_line(&session_a, region); + + // Click the last note with grid = its own duration: a full-cover overwrite. + let (_, pos, note_dur) = before.last().unwrap().clone(); + let at = click_for_position(&session_a, region, &pos, origin_y + 1.0); + let grid = GridResolution { + step: note_dur.clone(), + }; + let pitch = session_a + .staff_pitch_at(at) + .expect("a staff under the click"); + session_a + .insert_note_at(at, &grid) + .expect("the pencil overwrites the covered note"); + + let mut session_b = open_plain(seed); + session_b.caret = Some(Caret { + voice, + position: pos, + entry_duration: note_dur, + }); + session_b + .enter_pitch(cmn_pitch(pitch.nominal, pitch.octave)) + .expect("the caret overwrites the covered note"); + + assert_eq!( + session_a.score(), + session_b.score(), + "the caret's make-room overwrite reproduces the pencil's exactly" + ); + } + + /// t4: the MIDI table (contract minimum: 21/60/61/69/108). + #[test] + fn midi_note_to_pitch_table() { + let check = |note: u8, nominal: CmnNominal, alteration: i8, octave: i8| { + let pitch = midi_note_to_pitch(note); + match pitch.scale_position.position { + PitchSpacePosition::Cmn { + nominal: n, + alteration: a, + octave: o, + } => assert_eq!((n, a, o), (nominal, alteration, octave), "midi note {note}"), + other => panic!("midi note {note}: expected a CMN pitch, got {other:?}"), + } + }; + check(21, CmnNominal::A, 0, 0); // A0 + check(60, CmnNominal::C, 0, 4); // C4 + check(61, CmnNominal::C, 1, 4); // C#4 + check(69, CmnNominal::A, 0, 4); // A4 + check(108, CmnNominal::C, 0, 8); // C8 + } + + /// t5: `x_at_position` round-trips with `position_at` — every anchor onset + /// (exact, already on the grid), and one point three grid steps past the + /// last note (extrapolated, still on an exact grid multiple so there is no + /// snapping error to budget for). + #[test] + fn x_at_position_round_trips_with_position_at() { + let session = open_plain(0x5EED); + let region = a_clean_metric_region(&session); + let anchors = session.position_anchors(region, None); + assert!(anchors.len() >= 2, "at least two onsets to fix a scale"); + let (_, _, origin_y) = region_staff_line(&session, region); + let y = origin_y + 1.0; + let step = MusicalDuration(anchors[1].0 .0.sub(&anchors[0].0 .0)); + let grid = GridResolution { step }; + + for (onset, x) in &anchors { + let gp = session + .position_at(Point::new(*x, y), &grid) + .expect("a metric region under the click"); + assert_eq!(&gp.position, onset, "the click snaps to this onset exactly"); + let x_back = session + .x_at_position(region, None, &gp.position) + .expect("the forward map resolves"); + assert!( + (x_back - x).abs() < 1e-2, + "round trip at onset {onset:?}: {x_back} vs {x}" + ); + } + + let (last_onset, last_x) = anchors.last().cloned().unwrap(); + let gap = last_x - anchors[anchors.len() - 2].1; + let reach = last_x + gap * 3.0; // three exact grid steps past the last note + let gp = session + .position_at(Point::new(reach, y), &grid) + .expect("empty space past the notes still resolves"); + assert!(gp.position > last_onset, "past the last onset"); + let x_back = session + .x_at_position(region, None, &gp.position) + .expect("the forward map extrapolates past the last anchor too"); + assert!( + (x_back - reach).abs() < 1e-2, + "extrapolated round trip: {x_back} vs {reach}" + ); + } + + /// t5 (segment selection, direct): `valid_score`'s onsets are uniformly + /// spaced, so every segment shares one slope and + /// `x_at_position_round_trips_with_position_at`'s extrapolation cannot + /// distinguish "the last segment" from "the wrong segment" — both give the + /// same answer on a straight line. This calls [`forward_x`] directly with + /// deliberately **non-uniform** anchors so a wrong-segment bug is + /// observable: extrapolating past the last anchor must use the *last* + /// segment's own slope, not the first's. + #[test] + fn forward_x_extrapolates_from_the_last_segment_not_the_first() { + let p = |n: i64, d: i64| MusicalPosition(RationalTime::new(n, d).unwrap()); + // Segment 0 (onset 0 -> 1/4): slope 40 x per whole note. + // Segment 1 (onset 1/4 -> 1/2, the *last* segment): slope 8 x per whole note. + let anchors = vec![(p(0, 1), 0.0_f32), (p(1, 4), 10.0), (p(1, 2), 12.0)]; + // One more 1/4 step past the last anchor. + let target = p(3, 4); + let x = forward_x(&anchors, &target); + assert!( + (x - 14.0).abs() < 1e-4, + "extrapolating from the last segment's slope (8 x/whole-note) from \ + (1/2, 12.0) by 1/4 more gives 14.0, got {x}" + ); + } + + /// t6 (undo): undo restores the score but leaves the caret exactly where + /// the entry advanced it -- undo does not move the caret (contract-pinned). + #[test] + fn undo_restores_the_score_but_leaves_the_caret_advanced() { + let mut session = open_plain(0x5EED); + let region = a_clean_metric_region(&session); + let voice = primary_voice(&session, region); + let before_score = session.score().clone(); + let before_events = voice_events(&session, voice); + + let (_, last_pos, last_dur) = before_events.last().unwrap().clone(); + let start = MusicalPosition(last_pos.0.add(&last_dur.0)); + session.caret = Some(Caret { + voice, + position: start.clone(), + entry_duration: dur(1, 4), + }); + session.enter_rest().expect("enters at the caret"); + let advanced = session.caret().expect("the caret is still set"); + assert_eq!( + advanced.position, + MusicalPosition(start.0.add(&RationalTime::new(1, 4).unwrap())), + "the caret advanced by the entry duration" + ); + assert_ne!( + session.score(), + &before_score, + "the entry changed the score" + ); + + session.undo().expect("undoes the entry"); + assert_eq!(session.score(), &before_score, "undo restores the score"); + assert_eq!( + session.caret(), + Some(advanced), + "undo does not move the caret" + ); + } + + /// t6 (vanish): a caret whose voice is deleted out from under it clears. + /// Built with a **real** op -- `DeleteVoiceOp` exists in `epiphany-ops` and + /// accepts an empty voice (Chapter 6 §6.10 DeleteVoice) -- rather than a + /// synthetic internal score swap: the contract asked for the synthetic + /// construction only "if no current op can remove a voice", and one does. + #[test] + fn caret_clears_when_its_voice_is_deleted() { + use epiphany_core::Voice; + use epiphany_ops::DeleteVoiceOp; + + let mut score = valid_score(0x5EED); + let fresh_voice: VoiceId = score.identity.mint(); + { + let staff_instance = &mut score.canvas.regions[0] + .content + .staff_based_mut() + .expect("the fixture's region is staff-based") + .staff_instances[0]; + staff_instance.voices.push(Voice::user(fresh_voice)); + } + let mut session = EditorSession::open(score, Box::new(StubSolver)).expect("renders"); + session.caret = Some(Caret { + voice: fresh_voice, + position: MusicalPosition::origin(), + entry_duration: dur(1, 4), + }); + assert!(session.caret().is_some()); + + session + .apply(OperationKind::DeleteVoice(DeleteVoiceOp { + voice: fresh_voice, + })) + .expect("deletes the empty voice"); + + assert_eq!( + session.caret(), + None, + "the caret cleared when its voice vanished" + ); + } + /// Selects the first pitch (notehead) of `event` and returns its id. fn select_first_pitch_of(session: &mut EditorSession, event: EventId) -> PitchId { let mut buf: Vec<&IdentifiedPitch> = Vec::new(); diff --git a/crates/epiphany-editor-gui/DECISIONS.md b/crates/epiphany-editor-gui/DECISIONS.md index d8da294..814650f 100644 --- a/crates/epiphany-editor-gui/DECISIONS.md +++ b/crates/epiphany-editor-gui/DECISIONS.md @@ -331,3 +331,141 @@ pure for the same reason W2 needed to unit-test *something* about release-time dispatch). `cargo test -p epiphany-editor-gui` green (20/20, including all four golden tests) is this packet's regression gate for that untested surface, per the brief. + +## Note-entry mode + G5 (2026-07-24, T3-W2) + +Dispatched under `spec/CONTRACT_EDITOR_T3_CARET.md` §W2, over W1's caret seam +(`EditorSession::{caret, set_caret_at, set_entry_duration, advance, retreat, +enter_nominal, enter_pitch, enter_rest, x_at_position}`, `Caret`). `main.rs` +(mode, keys, palette gating, click dispatch, caret overlay, help/debug text) +and `goldens.rs` (the new G5 test) only — **no editor-core change was +needed**, confirmed by grep-checking the packet's own suggestion of a "tiny +pub accessor": every piece the caret overlay needs (`score().voices()`, +`score().staff_instances()`, `resolved().strokes`, `x_at_position()`) was +already public. + +**Mode exclusivity.** `pencil: bool` and the new `entry_mode: bool` are +mutually exclusive: turning either on always clears the other. Implemented +once, as a pure `toggle_exclusive(this, other) -> (bool, bool)` (mirrors +`resolve_release`'s role from T2-W2 — pulled out so the invariant itself is +unit-tested, not eyeballed at each of its four call sites: the P/N keys and +the two toolbar `toggle_value`s). `toggle_exclusive`'s contract is +intentionally asymmetric: turning `this` **on** forces `other` off; turning +`this` **off** leaves `other` exactly as it was (never assumes the +invariant already holds coming in, only guarantees it holds going out). + +**Letter-key interpretation is mode-gated**, pulled out as a second pure +function `resolve_letter(entry_mode, nominal) -> LetterAction` (`Enter`, +`AddChordNote`, or `None`) — the packet's own named candidate for "the +obvious testable pure fn". In entry mode every letter A–G enters that +natural; outside it, only `A` keeps its pre-existing "add chord note to the +anchor" meaning (B–G stay unbound outside entry mode, as they always were). +`handle_keys` reads whichever of the seven letter keys fired this frame (at +most one in practice — one key event — but the lookup takes the first match +either way) into `Keys::letter: Option`, then dispatches through +`resolve_letter`. + +**Key map, verified against the code rather than assumed from the +contract's own summary** (the contract said "←/→ ... they currently move +selection pitch", which does not match `handle_keys` as it stood — ↑/↓ move +pitch, ←/→ were unbound; the coordinator's dispatch caught this and it is +recorded here as the corrected, verified fact): + +* **N** — toggles note-entry mode (mutually exclusive with pencil). +* **A–G**, entry mode on — `enter_nominal`. **A**, entry mode off — the + existing "add chord note" (unchanged). **B–G**, entry mode off — no + binding (unchanged: they never had one). +* **R** — `enter_rest`, entry mode only. Verified free: `handle_keys` never + read `egui::Key::R` before this packet. +* **←/→** — `retreat`/`advance`, entry mode only. Both new bindings (neither + was bound before); outside entry mode they remain inert, same as before + this packet. +* **↑/↓** — unchanged in every mode: `move_selection_staff_step`. +* Duration palette (toolbar buttons `1`/`1/2`/`1/4`/`1/8`/`1/16`) — while + entry mode is on, `set_entry_duration`; otherwise unchanged + (`set_selection_duration`). Gated per-click on `self.entry_mode`, not + a separate widget. + +**Click dispatch.** `score_view`'s three-way branch is now `pencil` → +`insert_note_at` (unchanged) → else `entry_mode` → `set_caret_at` (new) → +else the existing select/toggle path. Pencil's branch keeps its early +`return` after `request_repaint()` (needed because `insert_note_at` mutates +the score, staling the hit-test map the overlay code below would otherwise +paint against this same frame). `set_caret_at` needs no such guard — it +mutates no score/layout state at all, so the overlay code safely runs in +the same frame using the just-updated caret. Rubber-band drag tracking +(`DragRect`) is now gated on `!self.pencil && !self.entry_mode` (widened +from `!self.pencil` alone) — an extension beyond the contract's literal +text (which only names "a click in entry mode"), decided for the same +reason pencil already excludes dragging: a click in either mode does +something other than select, so a drag gesture in either mode has no +selection to build either. Flagged as this packet's own call, not +contract-mandated. + +**The caret overlay's region/staff derivation** (`caret_segment`, next to +`selection_rect`): `caret.voice` → `(region, staff_instance)` via +`session.score().voices()` (matches `epiphany-editor-core`'s own internal +use of `Score::voices()`) → the staff's global `StaffId` via +`session.score().staff_instances()` → that staff's *own* rendered line +strokes, filtered from `session.resolved().strokes` by +`TypedObjectId::Staff(staff_id)` (not just any staff — a multi-staff score +needs the caret drawn on its own staff, not the topmost one) — min/max `y` +across those strokes is the "full staff height" span, no hardcoded staff- +space constant needed (unlike duplicating editor-core's private +`STAFF_SPAN`, this reads the *actual* rendered geometry). `x` comes +straight from `x_at_position(region, None, &caret.position)`. Drawn as a +2.0pt line in a distinct green (`rgb(0, 170, 90)`), separate from the +selection's blue and the anchor's orange. + +**G5 — the caret-entry golden.** `ten_measure_single_staff(0)`, the *same* +fixture and the *same* `scripted_insert_target` click point G2/G3 already +use (the contract's own ask: "the same extrapolation territory G2 used") — +reused verbatim rather than deriving a second target function. Drives +`set_caret_at` then four `enter_nominal(C/D/E/F)` calls at the caret's +default (quarter, from the 4/4 meter) entry duration. **Value assertions +before pixels**, per the contract: the grid step, the caret's post- +`set_caret_at` position (independently re-derived, not just asserted equal +to G2's own — though it does land on the same whole-note-10 slot G2 proves, +since it is the identical target on the identical fixture) and entry +duration, then each of the four entries' resulting caret position as an +exact `MusicalPosition`/`RationalTime` before any raster is taken. Baseline: +`goldens/ten_measure_caret_entry.png`, 57379 bytes — generated via +`EPIPHANY_BLESS_GOLDENS=1`, **not judged correct here**: per plan §Ruling +C's user deep-dive point, a new baseline is visually reviewed and approved +by the user before being committed. G1–G4's three baselines +(`ten_measure_open.png` 53638B, `ten_measure_insert.png` 54590B, +`ten_measure_slurs_castoff.png` 57891B) are confirmed byte-identical +(`git status` reports them untouched throughout this packet). + +**Mutations, each substituted, observed failing, then reversed** (never +`git checkout`): (1) `resolve_letter`'s mode gate (`if entry_mode` → +`if !entry_mode`) — kills both of its own tests (letters interpreted +backwards in and out of entry mode). (2) `toggle_exclusive`'s clear branch +(`other` always passed through unchanged) — kills +`toggle_exclusive_turning_on_clears_the_other` (`(true, true)` vs expected +`(true, false)`). (3) G5: skipping the D entry (substituting a hand-built +`EditOutcome{graph_changed: true, ..}` in place of ever calling +`enter_nominal`) — **the position assertion fires, not the pixel one**: +the test panics at `session.caret().map(|c| c.position) == Some(after)` +(`Some(MusicalPosition(41/4))` vs the expected `Some(MusicalPosition(21/2))`) +long before reaching `render_pixmap`/`assert_golden` at all — confirming +the contract's own ordering claim ("value assertions before pixels") is +real, not just documented. (4) G5's determinism double: perturbing the +second `render_pixmap` call's `px_per_staff_space` (12.0 → 13.0) — kills +the `assert_eq!(svg1, svg2, ...)` check (a large SVG-string diff), proving +the double is a real, load-bearing check and not vestigial boilerplate +copied from G1–G4. + +**Testing scope, stated honestly.** `resolve_letter` and `toggle_exclusive` +are pure and unit-tested directly, same discipline as `resolve_release`/ +`DragRect`. Everything else this packet added — the toolbar's two +`toggle_value` calls and their `.changed()` mutual-exclusion glue, the +`Keys` struct's new fields and their `ctx.input` reads, `do_set_caret_at`/ +`do_caret_step`/`set_entry_duration`'s own bodies, `score_view`'s click/drag +branching, the caret overlay's paint call, the debug panel and help text — +is **egui-side dispatch, reviewed but not unit-tested**, for the same +structural reason T2-W2/W4b's equivalent surfaces are not: there is no +headless way in this crate to synthesize an `egui::Context` frame or drive +`ctx.input`/painter calls. `cargo test -p epiphany-editor-gui` green +(25/25, including all five golden tests) is this packet's regression gate +for that untested surface. diff --git a/crates/epiphany-editor-gui/goldens/ten_measure_caret_entry.png b/crates/epiphany-editor-gui/goldens/ten_measure_caret_entry.png new file mode 100644 index 0000000000000000000000000000000000000000..2ab2df4a16c50476f3a44e92d2dfe96c49f59f4a GIT binary patch literal 57379 zcmeHw3wTu3x%MDRv`AA0jiM55wc5u^+A5;uGTMsh;i>#oG>@Vg{XIQYB1X;8+#nmJ z%7GM7JczpPNSY`@z5;xLmG&H{NjF?_93q@0I^^oggP0H@_H7>PMT>rZHj;=pRzq7@Eg6oSb6WuPo zGpE-zJ&$tsvu6Vp@i=5*KUf^Nx_DiV)tVEyt+?g3vep9ckVIr;Pjj8Sxz>JV%a%L0 z)=t{`gg?B`Ut8#JxzoSCz}s4oh+bRV$GiS9DLgxWereuWh3$*1c}c%23|(clq+9E! zrH1Z$_09UlOXB_!PI3vc+Se9$eow>K@LR&X;&RucS3UZFR>g@BH}H zU~%@p@c}#41Hu=)Gds1eW_?j2M(iBY8|y!>v2$xt|V=pM^f~g-%Xw8JoI3U3N6#{`*f9nP86!9n7+?ED8LoWL;-Q{mU6M`POaCJBEF< zv~=tHzZtauM;UvUXB6@4E(ao8;5~)M*!Gn#eBX$=>=DyDhIj56Q@1v`@aMkS=ezFf z?%8DrGrd=O14F&N*A$dj6*O;=gDg8YIq%)yywUR6_}3F2sr3_)a-q}*Yu-NTk(``f zx$T>ZKLovG{OCs4=e6Ce@Tue1j4uClbaR!}Sygs!>DK&<@gP^*Qh)oU86C?#iO=_^ zJl}2Vd|e~UO7Q6n2za^Q-b+tlFOF4Xu|HRR_l$4+_2x??<+04WDILtn~Xiq-_D~}zXz4}Z5=8XlbqzrM#aP_4F#*~3S7D1BiRspUTa}vYc1%* zoPA9M1=aV<~mVLOl34&M(#U8ZNUpX>Oh% zyVs1(fgnn&o?soEAhJAgbx6d+={#gHEn42Xv1YmOHz^R*n$ob$-LWkAf-iV>;oj21 zB5aOx;9!xn93mD1XiujkJ7tdRwTjMgdx<>{#Cdm4&fKK-S7x_gw7PwXcbmm zWkmN+IluY#l9t=~77k4v>Ui-6{YW`D?#?B>pnpyj5zxos)fcPs&Jp>LcmEixeN$f3 zwA2smE52hIdV93@!05mbD==gjBqBvfacf>Gmqiw#Vs(G4=zp>zc%ID^P&qX)2(I}{ zd1oHx3+Km(WMRgRW$yVtIkoMja%#EzstPAL+YCL|OC7E?n4*(65-k(@A?bLGePB!= zboS6k{scj>`#5QvUG*Y>`U|2Twn!!lcQH&HDxv8QRZu zDh>Ia@YF**aq{b1F{kpycS|Q1wnHt^5=vt4V{w&l-CLduhIM!VqCL5=X&UV? zhpKbrViL7nxwQyN$dPfds9a6ZQ}Wp!Oe+hfLAXDpgY{>zPWd|C@Lq4NXaAjIpVFeW z$A8CS^T(@Jts-4hihFmiTD^BwZewdbEO<0~hOg#I_ z?^b$y*L0NmI+|#a{#=Qlz_uj!^Am>cIP;FO+@~Qbz3(MpO8azJU%uQ8m(vCbm$vQo zhBI9m?Js-g*_V>i=jQ)yZjD&kp{xh?E-l^vrwPNRkr&(>6DVr?bXsB41|2weP-t6H zz`Is-y{Hvy9SqtB#qM^brEy_0eA&m-cS@ zB(3w4P*yW+xM91mO|GGVcGg!74f~;@tYJp~Ca7hYQp6JMl`+ zb{D>2_Y)Ee&i4k-qSCT21*Hv~I=67o+*9B0vXcab^8fdKl0TXp3X{71w7E+ zP0*hFiLw(9b$Cj<+%?~(XAm)atiCsDTP{_7SDt7X+_ZSl_QreqpEdeh!7>*Rhs1bozLRZp}`U?Q(ra`Mz;HN5NY6- zX@OssoeMu0dOHtlqCs2ro|aoY>ql7!M)jOl)RgSrV=v5S0k1T5@p>gu6HSUD=! z&YNLy9UV<-+$hIKE}M|2ZBkJZ%HLNOy@-&g3!7k>`m!Rx4c03>g?mQS-jiDh?~rN= zGJaU8IvDLC+rQNh)H18|$Q~H8t_j+IH{SZ!r)m}IU8_{)p~R_!$I$nrfb{)X8}=fR zC?{aF#BE{Lc{7yJB(haqW*Dl8wU>4#1K!8a*Oyz<)Ly+-Vbg1Ne9l1eF6~+w^fTRf z=dQ9n5kWY5>rt3t@BU`cTcVnM`#yjD9+ID&R$4{hAI}g&BfGg~WJ2Rmwxf#1~x%ngx~ z!~KEb1O$m7`St@{;%EG*&0q7`_^e%k{wV+DA@KTKblF4?04XMe^T+;e(TfQaI!5dE zu6|zRsV)b!98cU&fsKZ9NeZ(sm4~lRn%bIQ(pux*sJ+8b)`Kv*v3}kNPKL+}^|-FT zwu*?ZM0exlHLxk^4A|EJAWp4S!e+r{rI?1f$wY7@Xyq@=hlm$nZ#A9P&=`7cu>gnY zjxKu3K_`R$U!STe%xkK*LwF5y49(k5F^Qo?VB}OdCFwNt(&p!RI^7eA&9j`Oo?Kp=+Tk;w+ zh&T9eGomqusb#!#j0<&WPwU6jUjHhFN&gFPYPv>6>;u(XJIej@5jNcJeqU*{P)>Uf zPf@Akv{mF-%&e%t7Dp7{unC%^;o+qGXykj)ZWvGJCFh;!^8q7tPA=TgSbL@~w-1jL zC^(H#gypDvS%b{wsmH4cu#TdqfUe!vG`A39zEDkAi~Ag5>CSly`Ps(AC!#2FXEVowSfFY0-$k_oWDlr$3>txmDeDl4TaYc zbFiLMi)2EC6Axa`Tv$X7iP+d@DcsNi$J$=>2hX9Np{B5kXxsy5WNcVmCnONza|B9c z3SnTle;v(#?|KtJYX;aytcX9h`>2aG_IR>J3{~!+^VDA3A`l^*JB|2x*CJg6k71Hu zEZBFO0p?jnM+{IVh6-J@k)dxx7aoU{+@I4)MTE=sfJDtN%P4y{po0AjI5ROh5Kz$` z4FQF2s40|ug{<|=r1I}llD@~6{T_VT-3;=9)?K1}3uj^pwK{{d_ZsDS4=ZhilQcy# zcypJO=3mSt^tu(2g&>L5Z}@d0T91(x$s789WO|aq93oggE#yMRD<}z!>`q`$c`i%61cFdPD*9m?%h3{MyO*NtBzED6Z_pum2z7*MM}HtcA!U z9UPRAxS4~+DwI8+T(G)+$?BOb04QW?A2QaIK(5Xb9CG@wo##o`*>Oz=7P()fhVH=} zzhCj&f$T*}+tG6`Yi1}6H3Y~8Jb~~7#4bFbY_DSJwpSb5FQbvwQ6A0f?6<~HsXeio zgo#f|jD5Q}^7NQ9MFuIO(zKOEwsf^|mx)pI&g2i?VLq&5QWKrDkJo7;Itg*S3UkCq z1SJxqB6oI7uHZ8D8#6R=*o2;#=#P`3Q$h8CcL+^*S|>+~H=z=mqcI+x{iq`x36sf+ zwGu%SNnsr!L4Bl_yO={G z=Im0U{evVmfkJoKN0J;SDJ%YU1Q@Sj>N6{doa}`CioCUJ+LKK3T_uyG9YCo-2*#*8 zaJhS(cLTwGh#`j=4P?p?HmuNN8dF!9q?tV?Jnh@B3n8)cqSee~^>^XOD z)PE60Cmq3^2wl^_V?TAVT37n;&0SGFRMbaO;-c_n*A%`b3-(3-XaaaF{WzlB#51PG z)dAaU6x7bFJQ^6c*srT-XPikC2Syl|r4PUG*iu;+Pf+G*eCEYri#k@;e_m2p~lB zfk&AO=w@L9OVC^^R@S>eQ?wXoz#TdXb8+4Ihf6>2)>^%z<-S9ahg2RNCdX2?0LEV; z58X(FkBxMj4>Zb_V$AaRN+WH(vxLX~hppROcss9K%9sZ>aIIT`B-mQ_k zyxwbGzvms6%eoAqCbj)YeU$8u%&a#E|=0e~`4C?Wa4 zRw>}7*h8Nd!h{zaWJasobB}Mv_G@WHhfz?$@-*-MI-xtk>`@2tXl? z*bt}3)2E^%1wk^bly=F!tquEfi%N6*Y8vrwmSss=hi4rFeXNB02(_m`nGrEW6jo^c zB-mLjaA4{W%%n$|^ik5jxP38#tqfWlvQsmKRc`W^sGlGh1upOb9HTI z8#>P5_JelAtmHNVT8-UCv~iKwc2{s4Sx{P;KIJrd?KCHrOK@aN6AB<$ug^WtEia_SywQk4EYk>OT9(hOUAr!ky}ni`b-2yyO5{IgTav> z#VG;`xsNSUgk>MYOFA7j&=>~!cz8ELy4d@ZD;#i@UfYGy*Yo>I#Y9HfvwVDBcL#t7+~0|tDn z?Blb_PZ8J2``D!Lswp45Gx=mj2Kur}{orfUn?=CYQmomjq$4)mfz0R|*pL<45Za$+ z|8hmJ@gP#Gcpbp6pQnXu(mwp#%?^eY9=cB+g-6m8Py0aF6Gx(BLPM$PcxzT4zf~rg zSikbx9bTXIx2-0mH#lpVGq+&n+G0Z_LT6qP0O?dg!p z$(0DN+|Nuhh=agesJx#({Hwd~n>2~8b13Ic&@gIrNwA4ejAd?fH5!;wtAxOUV(r(f zdS2X&s$Q$G=Ek(x82MoPfqOH)TA%SJ3PSHj?l_bp2x4Ko&eyw4=em~MJ>TAo zaFXguky5FxxE#s~meJ!;<1};m9fF5|%oq>|zEC;waiKFjh$TX>Z_;M&eRb87$?bnh zc8Wl}4Jy|%JXp?V4_U^!!}#&zDS^o)@$V(wXd*qR%*<)e4z*k1w9P_w?Cr(i$$&*; zifh-)R&Z~_t7yu}K9sBza!OllobP^ioWS|o1BKWRfR-sR(q=-%GBC%%UQ zhQ0*s6C`INP*HR&yBpHqvH z{W8C(^*T?bb^c0i;OdeC|5EZcJwrb#AtQml{Ut_7;2iD5dNy8ZSE<-hg?EEN1mu(` z5rhXDGRNsvm~aKOPNsZ1iV7>_(O!dnyKMA+RSRYhqylmly6g8DwZG@%!h>}>k9S=R zC7W+Bw;X3jaxR)E1)cvQZ|xiy+fhYoeC>e-;cjPUH`*YEmkSl{ddc1X5>2b{U}lVw zdF+2cmduq38tk1l%ODx2NeM}YZ$f%U&!V)&!O0(9T>i<$k)wtRfkqO^5z5q#vQrJ0 z*8J4;VSjW4mI4qGbNvEu%vvrLUDURxID9W#KU%Y5>GT|@907$S9?67_w5JL$fuP0{ zVcN=TTVE)O3S~}F8qk`~rgt!nf>nmVm28}Qf575|!fe1}9As31|oK}EcXD6cP zCWVLcG{`qzac3f}bL0)8xdPLzr8FuIfe9vTV-G86$k$;K`tS2?!&8Z>N2_TPh^v# zxyh`ku4Sd{>;qZ~o~fMW?d~tNCNvt4*nbe`{H9awk2M#$9UaP-9kr~&tlgS@K=5Xd^HrQFB&MGEpIfirkc%z{t9F*K?}E1HwJJ(Py26<6Au>HafM(fTia z>e6q({)0YkBl;J9>5E9$8mWesZik~}8zWr??FwTP=1mG45l!pA)OH7*2p~N&`{_e$ zhOuCl7kk)^hYf}9W)yFBc=znuAUg(tuE$8ZM>E=f-tn+w0Yc_hH{$qdx>qs3jXp@n zaI|nl$5B}cbPZfI*WYiwc=gWFtN&=t5{Z#C<)b8ogd1AnIyljbGl?Ce3u*`TP9(+w zKio-H&4=?4soN$WpiZoz74|Hi5XyACeRLt}Jq3O3?9KdFE9wH-5G*`cbOc+LP99NL zNOz6#%wlp3H1$fqf|uof@FSUa8M)tW+g4nqeLef6#8HM}^wW>PB{) ztCF(w{4{JywikGZ(`wA=X40`G+HTVxD7MO&_Wv4FfVK9sfzH?gDh-PxfN z$K<}%zpaW86ZKG1JH_cTS^$v0tsU?Q!^>5=va!YXzcC{ijd?qD!n0?`$FmEskOl~i zZy(#KEx*0_0?+;nLYZ<}BX&<4@gJgeBIzg0|MW3CoAli{+isg3eAW{@Ej*Y6%4Fn^ z;wtz(HHf&15DcD~@syw}=n#O8t_wD+CPiq*C)(=O{Ixf|a1~l`r7bSaUs75F^(_X3 zLLu#ebaxr0_3KU>8RKHQ=7gQVJ!MakG~TU$*)cOpROn&4HLJM&Z^h2S8uu%;lD61b zUwG9GHJ%-4utyavrlsmu=-oj79KQ>B$jg*EX^WMza%%t1Z!T#D;nSTKgVsJ-ifOzoXGKFD zX;W#78{1!Ptf9DWJfIX_+DzbpBsfSx`$eaw2z8xZRt4$6F3#gLRoZJ6*bx8l95mrZ z3E{|Q<2AHq8!0`#qiqvkRPglnxVN+4l=A_X6xFNE3lG(8bc<>PCFT~vSg5cTfzU*_ z)=tPBtF0JNSL69JQYu+f0A>zZIyK`CL3F|8{=gje?;}9ME_l!5>`<-gD1}`PG3kLsoUpna|5h`q}KbnoC?I%+sSBoJW zRT;RT7|s--Ml1te3%P!X2MG#&Mczm294R=1%?@LO@-w^$egI_^vSThQr(-!9eT zAZPb_<>(J{qhOg%106m4M(s`nX&><%nHXx##H;c@5&QD_$k&i+jEC9t6~vTV$h0T? z$>EZl0pwsgRAD|J9XB!&1>nsDR47X>U(kH=g3ZF0e7h0tFS!r9P;}Wl0)i(A1J!3OjC?n;MIMX3~}wfK(&u z&9ZGFYzLd-xrHy}zX*TV)sgdHfjRi?&yI1?*Q4`;$C-?P3RProyVxa@eAo&Ai-tS^ zEEkW1&?ELcy?nPewgCl0{*l;eu{-%_Em7~nu}Bc-qq_ialSG7gA4wgwhFAfsDDh+3 z8uraJlF?5%YHP5MH+R$7Jr=#wy#TyNmm%&Ekc6PsY%%A6gYgmCt+`nn_gDnjf})r- zN|9Y$53VFgri-v4uSjeVGWJ*S=+#w_6>1R_8P;N270N5(6X+4NNNB`i)<1|F67@-T z5nPAqPIDDhPcqZ(#K1-{fY4BuOQp?g6V61ef?3)8D;GdlfGyCk6X=JMi8~4t77~=3 zLgpItFdAb~(_%+q*Qj^pRc;6PfGp#&^7ra>>AYBvMDKx<0pIdS>8?T9+vDg=Q4!!i zgip{7mdm6RUepe23yRS(JW%+q)`2}QR{Mp$0>V!@h#yn@*yo|asP|KfDUdCj3Hy9B zHo>=)DhZmn-D3ZSo#dy&%lkJulnx>U39synMvIBYF)X$(6Xr@ZUqr@b^cCX*ZLGWW z)NXG?3)?PV~igWE0`v-Rn%EDhhSkjugFupv{;=ASxNfEeoGIGTF)YHG(OAaVo5JzmJ zZ7JF?z8hAmV0@+A7xa#64XYv)a$s|fj<|AQ3OibRB^W`$IV#dX4r=l2p(O+Z5Jwzo zU>QRj#&@~*t8k@az~M=7IgJ4xiWud<(u}sAa$qU~GHueULiY*koG4^R@+e99WCrg$ zBt~o>^CXO{Kt(IYchx$Q8I{@jKZFM|=77RYRelK$&}g{_nSjjzV3@QSe4s;xVzr*m zv!*aA;?e5tDr>g3j^Q~uIXeuwDcUijRyTVP;3VM zQ?dQ566MV5)HO_-tWB?NW9YDR16b!7#^~S&NIhD2m z{66vngUwhD<@t9ayTe(W;iUA5>}!nFod3_r`}T_NxhmM|MB4yl|FbB`o{i2s=YIi3 zd%uJNisTW;v_NFjZL_GF;!ZPO&@8e}S7R2Xa7y>mNc=%#h(X@dQShO39Rbx@PQC=? z8-Zfx+*0W1955rTBxanXzE{aljY1vC^-$JPu-gy%26CD4;b>N7WMBQ~_b3y>OKAQ% zhFVOoS|?3K+k4I*iP16NeL<>WMq102vGbZFR~+Vp0z!=zdh)a7+679OucJIK+sHOj zo=^C42WFO>%4yP@Pc1=<&| z!|ai{w~?H*9%eWByNB6pdo;VkCKN<-SexVf(Cn+Fy6(l~wyDY65G7+ovE(3PYmr%r zw;^f_m;z)y7vs^pIcO7|P8rw_5w!xg&OF4DgP2FkTn%KLxHlrFg69l~-o6Bh$jEwP zdPjH}GDMI@%T$^mhgty?L2hh#$(?Wh`wV@gl7nKOiF6bY2T2S`mG;RB&F4u-f`#=2 zaxFl@Wx(WoKD7jFGBT(<`+zyKQ7?*Kckc)OGmGpB?BZe)}A9+2E4o|OKPLPDaUittO^^JtX99>giF60v4l zi1!_gH;wLkFLlGOfAmaSh=mA4=>$X~Wr^mbBxhIw?M;RZ>^P5ZnA-qb?TISZAthy( z9OeVb6hd;Co2Z+ExZ4wT;NWO%;|a_}MieDUZmgDI^eCt_XP(NOR0TO>XzHOq=*7ro z`om%T3BzjFD>K@3%-$apM5u^|#_ewt;I)>+eN5C{%&D88a~Vlck6mWqCpwN3EYZ~z zE5saBp^1ba=xOnY0~}ZDVBT_%V^fz;C|u6{X^7?T9%k>+?DP(t z5>i!qs*%DT&EEM%r1_wu^LsQq9N8YtE*70w|2<~>!Au&sPA8BaGyc$#wuHuVj~UN| zxp4JQDc?;fC6bz;SM3~E}433XRA?6AMACGjSKhdEsLM@}uhfHmit&O`ZAvj4O7P;91#7Z(9%Xb)<@Uc*yKrFyn z0bY*gS> zcL}$We*VaaZM{cK<@+Fa&if@WUsn@r#U~IPcd^{tz9a@4jsxg}#G?^2V9Ana9||TB zUtzMZ^&GlMG7tlF+`bLPd+d0?{PGE`3PyET%kR?G2V^urU*tfF{VA^>0G?fa4F&R7!q zCb-wVJbEqzDPI$$AH_Xq^aWvp;W3GhDYe_SO)Y3FpF3?@@4w#rf9L&FT9BYs*M_sQ zc7BUFk3DHOlGiTLh3;~2bf(RB8Hul}gOHZihru%_mr`~z7$|Y8Zy>8!M@4jqq7sR8 zk#zo;miJB$o=&rK{;1s%-)y6}!26|2iT_;s6CLIQV0vm5siT&pKXx+uyw>}r3OXjV z<_+lkFu6}_MJ=c$821`%bg{NhKhh7z4WyZqO8B=Dg7Sx1*>;^T_EK!FcsyKRk==nB zRJLPGS-&nd{J_?ITei;n#8WTIn97*!9*S&N@6MRE zdD*(aO_iP|)*Tz*F*ep1Sm|d{ zGy8?JCpdfY;w@JCRlB^ha{*@Z)zEw|<0%Cze_dBH7HURix%W#fpm!59_a`v5-bJuf zpk_>{KpL`x$j8JRf=rPo)GQ5Gf1Vh!EL8=@_~hPk6=6R|RDc{H`1s#3xm zp$V$mU1i*(^NY@^j=cyq2TprL3b<}|mE#93d9Kd7y{_^=V#w8^sRZD>2?6c4Q`EUDgy5bkbzN@FS zUom8B?oZ6{&GBol_-$xyYN(feyAEJ*fK_axIqwh9EhhPBGxbAb0^Mh3A0nB{zCzQ{ zv8BCBA@>+_%we}RluEgw)Y2%Jj`?tG)zjXcUk9(Z2eMfe8$w2VSar@m9=(p%9W4%T zZ(?>3TE@!IL*4X0@dus*)sDO3*tCx>S`g%}|34G3_LgpsTvW3?*}AIn;Qfso;u^BV zbs<;nuhse8DoCa*NKG4bVb!VOf{|wB`o(y2qqex z2<{8GpFUE{B*vOCER%20Oi6oJ8y~Mg0gQ^yz^QH&b&Jri^&XpTp-(1a;;LVobqU%? zr5-In9W1U@Hr&^-DeZf;pLC39?}}jY4M$og65~>SJ6@y{U6dg(d1r@|q!vDBZ114j_$SD6PGepQO%f_XJ_+F`C&9In9d|L~B>D>UPOy9~jQABBR%4{+ zieM>b%5h`{88*g}M&4mObU4{a0mI()V*RVvLBDGrX*AP`!$GlR zH2Qn3!kW&BOzf3{He)y%7kvZtxg@-3WKUR2zV)jylOURGhH~*J(vgei;bzh0t?C`o zbery@z#w259vR5ZjN&cjfw6^u!UJO?y6cU)y{r~;0n{}Jggz&eijXabIVLW)y3`W< z>;C{v+rjs=O_64$^1(<;(4f7VVox_4{nFz?0ZK$zr&C0*Uz{MwBb?NQ6F6bw${|Dk zwwyU}5=B7T8K*eXHhhfPPxI%`|MYhH=;7t$7*6IS(sC_HuA<_%8RGamJ#S>j8fK-b zVXX-gSOT8ki*(ZSJ+H;t3@zU@wE0s1{!2fv^<;k2e`cS5c&X%7`&qW)#N0BU!Urszzt$Ow7j9^*RY9Wg^7sW|b_2(HXvtI64voRsy0ajHjhw?Q%P(SN%Cnbq z-_YVRW_dmDxyRd(Vp>Ca>r}QJI-3Y{Vbgc zRHsXR6Z%%gec)UD-kgH7<>9L4S4It=N!8-nq0OEF%GW3Hmo!7uaMEeoOrR%}c`OF^ zZos5vPJ$`BXi&ImdfF&rIs8d-!h@o=gh%y1G4dcDHCf#Mci3)!5uvf<6WZ`3U) zc&*D{$m5dII&)HZFz*J9!KsVyy_DMdhgZuF-cDU%{{YP{DmIt(qxLDPSH^0_IQ-?q zy=6b7bUO53y#FeLWhM7eQ|!p)B74-d&d}%;D^^@})m3Y*v~M95a@^`jWB%wsJ@j>9 zyQa5UtupJ?8Q(j0ikhbA-OwQ$>P$*}#h~CFdhn?+QFY;$LK*xEn82L1dYHd(6bn;b z;_sI5FV))Ij|-SYTgA4!`=Rx(F*&(tqHG){^X{M^9BYggsu!1QyVwuLb$He)~x zrFb_e=dU3H&z)J&=;8TTGFKE#M%S3BeO6cQ3-ONHA_h~S(%zq>eXXa z%U)rw6Q@vkb||rwF^+?d=tLTWPSX}E3JX?gfsj(UvFwA5G}X)zqeb<`994tZp=dR+ z8#74*$bU}|I?*b15LD`1N>lh#*sQS?b=etvp3Rs+*)kF%+&E&?nzeN4r5O#oGM;q) z;Pe&2e66^|o{RzHSH40CMj*msoXK8t?#;%FeNj8I#h44a!Z{IqLt{Z^-;Tta;TO!x zE@@VMzwIyduoz<=YQ_N0=HbZt5*Z5;X)>NRlgfm$+nD2-vL180Iom!Y4|8wUP*SGm z_9ABjkTY0TbNk3<&T)a%K;JZCB#g^X8B*+@(8vWut|s61odLO1J( zc?;c+FXkI$N{d78c1NOiv#9Tv2e!=G7d=kc!w3Hb*wG~Pd>V2i36nRZT2;aOV z89J%G!)q6O_~Ouayx03%s~=d`4+$fEwqfj9E);mUoczaO_l2$1*~!jW#s4_*m(M_3 z_t{1i9fnRrD}5{EpR}%$^~o|9tnprd-BW*NAkS&!19vLhOC|+Xyx~75r+V$p3-%8P z5B~9EAM7M-#`ZrvppGdB3lOR9Y5pb=_xZ;jHidowQ*Uvq;jS^ zyu)4VuZhMv{1r8#rMu@tICfAihj5zeM)6I=aIVTRFJbe|9Wo?*7xVX z&ajq7Ls72DmFZJXgRk3f4NaF{$hFm3^M1;kT0B1#4IS}y48QsqqYZ-I=y*A^?ZDU- zoZ%osPfY=^)6x9g{VAti!F;c7Ob%~LeqL5Ni>s8*N!5+u8KNQ$9N5UWYVF`4-sSX9 z1kQTUOF8DS7f^?6uk6Y+cU$jpch|_77Kh#|ZYpVcD6RD&nPqR>C)mpTTryWj=Q?+& zH==5u9e{8Pp8AHu*(a{HR7_{ z1C?Rg$@ifRR!Dh`bIJC4+lsu~q>#j#rOJK^iBS;yXlzmOxz<&I4~OQxfAfO3yc=Z5 z?X#k+a{oe|R7m3DXg2TA#LrJorj~v>%p(BeF>7 z8k(UI7R7+z!f2zeU=^Bovl_@jw(L_FyY4vW?6aFvtmYIUe((1+~-u5MQah-9x)s4|Wp*?W z!4sF3VP(s2ZQ!skvb4jA&Ss%_=fYvzPYPwqlb5F#3?8&wJvYLM+=|w%1!<1teco5c zTP7b+^WzfVP)5sgl}YUh*Z5-2*7qF28=Y?wG!n;Auy)1i0+sK|BdZgyX!*-sR%=yS8AC^feaQEQM7u5?A0V8ptv_2ce zW^3$QKW>i9^X>W#y2bJSY{IwCm_KirYAwK>m>g*j6dh!(hXf8R%%|53%*~o5=k2A! z5DOE~CF{M!@9M1dP~wFqba0ej?WC=8wPBBH>pjLEHEfI>5U-r*mod(;F`6Of36rNS zHf)TxR~y?eql=wVY1kN(t8%8K&>SeLSCNVE;KKSvg)@nwj?^n0mC}F^6 zj3a-N(M1T0yCqE>D`<)=%#S%C&vvLmP6Ho{_IVT_L{O{P4iBOsV?gY%VMZIIf-X1y z7f-!iRF4LDpgMBB^|+epnLBoh+A$$U8SxVU^j7xD>bowDeBN}652Wzr0N9_)z0H=ChD5hL~TGZ{4 zVKcP=ljKxXQl^j@5(xoIm%TzGpF&;S9weg!_Q)7VcaRdiq#1_h~cM;)3YC{kSG;Nsf1GiBk+qJ zLR>{zNGKv@y&f-}Nv+{{2|F>`7fOVj=KYLY2-_tzI(BSoI_s9-{mr1aifXh-TMi*M zNc^^@g}M3|mr^;viIf4s`WQc>VFbsk>>}eB_iC;hU>qYAvG`;%S9vCrv!E%04#vl6 z&nfE(Q6tAfz*seTx2R^7P`qQ!FDF^i_Z``+Mj;zA-KJZp0$F&;zDO7d1ojxWhKzuMrBZswFoWPp_ubEd z&Nvwms-Ht}an>NUw0Bil7$IP*i2Dt1BEessl*0;E`##40oO~n|LYAI_llE@hl-64* zU{DX;)a7UJml=*iv2xkDEOX{dBgAD-)Xe=DX_z`5swccu7aEM?dKgt- zk~f*0oKmVqddkk&*|%9*)1|$O)}lOdBCGy-;=h1kGoHc}VoE5wE#68kAI~KR0wtye zWi3%0n0qZn1Kl^WS*g~aDNd4T)Yh1%yxqkX$2p}jNI=eFgr$?WQg$^ZLhP{XLwQF#?L`Fq+2L6XL>9|2=4?Ff-tJL z3cUd+Y=JsRw!ukLnVrU%DAX!rT8*4LO1=ZvaY8cuVm_0uJ1px^!4`?d5a%Tmi9T~$ z-Ey1BNYIS7GnbsQ3`N|otE7Bac{onT$^x(r>_46$bmmm z45(jcUM@mi47QR&tVLWAWe&WfhSpof8sw;3N=oaGBnt)(y7@bQk3r&x_`UkO3(}S= z1v79R_9>)gvVqzeE{=8%C+Ll$GNz6NT+(Zsh;uHI*OACm2`tlxggez|DiRj>5Nt=} zW%v-j3>M49sD}{Guumc9c6Ur_^jmso5>#vy{lh>xj8@`5>^sFvRin?;*Z7;2A_lm= ziSf4<{|SO9>zf$=)tS?)=TVW(?b*O6m8Ex27Hy+)mQ=2K@6OXMamoMPIP#|JUj6l5 GkNtmEumC>* literal 0 HcmV?d00001 diff --git a/crates/epiphany-editor-gui/src/goldens.rs b/crates/epiphany-editor-gui/src/goldens.rs index 5dd421f..480b864 100644 --- a/crates/epiphany-editor-gui/src/goldens.rs +++ b/crates/epiphany-editor-gui/src/goldens.rs @@ -602,4 +602,100 @@ mod tests { assert_golden("ten_measure_slurs_castoff", &pixmap1); } + + /// **G5 — the note-entry caret loop** (`spec/CONTRACT_EDITOR_T3_CARET.md` + /// §W2). `ten_measure_single_staff(0)`, the same fixture and the same + /// [`scripted_insert_target`] click point G2/G3 use — "past the last + /// note" is exactly the extrapolation territory the contract asks G5 to + /// reuse — but driven through [`EditorSession::set_caret_at`] + + /// [`EditorSession::enter_nominal`] instead of the pencil's + /// `insert_note_at`. Locks the entry loop's visible result the same way + /// G2 locks the pencil's. + /// + /// **Value assertions before pixels** (contract's own ordering): the + /// grid/caret's exact position and entry duration are asserted as exact + /// rationals, then each of the four scripted entries' resulting caret + /// position, *before* any raster is taken — so a broken entry loop fails + /// as a named rational mismatch, not a mystery pixel diff. + #[test] + fn g5_ten_measure_caret_entry_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); + + // Same target G2 proves lands exactly at whole-note 10 with a quarter-note + // grid (the fixture's 4/4 meter) — re-derived here independently, not just + // assumed from that other test. + 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 caret = session + .set_caret_at(target) + .expect("the target sits over the staff"); + assert_eq!( + caret.entry_duration, grid.step, + "the caret's entry duration is initialized from the grid step" + ); + assert_eq!( + caret.position, + MusicalPosition(RationalTime::new(10, 1).expect("10/1 is valid")), + "the caret lands exactly at whole-note 10 — immediately after the \ + fixture's last note ends, with nothing to overwrite" + ); + + // C, D, E, F at the caret's quarter entry duration: each entry must both + // change the graph and advance the caret by exactly 1/4. + let expected_positions = [ + (10, 1, 41, 4), // 10 -> 10 + 1/4 = 41/4 + (41, 4, 21, 2), // 41/4 -> 42/4 = 21/2 + (21, 2, 43, 4), // 21/2 -> 43/4 + (43, 4, 11, 1), // 43/4 -> 44/4 = 11 + ]; + for (nominal, (before_n, before_d, after_n, after_d)) in + [CmnNominal::C, CmnNominal::D, CmnNominal::E, CmnNominal::F] + .into_iter() + .zip(expected_positions) + { + let before = MusicalPosition(RationalTime::new(before_n, before_d).expect("valid")); + assert_eq!( + session.caret().map(|c| c.position), + Some(before), + "{nominal:?}: the caret sits at the expected pre-entry position" + ); + let outcome = session + .enter_nominal(nominal) + .unwrap_or_else(|err| panic!("entering {nominal:?} at the caret: {err}")); + assert!( + outcome.graph_changed, + "{nominal:?}: entering into empty space changes the score" + ); + let after = MusicalPosition(RationalTime::new(after_n, after_d).expect("valid")); + assert_eq!( + session.caret().map(|c| c.position), + Some(after), + "{nominal:?}: the caret advances by exactly the entry duration (1/4)" + ); + } + + let (svg1, pixmap1) = render_pixmap(&session, 12.0); + let (svg2, pixmap2) = render_pixmap(&session, 12.0); + assert_eq!(svg1, svg2, "G5 determinism double: SVG bytes must match"); + assert_eq!( + pixmap1.data(), + pixmap2.data(), + "G5 determinism double: rasterized pixels must match" + ); + + assert_golden("ten_measure_caret_entry", &pixmap1); + } } diff --git a/crates/epiphany-editor-gui/src/main.rs b/crates/epiphany-editor-gui/src/main.rs index 0efb6e1..a8dff86 100644 --- a/crates/epiphany-editor-gui/src/main.rs +++ b/crates/epiphany-editor-gui/src/main.rs @@ -24,8 +24,8 @@ use eframe::egui; -use epiphany_core::NoteValue; -use epiphany_editor_core::{EditOutcome, EditorError, EditorSession, GridResolution}; +use epiphany_core::{CmnNominal, NoteValue, TypedObjectId}; +use epiphany_editor_core::{Caret, EditOutcome, EditorError, EditorSession, GridResolution}; use epiphany_engrave::Engraver; use epiphany_layout_ir::{BoundingBox, HitShape, LayoutObjectId, Point}; use epiphany_ops::{OperationKind, OperationPayload}; @@ -139,6 +139,50 @@ fn selection_rect( .map(|region| shape_rect(®ion.shape, vm)) } +/// The caret's on-screen vertical segment `(top, bottom)`, full staff height — +/// `None` if there is no caret, or its voice's staff geometry cannot be +/// resolved (its region/staff-instance vanished — should not happen once +/// `EditorSession` itself clears a caret whose voice vanished, but this stays +/// defensive; or `x_at_position` has too few rendered anchors to fix a scale). +/// +/// Derived entirely from `EditorSession`'s existing **public** surface +/// (`score()`, `resolved()`, `x_at_position()`) — no new core accessor was +/// needed, per the packet's own preference. The voice→(region, staff instance) +/// lookup mirrors `Score::voices()`'s own shape (used the same way inside +/// `epiphany-editor-core`); the staff's y-extent is read from the same +/// `resolved().strokes` staff-line records `EditorSession::staff_pitch_at`'s +/// own tests already inspect this way (`epiphany-editor-core/src/lib.rs`'s +/// `staff_pitch_at_reads_the_clicked_height`) — filtered to *this* caret's +/// staff specifically (by `StaffId`), not just any `TypedObjectId::Staff(_)`, +/// so a multi-staff score would still draw the caret on its own staff. +fn caret_segment(session: &EditorSession, vm: &ViewMap) -> Option<(egui::Pos2, egui::Pos2)> { + let caret = session.caret()?; + let (region, staff_instance) = session + .score() + .voices() + .find(|(_, _, v)| v.id == caret.voice) + .map(|(region, staff_instance, _)| (region, staff_instance))?; + let staff_id = session + .score() + .staff_instances() + .find(|(_, si)| si.id == staff_instance) + .map(|(_, si)| si.staff)?; + let x = session.x_at_position(region, None, &caret.position)?; + + let mut min_y = f32::INFINITY; + let mut max_y = f32::NEG_INFINITY; + for stroke in &session.resolved().strokes { + if matches!(stroke.provenance.source, TypedObjectId::Staff(id) if id == staff_id) { + min_y = min_y.min(stroke.from.y.0).min(stroke.to.y.0); + max_y = max_y.max(stroke.from.y.0).max(stroke.to.y.0); + } + } + if !min_y.is_finite() || !max_y.is_finite() { + return None; + } + Some((vm.world_to_screen(x, max_y), vm.world_to_screen(x, min_y))) +} + /// Accumulates a rubber-band drag's two screen-space corners (the point where /// the drag started and where the pointer currently is), pure and independent /// of any `egui::Response`/`Context` so it can be unit-tested headlessly. The @@ -223,6 +267,47 @@ fn resolve_release(drag: &DragRect, ctrl: bool) -> ReleaseAction { } } +/// What a letter key (A–G) means, given whether note-entry mode is on — the +/// mode-gated interpretation `spec/CONTRACT_EDITOR_T3_CARET.md` §W2 asks for, +/// pulled out pure (mirrors [`resolve_release`]) so the gate itself is +/// headlessly unit-testable, independent of `egui::Context`/`InputState`. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum LetterAction { + /// Note-entry mode is on: enter this natural at the caret + /// ([`EditorSession::enter_nominal`]). + Enter(CmnNominal), + /// Note-entry mode is off, and the letter is `A`: the existing "add a + /// chord note to the anchor" intent ([`EditorSession::add_note_to_selection`]), + /// unchanged from before this packet. + AddChordNote, + /// No binding: entry mode is off and the letter isn't `A` (B–G have no + /// meaning outside entry mode). + None, +} + +fn resolve_letter(entry_mode: bool, nominal: CmnNominal) -> LetterAction { + if entry_mode { + LetterAction::Enter(nominal) + } else if nominal == CmnNominal::A { + LetterAction::AddChordNote + } else { + LetterAction::None + } +} + +/// Toggles `this` (pencil or note-entry), forcing `other` off exactly when +/// `this` turns **on** — the mutual-exclusion invariant §W2 pins ("the two +/// modes are mutually exclusive"), pulled out pure so it is unit-tested +/// directly rather than eyeballed at each of the toggle's four call sites (two +/// keys, two toolbar buttons). Turning `this` off leaves `other` exactly as it +/// was, whatever that was — this function does not itself assume the +/// invariant holds going in, only that it holds coming out. +fn toggle_exclusive(this: bool, other: bool) -> (bool, bool) { + let new_this = !this; + let new_other = if new_this { false } else { other }; + (new_this, new_other) +} + /// The text of the most recent `egui::Event::Paste` in `events`, if any (T2 /// W4b) — pure and independent of `egui::Context`/`InputState`, so it is /// headlessly unit-testable; `EditorApp::handle_clipboard_events` is the only @@ -313,8 +398,14 @@ struct EditorApp { px_per_staff_space: f32, needs_render: bool, /// Pencil ("insert") mode: a click on the staff inserts a note at that pitch and - /// beat (with make-room overwrite) instead of selecting. + /// beat (with make-room overwrite) instead of selecting. Mutually exclusive with + /// `entry_mode` — see `toggle_exclusive` and `DECISIONS.md`. pencil: bool, + /// Note-entry mode (T3-W2): a click places the caret instead of selecting; A–G + /// enter a natural at the caret; the duration palette sets the caret's entry + /// duration instead of resizing a selection; ←/→ retreat/advance the caret; R + /// enters a rest. Mutually exclusive with `pencil`. + entry_mode: bool, /// The in-progress rubber-band drag, if any (`None` outside a drag, and always /// `None` in pencil mode — pencil's click-to-insert takes precedence and ignores /// dragging entirely). @@ -343,6 +434,7 @@ impl EditorApp { px_per_staff_space: 12.0, needs_render: true, pencil: false, + entry_mode: false, rubber_band: None, last_paste_text: None, status: "opened ten_measure_single_staff".to_string(), @@ -454,6 +546,51 @@ impl EditorApp { } } + /// A click in note-entry mode: places the caret at `world` + /// ([`EditorSession::set_caret_at`]) instead of selecting — pencil and + /// note-entry are mutually exclusive, so this is only ever reached with + /// pencil off. No score/layout mutation happens here, so no + /// `needs_render`: the caret overlay reads live session state fresh every + /// frame regardless of whether the score texture is stale. + fn do_set_caret_at(&mut self, world: Point) { + self.status = match self.session.set_caret_at(world) { + Ok(caret) => format!( + "caret set: voice {:?}, beat {:.3}, entry duration {:.3}", + caret.voice, + caret.position.0.to_f64(), + caret.entry_duration.0.to_f64() + ), + Err(err) => format!("caret: {err}"), + }; + } + + /// Runs a caret-only step ([`EditorSession::advance`]/[`EditorSession::retreat`]), + /// recording the outcome in the status line. Neither moves the score graph or + /// layout, so — like `do_set_caret_at` — this never sets `needs_render`; it is a + /// bespoke small handler (not `Self::run`) because these two return `Result`, not `Result`. + fn do_caret_step( + &mut self, + name: &str, + step: impl FnOnce(&mut EditorSession) -> Result, + ) { + self.status = match step(&mut self.session) { + Ok(caret) => format!("{name}: caret now at beat {:.3}", caret.position.0.to_f64()), + Err(err) => format!("{name}: {err}"), + }; + } + + /// The duration palette's action while note-entry mode is on: sets the caret's + /// entry duration ([`EditorSession::set_entry_duration`]) instead of resizing a + /// selection (`Self::toolbar`'s non-entry-mode branch keeps that meaning + /// unchanged). No score/layout mutation, so no `needs_render`. + fn set_entry_duration(&mut self, label: &str, value: NoteValue) { + self.status = match self.session.set_entry_duration(value.whole_note_fraction()) { + Ok(()) => format!("entry duration set to {label}"), + Err(err) => format!("entry duration {label}: {err}"), + }; + } + /// Consumes this frame's `egui::Event::Paste`, if any (the only mechanism /// egui 0.29 offers for reading OS paste content — see `DECISIONS.md`): /// caches it for the toolbar "Paste" button and immediately pastes it @@ -540,8 +677,10 @@ impl EditorApp { self.do_paste_from_cache(); } ui.separator(); - // Duration palette: set the selected note/rest's written value (make-room - // overwrite when lengthening). + // Duration palette: while note-entry mode is on, sets the caret's entry + // duration (`Self::set_entry_duration`); otherwise, its long-standing + // meaning — set the selected note/rest's written value (make-room overwrite + // when lengthening) — is unchanged. ui.label("Dur:"); for (label, value) in [ ("1", NoteValue::Whole), @@ -551,13 +690,33 @@ impl EditorApp { ("1/16", NoteValue::Sixteenth), ] { if ui.button(label).clicked() { - self.run(&format!("duration {label}"), |s| { - s.set_selection_duration(value.whole_note_fraction()) - }); + if self.entry_mode { + self.set_entry_duration(label, value); + } else { + self.run(&format!("duration {label}"), |s| { + s.set_selection_duration(value.whole_note_fraction()) + }); + } } } ui.separator(); - ui.toggle_value(&mut self.pencil, "✏ Pencil (insert)"); + // Pencil and note-entry are mutually exclusive (`toggle_exclusive`, + // `DECISIONS.md`): turning either on from the toolbar clears the other, + // exactly as the matching key (P / N) does. + if ui + .toggle_value(&mut self.pencil, "✏ Pencil (insert)") + .changed() + && self.pencil + { + self.entry_mode = false; + } + if ui + .toggle_value(&mut self.entry_mode, "🎹 Note entry") + .changed() + && self.entry_mode + { + self.pencil = false; + } ui.separator(); if ui .add(egui::Slider::new(&mut self.px_per_staff_space, 6.0..=28.0).text("zoom")) @@ -600,9 +759,26 @@ impl EditorApp { } } ui.separator(); + match self.session.caret() { + Some(caret) => { + ui.label(format!("caret voice: {:?}", caret.voice)); + ui.label(format!( + "caret position: beat {:.3}", + caret.position.0.to_f64() + )); + ui.label(format!( + "caret entry duration: {:.3}", + caret.entry_duration.0.to_f64() + )); + } + None => { + ui.label("caret: none"); + } + } + ui.separator(); ui.label(format!( "Click a notehead to select (replaces). Ctrl/Cmd-click toggles a member. Drag a \ - rubber-band to select every hit within it. Pencil mode: {}.\n\ + rubber-band to select every hit within it. Pencil mode: {}. Note-entry mode: {}.\n\ Delete and Transpose act on the whole selection; Move / Add chord / Insert after / \ duration act on the anchor (the drag/toggle reference point, drawn with the \ thicker accent stroke).\n\ @@ -611,14 +787,40 @@ impl EditorApp { reports that instead of pasting). The Paste button acts on the last clipboard text \ this app has seen (egui reads it only on an actual paste gesture); Ctrl/Cmd+V pastes \ immediately using that same text as it arrives.\n\ - Keys: Del delete · ↑/↓ staff-step move · +/− transpose · A add chord · I insert after · \ - P pencil · Ctrl/Cmd+C copy · Ctrl/Cmd+V paste · Ctrl/Cmd+Z undo · Ctrl/Cmd+Shift+Z or \ - Ctrl/Cmd+Y redo", - if self.pencil { "ON — click to insert" } else { "off" } + Note-entry mode (N; mutually exclusive with Pencil — turning one on turns the \ + other off): click places the caret (drawn as a green vertical line spanning its \ + staff), instead of selecting. A–G enter that natural at the caret, octave inferred \ + from the nearest earlier note in its voice. R enters a rest. ←/→ retreat/advance \ + the caret by its entry duration (no insertion). The duration palette sets the \ + caret's entry duration instead of resizing a selection while this mode is on.\n\ + Keys: Del delete · ↑/↓ staff-step move (always) · +/− transpose · A add chord \ + (off) / enter A (entry mode) · B–G enter note (entry mode only) · I insert after · \ + R enter rest (entry mode) · ←/→ retreat/advance caret (entry mode) · P pencil · \ + N note-entry · Ctrl/Cmd+C copy · Ctrl/Cmd+V paste · Ctrl/Cmd+Z undo · \ + Ctrl/Cmd+Shift+Z or Ctrl/Cmd+Y redo", + if self.pencil { + "ON — click to insert" + } else { + "off" + }, + if self.entry_mode { + "ON — click to place the caret" + } else { + "off" + } )); } fn handle_keys(&mut self, ctx: &egui::Context) { + const LETTER_KEYS: [(egui::Key, CmnNominal); 7] = [ + (egui::Key::A, CmnNominal::A), + (egui::Key::B, CmnNominal::B), + (egui::Key::C, CmnNominal::C), + (egui::Key::D, CmnNominal::D), + (egui::Key::E, CmnNominal::E), + (egui::Key::F, CmnNominal::F), + (egui::Key::G, CmnNominal::G), + ]; let k = ctx.input(|i| Keys { // Ctrl/Cmd-Z undo, Ctrl/Cmd-Shift-Z (or Ctrl/Cmd-Y) redo, read first so a // modified Z is not also taken as a plain key. @@ -627,13 +829,26 @@ impl EditorApp { && ((i.modifiers.shift && i.key_pressed(egui::Key::Z)) || i.key_pressed(egui::Key::Y)), delete: i.key_pressed(egui::Key::Delete) || i.key_pressed(egui::Key::Backspace), + // ↑/↓ keep their one meaning (staff-step move) in every mode; only ←/→ + // are new (T3-W2), and only mean anything in note-entry mode. move_up: i.key_pressed(egui::Key::ArrowUp), move_down: i.key_pressed(egui::Key::ArrowDown), + retreat: i.key_pressed(egui::Key::ArrowLeft), + advance: i.key_pressed(egui::Key::ArrowRight), sharp: i.key_pressed(egui::Key::Plus) || i.key_pressed(egui::Key::Equals), flat: i.key_pressed(egui::Key::Minus), - add: i.key_pressed(egui::Key::A), + // Whichever of A–G was pressed this frame, if any — `resolve_letter` + // decides what it means (mode-gated); at most one fires per frame in + // practice (one key event), but the search takes the first match either + // way, deterministic regardless. + letter: LETTER_KEYS + .into_iter() + .find(|(key, _)| i.key_pressed(*key)) + .map(|(_, nominal)| nominal), + rest: i.key_pressed(egui::Key::R), insert: i.key_pressed(egui::Key::I), pencil: i.key_pressed(egui::Key::P), + entry_toggle: i.key_pressed(egui::Key::N), // Copy is a plain boolean edge, same as every other shortcut here — // `do_copy` needs nothing egui delivered *to* it, only the current // selection, so it fits this file's usual `key_pressed` pattern. @@ -649,9 +864,16 @@ impl EditorApp { self.run_history("redo", |s| s.redo()); } if k.pencil { - self.pencil = !self.pencil; + (self.pencil, self.entry_mode) = toggle_exclusive(self.pencil, self.entry_mode); self.status = format!("pencil mode {}", if self.pencil { "on" } else { "off" }); } + if k.entry_toggle { + (self.entry_mode, self.pencil) = toggle_exclusive(self.entry_mode, self.pencil); + self.status = format!( + "note-entry mode {}", + if self.entry_mode { "on" } else { "off" } + ); + } if k.delete { self.run("delete", |s| s.delete_selection()); } @@ -661,14 +883,31 @@ impl EditorApp { if k.move_down { self.run("move down", |s| s.move_selection_staff_step(-1)); } + if k.retreat && self.entry_mode { + self.do_caret_step("retreat", |s| s.retreat()); + } + if k.advance && self.entry_mode { + self.do_caret_step("advance", |s| s.advance()); + } if k.sharp { self.run("transpose +1", |s| s.alter_selection(1)); } if k.flat { self.run("transpose -1", |s| s.alter_selection(-1)); } - if k.add { - self.run("add chord note", |s| s.add_note_to_selection()); + if let Some(nominal) = k.letter { + match resolve_letter(self.entry_mode, nominal) { + LetterAction::Enter(n) => { + self.run(&format!("enter {n:?}"), |s| s.enter_nominal(n)); + } + LetterAction::AddChordNote => { + self.run("add chord note", |s| s.add_note_to_selection()); + } + LetterAction::None => {} + } + } + if k.rest && self.entry_mode { + self.run("enter rest", |s| s.enter_rest()); } if k.insert { self.run("insert after", |s| s.insert_note_after_selection()); @@ -730,10 +969,11 @@ impl EditorApp { let vm = ViewMap::new(self.view_box, rect); - // Rubber-band drag tracking. Pencil mode ignores dragging entirely — it takes - // precedence, and its click-to-insert behavior below is unchanged — so no drag - // is ever started while it is on. - if !self.pencil { + // Rubber-band drag tracking. Pencil and note-entry mode both ignore dragging + // entirely — each takes precedence over selection while it is on (their own + // click-to-insert / click-to-place-caret behavior below is unchanged) — so no + // drag is ever started while either is on. + if !self.pencil && !self.entry_mode { if response.drag_started() { if let Some(pos) = response.interact_pointer_pos() { self.rubber_band = Some(DragRect::new(pos)); @@ -813,6 +1053,11 @@ impl EditorApp { self.run("insert note", |s| s.insert_note_at(world, &grid)); ui.ctx().request_repaint(); return; + } else if self.entry_mode { + // Places the caret; no score/layout mutation, so — unlike pencil's + // branch above — the hit-test map and texture stay valid and the + // overlay code below can run this same frame with no early return. + self.do_set_caret_at(world); } else { let ctrl = ui.input(|i| i.modifiers.command); self.resolve_click(world, &grid, ctrl); @@ -843,6 +1088,16 @@ impl EditorApp { ); } } + + // The note-entry caret: a vertical line spanning its staff's full height, at + // its exact musical position (`x_at_position` through `vm`) — distinct in + // color from both the selection (blue) and anchor (orange) strokes. + if let Some((top, bottom)) = caret_segment(&self.session, &vm) { + ui.painter().line_segment( + [top, bottom], + egui::Stroke::new(2.0_f32, egui::Color32::from_rgb(0, 170, 90)), + ); + } } } @@ -853,11 +1108,20 @@ struct Keys { delete: bool, move_up: bool, move_down: bool, + /// ←, note-entry mode only ([`EditorSession::retreat`]). + retreat: bool, + /// →, note-entry mode only ([`EditorSession::advance`]). + advance: bool, sharp: bool, flat: bool, - add: bool, + /// Whichever of A–G was pressed this frame, if any — see [`resolve_letter`]. + letter: Option, + /// R, note-entry mode only ([`EditorSession::enter_rest`]). + rest: bool, insert: bool, pencil: bool, + /// N, toggles note-entry mode. + entry_toggle: bool, copy: bool, } @@ -1031,6 +1295,60 @@ mod tests { assert_eq!(resolve_release(&drag, true), ReleaseAction::RubberBand); } + // T3 W2: `resolve_letter` (the mode-gated letter interpretation) and + // `toggle_exclusive` (the pencil/note-entry mutual-exclusion invariant) — + // both pure, headlessly unit-tested exactly like `resolve_release` above. + + #[test] + fn resolve_letter_enters_a_natural_in_entry_mode() { + for nominal in [ + CmnNominal::A, + CmnNominal::B, + CmnNominal::C, + CmnNominal::D, + CmnNominal::E, + CmnNominal::F, + CmnNominal::G, + ] { + assert_eq!(resolve_letter(true, nominal), LetterAction::Enter(nominal)); + } + } + + #[test] + fn resolve_letter_outside_entry_mode_only_a_adds_a_chord_note() { + assert_eq!( + resolve_letter(false, CmnNominal::A), + LetterAction::AddChordNote + ); + for nominal in [ + CmnNominal::B, + CmnNominal::C, + CmnNominal::D, + CmnNominal::E, + CmnNominal::F, + CmnNominal::G, + ] { + assert_eq!( + resolve_letter(false, nominal), + LetterAction::None, + "{nominal:?} has no binding outside entry mode" + ); + } + } + + #[test] + fn toggle_exclusive_turning_on_clears_the_other() { + assert_eq!(toggle_exclusive(false, true), (true, false)); + } + + #[test] + fn toggle_exclusive_turning_off_leaves_the_other_alone() { + assert_eq!(toggle_exclusive(true, false), (false, false)); + // Defensive: even from a (never-reachable-in-practice) state where both + // were somehow on, turning `this` off must not itself clear `other`. + assert_eq!(toggle_exclusive(true, true), (false, true)); + } + // T2 W4b: `paste_event_text`, the pure extraction helper over a frame's // `egui::Event`s that `handle_clipboard_events` calls. This is the one // piece of clipboard-wiring logic with no `egui::Context`/native-backend