-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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.
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
System object correction #26588
System object correction #26588
Conversation
Don't add system object layers to the partIdList, otherwise we get duplicate IDs
The condition was not met when undoing a move-up-system-layer command so the UI wasn't updating.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving this! Further changes to that first system marking row pending redesign + review; will log another issue when that's ready.
@@ -178,6 +178,7 @@ void SystemObjectsLayerTreeItem::onUndoStackChanged(const mu::engraving::ScoreCh | |||
} | |||
|
|||
if (shouldUpdateState) { | |||
m_systemObjectGroups = collectSystemObjectGroups(m_staff); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm afraid this change will have a negative impact on performance. It scans the entire score for system objects, and we will now call it every time when the user adds / removes / changes a system object. On a small score it doesn't matter as much, but on a larger one it may be more noticeable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
20250220_153116.mp4
This was the problem I was trying to solve. Dragging a system object layer on another should cancels one of them, which means the remaining one needs to update its elements and state. I've tried a different solution in the last commit, could you tell me if that looks better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks better, thank you! However, it can be optimized even further: instead of scanning each staff one by one, we can call collectSystemObjectGroups for all of them (outside of the for loop) and update the state of the items. But it's rather nice to have :)
even for the already existing ones
Resolves: #26580