Skip to content

Commit

Permalink
Fix GH#26523: Crash on deleting instrument if last staff contains fre…
Browse files Browse the repository at this point in the history
…tboard diagra

Backport of musescore#26703 plus fixing clazy warnings
  • Loading branch information
RomanPudashkin authored and Jojo-Schmitz committed Feb 24, 2025
1 parent 677f43b commit bc6f098
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 31 deletions.
17 changes: 13 additions & 4 deletions libmscore/fret.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1001,7 +1001,7 @@ void FretDiagram::setBarre(int string, int fret, bool add /*= false*/)

void FretDiagram::undoSetFretDot(int _string, int _fret, bool _add /*= true*/, FretDotType _dtype /*= FretDotType::NORMAl*/)
{
for (ScoreElement* e : linkList()) {
for (ScoreElement*& e : linkList()) {
FretDiagram* fd = toFretDiagram(e);
fd->score()->undo(new FretDot(fd, _string, _fret, _add, _dtype));
}
Expand All @@ -1013,7 +1013,7 @@ void FretDiagram::undoSetFretDot(int _string, int _fret, bool _add /*= true*/, F

void FretDiagram::undoSetFretMarker(int _string, FretMarkerType _mtype)
{
for (ScoreElement* e : linkList()) {
for (ScoreElement*& e : linkList()) {
FretDiagram* fd = toFretDiagram(e);
fd->score()->undo(new FretMarker(fd, _string, _mtype));
}
Expand All @@ -1026,7 +1026,7 @@ void FretDiagram::undoSetFretMarker(int _string, FretMarkerType _mtype)

void FretDiagram::undoSetFretBarre(int _string, int _fret, bool _add /*= false*/)
{
for (ScoreElement* e : linkList()) {
for (ScoreElement*& e : linkList()) {
FretDiagram* fd = toFretDiagram(e);
fd->score()->undo(new FretBarre(fd, _string, _fret, _add));
}
Expand Down Expand Up @@ -1138,7 +1138,7 @@ void FretDiagram::clear()

void FretDiagram::undoFretClear()
{
for (ScoreElement* e : linkList()) {
for (ScoreElement*& e : linkList()) {
FretDiagram* fd = toFretDiagram(e);
fd->score()->undo(new FretClear(fd));
}
Expand Down Expand Up @@ -1362,6 +1362,15 @@ QVariant FretDiagram::propertyDefault(Pid pid) const
return Element::propertyDefault(pid);
}

void FretDiagram::setTrack(int val)
{
Element::setTrack(val);

if (_harmony) {
_harmony->setTrack(val);
}
}

//---------------------------------------------------------
// endEditDrag
//---------------------------------------------------------
Expand Down
2 changes: 2 additions & 0 deletions libmscore/fret.h
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,8 @@ class FretDiagram final : public Element {
bool setProperty(Pid propertyId, const QVariant&) override;
QVariant propertyDefault(Pid) const override;

void setTrack(int val) override;

qreal userMag() const { return _userMag; }
void setUserMag(qreal m) { _userMag = m; }

Expand Down
20 changes: 12 additions & 8 deletions libmscore/harmony.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,9 @@
// the file LICENCE.GPL
//=============================================================================

#include "harmony.h"

#include "chordlist.h"
#include "fret.h"
#include "harmony.h"
#include "measure.h"
#include "mscore.h"
#include "part.h"
Expand All @@ -27,6 +26,8 @@
#include "utils.h"
#include "xml.h"

#include "global/log.h"

namespace Ms {

//---------------------------------------------------------
Expand Down Expand Up @@ -876,7 +877,7 @@ void Harmony::endEdit(EditData& ed)
showSpell = false;

if (links()) {
for (ScoreElement* e : *links()) {
for (ScoreElement*& e : *links()) {
if (e == this)
continue;
Harmony* h = toHarmony(e);
Expand Down Expand Up @@ -1272,7 +1273,10 @@ const ChordDescription* Harmony::getDescription(const QString& name, const Parse

const RealizedHarmony& Harmony::getRealizedHarmony()
{
Staff* st = staff();
const Staff* st = staff();
IF_ASSERT_FAILED(st) {
return _realizedHarmony;
}
int capo = st->capo(tick()) - 1;
int offset = (capo < 0 ? 0 : capo); //semitone offset for pitch adjustment
Interval interval = st->part()->instrument(tick())->transpose();
Expand All @@ -1282,7 +1286,7 @@ const RealizedHarmony& Harmony::getRealizedHarmony()
//Adjust for Nashville Notation, might be temporary
// TODO: set dirty on add/remove of keysig
if (_harmonyType == HarmonyType::NASHVILLE && !_realizedHarmony.valid()) {
Key key = staff()->key(tick());
Key key = st->key(tick());
//parse root
int rootTpc = function2Tpc(_function, key);

Expand Down Expand Up @@ -2048,7 +2052,7 @@ QString Harmony::generateScreenReaderInfo() const
for (auto const &r : symbolReplacements) {
// only replace when not preceded by backslash
QString s = "(?<!\\\\)" + r.first;
QRegularExpression re(s);
static QRegularExpression re(s);
aux.replace(re, r.second);
}
// construct string one character at a time
Expand All @@ -2072,9 +2076,9 @@ QString Harmony::generateScreenReaderInfo() const
QString extension = "";

#if QT_VERSION >= QT_VERSION_CHECK(5, 11, 0)
for (QString s : aux.split(">", Qt::SkipEmptyParts)) {
for (QString& s : aux.split(">", Qt::SkipEmptyParts)) {
#else
for (QString s : aux.split(">", QString::SkipEmptyParts)) {
for (QString& s : aux.split(">", QString::SkipEmptyParts)) {
#endif
if (!s.contains("blues"))
s.replace("b", QObject::tr(""));
Expand Down
43 changes: 24 additions & 19 deletions libmscore/rendermidi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ void Score::updateCapo()

void Score::updateChannel()
{
for (Staff* s : staves()) {
for (Staff*& s : staves()) {
for (int i = 0; i < VOICES; ++i)
s->clearChannelList(i);
}
Expand All @@ -169,7 +169,7 @@ void Score::updateChannel()
for (Segment* s = fm->first(SegmentType::ChordRest); s; s = s->next1(SegmentType::ChordRest)) {
for (const Element* e : s->annotations()) {
if (e->isInstrumentChange()) {
for (Staff* staff : *e->part()->staves()) {
for (Staff*& staff : *e->part()->staves()) {
for (int voice = 0; voice < VOICES; ++voice)
staff->insertIntoChannelList(voice, s->tick(), 0);
}
Expand Down Expand Up @@ -199,7 +199,7 @@ void Score::updateChannel()
}

for (Segment* s = fm->first(SegmentType::ChordRest); s; s = s->next1(SegmentType::ChordRest)) {
for (Staff* st : staves()) {
for (Staff*& st : staves()) {
int strack = st->idx() * VOICES;
int etrack = strack + VOICES;
for (int track = strack; track < etrack; ++track) {
Expand Down Expand Up @@ -278,9 +278,9 @@ static void playNote(EventMap* events, const Note* note, int channel, int pitch,
static_cast<qreal>(glissando->easeOut()) / 100.0);
easeInOut.timeList(static_cast<int>((timeDelta + timeStep * 0.5) / timeStep), int(timeDelta), &onTimes);
double nTimes = static_cast<double>(onTimes.size() - 1);
for (double time : onTimes) {
for (int& time : onTimes) {
int p = static_cast<int>((t / nTimes) * pitchDelta);
int timeStamp = std::min(onTime + int(time), offTime - 1);
int timeStamp = std::min(onTime + time, offTime - 1);
int midiPitch = (p * 16384) / 1200 + 8192;
NPlayEvent evb(ME_PITCHBEND, channel, midiPitch % 128, midiPitch / 128);
evb.setOriginatingStaff(staffIdx);
Expand Down Expand Up @@ -636,7 +636,12 @@ static void renderHarmony(EventMap* events, Measure const * m, Harmony* h, int t
{
if (!h->isRealizable())
return;

Staff* staff = m->score()->staff(h->track() / VOICES);
IF_ASSERT_FAILED(staff) {
return;
}

const Channel* channel = staff->part()->harmonyChannel();
IF_ASSERT_FAILED(channel)
return;
Expand Down Expand Up @@ -721,7 +726,7 @@ void MidiRenderer::collectMeasureEventsSimple(EventMap* events, Measure const *
events->registerChannel(channel);

qreal veloMultiplier = 1;
for (Articulation* a : chord->articulations()) {
for (Articulation*& a : chord->articulations()) {
if (a->playArticulation()) {
veloMultiplier *= instr->getVelocityMultiplier(a->articulationName());
}
Expand All @@ -730,15 +735,15 @@ void MidiRenderer::collectMeasureEventsSimple(EventMap* events, Measure const *
SndConfig config; // dummy

if (!graceNotesMerged(chord))
for (Chord* c : chord->graceNotesBefore())
for (Chord*& c : chord->graceNotesBefore())
for (const Note* note : c->notes())
collectNote(events, channel, note, veloMultiplier, tickOffset, st1, config);

for (const Note* note : chord->notes())
collectNote(events, channel, note, veloMultiplier, tickOffset, st1, config);

if (!graceNotesMerged(chord))
for (Chord* c : chord->graceNotesAfter())
for (Chord*& c : chord->graceNotesAfter())
for (const Note* note : c->notes())
collectNote(events, channel, note, veloMultiplier, tickOffset, st1, config);
}
Expand Down Expand Up @@ -814,7 +819,7 @@ void MidiRenderer::collectMeasureEventsDefault(EventMap* events, Measure const *

// Get a velocity multiplier
qreal veloMultiplier = 1;
for (Articulation* a : chord->articulations()) {
for (Articulation*& a : chord->articulations()) {
if (a->playArticulation()) {
veloMultiplier *= instr->getVelocityMultiplier(a->articulationName());
}
Expand All @@ -828,15 +833,15 @@ void MidiRenderer::collectMeasureEventsDefault(EventMap* events, Measure const *
//

if (!graceNotesMerged(chord))
for (Chord* c : chord->graceNotesBefore())
for (Chord*& c : chord->graceNotesBefore())
for (const Note* note : c->notes())
collectNote(events, channel, note, veloMultiplier, tickOffset, st1, config);

for (const Note* note : chord->notes())
collectNote(events, channel, note, veloMultiplier, tickOffset, st1, config);

if (!graceNotesMerged(chord))
for (Chord* c : chord->graceNotesAfter())
for (Chord*& c : chord->graceNotesAfter())
for (const Note* note : c->notes())
collectNote(events, channel, note, veloMultiplier, tickOffset, st1, config);
}
Expand Down Expand Up @@ -891,12 +896,12 @@ void Score::updateHairpin(Hairpin* h)
st->velocities().addRamp(tick, tick2, veloChange, method, direction);
break;
case Dynamic::Range::PART:
for (Staff* s : *st->part()->staves()) {
for (Staff*& s : *st->part()->staves()) {
s->velocities().addRamp(tick, tick2, veloChange, method, direction);
}
break;
case Dynamic::Range::SYSTEM:
for (Staff* s : qAsConst(_staves)) {
for (Staff*& s : _staves) {
s->velocities().addRamp(tick, tick2, veloChange, method, direction);
}
break;
Expand Down Expand Up @@ -999,7 +1004,7 @@ void Score::updateVelo()
Instrument* instr = chord->part()->instrument();

qreal veloMultiplier = 1;
for (Articulation* a : chord->articulations()) {
for (Articulation*& a : chord->articulations()) {
if (a->playArticulation()) {
veloMultiplier *= instr->getVelocityMultiplier(a->articulationName());
}
Expand Down Expand Up @@ -1946,7 +1951,7 @@ bool graceNotesMerged(Chord* chord)
{
if (findFirstTrill(chord))
return true;
for (Articulation* a : chord->articulations())
for (Articulation*& a : chord->articulations())
for (auto& oe : excursions)
if ( oe.atype == a->symId() )
return true;
Expand Down Expand Up @@ -1974,7 +1979,7 @@ void renderChordArticulation(Chord* chord, QList<NoteEventList> & ell, int & gat
renderNoteArticulation(events, note, false, trill->trillType(), trill->ornamentStyle());
}
else {
for (Articulation* a : chord->articulations()) {
for (Articulation*& a : chord->articulations()) {
if (!a->playArticulation())
continue;
if (!renderNoteArticulation(events, note, false, a->symId(), a->ornamentStyle()))
Expand All @@ -1996,7 +2001,7 @@ static bool shouldRenderNote(Note* n)
// The previous tied note probably has events for this note too.
// That is, we don't need to render this note separately.
return false;
for (Articulation* a : n->chord()->articulations()) {
for (Articulation*& a : n->chord()->articulations()) {
if (a->isOrnament()) {
return false;
}
Expand Down Expand Up @@ -2369,7 +2374,7 @@ void MidiRenderer::renderChunk(const Chunk& chunk, EventMap* events, const Conte
}

// create note & other events
for (Staff* st : score->staves()) {
for (Staff*& st : score->staves()) {
StaffContext sctx;
sctx.staff = st;
sctx.method = renderMethod;
Expand Down Expand Up @@ -2454,7 +2459,7 @@ bool MidiRenderer::canBreakChunk(const Measure* last)
// being properly rendered, disallow breaking
// chunk at repeat measure.
if (const Measure* next = last->nextMeasure())
for (const Staff* staff : score->staves()) {
for (Staff*& staff : score->staves()) {
if (next->isRepeatMeasure(staff))
return false;
}
Expand Down

0 comments on commit bc6f098

Please sign in to comment.