Skip to content

Fix #25070: support copy-paste in local time signatures #31314

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
Reedbeta wants to merge 19 commits into
base: master
Choose a base branch
from
Open

Fix #25070: support copy-paste in local time signatures #31314

Reedbeta wants to merge 19 commits into from
+507 −294

Conversation

@Reedbeta
Copy link
Contributor

@Reedbeta Reedbeta commented Dec 2, 2025 •
edited
Loading

Resolves: #16911
Resolves: #25070
Resolves: #31359

Support copy/paste in local time signatures, with the restriction that the source and destination ranges must all have the same time stretch ratio. Also enables features like "repeat selection", Alt+drag range copies, and explode which are built on copy/paste internally.

Tested:

  • Tuplets
  • Dynamics etc.
  • Spanners/lines
  • Multiple voices
  • Multiple staves
  • Explode/implode
  • Paste double/half duration
  • Working on the measure before a local time sig change
  • The measure after a local time sig change
  • The last measure

Some other minor changes in this PR:

  • Adds a natvis file for the Visual Studio debugger with visualizers for a few common types such as Fraction.
  • Fixes a handful of debug-only asserts that I ran into.
  • Use the "forbidden" cursor (🚫) when Alt+dragging to a location that you can't drop in (either because of local time signatures or another reason).
  • Setup time ticks properly for local time signatures (so Alt+drag will work).
  • Show error dialogs for explode/implode and repeat-selection when we can't do it due to local time signatures.
  • I signed the CLA
  • The title of the PR describes the problem it addresses
  • Each commit's message describes its purpose and effects, and references the issue it resolves
  • If changes are extensive, there is a sequence of easily reviewable commits
  • The code in the PR follows the coding rules
  • There are no unnecessary changes
  • The code compiles and runs on my machine, preferably after each commit individually
  • I created a unit test or vtest to verify the changes I made (if applicable)

@Reedbeta Reedbeta force-pushed the pr/paste-local-time-sig branch from c60d633 to ee7c790 Compare December 6, 2025 07:38
@Reedbeta Reedbeta marked this pull request as ready for review December 8, 2025 04:25
@Reedbeta Reedbeta mentioned this pull request Dec 8, 2025
Open
2 tasks
cbjeukendrup
cbjeukendrup reviewed Dec 12, 2025
View reviewed changes
cbjeukendrup
cbjeukendrup reviewed Dec 12, 2025
View reviewed changes
Copy link
Member

@cbjeukendrup cbjeukendrup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks very much for this comprehensive PR!

src/notation/view/notationviewinputcontroller.cpp

const bool canDrop = viewInteraction()->updateDropRange(ctx.logicClickPos);
m_view->asItem()->setCursor(canDrop ? Qt::DragCopyCursor : QCursor());
m_view->asItem()->setCursor(canDrop ? Qt::DragCopyCursor : Qt::ForbiddenCursor);
Copy link
Member

@cbjeukendrup cbjeukendrup Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know that the designers are not super fond of the "forbidden" cursor, so we might need to distinguish between "can't drop because you're not hovering any destination location" (which would show the default cursor) or "can't drop because there's something wrong with the hovered destination location" (which would show the "forbidden" cursor). But let's await testing.

(@avvvvve fyi)

@cbjeukendrup cbjeukendrup requested a review from avvvvve December 12, 2025 19:29
Reedbeta
Reedbeta commented Dec 13, 2025
View reviewed changes
src/engraving/dom/anchors.cpp
Comment on lines +90 to +95
TimeSig* timeSig = measure->score()->staff(staffIdx)->timeSig(measure->tick());
Fraction timeSigFrac = timeSig ? timeSig->sig() : measure->timesig();
Fraction halfDivision = Fraction(1, 2 * timeSigFrac.denominator());
if (timeSig) {
halfDivision /= timeSig->stretch();
}
Copy link
Contributor Author

@Reedbeta Reedbeta Dec 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One issue with this is that once the time ticks are populated, we see the union of the time ticks for all the different local time signatures, even though many of them are unusable as drop targets in one time signature or the other.

It would be nicer if we had a way to filter which time ticks are visible when drag-dropping, but I'm not sure how best to accomplish that.

@zacjansheski
Copy link
Contributor

zacjansheski commented Dec 15, 2025

Tested on MacOS 15, Windows 11, Ubuntu 22.04.3. Approved
#16911 FIXED
#25070 FIXED
#31359 FIXED

@Reedbeta
Fix crash selecting tuplets
98617f8
@Reedbeta
Add error handling for empty duration lists
b4b8c93 
- rather than crashing
@Reedbeta
Enable copy/paste in local time signatures
486c2c8
@Reedbeta
Enable drag and drop in local time signatures
987a945
@Reedbeta
Add correct time tick anchors in local time signatures
003b5db
@Reedbeta
Take voice offset as fraction in makeGap1
fdec3aa
@Reedbeta
Add time stretch to clipboard XML
1b5f40e 
- and check it matches the destination
@Reedbeta
Handle time stretch in toRhythmicDurationList
ae443d1
@Reedbeta
Add comments about which functions operate in stretched vs global time
c129ca4
@Reedbeta
Local/global time fixes
1117737
@Reedbeta
Add error dialogs when attempting to paste across local time sigs
9e6f47f
@Reedbeta
Add time stretch check for drag-drop destination
74054ee
@Reedbeta
Rollback on paste failure
2e8c4cf
@Reedbeta
Use forbidden cursor when can't drop a range
877bd01
@Reedbeta
Update musicxml export refs
3c7669e 
- new ones are equivalent, lower divisions value, arguably better than before
@Reedbeta
Check matching time stretch ratios for explode/implode
73133d7
@Reedbeta
Move canPasteStaff checks inside pasteStaff
20ae444
@Reedbeta Reedbeta force-pushed the pr/paste-local-time-sig branch from df39f81 to 20ae444 Compare December 15, 2025 19:17
@Reedbeta
Copy link
Contributor Author

Reedbeta commented Dec 15, 2025

Addressed the code review feedback, rebased and cleaned up history a bit.

@zacjansheski
Copy link
Contributor

zacjansheski commented Dec 18, 2025

Looks good!

Tested on MacOS 15, Windows 11, Ubuntu 22.04.3. Approved
#16911 FIXED
#25070 FIXED
#31359 FIXED

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@avvvvve avvvvve Awaiting requested review from avvvvve

@cbjeukendrup cbjeukendrup Awaiting requested review from cbjeukendrup

Assignees

@zacjansheski zacjansheski

Labels

None yet

Projects

None yet

Milestone

No milestone

3 participants

@Reedbeta @zacjansheski @cbjeukendrup