Fix #25165 - crash when changing measure with whole measure rest #809
Conversation
…egular time. When the actual duration of a measure is changed and either the starting or ending duration cannot be represented with a simple value (for instance 5/4) and the measure contains a single whole measure rest, a division by 0 results. See http://musescore.org/en/node/25165 for examples, discussion and analysis. Fixed. If the new measure value is different from the current time sig, rest(s) to represent the actual measure values are used instead of whole measure rests.
| score()->undo(new ChangeChordRestLen(rest, TDuration(TDuration::V_MEASURE))); | ||
| continue; | ||
| } | ||
| // if measure value did change, represent with rests actual measure value |
There was a problem hiding this comment.
I'm not sure why yet but if I change a measure from 4/4 to 5/4 using this code, I end up with a quarter rest and a whole rest. Trying to select the measure will only select the whole rest.
Without this patch, I end up with a whole rest and a quarter (which is better?), and selecting the measure works ok.
There was a problem hiding this comment.
Very strange. When I try the same (4/4 measure with a whole-measure rest changed to 5/4) I get a whole and a quarter rests: both rests are individually selectable and selecting the whole measure selects both rests.
Was the measure empty in the first place? The code in the patch is only used when the measure(s) being changed contain a single whole-measure rest only. In all other cases, there is no change from previous behaviour.
There was a problem hiding this comment.
Got it! I was fooled by the multiple semantic of ticks (segment tick is measure-relative, but most functions take score-relative ticks...). Another commit coming...
| // } | ||
| // } | ||
| // if measure value didn't change, stick to whole measure rest | ||
| if (_timesig == nf) { | ||
| score()->undo(new ChangeChordRestLen(rest, TDuration(TDuration::V_MEASURE))); |
There was a problem hiding this comment.
It should be ChangeChordRestDuration here.
There was a problem hiding this comment.
I admit I have not understood the implications of the difference between ChangeChordRestLen() and ChangeChordRestDuration().
In any case, I didn't change that specific line, but I kept it as it was (which does not mean it is necessarily correct, but...)
There was a problem hiding this comment.
Actually I think it was ChangeChordRestDuration and you are using ChangeChordRestLen. I believe the difference is that ChangeChordRestDuration is working nicely when the full measure rest shouldn't be a whole rest but something else.
There was a problem hiding this comment.
About ChangeChordRestLen() in line 1663, I tried to change it into ChangeChordRestDuration() and it does not work: the duration (in DurationElement::_duration) is updated but _durationType is not and the rest, while (probably) lasting the correct amount of time, if it was originally a whole-measure rest, is still displayed as a whole-measure rest.
About ChangeChordRestLen() in line 1655, it was originally so (see current, unpatched code line 1653); and this matches your last comment, as we are precisely trying to show the rest as a whole measure rest.
The comment in line 1653 is not precise, though: rather, it should says:
// if measure value matches time sig, stick to whole measure rest
I can update it, if you think it is important.
|
Except my remark about ChangeChordRestDuration, it seems to work better than without this fix. |
…g_metre_measure_rest_crashes Fix #25165 - crash when changing measure with whole measure rest
mgavioli
deleted the
fix_25165_pickup_meas_with_irreg_metre_measure_rest_crashes
branch
Fix #25165 - crash when changing the actual duration of measure with single whole-measure rest to or from irregular time.
When the actual duration of a measure is changed and either the starting or ending duration cannot be represented with a simple value (for instance 5/4) and the measure contains a single whole measure rest, a division by 0 results. See http://musescore.org/en/node/25165 for examples, discussion and analysis.
Fixed.
If the new measure value is different from the current time sig, rest(s) to represent the actual measure values are used instead of a single whole measure rest.