Merge pull request #25674 from RomanPudashkin/replace_instrument_crash

replace_instrument_crash
musescore · Dec 3, 2024 · 193e609 · 193e609
2 parents 20ecf84 + adbf731
commit 193e609
Show file tree

Hide file tree

Showing 18 changed files with 78 additions and 43 deletions.
diff --git a/src/engraving/dom/note.cpp b/src/engraving/dom/note.cpp
@@ -1939,7 +1939,11 @@ EngravingItem* Note::drop(EditData& data)
         DirectionV stemDirection = DirectionV::AUTO;
         if (staffGroup == StaffGroup::PERCUSSION) {
             const Drumset* ds = st->part()->instrument(segment->tick())->drumset();
-            stemDirection = ds->stemDirection(n->noteVal().pitch);
+            DO_ASSERT(ds);
+
+            if (ds) {
+                stemDirection = ds->stemDirection(n->noteVal().pitch);
+            }
         }
         ch->setStemDirection(stemDirection);
 

diff --git a/src/engraving/dom/noteentry.cpp b/src/engraving/dom/noteentry.cpp
@@ -434,7 +434,11 @@ Ret Score::putNote(const Position& p, bool replace)
     switch (staffGroup) {
     case StaffGroup::PERCUSSION: {
         const Drumset* ds = st->part()->instrument(s->tick())->drumset();
-        stemDirection     = ds->stemDirection(nval.pitch);
+        DO_ASSERT(ds);
+
+        if (ds) {
+            stemDirection = ds->stemDirection(nval.pitch);
+        }
         break;
     }
     case StaffGroup::TAB:

diff --git a/src/engraving/dom/paste.cpp b/src/engraving/dom/paste.cpp
@@ -312,7 +312,11 @@ static Note* prepareTarget(ChordRest* target, Note* with, const Fraction& durati
     DirectionV stemDirection = DirectionV::AUTO;
     if (staffGroup == StaffGroup::PERCUSSION) {
         const Drumset* ds = staff->part()->instrument(segment->tick())->drumset();
-        stemDirection = ds->stemDirection(with->noteVal().pitch);
+        DO_ASSERT(ds);
+
+        if (ds) {
+            stemDirection = ds->stemDirection(with->noteVal().pitch);
+        }
     }
 
     segment = target->score()->setNoteRest(segment, target->track(),

diff --git a/src/instrumentsscene/internal/instrumentsactionscontroller.cpp b/src/instrumentsscene/internal/instrumentsactionscontroller.cpp
@@ -74,11 +74,12 @@ void InstrumentsActionsController::changeInstrument()
     key.partId = instrumentChange->part()->id();
     key.tick = instrumentChange->tick();
 
-    RetVal<Instrument> instrument = selectInstrumentsScenario()->selectInstrument(key);
-    if (!instrument.ret) {
-        LOGE() << instrument.ret.toString();
+    RetVal<InstrumentTemplate> templ = selectInstrumentsScenario()->selectInstrument(key);
+    if (!templ.ret) {
+        LOGE() << templ.ret.toString();
         return;
     }
 
-    master->parts()->replaceInstrument(key, instrument.val);
+    Instrument instrument = Instrument::fromTemplate(&templ.val);
+    master->parts()->replaceInstrument(key, instrument);
 }
diff --git a/src/instrumentsscene/internal/selectinstrumentscenario.cpp b/src/instrumentsscene/internal/selectinstrumentscenario.cpp
@@ -36,7 +36,7 @@ RetVal<PartInstrumentListScoreOrder> SelectInstrumentsScenario::selectInstrument
     return selectInstruments(params);
 }
 
-RetVal<Instrument> SelectInstrumentsScenario::selectInstrument(const InstrumentKey& currentInstrumentKey) const
+RetVal<InstrumentTemplate> SelectInstrumentsScenario::selectInstrument(const InstrumentKey& currentInstrumentKey) const
 {
     StringList params {
         u"canSelectMultipleInstruments=false",
@@ -49,8 +49,7 @@ RetVal<Instrument> SelectInstrumentsScenario::selectInstrument(const InstrumentK
     }
 
     const InstrumentTemplate& templ = selectedInstruments.val.instruments.first().instrumentTemplate;
-
-    return RetVal<Instrument>::make_ok(Instrument::fromTemplate(&templ));
+    return RetVal<InstrumentTemplate>::make_ok(templ);
 }
 
 RetVal<PartInstrumentListScoreOrder> SelectInstrumentsScenario::selectInstruments(const StringList& params) const

diff --git a/src/instrumentsscene/internal/selectinstrumentscenario.h b/src/instrumentsscene/internal/selectinstrumentscenario.h
@@ -35,9 +35,8 @@ class SelectInstrumentsScenario : public notation::ISelectInstrumentsScenario
 
 public:
     muse::RetVal<notation::PartInstrumentListScoreOrder> selectInstruments() const override;
-    muse::RetVal<notation::Instrument> selectInstrument(const notation::InstrumentKey& currentInstrumentId = notation::InstrumentKey())
-    const
-    override;
+    muse::RetVal<notation::InstrumentTemplate> selectInstrument(
+        const notation::InstrumentKey& currentInstrumentId = notation::InstrumentKey()) const override;
 
 private:
     muse::RetVal<notation::PartInstrumentListScoreOrder> selectInstruments(const muse::StringList& params) const;

diff --git a/src/instrumentsscene/view/parttreeitem.cpp b/src/instrumentsscene/view/parttreeitem.cpp
@@ -200,14 +200,20 @@ void PartTreeItem::replaceInstrument()
     instrumentKey.instrumentId = m_instrumentId;
     instrumentKey.tick = Part::MAIN_INSTRUMENT_TICK;
 
-    RetVal<Instrument> selectedInstrument = selectInstrumentsScenario()->selectInstrument(instrumentKey);
-    if (!selectedInstrument.ret) {
-        LOGE() << selectedInstrument.ret.toString();
+    RetVal<InstrumentTemplate> templ = selectInstrumentsScenario()->selectInstrument(instrumentKey);
+    if (!templ.ret) {
+        LOGE() << templ.ret.toString();
         return;
     }
 
-    const Instrument& newInstrument = selectedInstrument.val;
-    masterNotation()->parts()->replaceInstrument(instrumentKey, newInstrument);
+    Instrument instrument = Instrument::fromTemplate(&templ.val);
+
+    const StaffType* staffType = templ.val.staffTypePreset;
+    if (!staffType) {
+        staffType = StaffType::getDefaultPreset(templ.val.staffGroup);
+    }
+
+    masterNotation()->parts()->replaceInstrument(instrumentKey, instrument, staffType);
 }
 
 void PartTreeItem::resetAllFormatting()

diff --git a/src/notation/inotationparts.h b/src/notation/inotationparts.h
@@ -77,7 +77,8 @@ class INotationParts
     virtual void insertPart(Part* part, size_t index) = 0;
 
     virtual void replacePart(const muse::ID& partId, Part* newPart) = 0;
-    virtual void replaceInstrument(const InstrumentKey& instrumentKey, const Instrument& newInstrument) = 0;
+    virtual void replaceInstrument(const InstrumentKey& instrumentKey, const Instrument& newInstrument,
+                                   const StaffType* newStaffType = nullptr) = 0;
     virtual void replaceDrumset(const InstrumentKey& instrumentKey, const Drumset& newDrumset, bool undoable = true) = 0;
 
     virtual muse::async::Notification partsChanged() const = 0;

diff --git a/src/notation/internal/masternotationparts.cpp b/src/notation/internal/masternotationparts.cpp
@@ -184,7 +184,8 @@ bool MasterNotationParts::appendLinkedStaff(Staff* staff, const muse::ID& source
     return true;
 }
 
-void MasterNotationParts::replaceInstrument(const InstrumentKey& instrumentKey, const Instrument& newInstrument)
+void MasterNotationParts::replaceInstrument(const InstrumentKey& instrumentKey, const Instrument& newInstrument,
+                                            const StaffType* newStaffType)
 {
     TRACEFUNC;
 
@@ -195,10 +196,10 @@ void MasterNotationParts::replaceInstrument(const InstrumentKey& instrumentKey,
 
     mu::engraving::Interval oldTranspose = part ? part->instrument()->transpose() : mu::engraving::Interval(0, 0);
 
-    NotationParts::replaceInstrument(instrumentKey, newInstrument);
+    NotationParts::replaceInstrument(instrumentKey, newInstrument, newStaffType);
 
     for (INotationPartsPtr parts : excerptsParts()) {
-        parts->replaceInstrument(instrumentKey, newInstrument);
+        parts->replaceInstrument(instrumentKey, newInstrument, newStaffType);
     }
 
     // this also transposes all linked parts

diff --git a/src/notation/internal/masternotationparts.h b/src/notation/internal/masternotationparts.h
@@ -42,7 +42,8 @@ class MasterNotationParts : public NotationParts
     bool appendStaff(Staff* staff, const muse::ID& destinationPartId) override;
     bool appendLinkedStaff(Staff* staff, const muse::ID& sourceStaffId, const muse::ID& destinationPartId) override;
 
-    void replaceInstrument(const InstrumentKey& instrumentKey, const Instrument& newInstrument) override;
+    void replaceInstrument(const InstrumentKey& instrumentKey, const Instrument& newInstrument,
+                           const StaffType* newStaffType = nullptr) override;
     void replaceDrumset(const InstrumentKey& instrumentKey, const Drumset& newDrumset, bool undoable = true) override;
 
 private:

diff --git a/src/notation/internal/notationinteraction.cpp b/src/notation/internal/notationinteraction.cpp
@@ -1640,13 +1640,14 @@ bool NotationInteraction::selectInstrument(mu::engraving::InstrumentChange* inst
         return false;
     }
 
-    RetVal<Instrument> selectedInstrument = selectInstrumentScenario()->selectInstrument();
-    if (!selectedInstrument.ret) {
+    RetVal<InstrumentTemplate> templ = selectInstrumentScenario()->selectInstrument();
+    if (!templ.ret) {
         return false;
     }
 
+    Instrument newInstrument = Instrument::fromTemplate(&templ.val);
     instrumentChange->setInit(true);
-    instrumentChange->setupInstrument(&selectedInstrument.val);
+    instrumentChange->setupInstrument(&newInstrument);
 
     return true;
 }

diff --git a/src/notation/internal/notationparts.cpp b/src/notation/internal/notationparts.cpp
@@ -598,7 +598,7 @@ void NotationParts::replacePart(const ID& partId, Part* newPart)
     notifyAboutPartReplaced(part, newPart);
 }
 
-void NotationParts::replaceInstrument(const InstrumentKey& instrumentKey, const Instrument& newInstrument)
+void NotationParts::replaceInstrument(const InstrumentKey& instrumentKey, const Instrument& newInstrument, const StaffType* newStaffType)
 {
     TRACEFUNC;
 
@@ -617,15 +617,16 @@ void NotationParts::replaceInstrument(const InstrumentKey& instrumentKey, const
         for (staff_idx_t staffIdx = 0; staffIdx < part->nstaves(); ++staffIdx) {
             Staff* staff = part->staves().at(staffIdx);
             StaffConfig config = staffConfig(staff->id());
+            StaffConfig newConfig = config;
 
-            mu::engraving::ClefTypeList newClefTypeList = newInstrument.clefType(staffIdx);
-
-            if (config.clefTypeList == newClefTypeList) {
-                continue;
+            newConfig.clefTypeList = newInstrument.clefType(staffIdx);
+            if (newStaffType) {
+                newConfig.staffType = *newStaffType;
             }
 
-            config.clefTypeList = newClefTypeList;
-            doSetStaffConfig(staff, config);
+            if (config != newConfig) {
+                doSetStaffConfig(staff, newConfig);
+            }
         }
     } else {
         mu::engraving::InstrumentChange* instrumentChange = findInstrumentChange(part, instrumentKey.tick);

diff --git a/src/notation/internal/notationparts.h b/src/notation/internal/notationparts.h
@@ -71,7 +71,8 @@ class NotationParts : public INotationParts, public muse::async::Asyncable
     void insertPart(Part* part, size_t index) override;
 
     void replacePart(const muse::ID& partId, Part* newPart) override;
-    void replaceInstrument(const InstrumentKey& instrumentKey, const Instrument& newInstrument) override;
+    void replaceInstrument(const InstrumentKey& instrumentKey, const Instrument& newInstrument,
+                           const StaffType* newStaffType = nullptr) override;
     void replaceDrumset(const InstrumentKey& instrumentKey, const Drumset& newDrumset, bool undoable = true) override;
 
     muse::async::Notification partsChanged() const override;

diff --git a/src/notation/iselectinstrumentscenario.h b/src/notation/iselectinstrumentscenario.h
@@ -35,7 +35,7 @@ class ISelectInstrumentsScenario : MODULE_EXPORT_INTERFACE
     virtual ~ISelectInstrumentsScenario() = default;
 
     virtual muse::RetVal<PartInstrumentListScoreOrder> selectInstruments() const = 0;
-    virtual muse::RetVal<Instrument> selectInstrument(const InstrumentKey& currentInstrumentKey = InstrumentKey()) const = 0;
+    virtual muse::RetVal<InstrumentTemplate> selectInstrument(const InstrumentKey& currentInstrumentKey = InstrumentKey()) const = 0;
 };
 }
 

diff --git a/src/notation/notationtypes.h b/src/notation/notationtypes.h
@@ -446,6 +446,11 @@ struct StaffConfig
 
         return equal;
     }
+
+    bool operator!=(const StaffConfig& conf) const
+    {
+        return !(*this == conf);
+    }
 };
 
 struct TransposeOptions

diff --git a/src/notation/view/widgets/editstaff.cpp b/src/notation/view/widgets/editstaff.cpp
@@ -579,14 +579,22 @@ void EditStaff::applyPartProperties()
 
 void EditStaff::showReplaceInstrumentDialog()
 {
-    RetVal<Instrument> selectedInstrument = selectInstrumentsScenario()->selectInstrument(m_instrumentKey);
-    if (!selectedInstrument.ret) {
-        LOGE() << selectedInstrument.ret.toString();
+    RetVal<InstrumentTemplate> templ = selectInstrumentsScenario()->selectInstrument(m_instrumentKey);
+    if (!templ.ret) {
+        LOGE() << templ.ret.toString();
         return;
     }
 
-    m_instrument = selectedInstrument.val;
+    const StaffType* staffType = templ.val.staffTypePreset;
+    if (!staffType) {
+        staffType = StaffType::getDefaultPreset(StaffGroup::STANDARD);
+    }
+
+    m_instrument = Instrument::fromTemplate(&templ.val);
+    m_staff->setStaffType(Fraction(0, 1), *staffType);
+
     updateInstrument();
+    updateStaffType(*staffType);
 }
 
 void EditStaff::editStringDataClicked()

diff --git a/src/stubs/instrumentsscene/selectinstrumentscenariostub.cpp b/src/stubs/instrumentsscene/selectinstrumentscenariostub.cpp
@@ -30,7 +30,7 @@ RetVal<PartInstrumentListScoreOrder> SelectInstrumentsScenarioStub::selectInstru
     return make_ret(Ret::Code::NotSupported);
 }
 
-RetVal<Instrument> SelectInstrumentsScenarioStub::selectInstrument(const notation::InstrumentKey&) const
+RetVal<InstrumentTemplate> SelectInstrumentsScenarioStub::selectInstrument(const notation::InstrumentKey&) const
 {
     return make_ret(Ret::Code::NotSupported);
 }
diff --git a/src/stubs/instrumentsscene/selectinstrumentscenariostub.h b/src/stubs/instrumentsscene/selectinstrumentscenariostub.h
@@ -29,9 +29,8 @@ class SelectInstrumentsScenarioStub : public notation::ISelectInstrumentsScenari
 {
 public:
     muse::RetVal<notation::PartInstrumentListScoreOrder> selectInstruments() const override;
-    muse::RetVal<notation::Instrument> selectInstrument(const notation::InstrumentKey& currentInstrumentKey = notation::InstrumentKey())
-    const
-    override;
+    muse::RetVal<notation::InstrumentTemplate> selectInstrument(
+        const notation::InstrumentKey& currentInstrumentKey = notation::InstrumentKey()) const override;
 };
 }