Splitting irregular measures produce extra rests

I have verified this bug 2.0.3, 2.1-dev 9ca7d0b, and 3.0-dev 90f9d78.

I am noting that the resulting split measure have the correct duration according to their measure properties, but the problem is that unnecessary rests are added (as the title says).

I'm noting that in 2.0.3 and 2.1 that if created a time signature of 15/4 before adding the notes and apply the split, then the result is correct in 2.0.3 and 2.1-dev. Although in 3.0-dev I ran into this problem when split a breve and dotted whole:

Other steps (with 2.0.3)

1) "My First Score"
2) Right-click first measure -> Measure properties -> Change "Actual" duration to 6/4 -> Ok
3) Enter two quarter notes

The test file at this step: split irregular.mscz

4) Select the second quarter note D -> Edit -> Measure -> Split measure before selected note

Result: in the second measure: corruption (extra rest)

After save/close/ reload: 1 split.mscz

- Note I observe the same behavior as much as I go back in time
Eg, image below produced in May 2014 with 56177c3

So sounds like this behavior was never correctly implemented to being with. Should I take a stab at this?
It seems like musescore is only looking at the time signature, but needs to look at the measure's actual duration instead. Because if I did place a 6/4 time sig between steps 3 & 4 in your above comment, then I get valid result:

where that first measure's actual duration correctly becomes 1/4 and the second measures actual duration correctly becomes 5/4.

cmdSplitMeasure seems to be going along well

adjustToLen called for 1st measure with fraction 1/4 ...Good
adjustToLen called for 2nd measure with fraction 5/4 ...Good

until the very end "range.write(this, m1->tick());" which seems to be where to problem will be...

I'm also noting that happens with splitting an irregular measure containing only rests. For instance if have this irregular 6/4 measure:

and select the 2nd quarter note and split it, the result is:

Where what looks might look like a whole rest in the 2nd measure is actually a "measure rest" internally represented with a duration 5/4.

hmm...acutally I noticed in cmdSplitMeasure that m1 and m2 don't get their _irregular flag set true after
m1->adjustToLen(Fraction::fromTicks(ticks1));
m2->adjustToLen(Fraction::fromTicks(ticks2));
even though they end up with _len that doesn't match their _timesig. Never mind, it appears the original measure is not _irregular to begin with...and i think that _irregular means something different than what is in this issue's title...

oh, this is interested. In cmdSplitMeasures, after the operations:

m1->adjustToLen(Fraction::fromTicks(ticks1));
m2->adjustToLen(Fraction::fromTicks(ticks2));

the result is m1 contains 0 segments, while m2 (which had 0 segments before) gets 1 segment of ChordRest. Also intestering is that m2 has both _segments _first and _last defined as different non-null pointers, even though the size of _segments is 1 (so I'm confused why _last would be something other than NULL). Also interesting is that the first segment is a ChordRest of a quarter rest. Probably this quarter rest is the superfluous rest that is causing the bug...

That additional quarter note at tick 2000 at the end of of the new 2nd measure in this if block near the end of Measure::adjustToLen():

                  if ((n > 0) && (rFlag || voice == 0)) {
                        // add rest to measure
                        int rtick = tick() + nl - n;
                        int track = staffIdx * VOICES + voice;
                        s->setRest(rtick, track, Fraction::fromTicks(n), false, 0, false);
                        }

If I comment that block out, then my mtest passes. (I'm not saying I'll comment out that block...what I am saying is that we need to make sure that block doesn't execute when splitting an irregular measure)

just for reference, the commit that added that block was werner from Oct 2013 to fix #23230: Adjusting actual duration produces rests with incorrect values

I've changed the function definiton of Measure::adjustToLen to have a default boolean parameter "appendRestsIfNecessary" which is set to true by default, but which I'm calling with false when when calling this function from cmdSplitMeasures. That way cmdSplitMeasures doesn't add rests so my test passes, but any other functionality for everyone else who calls it remains the same.

Everything seems to be working. I am noting the following behavior, which results from behavior that was already in the code. If I start with

and split before the 3rd rest, then all the rests in each measure become merged into a single measure rest, of a duration equal to the duration of their irregular-length measure:

now to me, I'm not sure this is desired behavior...user may have organized the rests in that manner and would want to keep rests in same manner, but if this is indeed desired behavior, then I can't argue. Anyway, I'm just making a note for posterity...as it is I'm including this as one of my test cases, so if it gets merged then that will be solidified behavior...if that is not desired, then we should probably figure out a way to not have rests merged.

Or even #166096: Make individual beat rests when splitting a measure ;-)

Ok, thanks for making a note to that feature request...I added a comment that I agreed with it.

So that means I won't include that test case, since I don't want to be stuck with the behavior or rests being merged into measure rests.

Ok, my fix is ready for review...here is PR against 2.1: https://github.com/musescore/MuseScore/pull/3105

(I've only tested against 2.1-dev, but not 3.0-dev yet)

Fixed in branch 2.1, commit aa3261eb96

fix #183846 split irreg-len meas shouldn't add rest

This fixes an bug which produced corrupted measures when splitting irregular length measures. In Score::cmdSplitMeasure() after the newly split measures are first created, if their new actual length was greater than their nominal length, then Measure::adjustToLen() would append additional padding rests at the end of the measure. However, the subsequent range.write() assumed that the measures were entirely empty before copying contents from the original measure into the new measures. The extra padding rests were unnecessary, and caused the resulting measures to contain too many notes than their actual length, hence the corruption.

The fix here is to add a default boolean parameter to adjustToLen() called appendRestsIfNecessary which is true by default so as to not change behavior when it is called without specifiying the parameter. However, cmdSplitMeasure() will call adjustToLen with that boolean explicitly false, so that the new measures don't get unnecessary rests.

Fixed in branch 2.1, commit 5104373793

Merge pull request #3105 from ericfont/183846-SplitIrregular

fix #183846 split irreg-len meas shouldn't add padding rests, which cause corruption

Fixed in branch master, commit c08b67ff90

fix #183846 split irreg-len meas shouldn't add rest
This fixes an bug which produced corrupted measures when splitting irregular length measures. In Score::cmdSplitMeasure() after the newly split measures are first created, if their new actual length was greater than their nominal length, then Measure::adjustToLen() would append additional padding rests at the end of the measure. However, the subsequent range.write() assumed that the measures were entirely empty before copying contents from the original measure into the new measures. The extra padding rests were unnecessary, and caused the resulting measures to contain too many notes than their actual length, hence the corruption.

The fix here is to add a default boolean parameter to adjustToLen() called appendRestsIfNecessary which is true by default so as to not change behavior when it is called without specifiying the parameter. However, cmdSplitMeasure() will call adjustToLen with that boolean explicitly false, so that the new measures don't get unnecessary rests.

(deleted comment...I posted to wrong issue)

Automatically closed -- issue fixed for 2 weeks with no activity.