Skip to content

Don't trigger layout when adding elements to note - #34637

Open
miiizen wants to merge 1 commit into
musescore:mainfrom
miiizen:31166-accidentalPasteFreeze
Open

Don't trigger layout when adding elements to note#34637
miiizen wants to merge 1 commit into
musescore:mainfrom
miiizen:31166-accidentalPasteFreeze

Conversation

@miiizen

@miiizen miiizen commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Resolves: #31166
This should be done before we reach Note::add eg. as part of an undo command.
When pasting, we added an accidental to a note belonging to a segment without a valid tick. This caused us to lay the score out from the beginning.

@coderabbitai

coderabbitai Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 536ee7d8-10cd-4136-8329-2c3b6a9a2baa

📥 Commits

Reviewing files that changed from the base of the PR and between 8225a96 and 315946c.

📒 Files selected for processing (1)
  • src/engraving/dom/note.cpp
💤 Files with no reviewable changes (1)
  • src/engraving/dom/note.cpp

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

Note::add() and Note::remove() no longer call triggerLayout() after modifying engraving items. Their item-added and item-removed callbacks remain unchanged.

Merge Risk: ⚪ Minimal · up to 31594

This localized change prevents unnecessary layout work when adding elements to notes; no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description states the issue, motivation, and intended fix, but it omits the required repository checklist. Add the template checklist and complete each item, including CLA, coding standards, testing, unnecessary changes, and test coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the primary change: preventing layout triggers when adding elements to a note.
Linked Issues check ✅ Passed The change directly supports issue #31166 by preventing unnecessary layout during accidental paste operations in large scores.
Out of Scope Changes check ✅ Passed The changes are limited to removing layout triggers from Note::add and Note::remove, which matches the linked issue objective.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

Review ran into problems

🔥 Problems

Linked repositories: Public OSS repositories can only analyze public repositories installed in this organization. No linked repositories were analyzed; skipped musescore/muse_framework.git.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@miiizen
miiizen force-pushed the 31166-accidentalPasteFreeze branch from 8225a96 to 315946c Compare August 19, 2026 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Freeze when pasting range selections containing accidentals in large scores (~10-45 s)

3 participants