Bad layout of mmrest at start of system

• May 29, 2019 - 15:36
Reported version
3.1
Priority
P0 - Critical
Type
Functional
Frequency
Once
Severity
S3 - Major
Reproducibility
Always
Status
closed
Regression
No
Workaround
No
Project

1) load attached file
2) view bari sax part
3) change pitch of last note
4) save

Result: mmrest is laid out entirely wrong - no clef or key, too narrow, and next measure appears to start after the point where the mmrest should be:

mmrest-system.png

Oddly, though, if you then go back to the score, do a "save as" and use MSCX file type, then close the score, load that, and repeat the experiment, it works.

This has been frustrating trying to track down. I can reproduce with this score (excerpted from a larger one) but not if I try to create a similar one from scratch, and I still don't really understand what is happening. I know something is going wrong when we do an extra layout to generate the thumbnail for the MSCZ. The mmrest is having its header removed here:

https://github.com/musescore/MuseScore/blob/5136e5c03efedff79ed4f185ebd…

That's because curHeader is false (which it shoudn't be) and then the width is being set to 0 a few lines later because curWidth is 0 (which it shouldn't be). It seems these are set based on the underlying measure, not the mmrest.

I think it has to do with something I remember about disabling mmrests temporarily when saving, although I can't find this (only something about temporarily showing invisible staves if mmrests are enabled). But somehow things are getting out of sync - like we do half a layout with mmrests off then half with them on. Which makes no sense at all. As I said, this has been frustrating. Still working on it, but wanted to get this down.

Attachment Size
mmrest-debug-compressed-2.mscz 10 KB

Comments

Title Bad layout of mmrest at start of system Bad layout of mmrest at start of syste

Actually, I might be confusing myself, not sure it's really an extra layout for the thumbnail here. But during the layout we are performing, both startTick and endTick in the LayoutContext are 0. That's the key for a layout all, I think. The the test I have for lc.endTick < lc.prevMeasure->tick() is probably not good, if we're doing a layout all we need to just keep plowing ahead.

I have verified it works to skip this code is endTick is zero (probably safest to check <= 0). But I'm still too confused to want to submit that as a PR just yet.

Hmm, zero really is a perfectly valid value for endTick; it happens if you change the pitch of the first note of the score, for instance. But in such cases, an mmrest on the next system should have already been laid out. What's special about the case at hand is that the mmrest seems not to have been laid out, even though it pretty clearly was, both when we loaded the score and when we did the pitch change. So it isn't clear what is invalidating that layout. But, it's not just save - that just happens to have been the trigger in my original steps. Also, changing a note on the first system has the same effect. It shouldn't, we should be doing the range layout thing, finishing the first system, then and calling it good. Which is to say, the code here should work. The question is, why isn't the mmrest laid out by now?

Priority P1 - High P0 - Critical
Status active PR created

I finally figured it out. The issue isn't in the code shown above, but in the code a few lines earlier, where curHeader, curTrailer, and curWidth are being set. lc.nextMeasure at that point is the underlying measure, not the mmrest. The mmrest gets constructed immediately afterwards, in getNextMeasure(), presumably so we can check again whether it's still appropriate and how long it should be.

So, the fix is to check for an existing mmrest on nextMeasure and use that instead. Here is a PR:

https://github.com/musescore/MuseScore/pull/5081

This bug would have been around since the second beta so I'm a little surprised it hasn't been reported already, but it is pretty dependent on details of layout order and doesn't affect all mmrests at the beginning of systems, or at least not on all operations. Still, it's quite simple to reproduce from scratch now that I understand what is happening:

1) default empty score
2) enter notes into bars 1-4
3) enter note(s) into bar 8
4) press "M" to enable mmrests
5) change pitch of first note

Result:

mmrest-system-2.png

Fix version
3.2.0