Fix #33713: Full-measure rest collides with its own fermata#33790
Fix #33713: Full-measure rest collides with its own fermata#33790pyonpyoco wants to merge 3 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR updates collision detection in the full-measure rest layout engine. The measureShape.remove_if predicate in RestLayout::checkFullMeasureRestCollisions now captures fullMeasureRest and extends its removal logic to exclude FERMATA elements whose explicitParent segment and track match the full-measure rest, preventing those fermata symbols from being considered in measureShape collision checks. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsLinked repositories: Your configuration references 1 linked repositories, but your current plan allows 0. Analyzed ``, skipped 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. Comment |
| return shapeItem && ((shapeItem->isRest() && toRest(shapeItem)->isFullMeasureRest()) | ||
| || (shapeItem->type() == ElementType::FERMATA | ||
| && shapeItem->explicitParent() == fullMeasureRest->segment() | ||
| && shapeItem->track() == fullMeasureRest->track()) |
There was a problem hiding this comment.
I'm pretty sure that the Fermata can be safely removed from this Shape in any cases, without these checks? Cause the main purpose of this function is to check for collisions between the full measure rest and note/rests in other voices, while the Fermata position is checked elsewhere.
There was a problem hiding this comment.
Thanks. After rechecking the overall flow of this function, I understand that it is intended to handle cases with multiple voices, where a full-measure rest needs to be moved away from elements in other voices. Fermatas should not participate in this collision shape at all.
I simplified the condition accordingly.
b385c2b to
2bccead
Compare
Resolves: #33713
Problem
The issue was caused by
checkFullMeasureRestCollisions(). It removed the full-measure rest itself frommeasureShape, but did not remove the fermata associated with that same rest. As a result, the full-measure rest was checked against its own fermata and moved during collision avoidance.Solution
Exclude fermatas on the same segment and track as the full-measure rest being checked.
2026-06-12.165224.mp4