Skip to content

Commit

Permalink
Followup for PR musescore#6935, that included a bug.
Browse files Browse the repository at this point in the history 
`MeasureNumber`s and `MMRestRange`s were `delete`d in `measure::remove`,
which caused a crash when undoing the deletion of these elements.
  • Loading branch information
@ecstrema
ecstrema committed Dec 1, 2020
1 parent b628449 commit d4085ec
Show file tree
Hide file tree
Showing 8 changed files with 248 additions and 165 deletions.
5 changes: 3 additions & 2 deletions libmscore/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ add_library (
segmentlist.h select.h sequencer.h shadownote.h shape.h sig.h slur.h slurtie.h spacer.h spanner.h spannermap.h spatium.h
staff.h stafflines.h staffstate.h stafftext.h stafftextbase.h stafftype.h stafftypechange.h stafftypelist.h stem.h
stemslash.h stringdata.h style.h sym.h symbol.h synthesizerstate.h system.h systemdivider.h systemtext.h tempo.h
tempotext.h text.h mmrestrange.h measurenumber.h textbase.h textedit.h textframe.h textline.h textlinebase.h tie.h tiemap.h timesig.h
tempotext.h text.h mmrestrange.h measurenumberbase.h measurenumber.h textbase.h textedit.h textframe.h textline.h textlinebase.h tie.h tiemap.h timesig.h
tremolo.h tremolobar.h trill.h tuplet.h tupletmap.h types.h undo.h utils.h vibrato.h volta.h xml.h

segmentlist.cpp fingering.cpp accidental.cpp arpeggio.cpp
Expand All @@ -75,7 +75,8 @@ add_library (
score.cpp segment.cpp select.cpp shadownote.cpp slur.cpp tie.cpp slurtie.cpp
spacer.cpp spanner.cpp staff.cpp staffstate.cpp
stafftextbase.cpp stafftext.cpp systemtext.cpp stafftype.cpp stem.cpp style.cpp symbol.cpp
sym.cpp system.cpp stringdata.cpp tempotext.cpp text.cpp mmrestrange.cpp measurenumber.cpp textbase.cpp textedit.cpp
sym.cpp system.cpp stringdata.cpp tempotext.cpp text.cpp
mmrestrange.cpp measurenumber.cpp measurenumberbase.cpp textbase.cpp textedit.cpp
textframe.cpp textline.cpp textlinebase.cpp timesig.cpp
tremolobar.cpp tremolo.cpp trill.cpp tuplet.cpp
utils.cpp volta.cpp xmlreader.cpp xmlwriter.cpp mscore.cpp
Expand Down
2 changes: 0 additions & 2 deletions libmscore/measure.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -979,12 +979,10 @@ void Measure::remove(Element* e)

case ElementType::MEASURE_NUMBER:
_mstaves[e->staffIdx()]->setNoText(nullptr);
delete e;
break;

case ElementType::MMREST_RANGE:
_mstaves[e->staffIdx()]->setMMRangeText(nullptr);
delete e;
break;

case ElementType::SPACER:
Expand Down
138 changes: 4 additions & 134 deletions libmscore/measurenumber.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,9 @@ static const ElementStyle measureNumberStyle {
// MeasureNumber
//---------------------------------------------------------

MeasureNumber::MeasureNumber(Score* s, Tid tid, ElementFlags flags)
: TextBase(s, tid, flags)
MeasureNumber::MeasureNumber(Score* s, Tid tid)
: MeasureNumberBase(s, tid)
{
setFlag(ElementFlag::ON_STAFF, true);
initElementStyle(&measureNumberStyle);

setHPlacement(score()->styleV(Sid::measureNumberHPlacement).value<HPlacement>());
Expand All @@ -45,43 +44,9 @@ MeasureNumber::MeasureNumber(Score* s, Tid tid, ElementFlags flags)
// Copy constructor
//---------------------------------------------------------

MeasureNumber::MeasureNumber(const MeasureNumber& other): TextBase(other)
MeasureNumber::MeasureNumber(const MeasureNumber& other): MeasureNumberBase(other)
{
setFlag(ElementFlag::ON_STAFF, true);
initElementStyle(&measureNumberStyle);

setHPlacement(other.hPlacement());
}

//---------------------------------------------------------
// getProperty
//---------------------------------------------------------

QVariant MeasureNumber::getProperty(Pid id) const
{
switch (id) {
case Pid::HPLACEMENT:
return int(hPlacement());
default:
return TextBase::getProperty(id);
}
}

//---------------------------------------------------------
// setProperty
//---------------------------------------------------------

bool MeasureNumber::setProperty(Pid id, const QVariant& val)
{
switch (id) {
case Pid::HPLACEMENT:
setHPlacement(HPlacement(val.toInt()));
setLayoutInvalid();
triggerLayout();
return true;
default:
return TextBase::setProperty(id, val);
}
}

//---------------------------------------------------------
Expand All @@ -98,103 +63,8 @@ QVariant MeasureNumber::propertyDefault(Pid id) const
case Pid::HPLACEMENT:
return score()->styleV(Sid::measureNumberHPlacement);
default:
return TextBase::propertyDefault(id);
}
}

//---------------------------------------------------------
// readProperties
//---------------------------------------------------------

bool MeasureNumber::readProperties(XmlReader& xml)
{
if (readProperty(xml.name(), xml, Pid::HPLACEMENT))
return true;
else
return TextBase::readProperties(xml);
}

//---------------------------------------------------------
// layout
//---------------------------------------------------------

void MeasureNumber::layout()
{
setPos(QPointF());
if (!parent())
setOffset(0.0, 0.0);

// TextBase::layout1() needs to be called even if there's no measure attached to it.
// This happens for example in the palettes.
TextBase::layout1();
// this could be if (!measure()) but it is the same as current and slower
// See implementation of MeasureNumber::measure().
if (!parent())
return;

if (placeBelow()) {
qreal yoff = bbox().height();

// If there is only one line, the barline spans outside the staff lines, so the default position is not correct.
if (staff()->constStaffType(measure()->tick())->lines() == 1)
yoff += 2.0 * spatium();
else
yoff += staff()->height();

rypos() = yoff;
}
else {
qreal yoff = 0.0;

// If there is only one line, the barline spans outside the staff lines, so the default position is not correct.
if (staff()->constStaffType(measure()->tick())->lines() == 1)
yoff -= 2.0 * spatium();

rypos() = yoff;
}

if (hPlacement() == HPlacement::CENTER) {
// measure numbers should be centered over where there can be notes.
// This means that header and trailing segments should be ignored,
// which includes all timesigs, clefs, keysigs, etc.
// This is how it should be centered:
// |bb 4/4 notes-chords #| other measure |
// | ------18------ | other measure |

// x1 - left measure position of free space
// x2 - right measure position of free space

const Measure* mea = measure();

// find first chordrest
Segment* chordRest = mea->first(SegmentType::ChordRest);

Segment* s1 = chordRest->prevActive();
// unfortunately, using !s1->header() does not work
while (s1 && (s1->isChordRestType()
|| s1->isBreathType()
|| s1->isClefType()
|| s1->isBarLineType()
|| !s1->element(staffIdx() * VOICES)))
s1 = s1->prevActive();

Segment* s2 = chordRest->next();
// unfortunately, using !s1->trailer() does not work
while (s2 && (s2->isChordRestType()
|| s2->isBreathType()
|| s2->isClefType()
|| s2->isBarLineType()
|| !s2->element(staffIdx() * VOICES)))
s2 = s2->nextActive();

// if s1/s2 does not exist, it means there is no header/trailer segment. Align with start/end of measure.
qreal x1 = s1 ? s1->x() + s1->minRight() : 0;
qreal x2 = s2 ? s2->x() - s2->minLeft() : mea->width();

rxpos() = (x1 + x2) * 0.5;
return MeasureNumberBase::propertyDefault(id);
}
else if (hPlacement() == HPlacement::RIGHT)
rxpos() = measure()->width();
}

} // namespace MS
19 changes: 3 additions & 16 deletions libmscore/measurenumber.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,37 +13,24 @@
#ifndef __MEASURENUMBER_H__
#define __MEASURENUMBER_H__

#include "textbase.h"
#include "measurenumberbase.h"

namespace Ms {

//---------------------------------------------------------
// MeasureNumber
/// The basic element making measure numbers.
/// Reimplemented by MMRestRange
//---------------------------------------------------------

class MeasureNumber : public TextBase {

M_PROPERTY (HPlacement, hPlacement, setHPlacement) // Horizontal Placement
class MeasureNumber : public MeasureNumberBase {

public:
MeasureNumber(Score* = nullptr, Tid tid = Tid::MEASURE_NUMBER, ElementFlags flags = ElementFlag::NOTHING);
MeasureNumber(Score* = nullptr, Tid tid = Tid::MEASURE_NUMBER);
MeasureNumber(const MeasureNumber& other);

virtual ElementType type() const override { return ElementType::MEASURE_NUMBER; }
virtual MeasureNumber* clone() const override { return new MeasureNumber(*this); }

virtual QVariant getProperty(Pid id) const override;
virtual bool setProperty(Pid id, const QVariant& val) override;
virtual QVariant propertyDefault(Pid id) const override;

virtual bool readProperties(XmlReader&) override;

virtual void layout() override;
Measure* measure() const { return toMeasure(parent()); }

virtual bool isEditable() const override { return false; } // The measure numbers' text should not be editable
};

} // namespace Ms
Expand Down
Loading

0 comments on commit d4085ec

Please sign in to comment.