Fix issues discovered while moving grip handles around on trills #8600

wizofaus · 2021-07-12T21:58:05Z

Resolves: https://musescore.org/en/node/322957 #8592

Two unrelated issues, neither really to do with trill lines specifically at all but both discovered at the same time:

a) if you drag the grip handle from any line into a 0-stave system (e.g. for a V-frame, as new scores have by default at the top of the score), it crashes
b) a bug in the layout engine where it reused cached information from a previous layout caused erratic behaviour when dragging grip handles around, though it could be triggered in other ways,

[ x ] I signed CLA
[ x ] I made sure the code in the PR follows the coding rules
[ x ] I made sure the code compiles on my machine
[ x ] I made sure there are no unnecessary changes in the code
[ x ] I made sure the title of the PR reflects the core meaning of the issue you are solving
[ x ] I made sure the commit message(s) contain a description and answer the question "Why do those changes fix that particular issue?" or "Why are those changes really necessary as improvements?"
[ x ] I made sure the commit message title starts with "fix #424242:" if there is a related issue
I created the test (mtest, vtest, script test) to verify the changes I made

- fix issues discovered while moving grip handles around on trills

wizofaus · 2021-07-12T21:59:12Z

src/engraving/libmscore/layout.cpp

@@ -2156,6 +2156,9 @@ System* Score::getNextSystem(LayoutContext& lc)
    if (!isVBox) {
        int nstaves = Score::nstaves();
        system->adjustStavesNumber(nstaves);
+        for (int i = 0; i < nstaves; ++i) {
+            system->staff(i)->setShow(score()->staff(i)->show());
+        }


without this, it "remembers" which staves were hidden for this system on the previous layout, but that changes whether it takes into account the instrument name when calculating the width, meaning you can get a different result depending on the state of the cache.

This change, backported to the 3.x branch, causes MuseScore(3.exe) to crash (in Debug builds, QtCreator, MinGW)), on opening any score due to a failed assertion in Qt:
Fatal: ASSERT failure in QList::operator[]: "index out of range", file C:/Qt/5.15.2/mingw81_64/include/QtCore/qlist.h, line 575

Not sure how relevant this is to the master branch though.

I worked this fix out in MU3 and if it's crashing there (which it wasn't for me) it's wrong in MU4 too - I'll have a look when I can.

It certainly crashes for me. The branch I tried it on is not a plain 3.x with just that commit though, so there might be interaction with other commits.
I'm trying that now...

Oops, I added these 3 lines a line too late (before the system->adjustStavesNumber(nstaves); rather than after), downside of an all-manual cherry-pick. Have a brown bag for me?
Sorry for the noise...

Yeah I was pretty sure it was OK! Took me hours to work out what that problem was too BTW, surprised it was such a simple change in the end, and a bit surprised it doesn't cause more weird layout issues.

wizofaus · 2021-07-12T22:00:12Z

src/engraving/libmscore/line.cpp

@@ -432,7 +432,7 @@ Segment* LineSegment::findSegmentForGrip(Grip grip, PointF pos) const
    System* sys = oldSeg->system();
    const QList<System*> foundSystems = score()->searchSystem(pos, sys, spacingFactor);

-    if (!foundSystems.empty() && !foundSystems.contains(sys)) {
+    if (!foundSystems.empty() && !foundSystems.contains(sys) && foundSystems[0]->staves()->size()) {


ignore 0-staff systems (used for V-Boxes etc.) when trying to find system during grip handle dragging

…p handles around on trills Two unrelated issues, neither really to do with trill lines specifically at all but both discovered at the same time: a) if you drag the grip handle from any line into a 0-stave system (e.g. for a V-frame, as new scores have by default at the top of the score), it crashes b) a bug in the layout engine where it reused cached information from a previous layout caused erratic behaviour when dragging grip handles around, though it could be triggered in other ways, Backport of musescore#8600

Jojo-Schmitz · 2021-08-05T16:17:01Z

It also solves https://musescore.org/en/node/323559. Yet another good candidate for backporting to 3.x (I'd have it ready, no rocket science with just 4 likes of code ;-)) and a 3.6.3!

…p handles around on trills Two unrelated issues, neither really to do with trill lines specifically at all but both discovered at the same time: a) if you drag the grip handle from any line into a 0-stave system (e.g. for a V-frame, as new scores have by default at the top of the score), it crashes b) a bug in the layout engine where it reused cached information from a previous layout caused erratic behaviour when dragging grip handles around, though it could be triggered in other ways, Backport of musescore#8600

Fix musescore#8592 and also MU3 bug https://musescore.org/en/node/322957

fd2525a

- fix issues discovered while moving grip handles around on trills

wizofaus commented Jul 12, 2021

View reviewed changes

igorkorsukov approved these changes Jul 13, 2021

View reviewed changes

igorkorsukov merged commit ee618b0 into musescore:master Jul 13, 2021

Jojo-Schmitz mentioned this pull request Jul 23, 2021

[MU3 Issue] MuseScore 3.6.3 or 3.7.0, PRs to be merged and issues to be fixed. #7449

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix issues discovered while moving grip handles around on trills #8600

Fix issues discovered while moving grip handles around on trills #8600

wizofaus commented Jul 12, 2021 •

edited

Loading

wizofaus Jul 12, 2021

Jojo-Schmitz Jul 15, 2021

wizofaus Jul 15, 2021 •

edited

Loading

Jojo-Schmitz Jul 15, 2021 •

edited

Loading

Jojo-Schmitz Jul 15, 2021

wizofaus Jul 15, 2021

wizofaus Jul 12, 2021

Jojo-Schmitz commented Aug 5, 2021 •

edited

Loading

Fix issues discovered while moving grip handles around on trills #8600

Fix issues discovered while moving grip handles around on trills #8600

Conversation

wizofaus commented Jul 12, 2021 • edited Loading

wizofaus Jul 12, 2021

Choose a reason for hiding this comment

Jojo-Schmitz Jul 15, 2021

Choose a reason for hiding this comment

wizofaus Jul 15, 2021 • edited Loading

Choose a reason for hiding this comment

Jojo-Schmitz Jul 15, 2021 • edited Loading

Choose a reason for hiding this comment

Jojo-Schmitz Jul 15, 2021

Choose a reason for hiding this comment

wizofaus Jul 15, 2021

Choose a reason for hiding this comment

wizofaus Jul 12, 2021

Choose a reason for hiding this comment

Jojo-Schmitz commented Aug 5, 2021 • edited Loading

wizofaus commented Jul 12, 2021 •

edited

Loading

wizofaus Jul 15, 2021 •

edited

Loading

Jojo-Schmitz Jul 15, 2021 •

edited

Loading

Jojo-Schmitz commented Aug 5, 2021 •

edited

Loading