Dragging a volta on a staff other than the first causes problems

• Apr 16, 2015 - 13:22
Type
Functional
Severity
S3 - Major
Status
closed
Project

Nightly eb044f1/ Windows7

1) " My First Score" -> "I" -> Add a staff (bass clef) -> OK

2) Drag a volta VIA the second staff (not directly on the first, if not, it works as expected)
So, you must see this :
volta .jpg
Or, as the screencast of this related thread (third message before end) : https://musescore.org/en/node/56441
3) Release the volta → it goes as expected on the first staff
4) Edit the volta for extend it on the second measure → Shift + right arrow
5) Escape
Result : the volta appears twice on the two staves. Same result with three staves (and so on), after dragging the volta via the third staff
volta3.jpg

6) Then, made Undo → Undo → Undo (three times Undo)

Result : crash


Comments

Is this fix resolves the crash (only the crash after undo)? Or also the twice display of the volta after dragging on the second staff?
Because the crash occurs in August (between 9 and 18, need to further investigation, but it is fixed now, so...) while the twice display of the volta appears from July 29 (possibly from this Nightly:8d3d3f2, or maybe with the last commit of July 28? : https://github.com/musescore/MuseScore/commit/ae475eb0fd8cdc787cd2858ee…)

Status (old) fixed active

Hmm, it actually does something I woulnd't have expected at all. I'm not sure if this is the intended behavior or not, so I am reopening the issue. If this is the new "correct", that's OK, but I think there will be room for improvement.

As things stand, with this change, if you add a volta to a staff other than the top, it actually gets added to the staff you added it to. So you can actually add vltas to as many staves as you want. It seems to work OK with respect to Hide Empty Staves (no worse than previously, anyhow). So it might be seen as OK and even good to be able to add a volta to each section of an orchestra score if you want.

However, if you do add voltas to the same measure in different staves, then in the parts, there are multiple copies of the volta. That probably should not be allowed to happen.

I think there are probably places in the code where we assume voltas live in track 0 only (top staff), and I am a bit afriad that allowing voltas to be added on non-top staves is going to cause other worse problems. If not, great - but we should fix prpbably make it so parts have only one copy of the volta somehow.

Examining the code, I am inclined to think this was inadvertent and it was not really intended to be adding the volta anywhere but the top staff.

At this point in the code we are assigning track2, but we never explicitly set track:

https://github.com/musescore/MuseScore/blob/b482d00c652c9a1abf42d6e2c3e…

It had been set to the the staff to which the element was dragged back in cmdAddSpanner, but the code here in undoAddElement that sets track is now being bypassed. So without fully understandings the ins and outs, I would propose we set the track here the same basic way we are setting track2:


nsp->setTrack(staffIdx * VOICES + (sp->track() % VOICES));

But I have to emphasize, I don't really understand the staff index / track calculations going on here. After all, shouldn't all of the element types be in track 0 / sitaffIdx 0? At this point, we are populating a list of staves with staff(0) for each linked score:

https://github.com/musescore/MuseScore/blob/b482d00c652c9a1abf42d6e2c3e…

But then a few lines later, we initialize staffIdx from score->staffIdx(staff) as if we didn't know perfectly well it is going to be 0:

https://github.com/musescore/MuseScore/blob/b482d00c652c9a1abf42d6e2c3e…

Looks like this code was last year to fix an issue with voltas, and maybe it's just being very conservative and simply use "0" would have been OK. Doesn't seem to hurt anything - but I think it shows we do need to set track accordingly, otherwise track & track2 will be on different staves when there is logical reason for this I can see.

Title Dragging a volta on a staff other than the first causes a crash after undo Dragging a volta on a staff other than the first causes problems

(problems indeed, but no more crash)

Yes, thanks, I did vaguely recall there had been a littrle back and forth on this. And I did think about suggesting we set track to 0 when adding further upstream. Not a bad idea in any case, I think, but given how oddly specific the code I pointed is (rather than just assuming 0), I think we should probably still be consistent about this and set track correctly in undoAddElement(). If I don't hear otherwise, I'll probably just submit a PR for it.

If we were do want to support voltas on staves other than the top, we would as I suggested want to fix the parts issue, and that would probably involve changes to this same section of code.

Status (old) patch (code needs review) fixed

It is from my perspective. New behavior works well, I think.

One quirk worth mentionining: if you have a volta on a non-top staff, it is not propagated to any parts. This is good if there is also a volta on the top staff, since that one will be propagated to all parts. But what if there is not *also* a volta on the top staff - only the interior one? This won't be propagated to parts at all. I think that's probably fine, as I'm not sure what the use case for this would even be, but thought it worth mentioning. Not sure it would make sense to try to include logic to link the non-top-staff volta only in cases where there is no top-staff volta. Wed have to worry about about cases where there are voltas on multiple staves but they have different spans, different text, or whatever. Can of worms best left unopened, probably.

Sometimes I'll see, on a large score, a volta on the top staff, but also repeated for a different section - perhaps a piano accompaniment or string section. It sounds like this use-case would be supported (simply duplicate the volta on the main score, generated parts will have the top volta), but just wanted to confirm.

Yes, this works in the current build. Only the volta attached to the top staff gets copied to parts - and they continue to be copied to *all* parts. But you can add voltas to other staves in the score as well, such as for the string section in an orchestra. Those will appear in the score only