Skip to content

Commit

Permalink
Followup for PR #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 @vpereverzev
ecstrema authored and vpereverzev committed Dec 2, 2020
1 parent 21fede0 commit 01a2b7f
Show file tree
Hide file tree
Showing 8 changed files with 283 additions and 175 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
155 changes: 15 additions & 140 deletions libmscore/measurenumber.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,24 @@
// MuseScore
// Music Composition & Notation
//
// Copyright (C) 2011-2014 Werner Schweer
// Copyright (C) 2020 MuseScore BVBA and others
//
// This program is free software; you can redistribute it and/or modify
// it under the terms of the GNU General Public License version 2
// as published by the Free Software Foundation and appearing in
// the file LICENCE.GPL
// it under the terms of the GNU General Public License version 2.
//
// This program is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with this program; if not, write to the Free Software
// Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
//=============================================================================

#include "score.h"
#include "measurenumber.h"
// #include "xml.h"
#include "measure.h"
#include "staff.h"

namespace Ms {

Expand All @@ -31,10 +36,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 +49,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 +68,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
34 changes: 14 additions & 20 deletions libmscore/measurenumber.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,48 +2,42 @@
// MuseScore
// Music Composition & Notation
//
// Copyright (C) 2014 Werner Schweer
// Copyright (C) 2020 MuseScore BVBA and others
//
// This program is free software; you can redistribute it and/or modify
// it under the terms of the GNU General Public License version 2
// as published by the Free Software Foundation and appearing in
// the file LICENCE.GPL
// it under the terms of the GNU General Public License version 2.
//
// This program is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with this program; if not, write to the Free Software
// Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
//=============================================================================

#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 01a2b7f

Please sign in to comment.