Add drag-handle overlay to edit per-note playback start/duration offsets - #34569
Draft
tharos-devs wants to merge 26 commits into
Draft
Add drag-handle overlay to edit per-note playback start/duration offsets#34569tharos-devs wants to merge 26 commits into
tharos-devs wants to merge 26 commits into
Conversation
Adds two new per-note properties (playbackStartOffset, playbackDurationOffset) that let users nudge a note's audio playback timing independently of its notated position and duration. Exposed in the Properties panel's Play section and on the plugins API. Resolves musescore#34545
- Note::Note(const Note&, bool) did not copy m_playbackStartOffset / m_playbackDurationOffset, so cloning a note (copy-paste, duplication, linked parts) silently reset both offsets to 0. - effectivePlaybackStartTime()/effectivePlaybackDuration() had no lower bound. Since the two offsets are set independently (Properties panel spinboxes each range -1920..1920 with no cross-validation), an inconsistent combination could produce a negative effective start tick or a non-positive effective duration, both unguarded downstream in NoteRenderer. The duration formula intentionally keeps "- playbackStartOffset()" so that the effective end time (chordTick + chordTicks + durationOffset) does not depend on the start offset - this keeps start/duration independently adjustable, which upcoming UI work relies on. Clamping is applied at the computation itself rather than changing the formula, so every caller (Properties panel, plugins, future UI) is protected centrally.
tharos-devs
force-pushed
the
feature/note-offset-drag-handles
branch
from
August 13, 2026 12:05
1b030ad to
e77b97b
Compare
Adds an on-canvas alternative to the Properties panel spinboxes for editing
the playbackStartOffset/playbackDurationOffset introduced in
"Add per-note playback start/duration offset properties": a toggleable
overlay ("Note offsets" toolbar action, alongside Automation) draws a small
rounded rectangle above each note, anchored on the note's own position.
Dragging the rectangle's left/right edge adjusts the start/duration offset
respectively, with a live preview during the drag and a single undo/redo
step on release. Dragging a note that is part of a multi-note selection
applies the same tick delta to every selected note. The notation dims while
the mode is active, matching the existing Automation overlay's behavior.
New files:
- INotationNoteOffsets / NotationNoteOffsets: edit-mode toggle state,
mirroring INotationAutomation.
- NotationNoteOffsetController: builds/positions one overlay per staff from
the actual note layout (not a fixed lane), converts drag input to ticks
via segment interpolation, and commits offset changes through the
existing undoChangeProperty/Pid machinery (no new UndoableCommand needed).
- NoteOffsetOverlay: QQuickPaintedItem drawing the rectangles/handles and
doing its own hit-testing, independent of the Grip/NotationInteraction
edit path - the same pattern PolylinePlot uses for automation.
- segmentcanvasinterpolation.{h,cpp}: canvasX<->tick interpolation shared
between the automation and note-offset controllers (previously
duplicated).
Wiring follows the existing "toggle-automation" action end to end
(notationuiactions.cpp, notationactioncontroller.cpp,
notationcommandsregister.cpp, notationcommandsstate.cpp,
notationtoolbarmodel.cpp) and reuses AbstractNotationPaintView's existing
per-view-matrix redraw/dim hooks.
tharos-devs
force-pushed
the
feature/note-offset-drag-handles
branch
from
August 13, 2026 14:16
e77b97b to
8990603
Compare
Live-dragging a style value (e.g. "Staff space (sp)" in Page Settings) relayouts the score without going through changesChannel(), so the overlay's cached note positions went stale and stopped tracking the rescaled notation in real time.
timestampAndDurationFromStartAndDurationTicks() was called with a hardcoded tick-position offset of 0 instead of ctx.positionTickOffset, so every note (not just ones with a non-zero playback offset) played at first-playthrough timing on repeat/volta/D.C. passes. effectivePlaybackDuration() also independently recomputed from the raw, unclamped playbackStartOffset() instead of the same (possibly clamped) start effectivePlaybackStartTime() returns, so the two could disagree once the start clamp kicked in, making the note play longer than its clamped start implied.
- Compare SysStaffKey's System pointer by address only, never dereferencing it - a stale key still held from a previous rebuild had a dangling System* once Page/Continuous view mode switching destroyed and recreated every System, crashing on MeasureBase::index(). - Skip tied-continuation notes when building overlay handles: their own playback offset is ignored by NoteRenderer::shouldRender() in most cases, so a handle there could never actually do anything. - Disconnect from a document's viewModeChanged/styleChanged the moment a different document becomes current, instead of leaking one subscription per document ever opened in the session. - Don't delete an overlay that's mid-drag (holding the mouse grab) when a rebuild is triggered; defer until the drag finishes. - Handle mouseUngrabEvent so a stolen mouse grab mid-drag doesn't leave the overlay stuck thinking a drag is still in progress. - Consolidate the three parallel per-staff maps (overlay/notes/band rect) into one map to a per-staff struct, and reuse an existing overlay item in place instead of destroying and recreating every overlay on every rebuild. - Mutate a single rect in place during a drag instead of copying the whole staff's rect vector out and back on every mouse-move.
6 tasks
Continuation-line indentation was off by one space, flagged by the codestyle CI check.
This branch doesn't depend on the (not yet merged) MuseSampler velocity fix, so pin it to a commit that's actually on musescore/muse_framework:main - the check_muse_framework CI check rejects fork-only commits.
Note::effectivePlaybackStartTime()/effectivePlaybackDuration() derived the note's playback window from the chord's own tick()/ticks(), which is correct for an ordinary note but not for a grace note, an arpeggio note, or a note inside a repeated section: their actual playback window is computed separately (see GraceChordCtx::buildCtx and the repeat-aware positionTickOffset handling) and can differ substantially from the chord's notated tick/duration. Recomputing from chord tick/ticks discarded that and collapsed grace notes back onto their principal note's timing, breaking 16 unit tests. Apply playbackStartOffset()/playbackDurationOffset() directly in NoteRenderer::render() instead, on top of the RenderingContext's already-correct nominal tick range, and only when an offset is actually set (so unedited notes take the exact same code path as before this feature existed).
Continuation-line indentation was off by one space, flagged by the codestyle CI check.
Note::playbackStartOffset()/playbackDurationOffset() were only wired into read460/tread.cpp's XML reader, but files saved by this app go through read500 (the current format version, dispatched by RWRegister::reader() for version >= 500) - read460 is only ever used to open older 4.60-4.99 files, which can never contain this property in the first place since it didn't exist yet. The property was written correctly (twrite.cpp) but silently dropped on reload because the reader that actually matters never looked for the tag, resetting both values to 0 every time a file was saved and reopened. Moved the read hooks to read500/tread.cpp, removed the dead ones from read460, and added a save/reload regression test (writeReadElement round-trip) to Engraving_NoteTests.note.
createOverlayForStaff() anchored the nominal (zero-offset) right edge on "the next ChordRest segment", but that segment is shared across every voice/track on the staff. If a different voice had a shorter simultaneous note (e.g. an eighth note under a quarter note), its segment became the "next" one for every voice, cutting the longer note's rectangle down to the shorter note's end tick instead of its own. Anchor the right edge on this chord's own end tick via canvasXFromTick instead, which already correctly interpolates/snaps to the real segment at that tick regardless of which voice created it.
Match the note-velocity overlay's convention: green when both playback offsets are at their default (0), orange as soon as either the start or duration offset has been user-modified, blue when the note is selected (taking priority over the modified color, same as velocity). Both offsets already round-trip correctly through save/reload, so the color - computed live from those persisted values on every rebuild - does too.
Mirrors the existing Automation context-menu entry: only appears when note-offset edit mode is enabled (NotationContextMenuModel::loadItems), and resets both playback offsets to 0 for every selected note via the undo stack, so notes revert to the system-default (unmodified) offset.
The edge-drag handle nubs were always a fixed gray regardless of whether the pill body was showing default/modified/selected color, making them visually disconnected from the rectangle they belong to. Handles now use a darker shade of the same state color (green/orange/ blue) as the pill body they're attached to.
Previously a tied-into note (note->tieBack()) got no overlay entry at all, so the rectangle stopped at the first note's own duration and left a visual gap over the rest of the tied chain, even though playback already correctly sounds through the whole chain. NoteEntry now tracks headNote/tailNote/anchorNote instead of a single note. A chain-head's rectangle extends to the tail's own end tick (walked via Note::lastTiedNote(), playback-accurate to match NoteRenderer::renderNormalTie()), with the right/duration handle shown only if the tail resolves within the same System. A new branch handles a tied-continuation note whose *predecessor* lives in a different System: it gets its own continuation fragment starting at that System's own left edge, which is what actually closes the gap for a tie crossing a system or page break - each System still only knows its own coordinate space, so a single rectangle can't literally span two of them, but a fragment per System reads as continuous. Only the chain's first note owns the left/start handle and only its last note owns the right/duration handle; everything in between has neither, matching the fact that dragging duration from the middle of a tie chain (or start from past its own end) has no sensible meaning. Property writes always target the chain's head note regardless of which fragment/handle was actually dragged, since only the head's own offset is ever honored during playback. Also added two one-sided drag clamps, both only active for a real tie (tailNote != headNote): the duration handle can't shrink the total span to end before the tail note's own start, and the start handle can't push the start past the head note's own end.
The "Start offset"/"Duration offset" spinboxes read and wrote Pid::PLAYBACK_START_OFFSET/DURATION_OFFSET on the exact selected note with no tie-awareness at all: selecting a tied-continuation note let you type a value that was silently ignored during playback (only the chain's head note's own offset is ever honored) and, now that the on-canvas overlay is tie-aware too, disagreed with what it shows for that same chain. NotePlaybackModel::headNoteElements() now remaps each selected note to its firstTiedNote(false) for both directions, via a custom onPropertyChangedCallBack passed to buildPropertyItem - ties into an extension point PropertiesPanelAbstractModel already supported, no base-class changes needed. A note that is neither its chain's head nor its tail (a middle link in a 3+-note chain) owns no handle at all in the overlay, so it's dropped from the list entirely rather than redirected, leaving both spinboxes disabled for it instead of quietly editing a value it has no visual handle for. Also fixed a separate, pre-existing refresh bug found while testing this: committing a property change (e.g. releasing a drag) never live-updated these spinboxes without leaving and re-entering the Properties tab. GeneralSettingsModel::onNotationChanged() only ever reloaded its own four Pids and never forwarded the notification down into m_playbackProxyModel (unlike onCurrentNotationChanged() right below it, which does forward to both nested models), and NotePlaybackModel never overrode onNotationChanged() to react even if it had been forwarded. Both gaps are now closed for NotePlaybackModel; PlaybackProxyModel's other sibling models (arpeggio/fermata/breath/ glissando/gradual tempo change) have the same gap and are left as a known, separate follow-up.
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
4 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
Adds a visual, on-canvas way to edit the playbackStartOffset/
playbackDurationOffset properties introduced in #34546, as an alternative
to the Properties panel spinboxes.
Architecture
Mirrors the existing Automation overlay pattern (
NotationAutomationController+PolylinePlot), simplified since offset persistence goes through the existing genericundoChangeProperty/Pidmechanism rather than a dedicated curve/undo command:INotationNoteOffsets/NotationNoteOffsets: edit-mode toggle state.NotationNoteOffsetController: builds/positions one overlay per staff from actual note layout, converts drag input to ticks via segment interpolation, commits throughundoChangeProperty.NoteOffsetOverlay: QQuickPaintedItem drawing rectangles/handles and doing mouse hit-testing, independent of the Grip/NotationInteraction path.segmentcanvasinterpolation.{h,cpp}: canvasX<->tick interpolation shared between automation and note-offset controllers.Test plan
Depends on #34546 — please merge that one first.