fix #289693: correct calculation of system fill #5066
Merged
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
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/289693
This started with #4949, where I fixed what was undeniably a bug in the system distance calculation, where we were inexplicably striving for consistent distance between systems top to top. Meaning if you had page where some systems had only one staff and others had two, you'd get more space below the one-staff systems. This was wrong, previous versions of MuseScore didn't do that, published music doesn't do that, so I solved that problem by essentially removing that code (which uncovered another problem I fixed just before RC).
However, as I started doing some work on spacers for unrelated reasons, I realized my change had a very bad effect in that the effect of spacers is no longer as predictable as it should be. After some investigation/debugging, I now realize that the code I removed was actually almost correct to begin with. It just should have been doing the exact same calculation, but instead of equalizing the distance from top to top, it should have been equalizing the distance from bottom to top. Meaning, instead of removing that code, all I really should have done is subtract off the system height at a couple of places in the calculation.
So that is what this PR does. It reinstates the original code (with better comments) but with that small correction, and now it works as I believe it should have all along. The changes are thus actually smaller than they appear, some of it is just removing some of the stuff I had added in an effort to make the original code something that would respond to a style setting.