Fix #18869 : restore original clef on ChangeStaffType undo

When a staff type change is undone, the previous staff clef is not restored, if it has been changed to suit the new staff type. Fixed by storing the original initial clef and restoring it on undo. Should fix the following issues: #18869, #23374, #24294 and, possibly others too. - Added to ChangeStaffType class an initialClef member variable to hold the original staff initial clef type list. - ChangeStaffType::flip() split into undo() and redo(), as the managing the clef and setting the staff type must be applied in reversed order in the two cases. - Clef::layout() now hides a clef not compatible with the staff group, if the clef is not generated or changes it to the same clef as the staff initial clef if generated - TAB ad-hoc code in Clef::layout() to check for compatibility between the clef type and the staff group generalized to all groups removed. - Fixed GuitarPro test reference scores, which all contained wrong clefs for the TAB staves. Tested with a few 2.0 and 1.3 scores; more tests are probably necessary.
musescore · May 26, 2014 · 93e31dc · 93e31dc
1 parent b4e6343
commit 93e31dc
Show file tree

Hide file tree

Showing 27 changed files with 117 additions and 80 deletions.
diff --git a/libmscore/clef.cpp b/libmscore/clef.cpp
@@ -145,64 +145,63 @@ void Clef::setSelected(bool f)
 void Clef::layout()
       {
       // determine current number of lines and line distance
-      int lines      = 5;                       // assume a resonable default
-      qreal lineDist = 1.0;
-
-      Staff* stf = staff();
-      StaffType* staffType = nullptr;
-      if (stf && stf->staffType()) {
+      int   lines       = 5;              // assume resonable defaults
+      qreal lineDist    = 1.0;
+
+      Staff*      stf         = staff();
+      StaffType*  staffType   = nullptr;
+      Segment*    clefSeg     = static_cast<Segment*>(parent());
+      // check clef visibility and type compatibility
+      if (clefSeg && stf && stf->staffType()) {
+            bool        bHide;
+            // check staff type allows clef display
             staffType = staff()->staffType();
-            if (!staffType->genClef()) {        // if no clef, set empty bbox and do nothing
-                  qDeleteAll(elements);
-                  elements.clear();
-                  setbbox(QRectF());
-                  return;
-                  }
-
-            // tablatures:
-            if (staffType->group() == TAB_STAFF_GROUP) {
-                  // if current clef type not compatible with tablature,
-                  // set tab clef according to score style
-                  if (ClefInfo::staffGroup(clefType()) != TAB_STAFF_GROUP)
-                        setClefType( ClefType(score()->styleI(ST_tabClef)) );
+            bHide = !staffType->genClef();
+
+            // check clef is compatible with staff type group
+            int tick = clefSeg->tick();
+            if (ClefInfo::staffGroup(clefType()) != staffType->group()) {
+                  if (tick > 0 && !generated()) // if clef is not generated, hide it
+                        bHide = true;
+                  else                          // if generated, replace with initial clef type
+                        // TODO : instead of initial staff clef (which is assumed to be compatible)
+                        // use the last compatible clef previously found in staff
+                        _clefTypes = stf->clefTypeList(0);
                   }
 
-            //
-            // all staff types
             //
             // courtesy clef
             //
             bool showClef = true;
-            Segment* clefSeg = static_cast<Segment*>(parent());
-            if (clefSeg) {
-                  int tick = clefSeg->tick();
-                  // only if there is a clef change
-                  if (stf->clef(tick) != stf->clef(tick-1)) {
-                        // locate clef at the begining of next measure, if any
-                        Clef*       clefNext    = nullptr;
-                        Segment*    clefSegNext = nullptr;
-                        Measure*    meas        = static_cast<Measure*>(clefSeg->parent());
-                        Measure*    measNext    = meas->nextMeasure();
-                        if (measNext) {
-                              clefSegNext = measNext->findSegment(Segment::SegClef, tick);
-                              if (clefSegNext)
-                                    clefNext = static_cast<Clef*>(clefSegNext->element(track()));
-                              }
-                        // show this clef if: it is not a courtesy clef (no next clef or not at the end of the measure)
-                        showClef = !clefNext || (clefSeg->tick() != meas->tick() + meas->ticks())
-                              // if courtesy clef: show if score has courtesy clefs on
-                              || ( score()->styleB(ST_genCourtesyClef)
-                              // AND measure is not at the end of a repeat or of a section
-                              && !( (meas->repeatFlags() & Repeat::END) || meas->sectionBreak() )
-                              // AND this clef has courtesy clef turned on
-                              && showCourtesy() );
-                        if (!showClef)    {     // if no clef, set empty bbox and do nothing
-                              qDeleteAll(elements);
-                              elements.clear();
-                              setbbox(QRectF());
-                              return;
-                              }
+            // only if there is a clef change
+            if (!bHide && tick > 0 && stf->clef(tick) != stf->clef(tick-1)) {
+                  // locate clef at the begining of next measure, if any
+                  Clef*       clefNext    = nullptr;
+                  Segment*    clefSegNext = nullptr;
+                  Measure*    meas        = static_cast<Measure*>(clefSeg->parent());
+                  Measure*    measNext    = meas->nextMeasure();
+                  if (measNext) {
+                        clefSegNext = measNext->findSegment(Segment::SegClef, tick);
+                        if (clefSegNext)
+                              clefNext = static_cast<Clef*>(clefSegNext->element(track()));
                         }
+                  // show this clef if: it is not a courtesy clef (no next clef or not at the end of the measure)
+                  showClef = !clefNext || (clefSeg->tick() != meas->tick() + meas->ticks())
+                        // if courtesy clef: show if score has courtesy clefs on
+                        || ( score()->styleB(ST_genCourtesyClef)
+                        // AND measure is not at the end of a repeat or of a section
+                        && !( (meas->repeatFlags() & Repeat::END) || meas->sectionBreak() )
+                        // AND this clef has courtesy clef turned on
+                        && showCourtesy() );
+                  bHide |= !showClef;
+                  }
+
+            // if clef not to show or not compatible with staff group
+            if (bHide) {
+                  qDeleteAll(elements);         // set empty bbox and do nothing
+                  elements.clear();
+                  setbbox(QRectF());
+                  return;
                   }
 
             lines = staffType->lines();         // init values from staff type

diff --git a/libmscore/cleflist.cpp b/libmscore/cleflist.cpp
@@ -57,7 +57,7 @@ void ClefList::setClef(int tick, ClefTypeList ctl)
       {
       if (clef(tick) == ctl)
             return;
-      if (clef(tick-1) == ctl)
+      if (tick > 0 && clef(tick-1) == ctl)
             erase(tick);
       else  {
             auto i = find(tick);

diff --git a/libmscore/undo.cpp b/libmscore/undo.cpp
@@ -2357,14 +2357,51 @@ void ChangeStaff::flip()
       }
 
 //---------------------------------------------------------
-//   flip
+//   ChangeStaffType::undo / redo
 //---------------------------------------------------------
 
-void ChangeStaffType::flip()
+void ChangeStaffType::redo()
+      {
+      initialClef = staff->clefTypeList(0);
+      StaffType st = *staff->staffType();
+      staff->setStaffType(&staffType);
+      staffType = st;
+      }
+
+void ChangeStaffType::undo()
       {
       StaffType st = *staff->staffType();
       staff->setStaffType(&staffType);
       staffType = st;
+
+      // restore initial clef, both in the staff clef map...
+      staff->setClef(0, initialClef);
+
+      // ...and in the score itself (code mostly copied from undoChangeClef() )
+      // TODO : add a single function adding/setting a clef change in score?
+      // possibly directly in ClefList?
+      int tick = 0;
+      Score* score = staff->score();
+      Measure* measure = score->tick2measure(tick);
+      if (!measure) {
+            qDebug("measure for tick %d not found!", tick);
+            return;
+            }
+      Segment* seg = measure->findSegment(Segment::SegClef, tick);
+      int track    = staff->idx() * VOICES;
+      Clef* clef   = static_cast<Clef*>(seg->element(track));
+      if (clef) {
+            clef->setGenerated(false);
+            clef->setClefType(initialClef);
+            }
+      else {
+            clef = new Clef(score);
+            clef->setTrack(track);
+            clef->setClefType(initialClef);
+            clef->setParent(seg);
+            seg->add(clef);
+            }
+//      cmdUpdateNotes();                         // needed?
       }
 
 //---------------------------------------------------------

diff --git a/libmscore/undo.h b/libmscore/undo.h
@@ -752,13 +752,14 @@ class ChangeStaff : public UndoCommand {
 //---------------------------------------------------------
 
 class ChangeStaffType : public UndoCommand {
-      Staff*     staff;
-      StaffType  staffType;
-
-      void flip();
+      ClefTypeList initialClef;
+      Staff*       staff;
+      StaffType    staffType;
 
    public:
       ChangeStaffType(Staff* s, const StaffType& t) : staff(s), staffType(t) {}
+      virtual void undo();
+      virtual void redo();
       UNDO_NAME("ChangeStaffType")
       };
 

diff --git a/mtest/guitarpro/arpeggio_up_down.gp4-ref.mscx b/mtest/guitarpro/arpeggio_up_down.gp4-ref.mscx
@@ -408,7 +408,7 @@
       <Staff id="2">
         <Measure number="1">
           <Clef>
-            <concertClefType>G8vb</concertClefType>
+            <concertClefType>TAB2</concertClefType>
             <transposingClefType>TAB2</transposingClefType>
             </Clef>
           <KeySig>

diff --git a/mtest/guitarpro/basic-bend.gp5-ref.mscx b/mtest/guitarpro/basic-bend.gp5-ref.mscx
@@ -304,7 +304,7 @@
       <Staff id="2">
         <Measure number="1">
           <Clef>
-            <concertClefType>G8vb</concertClefType>
+            <concertClefType>TAB2</concertClefType>
             <transposingClefType>TAB2</transposingClefType>
             </Clef>
           <KeySig>

diff --git a/mtest/guitarpro/bend.gp3-ref.mscx b/mtest/guitarpro/bend.gp3-ref.mscx
@@ -494,7 +494,7 @@
       <Staff id="2">
         <Measure number="1">
           <Clef>
-            <concertClefType>G8vb</concertClefType>
+            <concertClefType>TAB2</concertClefType>
             <transposingClefType>TAB2</transposingClefType>
             </Clef>
           <Chord>

diff --git a/mtest/guitarpro/bend.gp4-ref.mscx b/mtest/guitarpro/bend.gp4-ref.mscx
@@ -502,7 +502,7 @@
       <Staff id="2">
         <Measure number="1">
           <Clef>
-            <concertClefType>G8vb</concertClefType>
+            <concertClefType>TAB2</concertClefType>
             <transposingClefType>TAB2</transposingClefType>
             </Clef>
           <KeySig>

diff --git a/mtest/guitarpro/bend.gp5-ref.mscx b/mtest/guitarpro/bend.gp5-ref.mscx
@@ -502,7 +502,7 @@
       <Staff id="2">
         <Measure number="1">
           <Clef>
-            <concertClefType>G8vb</concertClefType>
+            <concertClefType>TAB2</concertClefType>
             <transposingClefType>TAB2</transposingClefType>
             </Clef>
           <KeySig>

diff --git a/mtest/guitarpro/copyright.gp3-ref.mscx b/mtest/guitarpro/copyright.gp3-ref.mscx
@@ -275,7 +275,7 @@ All Rights Reserved - International Copyright Secured</metaTag>
       <Staff id="2">
         <Measure number="1">
           <Clef>
-            <concertClefType>G8vb</concertClefType>
+            <concertClefType>TAB2</concertClefType>
             <transposingClefType>TAB2</transposingClefType>
             </Clef>
           <Chord>

diff --git a/mtest/guitarpro/copyright.gp4-ref.mscx b/mtest/guitarpro/copyright.gp4-ref.mscx
@@ -283,7 +283,7 @@ All Rights Reserved - International Copyright Secured</metaTag>
       <Staff id="2">
         <Measure number="1">
           <Clef>
-            <concertClefType>G8vb</concertClefType>
+            <concertClefType>TAB2</concertClefType>
             <transposingClefType>TAB2</transposingClefType>
             </Clef>
           <KeySig>

diff --git a/mtest/guitarpro/copyright.gp5-ref.mscx b/mtest/guitarpro/copyright.gp5-ref.mscx
@@ -283,7 +283,7 @@ All Rights Reserved - International Copyright Secured</metaTag>
       <Staff id="2">
         <Measure number="1">
           <Clef>
-            <concertClefType>G8vb</concertClefType>
+            <concertClefType>TAB2</concertClefType>
             <transposingClefType>TAB2</transposingClefType>
             </Clef>
           <KeySig>

diff --git a/mtest/guitarpro/dynamic.gp5-ref.mscx b/mtest/guitarpro/dynamic.gp5-ref.mscx
@@ -492,7 +492,7 @@
       <Staff id="2">
         <Measure number="1">
           <Clef>
-            <concertClefType>G8vb</concertClefType>
+            <concertClefType>TAB2</concertClefType>
             <transposingClefType>TAB2</transposingClefType>
             </Clef>
           <KeySig>

diff --git a/mtest/guitarpro/ghost_note.gp3-ref.mscx b/mtest/guitarpro/ghost_note.gp3-ref.mscx
@@ -560,7 +560,7 @@
       <Staff id="2">
         <Measure number="1">
           <Clef>
-            <concertClefType>F8vb</concertClefType>
+            <concertClefType>TAB2</concertClefType>
             <transposingClefType>TAB2</transposingClefType>
             </Clef>
           <Chord>

diff --git a/mtest/guitarpro/grace.gp5-ref.mscx b/mtest/guitarpro/grace.gp5-ref.mscx
@@ -763,7 +763,7 @@
       <Staff id="2">
         <Measure number="1">
           <Clef>
-            <concertClefType>G</concertClefType>
+            <concertClefType>TAB2</concertClefType>
             <transposingClefType>TAB2</transposingClefType>
             </Clef>
           <KeySig>

diff --git a/mtest/guitarpro/heavy-accent.gp5-ref.mscx b/mtest/guitarpro/heavy-accent.gp5-ref.mscx
@@ -294,7 +294,7 @@
       <Staff id="2">
         <Measure number="1">
           <Clef>
-            <concertClefType>G8vb</concertClefType>
+            <concertClefType>TAB2</concertClefType>
             <transposingClefType>TAB2</transposingClefType>
             </Clef>
           <KeySig>

diff --git a/mtest/guitarpro/sforzato.gp4-ref.mscx b/mtest/guitarpro/sforzato.gp4-ref.mscx
@@ -308,7 +308,7 @@
       <Staff id="2">
         <Measure number="1">
           <Clef>
-            <concertClefType>G8vb</concertClefType>
+            <concertClefType>TAB2</concertClefType>
             <transposingClefType>TAB2</transposingClefType>
             </Clef>
           <KeySig>

diff --git a/mtest/guitarpro/slur.gp4-ref.mscx b/mtest/guitarpro/slur.gp4-ref.mscx
@@ -322,7 +322,7 @@
       <Staff id="2">
         <Measure number="1">
           <Clef>
-            <concertClefType>G8vb</concertClefType>
+            <concertClefType>TAB2</concertClefType>
             <transposingClefType>TAB2</transposingClefType>
             </Clef>
           <KeySig>

diff --git a/mtest/guitarpro/tempo.gp3-ref.mscx b/mtest/guitarpro/tempo.gp3-ref.mscx
@@ -524,7 +524,7 @@
       <Staff id="2">
         <Measure number="1">
           <Clef>
-            <concertClefType>G8vb</concertClefType>
+            <concertClefType>TAB2</concertClefType>
             <transposingClefType>TAB2</transposingClefType>
             </Clef>
           <Chord>

diff --git a/mtest/guitarpro/tempo.gp4-ref.mscx b/mtest/guitarpro/tempo.gp4-ref.mscx
@@ -532,7 +532,7 @@
       <Staff id="2">
         <Measure number="1">
           <Clef>
-            <concertClefType>G8vb</concertClefType>
+            <concertClefType>TAB2</concertClefType>
             <transposingClefType>TAB2</transposingClefType>
             </Clef>
           <KeySig>

diff --git a/mtest/guitarpro/tempo.gp5-ref.mscx b/mtest/guitarpro/tempo.gp5-ref.mscx
@@ -532,7 +532,7 @@
       <Staff id="2">
         <Measure number="1">
           <Clef>
-            <concertClefType>G8vb</concertClefType>
+            <concertClefType>TAB2</concertClefType>
             <transposingClefType>TAB2</transposingClefType>
             </Clef>
           <KeySig>

diff --git a/mtest/guitarpro/testIrrTuplet.gp4-ref.mscx b/mtest/guitarpro/testIrrTuplet.gp4-ref.mscx
@@ -406,7 +406,7 @@
       <Staff id="2">
         <Measure number="1">
           <Clef>
-            <concertClefType>G8vb</concertClefType>
+            <concertClefType>TAB2</concertClefType>
             <transposingClefType>TAB2</transposingClefType>
             </Clef>
           <KeySig>

diff --git a/mtest/guitarpro/tremolos.gp5-ref.mscx b/mtest/guitarpro/tremolos.gp5-ref.mscx
@@ -324,7 +324,7 @@
       <Staff id="2">
         <Measure number="1">
           <Clef>
-            <concertClefType>G8vb</concertClefType>
+            <concertClefType>TAB2</concertClefType>
             <transposingClefType>TAB2</transposingClefType>
             </Clef>
           <KeySig>

diff --git a/mtest/guitarpro/trill.gp4-ref.mscx b/mtest/guitarpro/trill.gp4-ref.mscx
@@ -294,7 +294,7 @@
       <Staff id="2">
         <Measure number="1">
           <Clef>
-            <concertClefType>G8vb</concertClefType>
+            <concertClefType>TAB2</concertClefType>
             <transposingClefType>TAB2</transposingClefType>
             </Clef>
           <KeySig>

diff --git a/mtest/guitarpro/volta.gp3-ref.mscx b/mtest/guitarpro/volta.gp3-ref.mscx
@@ -1778,7 +1778,7 @@
       <Staff id="2">
         <Measure number="1">
           <Clef>
-            <concertClefType>G8vb</concertClefType>
+            <concertClefType>TAB2</concertClefType>
             <transposingClefType>TAB2</transposingClefType>
             </Clef>
           <Chord>

diff --git a/mtest/guitarpro/volta.gp4-ref.mscx b/mtest/guitarpro/volta.gp4-ref.mscx
@@ -1458,7 +1458,7 @@
       <Staff id="2">
         <Measure number="1">
           <Clef>
-            <concertClefType>G8vb</concertClefType>
+            <concertClefType>TAB2</concertClefType>
             <transposingClefType>TAB2</transposingClefType>
             </Clef>
           <KeySig>