Skip to content
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

"Last system fill threshold" should also apply to section breaks #10577

Merged

Conversation

mike-spa
Copy link
Contributor

This is a tiny fix.
The setting "Last system fill threshold" (from Style -> Page), by which we don't justify the last system if its width is smaller than a given amount, should also apply to section breaks.
For reference, this is what the program does now (and also MU3.6)
Screenshot from 2022-02-07 10-52-09
but this would be the correct layout
Screenshot from 2022-02-07 10-53-49

I don't see any obvious downsides to making this the default behaviour. @oktophonie

@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented Feb 15, 2022

Fixes https://musescore.org/en/node/81856
That request goes a step further though, asking for a section break property whether or not to follow that setting.
And for the last system (of score or section) to use it regardless of trailing vertical- or text frames, but that might be covered, by just applying a section break to the last measure of the score (easy workaround at least), it is covered for a section break already.

I wonder what that crash in the utests is about
(my backport to 3.x crashes too, not sure whether that's good or bad news)
Edit: cause and fix found, see #10577 (comment)

@@ -381,8 +381,9 @@ System* LayoutSystem::collectSystem(const LayoutOptions& options, LayoutContext&
// proportional to the difference between the current length and the target length.
// After few iterations, the length will converge to the target length.
qreal newRest = systemWidth - curSysWidth;
if (ctx.curMeasure == 0 && ((curSysWidth / systemWidth) <= score->styleD(Sid::lastSystemFillLimit))) {
// We do not stretch last system if curSysWidth is <= lastSystemFillLimit
if ((ctx.curMeasure == 0 || system->lastMeasure()->sectionBreak())
Copy link
Contributor

@Jojo-Schmitz Jojo-Schmitz Feb 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could use lm->sectionBreak() here. And check whether lm exists first... might fix that crash?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually that check does fix the crash: if ((ctx.curMeasure == 0 || (lm && lm->lastMeasure()->sectionBreak()))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup, good catch! thanks

Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Feb 15, 2022
@mike-spa mike-spa force-pushed the lastSystemFillLimit_sectionBreaks branch from d019dca to 189e873 Compare February 15, 2022 16:32
@RomanPudashkin RomanPudashkin merged commit 7beb89f into musescore:master Feb 17, 2022
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Feb 24, 2022
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request May 12, 2022
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Mar 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants