fix #289548: incorrect initial layout of added articulations #5056
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
anatoly-os
added a commit
that referenced
this pull request
May 23, 2019
…tion fix #289548: incorrect initial layout of added articulations
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See https://musescore.org/en/node/289548
This is a regression caused by my continuous view performance optimization. Specifically, the change I made to articulation layout in order to fix an issue that was specific to continuous view. But I didn't actually need all the code I actually added to fix that issue. One bit I added - thinking it would be a nice little code simplification that would also help facilitate some future improvement - turns out to have caused this regression. I had moved the articulation layout slightly earlier, but it turns out this doesn't work because we haven't yet assigned the direction (up/down) and thus haven't necessarily chosen the proper symId, which throws off the bbox. Thus, the initial layout is bad, but later on we do calculate direction and assign the correct symID and so subsequent layouts are OK. if lter the direction is fliped, then the same glitch happens and fixes itself again.
This PR fixes the issue by reverting that portion of my continuous view change - moving articulation layout back to Chord::layoutArticulations() instead of Chord::layoutPitched(). As I said, this change wasn't needed anyhow, and performance is the same either way - even though there are multiple call sites now, each articulation still gets laid out only once (and it's a very cheap operation anyhow).