Add Staccato, Accent, and Tenuto ornaments - #35
Merged
Merged
Conversation
Wires up three more of the dead OrnamentType articulation values (Glissando remains, left for its own step since pitch-bend is genuinely harder). Each gets the full ribbon/Edit-menu/context-menu pattern plus a placeholder glyph (stac/acc/ten, matching this codebase's existing text-abbreviation convention rather than real notation symbols). Playback effects live in OrnamentPlaybackService.ApplyArticulation, a single post-process pass over whatever events a note already expanded into (a plain note, or every segment of a trill/turn/ mordent/grace note) rather than a new branch in the ornament expansion chain: staccato halves the sounding duration (the next note's start time comes from the nominal duration, so this just leaves a gap, not a timing shift), accent and tenuto boost velocity (accent more than tenuto), both clamped to the MIDI max. For layout, all three are registered into NoteTopAnnotationPlanner's existing HasCenterOrnament stacking switch (same as Trill/Turn/ Mordent) so they correctly clear any accidental/octave-dot glyph on the same note, instead of any new anchor-position math -- carries none of the layout-band risk flagged elsewhere in ROADMAP.md for the accidental slash-convention work. Verified via the same sandbox pipeline as every prior change this session: dotnet build succeeds against the real project/package graph, dotnet format --verify-no-changes passes on both projects, and new xUnit tests cover the glyph mapping and each playback effect (including composed with a trill and clamped at the MIDI velocity max). dotnet test itself cannot execute in this sandbox; real confirmation comes from GitHub Actions CI. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pguj4XSScE141p1ScWoqEr
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Continuing the Indonesian notasi angka gap list (
ROADMAP.md), the rest of phase 4 (the deadOrnamentTypearticulation values), minus Glissando.context-menu pattern (matching Grace/Trill/Turn/Mordent/Fermata/BreathMark), with a new
placeholder glyph each (
stac/acc/ten, following this codebase's existing text-abbreviationconvention rather than real notation symbols -- see the class doc comment on
OrnamentService.GetPlaceholderGlyph).OrnamentPlaybackService.ApplyArticulation, a singlepost-process pass applied uniformly over whatever events a note already expanded into (a plain
note, or every segment of a trill/turn/mordent/grace note) rather than a new branch in the
ornament-expansion if/else-if chain: staccato halves the sounding duration (the next note's
start time comes from the nominal duration elsewhere in the schedule, so this only leaves a
gap, never shifts timing), accent and tenuto boost velocity (accent more than tenuto), both
clamped to the MIDI max (127).
NoteTopAnnotationPlanner's existingHasCenterOrnamentstacking switch (same mechanism Trill/Turn/Mordent already use) so theycorrectly clear any accidental/octave-dot glyph on the same note, instead of any new
anchor-position math -- carries none of the layout-band risk flagged elsewhere in
ROADMAP.mdfor the accidental slash-convention work.
for its own separate step, per the roadmap's existing note.
Test plan
Verified via the same sandbox pipeline as every prior change this session:
dotnet buildsucceeds against the real project/package graph (0 errors/warnings).dotnet format --verify-no-changespasses on both projects.halving without moving the start time, accent's and tenuto's velocity boosts, tenuto's boost
being smaller than accent's), staccato composed with a trill (shortens every segment), and
velocity clamping at the MIDI maximum when accent and tenuto combine.
Not verified in this sandbox: the new glyphs' actual on-screen appearance hasn't been visually
confirmed on a real Windows machine, same caveat as every prior UI change this session.
dotnet testitself cannot execute here (no WindowsDesktop runtime pack); real confirmation of the newtests comes from GitHub Actions CI.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Pguj4XSScE141p1ScWoqEr
Generated by Claude Code