Skip to content

Commit

Permalink
Fix crash when exporting to MusicXML after toggling MMRests
Browse files Browse the repository at this point in the history 
The problem: when MMRests are disabled, measures still keep a reference to their MMRest measure (in order to preserve that MMRest measure and its properties). This means that the `mmRest1` method might return this MMRest, even when MMRests are disabled (so the method would be expected to return the measure itself). This is fixed by checking inside that method if MMRests are actually enabled.

Additionally, this commit gives that method a slightly nicer name: `coveringMMRestOrThis()`.

Resolves: musescore#17819, backport of musescore#18038
  • Loading branch information
@cbjeukendrup @Jojo-Schmitz
cbjeukendrup authored and Jojo-Schmitz committed Jun 21, 2023
1 parent 53b10f0 commit 5ca2b43
Show file tree
Hide file tree
Showing 8 changed files with 39 additions and 34 deletions.
24 changes: 12 additions & 12 deletions importexport/musicxml/exportxml.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1804,7 +1804,7 @@ static void writeBarlineFermata(const BarLine* const barline, XmlWriter& xml, co

void ExportMusicXml::barlineRight(const Measure* const m, const int strack, const int etrack)
{
const Measure* mmR1 = m->mmRest1(); // the multi measure rest this measure is covered by
const Measure* mmR1 = m->coveringMMRestOrThis(); // the multi measure rest this measure is covered by
const Measure* mmRLst = mmR1->isMMRest() ? mmR1->mmRestLast() : 0; // last measure of replaced sequence of empty measures
// note: use barlinetype as found in multi measure rest for last measure of replaced sequence
BarLineType bst = m == mmRLst ? mmR1->endBarLineType() : m->endBarLineType();
Expand Down Expand Up @@ -5161,7 +5161,7 @@ static bool elementRighter(const Element* e1, const Element* e2)

static void measureStyle(XmlWriter& xml, Attributes& attr, const Measure* const m)
{
const Measure* mmR1 = m->mmRest1();
const Measure* mmR1 = m->coveringMMRestOrThis();
if (m != mmR1 && m == mmR1->mmRestFirst()) {
attr.doAttr(xml, true);
xml.stag("measure-style");
Expand Down Expand Up @@ -5808,7 +5808,7 @@ void ExportMusicXml::print(const Measure* const m, const int partNr, const int f
// in the replacing measure
// note: for a normal measure, mmRest1 is the measure itself,
// for a multi-meaure rest, it is the replacing measure
const Measure* mmR1 = m->mmRest1();
const Measure* mmR1 = m->coveringMMRestOrThis();
const System* system = mmR1->system();

// Put the system print suggestions only for the first part in a score...
Expand Down Expand Up @@ -6433,7 +6433,7 @@ static System* findLastSystemWithMeasures(const Page* const page)

static bool isFirstMeasureInSystem(const Measure* const measure)
{
const auto system = measure->mmRest1()->system();
const auto system = measure->coveringMMRestOrThis()->system();
const auto firstMeasureInSystem = system->firstMeasure();
const auto realFirstMeasureInSystem = firstMeasureInSystem->isMMRest() ? firstMeasureInSystem->mmRestFirst() : firstMeasureInSystem;
return measure == realFirstMeasureInSystem;
Expand All @@ -6444,12 +6444,12 @@ static bool isFirstMeasureInSystem(const Measure* const measure)

static bool isFirstMeasureInLastSystem(const Measure* const measure)
{
const auto system = measure->mmRest1()->system();
const auto system = measure->coveringMMRestOrThis()->system();
const auto page = system->page();

/*
Notes on multi-measure rest handling:
Function mmRest1() returns either the measure itself (if not part of multi-measure rest)
Function coveringMMRestOrThis() returns either the measure itself (if not part of multi-measure rest)
or the replacing multi-measure rest measure.
Using this is required as a measure that is covered by a multi-measure rest has no system.
Furthermore, the first measure in a system starting with a multi-measure rest is the a multi-
Expand Down Expand Up @@ -6479,7 +6479,7 @@ static bool systemHasMeasures(const System* const system)

static std::vector<TBox*> findTextFramesToWriteAsWordsAbove(const Measure* const measure)
{
const auto system = measure->mmRest1()->system();
const auto system = measure->coveringMMRestOrThis()->system();
const auto page = system->page();
const auto systemIndex = page->systems().indexOf(system);
std::vector<TBox*> tboxes;
Expand All @@ -6503,7 +6503,7 @@ static std::vector<TBox*> findTextFramesToWriteAsWordsAbove(const Measure* const

static std::vector<TBox*> findTextFramesToWriteAsWordsBelow(const Measure* const measure)
{
const auto system = measure->mmRest1()->system();
const auto system = measure->coveringMMRestOrThis()->system();
const auto page = system->page();
const auto systemIndex = page->systems().indexOf(system);
std::vector<TBox*> tboxes;
Expand Down Expand Up @@ -6574,17 +6574,17 @@ void ExportMusicXml::writeMeasureTracks(const Measure* const m,
const bool isLastPart = (partIndex == (_score->parts().size() - 1));
if (!tboxesAboveWritten && isFirstPart) {
for (const auto tbox : tboxesAbove) {
// note: use mmRest1() to get at a possible multi-measure rest,
// note: use coveringMMRestOrThis() to get at a possible multi-measure rest,
// as the covered measure would be positioned at 0,0.
tboxTextAsWords(tbox->text(), 0, tbox->text()->canvasPos() - m->mmRest1()->canvasPos());
tboxTextAsWords(tbox->text(), 0, tbox->text()->canvasPos() - m->coveringMMRestOrThis()->canvasPos());
}
tboxesAboveWritten = true;
}
if (!tboxesBelowWritten && isLastPart && (etrack - VOICES) <= st) {
for (const auto tbox : tboxesBelow) {
const auto lastStaffNr = st / VOICES;
const auto sys = m->mmRest1()->system();
auto textPos = tbox->text()->canvasPos() - m->mmRest1()->canvasPos();
const auto sys = m->coveringMMRestOrThis()->system();
auto textPos = tbox->text()->canvasPos() - m->coveringMMRestOrThis()->canvasPos();
if (lastStaffNr < sys->staves()->size()) {
// convert to position relative to last staff of system
textPos.setY(textPos.y() - (sys->staffCanvasYpage(lastStaffNr) - sys->staffCanvasYpage(0)));
Expand Down
2 changes: 1 addition & 1 deletion libmscore/cmd.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2522,7 +2522,7 @@ Element* Score::move(const QString& cmd)
ftm = firstTrailingMeasure(&cr) ? firstTrailingMeasure(&cr) : lastMeasure();
if (ftm) {
if (score()->styleB(Sid::createMultiMeasureRests) && ftm->hasMMRest())
ftm = ftm->mmRest1();
ftm = ftm->coveringMMRestOrThis();
el = !cr ? ftm->first()->nextChordRest(0, false) : ftm->first()->nextChordRest(trackZeroVoice(cr->track()), false);
}
// Note: Due to the nature of this command as being preparatory for input,
Expand Down
5 changes: 3 additions & 2 deletions libmscore/line.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -989,8 +989,9 @@ QPointF SLine::linePos(Grip grip, System** sys) const
}
}
}
if (score()->styleB(Sid::createMultiMeasureRests))
m = m->mmRest1();

m = m->coveringMMRestOrThis();

Q_ASSERT(m->system());
*sys = m->system();
}
Expand Down
13 changes: 9 additions & 4 deletions libmscore/measure.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3367,13 +3367,18 @@ Measure* Measure::mmRestLast() const
}

//---------------------------------------------------------
// mmRest1
// return the multi measure rest this measure is covered
// by
// coveringMMRestOrThis
// if multi-measure rests are enabled,
// and this measure is covered by an MMRest,
// return that MMRest.
// otherwise, return the measure itself.
//---------------------------------------------------------

const Measure* Measure::mmRest1() const
const Measure* Measure::coveringMMRestOrThis() const
{
if (!score()->styleB(Sid::createMultiMeasureRests))
return this;

if (_mmRest)
return _mmRest;
if (_mmRestCount != -1)
Expand Down
2 changes: 1 addition & 1 deletion libmscore/measure.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ class Measure final : public MeasureBase {
bool hasMMRest() const { return _mmRest != 0; }
bool isMMRest() const { return _mmRestCount > 0; }
Measure* mmRest() const { return _mmRest; }
const Measure* mmRest1() const;
const Measure* coveringMMRestOrThis() const;
void setMMRest(Measure* m) { _mmRest = m; }
int mmRestCount() const { return _mmRestCount; } // number of measures _mmRest spans
void setMMRestCount(int n) { _mmRestCount = n; }
Expand Down
2 changes: 1 addition & 1 deletion libmscore/measurebase.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@ MeasureBase* MeasureBase::prevMM() const
if (_prev
&& _prev->isMeasure()
&& score()->styleB(Sid::createMultiMeasureRests)) {
return const_cast<Measure*>(toMeasure(_prev)->mmRest1());
return const_cast<Measure*>(toMeasure(_prev)->coveringMMRestOrThis());
}
return _prev;
}
Expand Down
21 changes: 10 additions & 11 deletions libmscore/score.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1708,12 +1708,7 @@ Measure* Score::lastMeasure() const
Measure* Score::lastMeasureMM() const
{
Measure* m = lastMeasure();
if (m && styleB(Sid::createMultiMeasureRests)) {
Measure* m1 = const_cast<Measure*>(toMeasure(m->mmRest1()));
if (m1)
return m1;
}
return m;
return m ? const_cast<Measure*>(m->coveringMMRestOrThis()) : nullptr;
}

//---------------------------------------------------------
Expand Down Expand Up @@ -4050,7 +4045,7 @@ ChordRest* Score::cmdNextPrevSystem(ChordRest* cr, bool next)
{
auto newCR = cr;
auto currentMeasure = cr->measure();
auto currentSystem = currentMeasure->system() ? currentMeasure->system() : currentMeasure->mmRest1()->system();
auto currentSystem = currentMeasure->system() ? currentMeasure->system() : currentMeasure->coveringMMRestOrThis()->system();
if (!currentSystem)
return cr;
auto destinationMeasure = currentSystem->firstMeasure();
Expand All @@ -4060,7 +4055,9 @@ ChordRest* Score::cmdNextPrevSystem(ChordRest* cr, bool next)
if (next) {
if ((destinationMeasure = currentSystem->lastMeasure()->nextMeasure())) {
// There is a next system present: get it and accommodate for MMRest
currentSystem = destinationMeasure->system() ? destinationMeasure->system() : destinationMeasure->mmRest1()->system();
currentSystem = destinationMeasure->system()
? destinationMeasure->system()
: destinationMeasure->coveringMMRestOrThis()->system();
if ((destinationMeasure = currentSystem->firstMeasure()))
if ((newCR = destinationMeasure->first()->nextChordRest(trackZeroVoice(cr->track()), false)))
cr = newCR;
Expand Down Expand Up @@ -4089,7 +4086,9 @@ ChordRest* Score::cmdNextPrevSystem(ChordRest* cr, bool next)
if (!(destinationMeasure = destinationMeasure->prevMeasure()))
if (!(destinationMeasure = destinationMeasure->prevMeasureMM()))
return cr;
if (!(currentSystem = destinationMeasure->system() ? destinationMeasure->system() : destinationMeasure->mmRest1()->system()))
if (!(currentSystem = destinationMeasure->system()
? destinationMeasure->system()
: destinationMeasure->coveringMMRestOrThis()->system()))
return cr;
destinationMeasure = currentSystem->firstMeasure();
}
Expand Down Expand Up @@ -4220,7 +4219,7 @@ Element* Score::getScoreElementOfMeasureBase(MeasureBase* mb) const
else if ((currentMeasure = mb->findMeasure())) {
// Accommodate for MMRest
if (score()->styleB(Sid::createMultiMeasureRests) && currentMeasure->hasMMRest())
currentMeasure = currentMeasure->mmRest1();
currentMeasure = currentMeasure->coveringMMRestOrThis();
if ((cr = currentMeasure->first()->nextChordRest(0, false)))
el = cr;
}
Expand Down Expand Up @@ -4309,7 +4308,7 @@ ChordRest* Score::cmdTopStaff(ChordRest* cr)
if (destinationMeasure) {
// Accommodate for MMRest
if (score()->styleB(Sid::createMultiMeasureRests) && destinationMeasure->hasMMRest())
destinationMeasure = destinationMeasure->mmRest1();
destinationMeasure = destinationMeasure->coveringMMRestOrThis();
// Get first ChordRest of top staff
cr = destinationMeasure->first()->nextChordRest(0, false);
}
Expand Down
4 changes: 2 additions & 2 deletions mscore/scoreview.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1462,7 +1462,7 @@ void ScoreView::paint(const QRect& r, QPainter& p)
// segment is in a measure that has not been laid out yet
// this can happen in mmrests
// first chordrest segment of mmrest instead
const Measure* mmr = ss->measure()->mmRest1();
const Measure* mmr = ss->measure()->coveringMMRestOrThis();
if (mmr && mmr->system())
ss = mmr->first(SegmentType::ChordRest);
else
Expand Down Expand Up @@ -1515,7 +1515,7 @@ void ScoreView::paint(const QRect& r, QPainter& p)
system2 = s->measure()->system();
if (!system2) {
// as before, use mmrest if necessary
const Measure* mmr = s->measure()->mmRest1();
const Measure* mmr = s->measure()->coveringMMRestOrThis();
if (mmr)
system2 = mmr->system();
if (!system2)
Expand Down

0 comments on commit 5ca2b43

Please sign in to comment.