System divider goes away

• Dec 8, 2019 - 09:05
Reported version
3.2
Type
Functional
Frequency
Once
Severity
S4 - Minor
Reproducibility
Always
Status
closed
Regression
No
Workaround
No
Project

When inserting a staff spacer fixed down (between two systems, to reduce the distance slightly), the system divider goes away.


Comments

Weird, I can reproduce on this score, but not easily in a score I create from scratch. If I keep adding and deleting the spacer in my own score, it works and works and works and works and works then suddenly fails, Maybe autosave is involved? Not really clear.

Here is why it's why happening:

https://github.com/musescore/MuseScore/commit/beda6eecac974657fe6ba227d…

For some reason, the check to add system dividers is skipped if there if the top system hasFixedDownDistance(), which makes zero sense to me. The reason the spacer works at first is the system isn't actually laid out again when adding the spacer, which is potentially fine because there really shouldn't be a need to. But not laying out the system means the fixedDownDistance flag never gets set, which also means some other things that depend on it don't work right (including the position of the divider). So really, we need to trigger a layout when a fixed divider is added, or at least set that flag. But there is still more to this, we never even reach that point in the code for scores where these are the only systems on the page, because if the only system gaps on the page are controlled by fixed spacers, sList is empty and we return earlier, having first deleted the divers. Not sure when that early return is appropriate, but it isn't here.

Bottom line, I see three things that need to happen to fix this, the first (removing the bad check for hasFixedDownDistance) is probably safe, the second (making sure that flag is set in the first place) is almost certainly good, but I'm not so sure about removing the early return - I won't be surprised if something breaks, very possibly with a crash - if that is actually needed.

The early return happens in single page view, so indeed, system dividers don't work there. Can't think of a good reason for that. But here, I can see the early makes sense, we don;t need to do much of the process that follows - but we do need dividers. So instead of removing them unconditionally, probably we need a call to checkDivider.

So I think I now understand enough to do a fix.

Status PR created fixed

Fixed in branch master, commit 1c15bb0e49

_fix #298273: divider not displayed in some cases

Resolves: https://musescore.org/en/node/298273

System dividers were not being displayed in certain cases:
if a fixed spacer is used, or in single page view.
In addition, dividers were displaying that shouldn't be
if layout changes and a system that was formerly not last on page
suddenly becomes last on page,
This is due to a series of errors in layoutPage()
where the dividers are managed.
This fix involves a number of aspects:
1) checkDivider now takes an extra boolean parameter to force deletion
2) we always call checkDivider with that parameter set to true
for the last system of a page
3) in the case where we don't stretch system distance
(the clause checking sList, noVerticalStretch, or System layout mode),
don't just remove dividers, but do the normal checkDivider call,
which adds or removes dividers as appropriate
4) in the calls to checkDivider at the end of the function
(which handle the normal case of non-final systems on the page),
dion't skip the checkDivider calls if a system hasFixedDownDistance.
I believe that check was added because it is appropriate in other places
that also check vBox, so it may have looked like this code should match.
But it shouldn't, there is no reason to skip dividers in this case.
Only the stretch calculations should be skipped._

Fix version
3.5.0