Add drag-handle overlay to edit per-note playback start/duration offsets - #1
Open
tharos-devs wants to merge 17 commits into
Open
Add drag-handle overlay to edit per-note playback start/duration offsets#1tharos-devs wants to merge 17 commits into
tharos-devs wants to merge 17 commits into
Conversation
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.
- 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.
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
1 participant
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
Builds on musescore#34546 (feature/note-offsets) to add a visual, on-canvas way to edit the playbackStartOffset/playbackDurationOffset properties introduced there, 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 (previously duplicated inside notationautomationcontroller.cpp).Test plan
Depends on musescore#34546 — please review/merge that one first.