Merge pull request #4268 from dmitrio95/87241-corrupted-files-and-sel…

…ection

fix #87241, fix #279064: Remove invalid pointers to elements from selection and avoid complete data loss in case of crash on saving .mscz file

libmscore/cmd.cpp

@@ -2481,16 +2481,14 @@ void Score::cmdExplode()
                   lastStaff = qMin(nstaves(), srcStaff + n);
                   }
 
-            // make our own copy of selection, since pasting modifies actual selection
-            Selection srcSelection(selection());
-
+            const QByteArray mimeData(selection().mimeData());
             // copy to all destination staves
             Segment* firstCRSegment = startMeasure->tick2segment(startMeasure->tick());
             for (int i = 1; srcStaff + i < lastStaff; ++i) {
                   int track = (srcStaff + i) * VOICES;
                   ChordRest* cr = toChordRest(firstCRSegment->element(track));
                   if (cr) {
-                        XmlReader e(srcSelection.mimeData());
+                        XmlReader e(mimeData);
                         e.setPasteMode(true);
                         pasteStaff(e, cr->segment(), cr->staffIdx());
                         }

libmscore/edit.cpp

@@ -1254,7 +1254,7 @@ void Score::cmdAddTie()
 
 void Score::cmdAddOttava(OttavaType type)
       {
-      Selection sel = selection();
+      const Selection& sel = selection();
       // add on each staff if possible
       if (sel.isRange() && sel.staffStart() != sel.staffEnd() - 1) {
             for (int staffIdx = sel.staffStart() ; staffIdx < sel.staffEnd(); ++staffIdx) {

libmscore/element.cpp

@@ -186,14 +186,7 @@ Element::Element(const Element& e)
 
 Element::~Element()
       {
-#if 0
-      if (score() &&  flag(ElementFlag::SELECTED)) {
-            if (score()->selection().elements().removeOne(this))
-                  printf("remove element from selection\n");
-            else
-                  printf("element not in selection\n");
-            }
-#endif
+      Score::onElementDestruction(this);
       }
 
 //---------------------------------------------------------

libmscore/layout.cpp

@@ -3865,8 +3865,7 @@ void Score::doLayoutRange(int stick, int etick)
             for (System* s : _systems) {
                   for (Bracket* b : s->brackets()) {
                         if (b->selected()) {
-                              _selection.elements().removeOne(b);
-                              _selection.updateState();
+                              _selection.remove(b);
                               setSelectionChanged(true);
                               }
                         }

libmscore/score.cpp

@@ -74,6 +74,7 @@
 namespace Ms {
 
 MasterScore* gscore;                 ///< system score, used for palettes etc.
+std::set<Score*> Score::validScores;
 
 bool scriptDebug     = false;
 bool noSeq           = false;
@@ -244,6 +245,7 @@ void MeasureBaseList::change(MeasureBase* ob, MeasureBase* nb)
 Score::Score()
    : ScoreElement(this), _is(this), _selection(this), _selectionFilter(this)
       {
+      Score::validScores.insert(this);
       _masterScore = 0;
       Layer l;
       l.name          = "default";
@@ -265,6 +267,7 @@ Score::Score()
 Score::Score(MasterScore* parent)
    : Score{}
       {
+      Score::validScores.insert(this);
       _masterScore = parent;
       if (MScore::defaultStyleForParts())
             _style = *MScore::defaultStyleForParts();
@@ -301,6 +304,7 @@ Score::Score(MasterScore* parent)
 Score::Score(MasterScore* parent, const MStyle& s)
    : Score{parent}
       {
+      Score::validScores.erase(this);
       _style  = s;
       }
 
@@ -353,6 +357,22 @@ Score* Score::clone()
       return score;
       }
 
+//---------------------------------------------------------
+//   Score::onElementDestruction
+//    Ensure correct state of the score after destruction
+//    of the element (e.g. remove invalid pointers etc.).
+//---------------------------------------------------------
+
+void Score::onElementDestruction(Element* e)
+      {
+      Score* score = e->score();
+      if (!score || Score::validScores.find(score) == Score::validScores.end()) {
+            // No score or the score is already deleted
+            return;
+            }
+      score->selection().remove(e);
+      }
+
 //---------------------------------------------------------
 //   addMeasure
 //---------------------------------------------------------

libmscore/score.h

@@ -387,6 +387,7 @@ class Score : public QObject, public ScoreElement {
             };
 
    private:
+      static std::set<Score*> validScores;
       int _linkId { 0 };
       MasterScore* _masterScore { 0 };
       QList<MuseScoreView*> viewer;
@@ -560,6 +561,8 @@ class Score : public QObject, public ScoreElement {
       virtual bool isMaster() const  { return false;        }
       virtual bool readOnly() const;
 
+      static void onElementDestruction(Element* se);
+
       virtual inline QList<Excerpt*>& excerpts();
       virtual inline const QList<Excerpt*>& excerpts() const;
 
@@ -758,7 +761,7 @@ class Score : public QObject, public ScoreElement {
       bool saveFile(QFileInfo& info);
       bool saveFile(QIODevice* f, bool msczFormat, bool onlySelection = false);
       bool saveCompressedFile(QFileInfo&, bool onlySelection);
-      bool saveCompressedFile(QIODevice*, QFileInfo&, bool onlySelection, bool createThumbnail = true);
+      bool saveCompressedFile(QFileDevice*, QFileInfo&, bool onlySelection, bool createThumbnail = true);
       bool exportFile();
 
       void print(QPainter* printer, int page);

libmscore/scorefile.cpp

@@ -546,7 +546,7 @@ QImage Score::createThumbnail()
 //    file is already opened
 //---------------------------------------------------------
 
-bool Score::saveCompressedFile(QIODevice* f, QFileInfo& info, bool onlySelection, bool doCreateThumbnail)
+bool Score::saveCompressedFile(QFileDevice* f, QFileInfo& info, bool onlySelection, bool doCreateThumbnail)
       {
       MQZipWriter uz(f);
 
@@ -572,6 +572,14 @@ bool Score::saveCompressedFile(QIODevice* f, QFileInfo& info, bool onlySelection
       //uz.addDirectory("META-INF");
       uz.addFile("META-INF/container.xml", cbuf.data());
 
+      QBuffer dbuf;
+      dbuf.open(QIODevice::ReadWrite);
+      saveFile(&dbuf, true, onlySelection);
+      dbuf.seek(0);
+      uz.addFile(fn, dbuf.data());
+      f->flush(); // flush to preserve score data in case of
+                  // any failures on the further operations.
+
       // save images
       //uz.addDirectory("Pictures");
       foreach (ImageStoreItem* ip, imageStore) {
@@ -620,11 +628,6 @@ bool Score::saveCompressedFile(QIODevice* f, QFileInfo& info, bool onlySelection
       if (_audio)
             uz.addFile("audio.ogg", _audio->data());
 
-      QBuffer dbuf;
-      dbuf.open(QIODevice::ReadWrite);
-      saveFile(&dbuf, true, onlySelection);
-      dbuf.seek(0);
-      uz.addFile(fn, dbuf.data());
       uz.close();
       return true;
       }

libmscore/select.cpp

@@ -319,9 +319,10 @@ void Selection::clear()
 
 void Selection::remove(Element* el)
       {
-      _el.removeOne(el);
+      const bool removed = _el.removeOne(el);
       el->setSelected(false);
-      updateState();
+      if (removed)
+            updateState();
       }
 
 //---------------------------------------------------------

libmscore/select.h

@@ -157,7 +157,6 @@ class Selection {
       void setState(SelState s);
 
       const QList<Element*>& elements() const { return _el; }
-      QList<Element*>& elements()             { return _el; }
       std::vector<Note*> noteList(int track = -1) const;
 
       const QList<Element*> uniqueElements() const;

mscore/palette.cpp

@@ -451,7 +451,7 @@ void Palette::applyPaletteElement(PaletteCell* cell, Qt::KeyboardModifiers modif
       Score* score = mscore->currentScore();
       if (score == 0)
             return;
-      Selection sel = score->selection();       // make a copy of the list
+      const Selection& sel = score->selection();       // make a copy of the list
       if (sel.isNone())
             return;
 
@@ -753,8 +753,7 @@ void Palette::mouseDoubleClickEvent(QMouseEvent* ev)
       Score* score = mscore->currentScore();
       if (score == 0)
             return;
-      Selection sel = score->selection();       // make a copy of the list
-      if (sel.isNone())
+      if (score->selection().isNone())
             return;
 
       applyPaletteElement(cellAt(i), ev->modifiers());

mscore/pianotools.cpp

@@ -184,7 +184,7 @@ void HPiano::releasePitch(int pitch)
 //   changeSelection
 //---------------------------------------------------------
 
-void HPiano::changeSelection(Selection selection)
+void HPiano::changeSelection(const Selection& selection)
       {
       for (PianoKeyItem* key : keys) {
             key->setHighlighted(false);
@@ -533,7 +533,7 @@ bool HPiano::gestureEvent(QGestureEvent *event)
 //   changeSelection
 //---------------------------------------------------------
 
-void PianoTools::changeSelection(Selection selection)
+void PianoTools::changeSelection(const Selection& selection)
       {
       _piano->changeSelection(selection);
       }

mscore/pianotools.h

@@ -79,7 +79,7 @@ class HPiano : public QGraphicsView {
       void pressPitch(int pitch);
       void releasePitch(int pitch);
       void clearSelection();
-      void changeSelection(Selection selection);
+      void changeSelection(const Selection& selection);
       void updateAllKeys();
       virtual QSize sizeHint() const;
 
@@ -110,7 +110,7 @@ class PianoTools : public QDockWidget {
       void releasePitch(int pitch)  { _piano->releasePitch(pitch); }
       void heartBeat(QList<const Note*> notes);
       void clearSelection();
-      void changeSelection(Selection selection);
+      void changeSelection(const Selection& selection);
       };
 
 

mscore/pianoview.cpp

@@ -937,7 +937,8 @@ void PianoView::selectNotes(int startTick, int endTick, int lowPitch, int highPi
       //score->masterScore()->cmdState().reset();      // DEBUG: should not be necessary
       score->startCmd();
 
-      Selection selection(score);
+      Selection& selection = score->selection();
+      selection.deselectAll();
 
       for (int i = 0; i < noteList.size(); ++i) {
             PianoItem* pi = noteList[i];
@@ -967,7 +968,6 @@ void PianoView::selectNotes(int startTick, int endTick, int lowPitch, int highPi
                   selection.add(pi->note());
             }
 
-      score->setSelection(selection);
       for (MuseScoreView* view : score->getViewer())
             view->updateAll();
 

mscore/scoreview.cpp

@@ -3308,7 +3308,7 @@ void ScoreView::cmdAddNoteLine()
 void ScoreView::cmdChangeEnharmonic(bool both)
       {
       _score->startCmd();
-      Selection selection = _score->selection();
+      Selection& selection = _score->selection();
       QList<Note*> notes = selection.uniqueNotes();
       for (Note* n : notes) {
             Staff* staff = n->staff();

mscore/timeline.cpp

@@ -1726,8 +1726,8 @@ void Timeline::drawSelection()
 
       std::set<std::tuple<Measure*, int, ElementType>> meta_labels_set;
 
-      const Selection selection = _score->selection();
-      QList<Element*> el = selection.elements();
+      const Selection& selection = _score->selection();
+      const QList<Element*>& el = selection.elements();
       for (Element* element : el) {
             if (element->tick() == -1)
                   continue;