Lines extended with Shift+Left over more than one measure not saved/restored

• Jan 26, 2016 - 18:20
Type
Functional
Severity
S2 - Critical
Status
closed
Project
Tags

Tested against 2.0.2 and master(6205e12)

1. Create a new score with at least one staff and 5 measures (or use the default My_First_Score)
2. Drag the prima volta from the palette to measure 4
3. Double click the just created volta
4. Left click once on the first handle of the volta (the leftmost)
5. Hold Shift and press Left twice
6. Save the score
7. Close the score
8. Reopen the score

Result: the volta has disappeared
Expectation: the volta is present in the same location as when saving the file

Additional info:
* when extending only one measure to the left, the volta is retained after reopening
* Issue is present with any of the voltas, not just with the prima volta
* original thread: https://musescore.org/en/node/95561


Comments

Apparently, when moving the start point of a line using Shift+left, we do set the tick correctly, and this sets the spanner map to dirty, with a comment in the source about that flag saying we need to do that if we move the start point, so that much is good. But I don't think we actually process that; in any event, after the edit operation is completely the spanner map still shows the original tick in the "first" field for this spanner. I think maybeSpannerMap::update() is probably supposed to be fixing this up, but isn't. I am not really clear on the relationship of the map and the tree and iterators and intervals and all that or it would probably be obvious. I'll take another look a bit later if no one else jumps in before then.

Title Volta extended with Shift+Left over more than one measure not saved/restored Lines extended with Shift+Left over more than one measure not saved/restored

making critical due to data loss

I notice a change there is one year, on 19 January 2015
Before, the result was this one after Shift -> Left back twice: the volta was not be extended and moved to measure 2
volta.jpg
But this volta was saved.

Since this commit (the first one of 19 January 2015): 16764d4, for fix ( #44466: Crash extending volta over mmrest starting a new system, the behaviour becomes this one observed currently: the volta is extended on three measures (image below produced with the same nightly), but is not preserved after save and reload.

volta2.jpg

Thanks for the analysis! It confirms my own thoughts, as the code in that commit is definitely dealing with the same issues. What still isn't clear to me is exactly what aspect of it changed for the worse - it was a pretty big change to how this is managed. But I'm still looking.

I also noticed the same issue recently with a slur (spanning several notes). I clicked on the left handle, used shift + left arrow to move it one note to the left, pressed escape and the slur disappeared. But I couldn't reproduce the issue.

Well, my belief is that SpannerMap::update() needs to looks for modified spanners (elements of the map where i.first != i.second->tick()) and update them. As far as I can tell, this requires removing the spanner from the map then re-inserting with the updated key - and presumably doing so in two passes so we don't remove from a data structure we are iterating over. But I'm still too unfamiliar with these data structures to know how to pull this off correctly and safely. So I will leave this to someone else.

I am pretty sure that this is the problem, though. Score::writeSegments() is looping through segments looking for spanners that start on that segment, but this is failing because the map has the wrong key. And SpannerMap::update() *is* being called after the edit, it just isn't fixing the bad key.

I'm not sure if this is a different bug, but in the attached score, extending leftward the slur in the second violin at measure 58 results in either the immediate disappearance of the slur or a crash, even though the slur stays within one measure. Deleting the slur and replacing it first makes no difference.

Attachment Size
Huron_River_Largo.mscz 159.35 KB

I did take another look at this, but basically just confirmed my original finding - the problem is in SpannerMap::update(), and the fix would be to update the keys for elements whose tick no longer matches the key. But actually maniuplating the data structure to do this is not as straightforward as I'd like. Well, it probably is for someone who really gets map & iterators, but that's not me. I did try, but it seems the "const" declaration on the function got in my way, and I don't understand why a function called "update" is declared "const", making me that much more unsure about how to do this.

FWIW, yesterday I had to change a score where I added bars to both 1st and 2nd endings and then extended the voltas of both using shift/left arrow.

I see no problem on reopening it today neither in the score or parts.

f51dc11

I've also added a small code comment in the pull request about the update method.
Its intention is to only update the internal binary search tree used by the find-methods, not to update the map itself. Hence it being const.

@Zack That seems to be a different issue.
I've just tested extending that slur on my version of the PR (which does fix the Volta issue), the Slur still disappears. I've also noted that the longest slur in that measure isn't in the violin2 part.

And more slur-corruption seems to be part of that score:
Debug: zero slur at tick 0(0) track 0 in measure 0-0
Debug: zero slur at tick 51840(51840) track 0 in measure 36-36
Debug: zero slur at tick 54720(54720) track 0 in measure 38-38
Debug: zero slur at tick 56160(56160) track 0 in measure 39-39
Debug: zero slur at tick 60480(60960) track 0 in measure 42-42
Debug: zero slur at tick 195840(195840) track 0 in measure 136-136
Debug: zero slur at tick 204480(204960) track 0 in measure 142-142
All triggered during computeBezier of the Slur

Indeed, that score seems to have some corruption of its scores. Was it perhaps created in an early experimental pre-release build? If so, there is probably nothing to be done. If it was created solely within 2.0 or later builds, then it is worth further investigation for sure.