Fix spanners' visible property propagation to new segments in Spanner::add()#33944
Fix spanners' visible property propagation to new segments in Spanner::add()#33944ajuncosa wants to merge 3 commits into
visible property propagation to new segments in Spanner::add()#33944Conversation
|
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 (2)
📝 WalkthroughWalkthroughTwo small visibility fixes in the engraving layer. In 🚥 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 |
fa688e6 to
42393a0
Compare
3b0e7f7 to
38ee919
Compare
…ing added in Spanner::add
…rty using getProperty and setProperty instead of getting it through visible() (to ensure that they both go through property delegates, if any)
38ee919 to
e9f04bf
Compare
Resolves: #33714
When an invisible tie is copied and pasted, the pasted tie loses its invisibility: it can't be toggled with the
vkey, and on save the<Tie>element is written with no<TieSegment>child (possibly the cause of it not rendering properly in musescore.com).A tie tracks visibility in two places that are meant to stay in sync: the
Tie(spanner) itself and itsTieSegment. These are normally kept aligned bySpanner::setVisible, which writes both.However, a
SpannerSegmentis layout-related, not durable. When a tie is added to a score (e.g. during paste, viaundoAddElement->eraseSpannerSegments), its segments are erased and re-generated from scratch during layout. When re-creating the segments (inSpanner::fixupSegments) we were not copying the tie's visible flag onto them, so the re-generated segments defaulted to visible, while the tie itself stayed invisible.This lack of sync caused the following:
getProperty(Pid::VISIBLE), which delegates to theSpanner, so the tie looked correct (i.e. grayed-out).vshortcut (cmdToggleVisible) was not working because it uses the segment's ownvisible()flag, then writes throughundoChangeProperty(VISIBLE), which delegates to the spanner (already invisible). The values match, so the change was ignored.visible()/setVisible()vs.getProperty()/setProperty()inconsisntencies may be present in other places throughout the code.setProperty()(which delegates to theSpanner).visible()), soisUserModified()was false and the<TieSegment>was never written.