fix #302316, fix #297501 - slurs and fingering

[njvdberg]

Resolves: #302316 - String number under slur interferes with slur.
Resolves: #297501 - Layout shift of slur after reload

When a score, containing a fingering under a slur is loaded, the slur crosses the fingering. Only after a re-layout the slur is drawn higher so it won't intersect the fingering (issue

The root cause of this issue is the shapes of the fingering elements where not available when the bezier of the slur is calculated. The fingering shapes are calculated after the slur bezier is calculated. This explains why a re-layout will solve the issue, then the fingering shapes are available. The shapes are calculated during the calculation of the note shapes but there was no call the fingering layout() method, making the bbox of the fingering shape invalid and therefor it is not added.

This issue is solved by, in Score::layoutSystemElements(), calling the Segment::createShapes() when lay-outing the fingering elements.

This solution also solves #302316 which also requires a re-layout after a String Fingering is added to note under a slur.

This PR replaces PR5797 which was just a quick solution.

• I signed CLA
• I made sure the code in the PR follows the coding rules
• I made sure the code compiles on my machine
• I made sure there are no unnecessary changes in the code
• I made sure the title of the PR reflects the core meaning of the issue you are solving
• 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?"
• 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

[MarcSabatella]

Wow, really great detective work! Your analysis makes perfect sense to me. Only question in my mind then is whether we can save a little time by only doing the createShapes() call if there are in fact chord-based fingerings that needed layout. So, move it one line up, preceded by "if (!fingerings.empty())" or whatever. I don't think adding the beams to the skyline will affect segments shapes, although if it does, you probably just fixed another bug by doing it unconditionally :-)

[njvdberg]

I was quite a search indeed :-). And when I than finally found the solution I must admit I forgot the potential performance issue. Thanks for the remark.

[njvdberg]

This PR is related to #5822 but has a different root cause and is therefor independent.

[njvdberg]

After looking into more, similar issues, e.g. #292614 and #297482, I start wandering whether an extra layout during read-in is a better solution instead of trying to solve this issues in the auto-layout.
Reason is the auto-layout algorithms expect everything to be layout already but during read this is not the case. E.g. If I add a slur interactively, everything is already there, there is a correct skyline, so there is nothing to stop to do a nice layout of the slur and update the skyline, up to the next edit action.
However, when reading a score, first all element are created and then the layout is started. Now the skyline is not correct, layout of certain elements (like beams) take notes in a chord as a reference but these are not layout yet so a wrong reference is used. This will be corrected during the first layout (if there is any!!!)

I see 4 solutions for this:

1. Continue solving all issues, hence complicating the layout algorithm.
2. Just add an extra layout during read (In fact, this was PR5797.
3. Redesign the auto-layout so it doesn't rely on previous layout elements.
4. Redesign the read of a score so it complies the expectations of auto-layout.

For me options 3 and 4 are not the way to go, it is a major rework which might touch more fundamentals of MuseScore.
The main objection against option 2 is a performance issue during read-in. However, option 1 messes up the code of the auto-layout (which is complex already) and introduces some hacks which makes the code even more complex and also introduce a (small) performance penalty but during interactive working. Personally I prefer a slower read performance of a score and a, slightly, faster interactive performance. Option 2 seems to best to me.

Any comments on this?

[dmitrio95]

I would admit that layout algorithm should be designed in a way in which applying this algorithm once makes the entire score (or its relevant part in case of partial layout) be fully correctly laid out. Therefore making layout algorithm rely on things which were set up by previous layouts is, in my opinion, an incorrect way to go. So I would say that we should indeed solve the issues within layout algorithm, be it by fixing them within the existing algorithm structure (which is indeed likely to complicate if further) or by doing some larger redesign of the algorithm or its parts (which requires more work, larger test scores base and might indeed be not something that would be done right now).

Adding extra layout on loading a score actually hides the real issue, and while the real issue is present it is likely that there is a way to break this algorithm during an interactive usage of the editor as well.

[MarcSabatella]

To me this is ready to be merged.

[RobFog]

To me this is ready to be merged.

@anatoly-os, go? :-)