Fix #65161 repeatlist unwind section by section

[ericfont]

This pull requests generates the repeat list of a score with section breaks by individually generating the repeat lists of the individual sections and combining them in order.

It seems jojo, marc, and I agree on this expected behavior - that repeats should be handled independently for each section - but last lasconic comments on the forum seem to disagree with this interpretation.

"start" and "end" tags are section aware, which is implemented by Score::searchLabelWithinSectionFirst. As Jojo suggested, I haven't forbidden jumps to outside the current section, but I haven't tested it either (and I don't really know what exactly expected behavior should be for those). So labels are first searched for in current section, and if not found there, then the old function Score::searchLabel will be used as a fallback, essentially allowing jumps to other sections.

Most of the functionality that was in RepeatList::unwind is moved to RepeatList::unwindSection, such that "unwind" only divides up the score into sections for unwindSection to handle.

Note: No RepeatSection will ever go through a section break, in my implementation, but are essentially split into two RepeatSections at the section break.

I added "repeat27.mscx" to mtest, is basically from Jojo's test example https://musescore.org/en/node/65161#comment-297721, but I modified movt 4 into a DS-al-Coda case since that case wasn't represented, and I removed superflous measurses. That test passes, but I haven't extensively tested other scores, and I notice test 32 "tools" won't pass on my machine...I don't know what that is and haven't looked into that...I'm curious to see if it passes on travis.

[ericfont]

interesting...test 32 passes on travis, but not on my arch linux machine. Could be another of those QT 5.5 bugs or something else with my setup.

[ericfont]

wait...looking at the travis output I see: ./gen: 97: ./gen: rekonq: not found
and then an exit 0. Maybe it didn't pass. Any ideas?

[lasconic]

./gen: rekonq is used to launch a browser once the vtests are done. There is no browser installed on travis, then the error. So, travis seems to be ok with test32

[ericfont]

oops...one minor error I now noticed when doing entire album of repeat tests 0-27...about to reupload fix and will include this combined album test as "repeat28.mscx"

regarding test 32, the "./gen rekonq not found" disappears after installing rkonq, however it turns out my arch linux machine is still failing that test with this error:

32/44 Test #32: tst_tools ........................***Failed    2.35 sec
--- undoSlashRhythm01-test.mscx 2015-08-14 05:51:37.075466322 -0400
+++ /home/e/MuseScore/mtest/libmscore/tools/undoSlashRhythm01-ref.mscx  2015-07-18 01:36:51.142327404 -0400
@@ -539,7 +539,7 @@
           </Rest>
         <tick>3840</tick>
         <Rest>
-          <offset x="0" y="-1.28006"/>
+          <offset x="0" y="-1.28044"/>
           <track>2</track>
           <small>1</small>
           <durationType>quarter</durationType>
@@ -960,7 +960,7 @@
           </Rest>
         <tick>3840</tick>
         <Rest>
-          <offset x="0" y="-1.28006"/>
+          <offset x="0" y="-1.28044"/>
           <track>6</track>
           <small>1</small>
           <durationType>quarter</durationType>
   <diff -u undoSlashRhythm01-test.mscx /home/e/MuseScore/mtest/libmscore/tools/undoSlashRhythm01-ref.mscx failed

so y offset of rest seems t be off by a small relative amount of .00038, so could very well be a floating point roundoff error, or again could be something with my qt5.5 or arch linux machine (or even a different version of my compiler).

[MarcSabatella]

Yes, I think you are right that this is some little system-dependent
quirk. The offset for the rest is dependent not just on floating point
math (which should not be off by even that much) but also font calculations
(height of the rest glyph). In theory the new font system for 2.0.2 and
beyond should make those less fragile, but not knowing what's involved, I
could still imagine Qt changes affecting them.

On Fri, Aug 14, 2015 at 6:06 AM Eric Fontaine notifications@github.com
wrote:

oops...one minor error I now noticed when doing entire album of repeat
tests 0-27...about to reupload fix and will include this combined album
test as "repeat28.mscx"

regarding test 32, the "./gen rekonq not found" disappears after
installing rkonq, however it turns out my arch linux machine is still
failing that test with this error:

32/44 Test #32: tst_tools ........................***Failed 2.35 sec
--- undoSlashRhythm01-test.mscx 2015-08-14 05:51:37.075466322 -0400
+++ /home/e/MuseScore/mtest/libmscore/tools/undoSlashRhythm01-ref.mscx 2015-07-18 01:36:51.142327404 -0400
@@ -539,7 +539,7 @@

3840

•      <offset x="0" y="-1.28006"/>
  

•      <offset x="0" y="-1.28044"/>
     <track>2</track>
     <small>1</small>
     <durationType>quarter</durationType>
  

  @@ -960,7 +960,7 @@
  
  3840
  

•      <offset x="0" y="-1.28006"/>
  

•      <offset x="0" y="-1.28044"/>
     <track>6</track>
     <small>1</small>
     <durationType>quarter</durationType>
  

  <diff -u undoSlashRhythm01-test.mscx /home/e/MuseScore/mtest/libmscore/tools/undoSlashRhythm01-ref.mscx failed

so y offset of rest seems t be off by a small relative amount of .00038,
so could very well be a floating point roundoff error, or again could be
something with my qt5.5 or arch linux machine (or even a different version
of my compiler).

—
Reply to this email directly or view it on GitHub
#2165 (comment).

[ericfont]

ok, it would be nice to have these tests pass with a warning if they are within some threshold, say .01 sp.

back to my test of combined album of repeat01.mscx through repeat27.mscx, I'm having issues with the section formed by repeat13.mscx, because it has a DC al Coda but does not have coda defined in that section. Just running repeat13.mscx by itself, since that coda is not defined, that jump is treated as invalid and thus ingnored. However my implementation searches through the whole combined score as fallback if it can't find a label in the current section, and since it finds another section's coda, it will thus takes the repeat. So that means that this section's repeat list is different depending on whether it is included in an album with another coda or not.

I'm going to exclude repeat13.mscx from my combined album test, since I don't think we've precisely defined the expected behavior for jumping to labels that are not found in the current section. That might be left for another feature request to nail down.

[ericfont]

this travis is going to fail, because I haven't finished debugging the rest of my combined album test "repeat.28", but I wanted to get my revised code up here in case you're looking at my code. The combined album repeats work correctly at least up till the 12th section. I would appreciate any comments on how to handle jumps to label outside of the current section, or if you think my current fallback strategy of simply searching the rest of the score is OK for now.

[ericfont]

sorry, I shouldn't have included this in the commit. However, it would be nice if at least the travis CI used -j2 since the travis runs on two cores, I believe.

[ericfont]

closing this PR, in favor of PR #2172