Skip to content

Fix assert on broken slurs #34041

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
miiizen wants to merge 3 commits into
base: main
Choose a base branch
from
+52 −37
Open

Fix assert on broken slurs #34041

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
586dd00
Don't add invalid slurs with no end to score
miiizen Jul 2, 2026
ba620a0
Make sure Chord's starting/ending spanners are kept up to date
miiizen Jul 2, 2026
c138d03
Fix XML import crash
miiizen Jul 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 37 additions & 37 deletions src/engraving/dom/spanner.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -723,18 +723,7 @@ PropertyValue Spanner::propertyDefault(Pid propertyId) const

void Spanner::computeStartElement()
{
EngravingItem* oldStartElement = m_startElement;

doComputeStartElement();

if (oldStartElement && oldStartElement->isChord()) {
toChord(oldStartElement)->removeStartingSpanner(this);
}

Chord* startChord = m_startElement && m_startElement->isChord() ? toChord(m_startElement) : nullptr;
if (startChord) {
startChord->addStartingSpanner(this);
}
}

void Spanner::doComputeStartElement()
Expand All @@ -746,24 +735,24 @@ void Spanner::doComputeStartElement()
return;
}
if (systemFlag()) {
m_startElement = startSegment();
setStartElement(startSegment());
} else {
EngravingItem* startEl = startSeg->element(track());
if (startEl) {
m_startElement = startEl;
setStartElement(startEl);
} else {
m_startElement = startSeg->firstElement(track2staff(track()));
setStartElement(startSeg->firstElement(track2staff(track())));
}
}
}
break;

case Anchor::MEASURE:
m_startElement = score()->tick2measure(tick());
setStartElement(score()->tick2measure(tick()));
break;

case Anchor::CHORD:
m_startElement = startCR();
setStartElement(startCR());
break;
case Anchor::NOTE:
break;
Expand All @@ -777,22 +766,11 @@ void Spanner::doComputeStartElement()
void Spanner::computeEndElement()
{
if (score()->isPaletteScore()) {
m_endElement = nullptr;
setEndElement(nullptr);
return;
}

EngravingItem* oldEndElement = m_endElement;

doComputeEndElement();

if (oldEndElement && oldEndElement->isChord()) {
toChord(oldEndElement)->removeEndingSpanner(this);
}

Chord* endChord = m_endElement && m_endElement->isChord() ? toChord(m_endElement) : nullptr;
if (endChord) {
endChord->addEndingSpanner(this);
}
}

void Spanner::doComputeEndElement()
Expand All @@ -804,27 +782,27 @@ void Spanner::doComputeEndElement()
return;
}
if (systemFlag()) {
m_endElement = endSeg;
setEndElement(endSeg);
} else {
track_idx_t trackIdx = effectiveTrack2();
EngravingItem* endEl = endSeg->element(trackIdx);
if (endEl) {
m_endElement = endEl;
setEndElement(endEl);
} else {
m_endElement = endSeg->firstElement(track2staff(trackIdx));
setEndElement(endSeg->firstElement(track2staff(trackIdx)));
}
}
}
break;

case Anchor::MEASURE:
m_endElement = score()->tick2measure(tick2() - Fraction(1, 1920));
setEndElement(score()->tick2measure(tick2() - Fraction(1, 1920)));
break;

case Anchor::NOTE:
break;
case Anchor::CHORD:
m_endElement = endCR();
setEndElement(endCR());
break;
}
}
Expand Down Expand Up @@ -957,7 +935,7 @@ Chord* Spanner::startChord()
return nullptr;
}
if (!m_startElement) {
m_startElement = findStartChord();
setStartElement(findStartChord());
}

if (m_startElement && m_startElement->isChord()) {
Expand All @@ -977,7 +955,7 @@ Chord* Spanner::endChord()
return nullptr;
}
if (!m_endElement && type() == ElementType::SLUR) {
m_endElement = findEndChord();
setEndElement(findEndChord());
}

if (m_endElement && m_endElement->isChord()) {
Expand All @@ -996,7 +974,7 @@ ChordRest* Spanner::startCR()
assert(m_anchor == Anchor::SEGMENT || m_anchor == Anchor::CHORD);
if (!m_startElement || m_startElement->score() != score()) {
// TODO: This is a bit weird and prevents this method from being const...
m_startElement = findStartCR();
setStartElement(findStartCR());
}
return (m_startElement && m_startElement->isChordRest()) ? toChordRest(m_startElement) : nullptr;
}
Expand All @@ -1010,7 +988,7 @@ ChordRest* Spanner::endCR()
assert(m_anchor == Anchor::SEGMENT || m_anchor == Anchor::CHORD);
if ((!m_endElement || m_endElement->score() != score())) {
// TODO: This is a bit weird and prevents this method from being const...
m_endElement = findEndCR();
setEndElement(findEndCR());
}
return (m_endElement && m_endElement->isChordRest()) ? toChordRest(m_endElement) : nullptr;
}
Expand Down Expand Up @@ -1260,7 +1238,18 @@ void Spanner::setStartElement(EngravingItem* e)
assert(!e || e->isNote());
}
#endif
EngravingItem* oldStartElement = m_startElement;

m_startElement = e;

if (oldStartElement && oldStartElement->isChord()) {
toChord(oldStartElement)->removeStartingSpanner(this);
}

Chord* startChord = m_startElement && m_startElement->isChord() ? toChord(m_startElement) : nullptr;
if (startChord) {
startChord->addStartingSpanner(this);
}
}

//---------------------------------------------------------
Expand All @@ -1274,7 +1263,18 @@ void Spanner::setEndElement(EngravingItem* e)
assert(!e || e->isNote());
}
#endif
EngravingItem* oldEndElement = m_endElement;

m_endElement = e;

if (oldEndElement && oldEndElement->isChord()) {
toChord(oldEndElement)->removeEndingSpanner(this);
}

Chord* endChord = m_endElement && m_endElement->isChord() ? toChord(m_endElement) : nullptr;
if (endChord) {
endChord->addEndingSpanner(this);
}
}

//---------------------------------------------------------
Expand Down
12 changes: 12 additions & 0 deletions src/engraving/rw/read206/read206.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3653,6 +3653,18 @@ Ret Read206::readScoreFile(Score* score, XmlReader& e, ReadInOutData* out)
}
}

// Discard slurs that were started but never terminated
for (Spanner* sp : score->spannerList()) {
if (sp->isSlur() && sp->ticks() <= Fraction(0, 1) && sp->startElement() == sp->endElement()) {
LOGW() << "discarding slur (id " << ctx.spannerId(sp) << ") that has no end";
assert(sp->startElement() && sp->startElement()->isChord());

ctx.removeSpanner(sp);
score->removeSpanner(sp);
delete sp;
}
}

for (Staff* s : score->staves()) {
s->updateOttava();
}
Expand Down
3 changes: 3 additions & 0 deletions src/importexport/musicxml/internal/import/importmusicxmlpass2.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8339,6 +8339,7 @@ static void addSlur(const Notation& notation, SlurStack& slurs, ChordRest* cr, N
slurs[slurNo] = SlurDesc();

if (newSlur->endElement() == cr && note) {
newSlur->setEndElement(nullptr);
delete newSlur;

// Slur starts & ends on same chord - add lv instead
Expand All @@ -8350,6 +8351,7 @@ static void addSlur(const Notation& notation, SlurStack& slurs, ChordRest* cr, N
const Fraction tick2 = newSlur->endElement()->tick();
if (tick2 < tick) {
logger->logError(String(u"slur end is before slur start"), xmlreader);
newSlur->setEndElement(nullptr);
delete newSlur;
return;
}
Expand Down Expand Up @@ -8400,6 +8402,7 @@ static void addSlur(const Notation& notation, SlurStack& slurs, ChordRest* cr, N
slurs[slurNo] = SlurDesc();

if (newSlur->startElement() == cr && note) {
newSlur->setStartElement(nullptr);
delete newSlur;

// Slur starts & ends on same chord - add lv instead
Expand Down
Loading