"Select all similar" does not fully select elements attached to mmrests

• Sep 16, 2014 - 15:31
Type
Functional
Severity
S4 - Minor
Status
closed
Project

Ubuntu Studio 14.04, GIT commit: 3338af9

1) open attached score
2) "M" to enable mmrests
3) right click rehearsal letter "A"
4) select / all similar elements

Result: "A" and "B" appear selected, but "C" does not. If you toggle visibility or do any other sort of change at this point short of deleting, "C" is not affected. But if you switch off mmrests, you'll see the "C" is now invisible. If you then toggle mmrests back on and do the selection again, but this time press "Delete" instead of toggling visibility, "C" is deleted.

I think what is happening is that there are two (linked) "C"'s - one in the "real" measure, one in the mmrest, and the "select all similar" operation is missing the latter. Visibility is normally maintained independently for linked elements, so that explains why the real "C" is marked invisible but the mmrest version is not. Deletion of the real "C" deletes the mmrest version too, so that makes sense.

So I think all that is needed here is for "select / all similar" (and the other similar commands) to be mmrest-aware. I think this probably means, they should select both the real and mmrest versions of these items, but actually, a case could probably be made for *only* selecting the mmrest version (and I suspect that's easier to implement).

Attachment Size
select-mm-rest.mscz 1.54 KB

Comments

Looking at the code, I see two possible solutions:

1) enhance Score::scanElements() and Score::scanElementsInRange() to detect mmrests and traverse them as well. Actually, the latter function already traverses the mmrests, but it skips the underlying measures, so it has the opposite bug, but we would fix it the analogous way.

or

2) ehance Score::collectMatch() to traverse list of linksand add any elements found in the same score.

The second is less risky in that it is guaranteed to *only* affect "select similar" operations - those are the only places collectMatch() is used. And yet the first approach appeals to me in that Score::scanElements() is used in other places as well and I am thinking there it is at least as likely that changing the function in this way will fix other mmrest bugs as break anything. Still investigating.

Sure enough, I discovered several more bugs that are fixed if we make the change to Score::scanElements():

1) Change spatium while mmrests are enabled - you'll see a number of elements do *not* change size if attached to mmrests - clefs, key & time signatures, chord symbols, You can see this in an empty score - the clef & time signature won't change in response to the spatium change. That's because we are depending on scanElements to call spatiumChanged() for all affected elements. My proposed change to scanElements() fixes all of these but key signatures.

2) Change text style will not affect text items attached to mmrests (eg, rehearsal marks), because we are depending on scanElements to call textStyleChanged() for all affected elements.

3) Change musical symbol font between Emmentaler and Bravura while mmrests are enabled and score has "common" or "cut" time signature - time signature will not be updated to the correct glyph in the other font if attached to mmrest (eg, an empty score).

So I will making this change once I see I can address the remaining key signature issue.

Interesting (to me anyhow) - the reason the key signature doesn't fix itself is that after initial layout, mmrests are generally never laid out again until something changes to force the issue. That is, _minWidth2 is set and left alone, so we never call computeMinWidth() again. I added a spatiumChanged() method override for Measure to clear this value, and that fixes it.

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