From 2854caed4c57dcafc9747f8405c64af46615eae5 Mon Sep 17 00:00:00 2001 From: Leon Vinken Date: Thu, 12 Aug 2021 11:15:09 +0200 Subject: [PATCH] Fix #317099: [MusicXML import] replace assertions by proper error reporting The MusicXML parser contains several assertions checking start and end elements to verify the parser is still in sync. For end users they have no effect (resulting in incomplete import without any error), for a debug build they crash MuseScore when triggered. These assertions have been replaced by a dialog providing meaningful information (expected element name and type versus actual) to the end user and the developer. Other Q_ASSERTS have been replaced by `if` statements. Backport of #8833, part 1 --- importexport/musicxml/exportxml.cpp | 6 +- importexport/musicxml/importmxml.cpp | 51 ++++- .../musicxml/importmxmlnoteduration.cpp | 2 - importexport/musicxml/importmxmlnotepitch.cpp | 7 - importexport/musicxml/importmxmlpass1.cpp | 78 +++---- importexport/musicxml/importmxmlpass1.h | 4 +- importexport/musicxml/importmxmlpass2.cpp | 216 +++++++----------- importexport/musicxml/importmxmlpass2.h | 8 +- importexport/musicxml/importxml.cpp | 2 +- importexport/musicxml/importxmlfirstpass.cpp | 3 +- importexport/musicxml/musicxmlsupport.cpp | 25 ++ importexport/musicxml/musicxmlsupport.h | 3 +- 12 files changed, 189 insertions(+), 216 deletions(-) diff --git a/importexport/musicxml/exportxml.cpp b/importexport/musicxml/exportxml.cpp index aea6228b4b4af..000d4688aac2a 100644 --- a/importexport/musicxml/exportxml.cpp +++ b/importexport/musicxml/exportxml.cpp @@ -5668,7 +5668,8 @@ static MeasureBase* lastMeasureBase(const System* const system) MeasureBase* mb = nullptr; if (system) { const auto& measures = system->measures(); - Q_ASSERT(!(measures.empty())); + if (measures.empty()) + return nullptr; mb = measures.back(); } return mb; @@ -5683,7 +5684,8 @@ static bool hasPageBreak(const System* const system) const MeasureBase* mb = nullptr; if (system) { const auto& measures = system->measures(); - Q_ASSERT(!(measures.empty())); + if (measures.empty()) + return false; mb = measures.back(); } diff --git a/importexport/musicxml/importmxml.cpp b/importexport/musicxml/importmxml.cpp index 41c6d5087c106..0275a5637c57f 100644 --- a/importexport/musicxml/importmxml.cpp +++ b/importexport/musicxml/importmxml.cpp @@ -73,6 +73,30 @@ Score::FileError applyMusicXMLPVGStyles(Score* score, MxmlLogger* logger) } +//--------------------------------------------------------- +// musicXMLImportErrorDialog +//--------------------------------------------------------- + +/** + Show a dialog displaying the MusicXML import error(s). + */ + +static int musicXMLImportErrorDialog(QString text, QString detailedText) + { + QMessageBox errorDialog; + errorDialog.setIcon(QMessageBox::Question); + errorDialog.setText(text); + errorDialog.setInformativeText(QObject::tr("Do you want to try to load this file anyway?")); + errorDialog.setDetailedText(detailedText); + errorDialog.setStandardButtons(QMessageBox::Yes | QMessageBox::No); + errorDialog.setDefaultButton(QMessageBox::No); + return errorDialog.exec(); + } + +//--------------------------------------------------------- +// importMusicXMLfromBuffer +//--------------------------------------------------------- + Score::FileError importMusicXMLfromBuffer(Score* score, const QString& /*name*/, QIODevice* dev) { //qDebug("importMusicXMLfromBuffer(score %p, name '%s', dev %p)", @@ -92,21 +116,26 @@ Score::FileError importMusicXMLfromBuffer(Score* score, const QString& /*name*/, dev->seek(0); MusicXMLParserPass1 pass1(score, &logger); Score::FileError res = pass1.parse(dev); - if (res != Score::FileError::FILE_NO_ERROR) { - logger.logError(QString("Error during MusicXML import pass 1. Returning.")); - return res; - } + const auto pass1_errors = pass1.errors(); // pass 2 - dev->seek(0); MusicXMLParserPass2 pass2(score, pass1, &logger); - Score::FileError res2 = pass2.parse(dev); - if (res2 != Score::FileError::FILE_NO_ERROR) { - logger.logError(QString("Error during MusicXML import pass 2. Returning.")); - return res2; + if (res == Score::FileError::FILE_NO_ERROR) { + dev->seek(0); + res = pass2.parse(dev); + } + + // report result + const auto pass2_errors = pass2.errors(); + if (!(pass1_errors.isEmpty() && pass2_errors.isEmpty())) { + if (!MScore::noGui) { + const QString text { QObject::tr("Error(s) found, import may be incomplete.") }; + if (musicXMLImportErrorDialog(text, pass1.errors() + pass2.errors()) != QMessageBox::Yes) + res = Score::FileError::FILE_USER_ABORT; + } } - - return Score::FileError::FILE_NO_ERROR; + + return res; } } // namespace Ms diff --git a/importexport/musicxml/importmxmlnoteduration.cpp b/importexport/musicxml/importmxmlnoteduration.cpp index 2c64ed3a263a1..744f26e7ca47c 100644 --- a/importexport/musicxml/importmxmlnoteduration.cpp +++ b/importexport/musicxml/importmxmlnoteduration.cpp @@ -187,7 +187,6 @@ QString mxmlNoteDuration::checkTiming(const QString& type, const bool rest, cons void mxmlNoteDuration::duration(QXmlStreamReader& e) { - Q_ASSERT(e.isStartElement() && e.name() == "duration"); _logger->logDebugTrace("MusicXMLParserPass1::duration", &e); _specDura.set(0, 0); // invalid unless set correctly @@ -244,7 +243,6 @@ bool mxmlNoteDuration::readProperties(QXmlStreamReader& e) void mxmlNoteDuration::timeModification(QXmlStreamReader& e) { - Q_ASSERT(e.isStartElement() && e.name() == "time-modification"); _logger->logDebugTrace("MusicXMLParserPass1::timeModification", &e); int intActual = 0; diff --git a/importexport/musicxml/importmxmlnotepitch.cpp b/importexport/musicxml/importmxmlnotepitch.cpp index 0fe95475fb72f..a39b09d2779b5 100644 --- a/importexport/musicxml/importmxmlnotepitch.cpp +++ b/importexport/musicxml/importmxmlnotepitch.cpp @@ -29,8 +29,6 @@ namespace Ms { static Accidental* accidental(QXmlStreamReader& e, Score* score) { - Q_ASSERT(e.isStartElement() && e.name() == "accidental"); - bool cautionary = e.attributes().value("cautionary") == "yes"; bool editorial = e.attributes().value("editorial") == "yes"; bool parentheses = e.attributes().value("parentheses") == "yes"; @@ -61,9 +59,6 @@ static Accidental* accidental(QXmlStreamReader& e, Score* score) void mxmlNotePitch::displayStepOctave(QXmlStreamReader& e) { - Q_ASSERT(e.isStartElement() - && (e.name() == "rest" || e.name() == "unpitched")); - while (e.readNextStartElement()) { if (e.name() == "display-step") { const auto step = e.readElementText(); @@ -99,8 +94,6 @@ void mxmlNotePitch::displayStepOctave(QXmlStreamReader& e) void mxmlNotePitch::pitch(QXmlStreamReader& e) { - Q_ASSERT(e.isStartElement() && e.name() == "pitch"); - // defaults _step = -1; _alter = 0; diff --git a/importexport/musicxml/importmxmlpass1.cpp b/importexport/musicxml/importmxmlpass1.cpp index 1f0e4c93c21a6..2340df4774711 100644 --- a/importexport/musicxml/importmxmlpass1.cpp +++ b/importexport/musicxml/importmxmlpass1.cpp @@ -188,6 +188,19 @@ MusicXMLParserPass1::MusicXMLParserPass1(Score* score, MxmlLogger* logger) // nothing } +//--------------------------------------------------------- +// addError +//--------------------------------------------------------- + +void MusicXMLParserPass1::addError(const QString& error) + { + if (error != "") { + _logger->logError(error, &_e); + QString errorWithLocation = xmlReaderLocation(_e) + ' ' + error + '\n'; + _errors += errorWithLocation; + } + } + //--------------------------------------------------------- // initPartState //--------------------------------------------------------- @@ -373,7 +386,8 @@ bool MusicXMLParserPass1::determineStaffMoveVoice(const QString& id, const int m // make score-relative instead on part-relative Part* part = _partMap.value(id); - Q_ASSERT(part); + if (!part) + return -1; int scoreRelStaff = _score->staffIdx(part); // zero-based number of parts first staff in the score msTrack = (scoreRelStaff + s) * VOICES; @@ -409,7 +423,8 @@ bool MusicXMLParserPass1::hasPart(const QString& id) const int MusicXMLParserPass1::trackForPart(const QString& id) const { Part* part = _partMap.value(id); - Q_ASSERT(part); + if (!part) + return -1; int scoreRelStaff = _score->staffIdx(part); // zero-based number of parts first staff in the score return scoreRelStaff * VOICES; } @@ -1056,7 +1071,6 @@ static bool allStaffGroupsIdentical(Part const* const p) void MusicXMLParserPass1::scorePartwise() { - Q_ASSERT(_e.isStartElement() && _e.name() == "score-partwise"); _logger->logDebugTrace("MusicXMLParserPass1::scorePartwise", &_e); MusicXmlPartGroupList partGroupList; @@ -1141,6 +1155,7 @@ void MusicXMLParserPass1::scorePartwise() } } } + addError(checkAtEndElement(_e, "score-partwise")); } //--------------------------------------------------------- @@ -1154,7 +1169,6 @@ void MusicXMLParserPass1::scorePartwise() void MusicXMLParserPass1::identification() { - Q_ASSERT(_e.isStartElement() && _e.name() == "identification"); _logger->logDebugTrace("MusicXMLParserPass1::identification", &_e); while (_e.readNextStartElement()) { @@ -1366,7 +1380,6 @@ static QString nextPartOfFormattedString(QXmlStreamReader& e) void MusicXMLParserPass1::credit(CreditWordsList& credits) { - Q_ASSERT(_e.isStartElement() && _e.name() == "credit"); _logger->logDebugTrace("MusicXMLParserPass1::credit", &_e); const auto page = _e.attributes().value("page").toString().toInt(); // ignoring errors implies incorrect conversion defaults to the first page @@ -1413,7 +1426,6 @@ void MusicXMLParserPass1::credit(CreditWordsList& credits) credits.append(cw); } - Q_ASSERT(_e.isEndElement() && _e.name() == "credit"); } //--------------------------------------------------------- @@ -1515,7 +1527,6 @@ static void setPageFormat(Score* score, const PageFormat& pf) void MusicXMLParserPass1::defaults() { - Q_ASSERT(_e.isStartElement() && _e.name() == "defaults"); //_logger->logDebugTrace("MusicXMLParserPass1::defaults", &_e); double millimeter = _score->spatium()/10.0; @@ -1619,7 +1630,6 @@ void MusicXMLParserPass1::defaults() void MusicXMLParserPass1::pageLayout(PageFormat& pf, const qreal conversion) { - Q_ASSERT(_e.isStartElement() && _e.name() == "page-layout"); _logger->logDebugTrace("MusicXMLParserPass1::pageLayout", &_e); qreal _oddRightMargin = 0.0; @@ -1691,7 +1701,6 @@ void MusicXMLParserPass1::pageLayout(PageFormat& pf, const qreal conversion) void MusicXMLParserPass1::partList(MusicXmlPartGroupList& partGroupList) { - Q_ASSERT(_e.isStartElement() && _e.name() == "part-list"); _logger->logDebugTrace("MusicXMLParserPass1::partList", &_e); int scoreParts = 0; // number of score-parts read sofar @@ -1818,7 +1827,6 @@ void MusicXMLParserPass1::partGroup(const int scoreParts, MusicXmlPartGroupList& partGroupList, MusicXmlPartGroupMap& partGroups) { - Q_ASSERT(_e.isStartElement() && _e.name() == "part-group"); _logger->logDebugTrace("MusicXMLParserPass1::partGroup", &_e); bool barlineSpan = true; int number = _e.attributes().value("number").toInt(); @@ -1904,7 +1912,6 @@ void MusicXMLParserPass1::addInferredTranspose(const QString& partId) void MusicXMLParserPass1::scorePart() { - Q_ASSERT(_e.isStartElement() && _e.name() == "score-part"); _logger->logDebugTrace("MusicXMLParserPass1::scorePart", &_e); QString id = _e.attributes().value("id").toString().trimmed(); @@ -1969,8 +1976,6 @@ void MusicXMLParserPass1::scorePart() else skipLogCurrElem(); } - - Q_ASSERT(_e.isEndElement() && _e.name() == "score-part"); } //--------------------------------------------------------- @@ -1983,7 +1988,6 @@ void MusicXMLParserPass1::scorePart() void MusicXMLParserPass1::scoreInstrument(const QString& partId) { - Q_ASSERT(_e.isStartElement() && _e.name() == "score-instrument"); _logger->logDebugTrace("MusicXMLParserPass1::scoreInstrument", &_e); QString instrId = _e.attributes().value("id").toString(); @@ -2029,7 +2033,6 @@ void MusicXMLParserPass1::scoreInstrument(const QString& partId) else skipLogCurrElem(); } - Q_ASSERT(_e.isEndElement() && _e.name() == "score-instrument"); } //--------------------------------------------------------- @@ -2042,7 +2045,6 @@ void MusicXMLParserPass1::scoreInstrument(const QString& partId) void MusicXMLParserPass1::midiInstrument(const QString& partId) { - Q_ASSERT(_e.isStartElement() && _e.name() == "midi-instrument"); _logger->logDebugTrace("MusicXMLParserPass1::midiInstrument", &_e); QString instrId = _e.attributes().value("id").toString(); @@ -2102,7 +2104,6 @@ void MusicXMLParserPass1::midiInstrument(const QString& partId) else skipLogCurrElem(); } - Q_ASSERT(_e.isEndElement() && _e.name() == "midi-instrument"); } //--------------------------------------------------------- @@ -2117,7 +2118,8 @@ void MusicXMLParserPass1::midiInstrument(const QString& partId) static void setNumberOfStavesForPart(Part* const part, const int staves) { - Q_ASSERT(part); + if (!part) + return; int prevnstaves = part->nstaves(); if (staves > part->nstaves()) { part->setStaves(staves); @@ -2139,13 +2141,13 @@ static void setNumberOfStavesForPart(Part* const part, const int staves) void MusicXMLParserPass1::part() { - Q_ASSERT(_e.isStartElement() && _e.name() == "part"); _logger->logDebugTrace("MusicXMLParserPass1::part", &_e); const QString id = _e.attributes().value("id").toString().trimmed(); if (!_parts.contains(id)) { _logger->logError(QString("cannot find part '%1'").arg(id), &_e); skipLogCurrElem(); + return; } initPartState(id); @@ -2217,6 +2219,8 @@ void MusicXMLParserPass1::part() qPrintable(i.key()), qPrintable(i.value().toString())); } */ + + addError(checkAtEndElement(_e, "part")); } //--------------------------------------------------------- @@ -2259,7 +2263,6 @@ void MusicXMLParserPass1::measure(const QString& partId, VoiceOverlapDetector& vod, const int measureNr) { - Q_ASSERT(_e.isStartElement() && _e.name() == "measure"); _logger->logDebugTrace("MusicXMLParserPass1::measure", &_e); QString number = _e.attributes().value("number").toString(); @@ -2394,6 +2397,8 @@ void MusicXMLParserPass1::measure(const QString& partId, qPrintable(partId), qPrintable(number), qPrintable(mdur.print()), mdur.ticks()); */ _parts[partId].addMeasureNumberAndDuration(number, mdur); + + addError(checkAtEndElement(_e, "measure")); } //--------------------------------------------------------- @@ -2402,7 +2407,6 @@ void MusicXMLParserPass1::measure(const QString& partId, void MusicXMLParserPass1::print(const int measureNr) { - Q_ASSERT(_e.isStartElement() && _e.name() == "print"); _logger->logDebugTrace("MusicXMLParserPass1::print", &_e); const QString newPage = _e.attributes().value("new-page").toString(); @@ -2425,7 +2429,6 @@ void MusicXMLParserPass1::print(const int measureNr) void MusicXMLParserPass1::attributes(const QString& partId, const Fraction cTime) { - Q_ASSERT(_e.isStartElement() && _e.name() == "attributes"); _logger->logDebugTrace("MusicXMLParserPass1::attributes", &_e); int staves = 0; @@ -2506,7 +2509,6 @@ void MusicXMLParserPass1::attributes(const QString& partId, const Fraction cTime void MusicXMLParserPass1::clef(const QString& partId) { - Q_ASSERT(_e.isStartElement() && _e.name() == "clef"); _logger->logDebugTrace("MusicXMLParserPass1::clef", &_e); while (_e.readNextStartElement()) { @@ -2600,7 +2602,6 @@ static bool determineTimeSig(MxmlLogger* logger, const QXmlStreamReader* const x void MusicXMLParserPass1::time(const Fraction cTime) { - Q_ASSERT(_e.isStartElement() && _e.name() == "time"); QString beats; QString beatType; @@ -2637,8 +2638,6 @@ void MusicXMLParserPass1::time(const Fraction cTime) void MusicXMLParserPass1::transpose(const QString& partId, const Fraction& tick) { - Q_ASSERT(_e.isStartElement() && _e.name() == "transpose"); - Interval interval; while (_e.readNextStartElement()) { int i = _e.readElementText().toInt(); @@ -2672,8 +2671,6 @@ void MusicXMLParserPass1::transpose(const QString& partId, const Fraction& tick) void MusicXMLParserPass1::divisions() { - Q_ASSERT(_e.isStartElement() && _e.name() == "divisions"); - _divs = _e.readElementText().toInt(); if (!(_divs > 0)) _logger->logError("illegal divisions", &_e); @@ -2691,8 +2688,6 @@ void MusicXMLParserPass1::divisions() void MusicXMLParserPass1::direction(const QString& partId, const Fraction cTime) { - Q_ASSERT(_e.isStartElement() && _e.name() == "direction"); - // note: file order is direction-type first, then staff // this means staff is still unknown when direction-type is handled @@ -2766,8 +2761,6 @@ void MusicXMLParserPass1::directionType(const Fraction cTime, QList& starts, QList& stops) { - Q_ASSERT(_e.isStartElement() && _e.name() == "direction-type"); - while (_e.readNextStartElement()) { if (_e.name() == "octave-shift") { QString number = _e.attributes().value("number").toString(); @@ -2801,8 +2794,6 @@ void MusicXMLParserPass1::directionType(const Fraction cTime, else _e.skipCurrentElement(); } - - Q_ASSERT(_e.isEndElement() && _e.name() == "direction-type"); } //--------------------------------------------------------- @@ -2853,7 +2844,6 @@ void MusicXMLParserPass1::handleOctaveShift(const Fraction cTime, void MusicXMLParserPass1::notations(MxmlStartStop& tupletStartStop) { - Q_ASSERT(_e.isStartElement() && _e.name() == "notations"); //_logger->logDebugTrace("MusicXMLParserPass1::note", &_e); while (_e.readNextStartElement()) { @@ -2875,8 +2865,6 @@ void MusicXMLParserPass1::notations(MxmlStartStop& tupletStartStop) _e.skipCurrentElement(); // skip but don't log } } - - Q_ASSERT(_e.isEndElement() && _e.name() == "notations"); } //--------------------------------------------------------- @@ -3242,7 +3230,6 @@ void MusicXMLParserPass1::note(const QString& partId, VoiceOverlapDetector& vod, MxmlTupletStates& tupletStates) { - Q_ASSERT(_e.isStartElement() && _e.name() == "note"); //_logger->logDebugTrace("MusicXMLParserPass1::note", &_e); if (_e.attributes().value("print-spacing") == "no") { @@ -3310,7 +3297,8 @@ void MusicXMLParserPass1::note(const QString& partId, staff = _parts[partId].staffNumberToIndex(strStaff.toInt(&ok)); _parts[partId].setMaxStaff(staff); Part* part = _partMap.value(partId); - Q_ASSERT(part); + if (!part) + continue; if (!ok || staff < 0 || staff >= part->nstaves()) _logger->logError(QString("illegal or hidden staff '%1'").arg(strStaff), &_e); } @@ -3378,9 +3366,7 @@ void MusicXMLParserPass1::note(const QString& partId, vod.addNote((sTime + missingPrev).ticks(), (sTime + missingPrev + dura).ticks(), voice, staff); } - if (!(_e.isEndElement() && _e.name() == "note")) - qDebug("name %s line %lld", qPrintable(_e.name().toString()), _e.lineNumber()); - Q_ASSERT(_e.isEndElement() && _e.name() == "note"); + addError(checkAtEndElement(_e, "note")); } //--------------------------------------------------------- @@ -3394,7 +3380,6 @@ void MusicXMLParserPass1::note(const QString& partId, void MusicXMLParserPass1::notePrintSpacingNo(Fraction& dura) { - Q_ASSERT(_e.isStartElement() && _e.name() == "note"); //_logger->logDebugTrace("MusicXMLParserPass1::notePrintSpacingNo", &_e); bool chord = false; @@ -3420,8 +3405,6 @@ void MusicXMLParserPass1::notePrintSpacingNo(Fraction& dura) // cannot have a duration longer than the first note in the chord if (chord || grace) dura.set(0, 1); - - Q_ASSERT(_e.isEndElement() && _e.name() == "note"); } //--------------------------------------------------------- @@ -3434,7 +3417,6 @@ void MusicXMLParserPass1::notePrintSpacingNo(Fraction& dura) void MusicXMLParserPass1::duration(Fraction& dura) { - Q_ASSERT(_e.isStartElement() && _e.name() == "duration"); //_logger->logDebugTrace("MusicXMLParserPass1::duration", &_e); dura.set(0, 0); // invalid unless set correctly @@ -3462,7 +3444,6 @@ void MusicXMLParserPass1::duration(Fraction& dura) void MusicXMLParserPass1::forward(Fraction& dura) { - Q_ASSERT(_e.isStartElement() && _e.name() == "forward"); //_logger->logDebugTrace("MusicXMLParserPass1::forward", &_e); while (_e.readNextStartElement()) { @@ -3487,7 +3468,6 @@ void MusicXMLParserPass1::forward(Fraction& dura) void MusicXMLParserPass1::backup(Fraction& dura) { - Q_ASSERT(_e.isStartElement() && _e.name() == "backup"); //_logger->logDebugTrace("MusicXMLParserPass1::backup", &_e); while (_e.readNextStartElement()) { @@ -3508,7 +3488,6 @@ void MusicXMLParserPass1::backup(Fraction& dura) void MusicXMLParserPass1::timeModification(Fraction& timeMod) { - Q_ASSERT(_e.isStartElement() && _e.name() == "time-modification"); //_logger->logDebugTrace("MusicXMLParserPass1::timeModification", &_e); int intActual = 0; @@ -3546,7 +3525,6 @@ void MusicXMLParserPass1::timeModification(Fraction& timeMod) void MusicXMLParserPass1::rest() { - Q_ASSERT(_e.isStartElement() && _e.name() == "rest"); //_logger->logDebugTrace("MusicXMLParserPass1::rest", &_e); while (_e.readNextStartElement()) { diff --git a/importexport/musicxml/importmxmlpass1.h b/importexport/musicxml/importmxmlpass1.h index b7872a8597fd4..2e4e2152eb9fd 100644 --- a/importexport/musicxml/importmxmlpass1.h +++ b/importexport/musicxml/importmxmlpass1.h @@ -120,6 +120,7 @@ class MusicXMLParserPass1 { void initPartState(const QString& partId); Score::FileError parse(QIODevice* device); Score::FileError parse(); + QString errors() const { return _errors; } void scorePartwise(); void identification(); void credit(CreditWordsList& credits); @@ -183,7 +184,7 @@ class MusicXMLParserPass1 { private: // functions - // none + void addError(const QString& error); ///< Add an error to be shown in the GUI // generic pass 1 data QXmlStreamReader _e; @@ -198,6 +199,7 @@ class MusicXMLParserPass1 { QMap _instruments; ///< instruments for each part, mapped on part id Score* _score; ///< MuseScore score MxmlLogger* _logger; ///< Error logger + QString _errors; ///< Errors to present to the user bool _hasBeamingInfo; ///< Whether the score supports or contains beaming info QString _supportsTranspose; ///< Whether the score supports transposition info bool _hasInferredHeaderText; diff --git a/importexport/musicxml/importmxmlpass2.cpp b/importexport/musicxml/importmxmlpass2.cpp index bf79b348488b8..c81b56e6d8085 100644 --- a/importexport/musicxml/importmxmlpass2.cpp +++ b/importexport/musicxml/importmxmlpass2.cpp @@ -296,8 +296,10 @@ static void fillGap(Measure* measure, int track, const Fraction& tstart, const F static void fillGapsInFirstVoices(Measure* measure, Part* part) { - Q_ASSERT(measure); - Q_ASSERT(part); + if (!measure) + return; + if (!part) + return; Fraction measTick = measure->tick(); Fraction measLen = measure->ticks(); @@ -1375,6 +1377,19 @@ MusicXMLParserPass2::MusicXMLParserPass2(Score* score, MusicXMLParserPass1& pass // nothing } +//--------------------------------------------------------- +// addError +//--------------------------------------------------------- + +void MusicXMLParserPass2::addError(const QString& error) + { + if (error != "") { + _logger->logError(error, &_e); + QString errorWithLocation = xmlReaderLocation(_e) + ' ' + error + '\n'; + _errors += errorWithLocation; + } + } + //--------------------------------------------------------- // setChordRestDuration //--------------------------------------------------------- @@ -1626,7 +1641,8 @@ SpannerSet MusicXMLParserPass2::findIncompleteSpannersAtPartEnd() static void addArticLaissezVibrer(const Note* const note) { - Q_ASSERT(note); + if (!note) + return; auto chord = note->chord(); if (!hasLaissezVibrer(chord)) { Articulation* na = new Articulation(SymId::articLaissezVibrerBelow, chord->score()); @@ -1953,8 +1969,6 @@ Score::FileError MusicXMLParserPass2::parse() void MusicXMLParserPass2::scorePartwise() { - Q_ASSERT(_e.isStartElement() && _e.name() == "score-partwise"); - while (_e.readNextStartElement()) { if (_e.name() == "part") { part(); @@ -1982,6 +1996,8 @@ void MusicXMLParserPass2::scorePartwise() addCopyrightVBox(); cleanUpLayoutBreaks(_score, _logger); + + addError(checkAtEndElement(_e, "score-partwise")); } //--------------------------------------------------------- @@ -1994,8 +2010,6 @@ void MusicXMLParserPass2::scorePartwise() void MusicXMLParserPass2::partList() { - Q_ASSERT(_e.isStartElement() && _e.name() == "part-list"); - while (_e.readNextStartElement()) { if (_e.name() == "score-part") scorePart(); @@ -2013,8 +2027,6 @@ void MusicXMLParserPass2::partList() void MusicXMLParserPass2::scorePart() { - Q_ASSERT(_e.isStartElement() && _e.name() == "score-part"); - while (_e.readNextStartElement()) { if (_e.name() == "midi-instrument") _e.skipCurrentElement(); // skip but don't log @@ -2037,7 +2049,6 @@ void MusicXMLParserPass2::scorePart() void MusicXMLParserPass2::part() { - Q_ASSERT(_e.isStartElement() && _e.name() == "part"); const QString id = _e.attributes().value("id").toString(); if (!_pass1.hasPart(id)) { @@ -2166,6 +2177,8 @@ void MusicXMLParserPass2::part() // but may be incorrect for a percussion staff that does not use a percussion clef setStaffTypePercussion(msPart, drumset); } + + addError(checkAtEndElement(_e, "part")); } //--------------------------------------------------------- @@ -2426,7 +2439,6 @@ static bool hasTempoTextAtTick(const TempoMap* const tempoMap, const int tick) void MusicXMLParserPass2::measure(const QString& partId, const Fraction time) { - Q_ASSERT(_e.isStartElement() && _e.name() == "measure"); //QString number = _e.attributes().value("number").toString(); //qDebug("measure %s start", qPrintable(number)); @@ -2635,7 +2647,7 @@ void MusicXMLParserPass2::measure(const QString& partId, measure->setBreakMultiMeasureRest(true); } - Q_ASSERT(_e.isEndElement() && _e.name() == "measure"); + addError(checkAtEndElement(_e, "measure")); } //--------------------------------------------------------- @@ -2654,8 +2666,6 @@ void MusicXMLParserPass2::measure(const QString& partId, void MusicXMLParserPass2::attributes(const QString& partId, Measure* measure, const Fraction& tick) { - Q_ASSERT(_e.isStartElement() && _e.name() == "attributes"); - while (_e.readNextStartElement()) { if (_e.name() == "clef") clef(partId, measure, tick); @@ -2700,11 +2710,11 @@ static void setStaffLines(Score* score, int staffIdx, int stafflines) void MusicXMLParserPass2::staffDetails(const QString& partId) { - Q_ASSERT(_e.isStartElement() && _e.name() == "staff-details"); //logDebugTrace("MusicXMLParserPass2::staffDetails"); Part* part = _pass1.getPart(partId); - Q_ASSERT(part); + if (!part) + return; int staves = part->nstaves(); QString strNumber = _e.attributes().value("number").toString(); @@ -2769,7 +2779,6 @@ void MusicXMLParserPass2::staffDetails(const QString& partId) void MusicXMLParserPass2::staffTuning(StringData* t) { - Q_ASSERT(_e.isStartElement() && _e.name() == "staff-tuning"); //logDebugTrace("MusicXMLParserPass2::staffTuning"); // ignore if not a TAB staff @@ -2823,8 +2832,6 @@ void MusicXMLParserPass2::staffTuning(StringData* t) void MusicXMLParserPass2::measureStyle(Measure* measure) { - Q_ASSERT(_e.isStartElement() && _e.name() == "measure-style"); - while (_e.readNextStartElement()) { if (_e.name() == "multiple-rest") { int multipleRest = _e.readElementText().toInt(); @@ -2956,7 +2963,6 @@ void MusicXMLParserDirection::direction(const QString& partId, DelayedDirectionsList& delayedDirections, InferredFingeringsList& inferredFingerings) { - Q_ASSERT(_e.isStartElement() && _e.name() == "direction"); //qDebug("direction tick %s", qPrintable(tick.print())); _placement = _e.attributes().value("placement").toString(); @@ -3210,8 +3216,6 @@ void MusicXMLParserDirection::direction(const QString& partId, } } } - - Q_ASSERT(_e.isEndElement() && _e.name() == "direction"); } //--------------------------------------------------------- @@ -3225,8 +3229,6 @@ void MusicXMLParserDirection::direction(const QString& partId, void MusicXMLParserDirection::directionType(QList& starts, QList& stops) { - Q_ASSERT(_e.isStartElement() && _e.name() == "direction-type"); - while (_e.readNextStartElement()) { // Prevent multi-word directions from overwriting y-values. bool hasDefaultYCandidate = false; @@ -3287,8 +3289,6 @@ void MusicXMLParserDirection::directionType(QList& starts, else skipLogCurrElem(); } - - Q_ASSERT(_e.isEndElement() && _e.name() == "direction-type"); } //--------------------------------------------------------- @@ -3301,8 +3301,6 @@ void MusicXMLParserDirection::directionType(QList& starts, void MusicXMLParserDirection::sound() { - Q_ASSERT(_e.isStartElement() && _e.name() == "sound"); - _sndCoda = _e.attributes().value("coda").toString(); _sndDacapo = _e.attributes().value("dacapo").toString(); _sndDalsegno = _e.attributes().value("dalsegno").toString(); @@ -3325,8 +3323,6 @@ void MusicXMLParserDirection::sound() void MusicXMLParserDirection::dynamics() { - Q_ASSERT(_e.isStartElement() && _e.name() == "dynamics"); - while (_e.readNextStartElement()) { if (_e.name() == "other-dynamics") _dynamicsList.push_back(_e.readElementText()); @@ -4167,8 +4163,6 @@ void MusicXMLParserPass2::deleteHandledSpanner(SLine* const& spanner) QString MusicXMLParserDirection::metronome(double& r) { - Q_ASSERT(_e.isStartElement() && _e.name() == "metronome"); - r = 0; QString tempoText; QString perMinute; @@ -4356,8 +4350,6 @@ static void addBarlineToMeasure(Measure* measure, const Fraction tick, std::uniq void MusicXMLParserPass2::barline(const QString& partId, Measure* measure, const Fraction& tick) { - Q_ASSERT(_e.isStartElement() && _e.name() == "barline"); - QString loc = _e.attributes().value("location").toString(); if (loc == "") loc = "right"; @@ -4631,8 +4623,6 @@ static void flushAlteredTone(KeySigEvent& kse, QString& step, QString& alt, QStr void MusicXMLParserPass2::key(const QString& partId, Measure* measure, const Fraction& tick) { - Q_ASSERT(_e.isStartElement() && _e.name() == "key"); - QString strKeyno = _e.attributes().value("number").toString(); int keyno = -1; // assume no number (see below) if (strKeyno != "") { @@ -4721,27 +4711,6 @@ void MusicXMLParserPass2::key(const QString& partId, Measure* measure, const Fra void MusicXMLParserPass2::clef(const QString& partId, Measure* measure, const Fraction& tick) { - Q_ASSERT(_e.isStartElement() && _e.name() == "clef"); - - Part* part = _pass1.getPart(partId); - Q_ASSERT(part); - - // TODO: check error handling for - // - single staff - // - multi-staff with same clef - QString strClefno = _e.attributes().value("number").toString(); - const bool afterBarline = _e.attributes().value("after-barline") == "yes"; - int clefno = 0; // default - if (strClefno != "") - clefno = _pass1.getMusicXmlPart(partId).staffNumberToIndex(strClefno.toInt()); - if (clefno < 0 || clefno >= part->nstaves()) { - // conversion error (0) or other issue, assume staff 1 - // Also for Cubase 6.5.5 which generates clef number="2" in a single staff part - // Same fix is required in pass 1 and pass 2 - _logger->logError(QString("invalid clef number '%1'").arg(strClefno), &_e); - clefno = 0; - } - ClefType clef = ClefType::G; StaffTypes st = StaffTypes::STANDARD; @@ -4749,6 +4718,9 @@ void MusicXMLParserPass2::clef(const QString& partId, Measure* measure, const Fr int i = 0; int line = -1; + QString strClefno = _e.attributes().value("number").toString(); + const bool afterBarline = _e.attributes().value("after-barline") == "yes"; + while (_e.readNextStartElement()) { if (_e.name() == "sign") c = _e.readElementText(); @@ -4823,6 +4795,27 @@ void MusicXMLParserPass2::clef(const QString& partId, Measure* measure, const Fr else qDebug("clef: unknown clef ", qPrintable(c), line, i); // TODO + + Part* part = _pass1.getPart(partId); + if (!part) + return; + + // TODO: check error handling for + // - single staff + // - multi-staff with same clef + int clefno = 1; // default + if (strClefno != "") + clefno = strClefno.toInt(); + if (clefno <= 0 || clefno > part->nstaves()) { + // conversion error (0) or other issue, assume staff 1 + // Also for Cubase 6.5.5 which generates clef number="2" in a single staff part + // Same fix is required in pass 1 and pass 2 + _logger->logError(QString("invalid clef number '%1'").arg(strClefno), &_e); + clefno = 1; + } + // convert to 0-based + clefno--; + Clef* clefs = new Clef(_score); clefs->setClefType(clef); int track = _pass1.trackForPart(partId) + clefno * VOICES; @@ -4923,8 +4916,6 @@ static bool determineTimeSig(const QString beats, const QString beatType, const void MusicXMLParserPass2::time(const QString& partId, Measure* measure, const Fraction& tick) { - Q_ASSERT(_e.isStartElement() && _e.name() == "time"); - QString beats; QString beatType; QString timeSymbol = _e.attributes().value("symbol").toString(); @@ -4975,8 +4966,6 @@ void MusicXMLParserPass2::time(const QString& partId, Measure* measure, const Fr void MusicXMLParserPass2::divisions() { - Q_ASSERT(_e.isStartElement() && _e.name() == "divisions"); - _divs = _e.readElementText().toInt(); if (!(_divs > 0)) _logger->logError("illegal divisions", &_e); @@ -5438,8 +5427,6 @@ Note* MusicXMLParserPass2::note(const QString& partId, Tuplets& tuplets ) { - Q_ASSERT(_e.isStartElement() && _e.name() == "note"); - if (_e.attributes().value("print-spacing") == "no") { notePrintSpacingNo(dura); return 0; @@ -5511,6 +5498,7 @@ Note* MusicXMLParserPass2::note(const QString& partId, } else if (_e.name() == "notations") { notations.parse(); + addError(notations.errors()); } else if (_e.name() == "notehead") { noteheadColor.setNamedColor(_e.attributes().value("color").toString()); @@ -5569,6 +5557,9 @@ Note* MusicXMLParserPass2::note(const QString& partId, if (errorStr != "") _logger->logError(errorStr, &_e); + if (!_pass1.getPart(partId)) + return nullptr; + // At this point all checks have been done, the note should be added // note: in case of error exit from here, the postponed children // must still be skipped @@ -5579,7 +5570,7 @@ Note* MusicXMLParserPass2::note(const QString& partId, if (!_pass1.determineStaffMoveVoice(partId, staff, voice, msMove, msTrack, msVoice)) { _logger->logDebugInfo(QString("could not map staff %1 voice '%2'").arg(staff + 1).arg(voice), &_e); - Q_ASSERT(_e.isEndElement() && _e.name() == "note"); + addError(checkAtEndElement(_e, "note")); return 0; } @@ -5768,8 +5759,6 @@ Note* MusicXMLParserPass2::note(const QString& partId, if (cr) cr->setVisible(printObject); - Q_ASSERT(_pass1.getPart(partId)); - // handle notations if (cr) { notations.addToScore(cr, note, noteStartTime.ticks(), _slurs, _glissandi, _spanners, _trills, _ties); @@ -5848,9 +5837,7 @@ Note* MusicXMLParserPass2::note(const QString& partId, if (chord || grace) dura.set(0, 1); - if (!(_e.isEndElement() && _e.name() == "note")) - qDebug("name %s line %lld", qPrintable(_e.name().toString()), _e.lineNumber()); - Q_ASSERT(_e.isEndElement() && _e.name() == "note"); + addError(checkAtEndElement(_e, "note")); return note; } @@ -5866,7 +5853,6 @@ Note* MusicXMLParserPass2::note(const QString& partId, void MusicXMLParserPass2::notePrintSpacingNo(Fraction& dura) { - Q_ASSERT(_e.isStartElement() && _e.name() == "note"); //_logger->logDebugTrace("MusicXMLParserPass1::notePrintSpacingNo", &_e); bool chord = false; @@ -5893,7 +5879,7 @@ void MusicXMLParserPass2::notePrintSpacingNo(Fraction& dura) if (chord || grace) dura.set(0, 1); - Q_ASSERT(_e.isEndElement() && _e.name() == "note"); + addError(checkAtEndElement(_e, "note")); } //--------------------------------------------------------- @@ -5906,8 +5892,6 @@ void MusicXMLParserPass2::notePrintSpacingNo(Fraction& dura) void MusicXMLParserPass2::duration(Fraction& dura) { - Q_ASSERT(_e.isStartElement() && _e.name() == "duration"); - dura.set(0, 0); // invalid unless set correctly const auto elementText = _e.readElementText(); if (elementText.toInt() > 0) @@ -5928,8 +5912,6 @@ void MusicXMLParserPass2::duration(Fraction& dura) FiguredBassItem* MusicXMLParserPass2::figure(const int idx, const bool paren) { - Q_ASSERT(_e.isStartElement() && _e.name() == "figure"); - FiguredBassItem* fgi = new FiguredBassItem(_score, idx); // read the figure @@ -6000,8 +5982,6 @@ FiguredBassItem* MusicXMLParserPass2::figure(const int idx, const bool paren) FiguredBass* MusicXMLParserPass2::figuredBass() { - Q_ASSERT(_e.isStartElement() && _e.name() == "figured-bass"); - FiguredBass* fb = new FiguredBass(_score); bool parentheses = _e.attributes().value("parentheses") == "yes"; @@ -6057,8 +6037,6 @@ FiguredBass* MusicXMLParserPass2::figuredBass() FretDiagram* MusicXMLParserPass2::frame(qreal& defaultY, qreal& relativeY) { - Q_ASSERT(_e.isStartElement() && _e.name() == "frame"); - FretDiagram* fd = new FretDiagram(_score); int fretOffset = 0; @@ -6183,8 +6161,6 @@ FretDiagram* MusicXMLParserPass2::frame(qreal& defaultY, qreal& relativeY) void MusicXMLParserPass2::harmony(const QString& partId, Measure* measure, const Fraction sTime, DelayedDirectionsList& delayedDirections) { - Q_ASSERT(_e.isStartElement() && _e.name() == "harmony"); - int track = _pass1.trackForPart(partId); // placement: @@ -6407,8 +6383,6 @@ void MusicXMLParserPass2::harmony(const QString& partId, Measure* measure, const */ void MusicXMLParserPass2::beam(QMap& beamTypes) { - Q_ASSERT(_e.isStartElement() && _e.name() == "beam"); - bool hasBeamNo; int beamNo = _e.attributes().value("number").toInt(&hasBeamNo); QString s = _e.readElementText(); @@ -6426,8 +6400,6 @@ void MusicXMLParserPass2::beam(QMap& beamTypes) { void MusicXMLParserPass2::forward(Fraction& dura) { - Q_ASSERT(_e.isStartElement() && _e.name() == "forward"); - while (_e.readNextStartElement()) { if (_e.name() == "duration") duration(dura); @@ -6450,8 +6422,6 @@ void MusicXMLParserPass2::forward(Fraction& dura) void MusicXMLParserPass2::backup(Fraction& dura) { - Q_ASSERT(_e.isStartElement() && _e.name() == "backup"); - while (_e.readNextStartElement()) { if (_e.name() == "duration") duration(dura); @@ -6491,8 +6461,6 @@ void MusicXMLParserLyric::skipLogCurrElem() void MusicXMLParserLyric::parse() { - Q_ASSERT(_e.isStartElement() && _e.name() == "lyric"); - std::unique_ptr lyric { new Lyrics(_score) }; // TODO in addlyrics: l->setTrack(trk); @@ -6572,8 +6540,6 @@ void MusicXMLParserLyric::parse() && (l->syllabic() == Lyrics::Syllabic::SINGLE || l->syllabic() == Lyrics::Syllabic::END) ) _extendedLyrics.insert(l); - - Q_ASSERT(_e.isEndElement() && _e.name() == "lyric"); } //--------------------------------------------------------- @@ -6586,8 +6552,6 @@ void MusicXMLParserLyric::parse() void MusicXMLParserNotations::slur() { - Q_ASSERT(_e.isStartElement() && _e.name() == "slur"); - Notation notation = Notation::notationWithAttributes(_e.name().toString(), _e.attributes(), "notations"); _notations.push_back(notation); @@ -6602,8 +6566,6 @@ void MusicXMLParserNotations::slur() } _e.readNext(); - - Q_ASSERT(_e.isEndElement() && _e.name() == "slur"); } //--------------------------------------------------------- @@ -6703,8 +6665,6 @@ static void addSlur(const Notation& notation, SlurStack& slurs, ChordRest* cr, c void MusicXMLParserNotations::tied() { - Q_ASSERT(_e.isStartElement() && _e.name() == "tied"); - Notation notation = Notation::notationWithAttributes(_e.name().toString(), _e.attributes(), "notations"); // Make sure "stops" get processed before "starts" if (notation.attribute("type") == "stop") @@ -6717,9 +6677,6 @@ void MusicXMLParserNotations::tied() } _e.readNext(); - - Q_ASSERT(_e.isEndElement() && _e.name() == "tied"); - } //--------------------------------------------------------- @@ -6732,8 +6689,6 @@ void MusicXMLParserNotations::tied() void MusicXMLParserNotations::dynamics() { - Q_ASSERT(_e.isStartElement() && _e.name() == "dynamics"); - _dynamicsPlacement = _e.attributes().value("placement").toString(); while (_e.readNextStartElement()) { @@ -6744,8 +6699,6 @@ void MusicXMLParserNotations::dynamics() _e.readNext(); } } - - Q_ASSERT(_e.isEndElement() && _e.name() == "dynamics"); } //--------------------------------------------------------- @@ -6761,8 +6714,6 @@ void MusicXMLParserNotations::dynamics() void MusicXMLParserNotations::articulations() { - Q_ASSERT(_e.isStartElement() && _e.name() == "articulations"); - while (_e.readNextStartElement()) { SymId id { SymId::noSym }; if (convertArticulationToSymId(_e.name().toString(), id)) { @@ -6795,9 +6746,6 @@ void MusicXMLParserNotations::articulations() skipLogCurrElem(); } } - //qDebug("::notations tokenString '%s' name '%s'", qPrintable(_e.tokenString()), qPrintable(_e.name().toString())); - - Q_ASSERT(_e.isEndElement() && _e.name() == "articulations"); } //--------------------------------------------------------- @@ -6810,8 +6758,6 @@ void MusicXMLParserNotations::articulations() void MusicXMLParserNotations::ornaments() { - Q_ASSERT(_e.isStartElement() && _e.name() == "ornaments"); - bool trillMark = false; // while (_e.readNextStartElement()) { @@ -6875,8 +6821,6 @@ void MusicXMLParserNotations::ornaments() void MusicXMLParserNotations::technical() { - Q_ASSERT(_e.isStartElement() && _e.name() == "technical"); - while (_e.readNextStartElement()) { SymId id { SymId::noSym }; if (convertArticulationToSymId(_e.name().toString(), id)) { @@ -6913,8 +6857,6 @@ void MusicXMLParserNotations::technical() void MusicXMLParserNotations::harmonic() { - Q_ASSERT(_e.isStartElement() && _e.name() == "harmonic"); - Notation notation = Notation::notationWithAttributes(_e.name().toString(), _e.attributes(), "technical", SymId::stringsHarmonic); while (_e.readNextStartElement()) { @@ -6932,8 +6874,6 @@ void MusicXMLParserNotations::harmonic() if (notation.subType() != "") { _notations.push_back(notation); } - - Q_ASSERT(_e.isEndElement() && _e.name() == "harmonic"); } //--------------------------------------------------------- @@ -6990,13 +6930,9 @@ void MusicXMLParserNotations::addTechnical(const Notation& notation, Note* note) void MusicXMLParserNotations::mordentNormalOrInverted() { - Q_ASSERT(_e.isStartElement() && (_e.name() == "mordent" || _e.name() == "inverted-mordent")); - Notation notation = Notation::notationWithAttributes(_e.name().toString(), _e.attributes(), "ornaments"); notation.setText(_e.readElementText()); _notations.push_back(notation); - - Q_ASSERT(_e.isEndElement() && (_e.name() == "mordent" || _e.name() == "inverted-mordent")); } //--------------------------------------------------------- @@ -7010,13 +6946,9 @@ void MusicXMLParserNotations::mordentNormalOrInverted() void MusicXMLParserNotations::glissandoSlide() { - Q_ASSERT(_e.isStartElement() && (_e.name() == "glissando" || _e.name() == "slide")); - Notation notation = Notation::notationWithAttributes(_e.name().toString(), _e.attributes(), "notations"); notation.setText(_e.readElementText()); _notations.push_back(notation); - - Q_ASSERT(_e.isEndElement() && (_e.name() == "glissando" || _e.name() == "slide")); } //--------------------------------------------------------- @@ -7116,7 +7048,8 @@ static void addArpeggio(ChordRest* cr, const QString& arpeggioType, static void addTie(const Notation& notation, Score* score, Note* note, const int track, std::map& ties, MxmlLogger* logger, const QXmlStreamReader* const xmlreader) { - Q_ASSERT(note); + if (!note) + return; const QString& type = notation.attribute("type"); const QString& orientation = notation.attribute("orientation"); const QString& lineType = notation.attribute("line-type"); @@ -7366,6 +7299,23 @@ MusicXMLParserNotations::MusicXMLParserNotations(QXmlStreamReader& e, Score* sco // nothing } +//--------------------------------------------------------- +// addError +//--------------------------------------------------------- + +// notes: +// when the parser gets out of sync, only a single error stack is reported +// even when e.g. two incorrect notations are present +// line number will be added by pass 2 + +void MusicXMLParserNotations::addError(const QString& error) + { + if (error != "") { + _logger->logError(error, &_e); + _errors += error; + } + } + //--------------------------------------------------------- // skipLogCurrElem //--------------------------------------------------------- @@ -7449,8 +7399,6 @@ void MusicXMLParserNotations::combineArticulations() void MusicXMLParserNotations::parse() { - Q_ASSERT(_e.isStartElement() && _e.name() == "notations"); - while (_e.readNextStartElement()) { if (_e.name() == "arpeggiate") { _arpeggioType = _e.attributes().value("direction").toString(); @@ -7503,7 +7451,7 @@ void MusicXMLParserNotations::parse() */ combineArticulations(); - Q_ASSERT(_e.isEndElement() && _e.name() == "notations"); + addError(checkAtEndElement(_e, "notations")); } //--------------------------------------------------------- @@ -7622,8 +7570,6 @@ void MusicXMLParserNotations::addToScore(ChordRest* const cr, Note* const note, void MusicXMLParserPass2::stem(Direction& sd, bool& nost) { - Q_ASSERT(_e.isStartElement() && _e.name() == "stem"); - // defaults sd = Direction::AUTO; nost = false; @@ -7653,8 +7599,6 @@ void MusicXMLParserPass2::stem(Direction& sd, bool& nost) void MusicXMLParserNotations::fermata() { - Q_ASSERT(_e.isStartElement() && _e.name() == "fermata"); - Notation notation = Notation::notationWithAttributes(_e.name().toString(), _e.attributes(), "notations"); const auto fermataText = _e.readElementText(); @@ -7679,8 +7623,6 @@ void MusicXMLParserNotations::fermata() } else _logger->logError(QString("unknown fermata '%1'").arg(fermataText), &_e); - - Q_ASSERT(_e.isEndElement() && _e.name() == "fermata"); } //--------------------------------------------------------- @@ -7693,8 +7635,6 @@ void MusicXMLParserNotations::fermata() void MusicXMLParserNotations::tuplet() { - Q_ASSERT(_e.isStartElement() && _e.name() == "tuplet"); - QString tupletType = _e.attributes().value("type").toString(); // QString tupletPlacement = _e.attributes().value("placement").toString(); not used (TODO) QString tupletBracket = _e.attributes().value("bracket").toString(); @@ -7724,8 +7664,6 @@ void MusicXMLParserNotations::tuplet() _tupletDesc.shownumber = TupletNumberType::NO_TEXT; else _tupletDesc.shownumber = TupletNumberType::SHOW_NUMBER; - - Q_ASSERT(_e.isEndElement() && _e.name() == "tuplet"); } //--------------------------------------------------------- diff --git a/importexport/musicxml/importmxmlpass2.h b/importexport/musicxml/importmxmlpass2.h index f3b30edef6e9d..2e3d44bddabdb 100644 --- a/importexport/musicxml/importmxmlpass2.h +++ b/importexport/musicxml/importmxmlpass2.h @@ -214,12 +214,14 @@ class MusicXMLParserNotations { void addToScore(ChordRest* const cr, Note* const note, const int tick, SlurStack& slurs, Glissando* glissandi[MAX_NUMBER_LEVEL][2], MusicXmlSpannerMap& spanners, TrillStack& trills, std::map& ties); + QString errors() const { return _errors; } MusicXmlTupletDesc tupletDesc() const { return _tupletDesc; } QString tremoloType() const { return _tremoloType; } int tremoloNr() const { return _tremoloNr; } bool mustStopGraceAFter() const { return _slurStop || _wavyLineStop; } bool skipCombine(const Notation& n1, const Notation& n2); private: + void addError(const QString& error); ///< Add an error to be shown in the GUI void addNotation(const Notation& notation, ChordRest* const cr, Note* const note); void addTechnical(const Notation& notation, Note* note); void combineArticulations(); @@ -237,7 +239,8 @@ class MusicXMLParserNotations { void tuplet(); QXmlStreamReader& _e; Score* const _score; // the score - MxmlLogger* _logger; // the error logger + MxmlLogger* _logger; // the error logger + QString _errors; // errors to present to the user MusicXmlTupletDesc _tupletDesc; QString _dynamicsPlacement; QStringList _dynamicsList; @@ -261,6 +264,7 @@ class MusicXMLParserPass2 { public: MusicXMLParserPass2(Score* score, MusicXMLParserPass1& pass1, MxmlLogger* logger); Score::FileError parse(QIODevice* device); + QString errors() const { return _errors; } // part specific data interface functions void addSpanner(const MusicXmlSpannerDesc& desc); @@ -269,6 +273,7 @@ class MusicXMLParserPass2 { void deleteHandledSpanner(SLine* const& spanner); private: + void addError(const QString& error); ///< Add an error to be shown in the GUI void initPartState(const QString& partId); SpannerSet findIncompleteSpannersAtPartEnd(); Score::FileError parse(); @@ -319,6 +324,7 @@ class MusicXMLParserPass2 { Score* const _score; // the score MusicXMLParserPass1& _pass1; // the pass1 results MxmlLogger* _logger; ///< Error logger + QString _errors; ///< Errors to present to the user // part specific data (TODO: move to part-specific class) diff --git a/importexport/musicxml/importxml.cpp b/importexport/musicxml/importxml.cpp index 1fa8a82d4ced7..1e3ae9ef2e475 100644 --- a/importexport/musicxml/importxml.cpp +++ b/importexport/musicxml/importxml.cpp @@ -233,7 +233,7 @@ static Score::FileError doValidateAndImport(Score* score, const QString& name, Q // actually do the import importMusicXMLfromBuffer(score, name, dev); - //qDebug("importMusicXml() return %d", int(res)); + //qDebug("res %d", static_cast(res)); return res; } diff --git a/importexport/musicxml/importxmlfirstpass.cpp b/importexport/musicxml/importxmlfirstpass.cpp index 91d67cd7847ab..23d045f6a00ff 100644 --- a/importexport/musicxml/importxmlfirstpass.cpp +++ b/importexport/musicxml/importxmlfirstpass.cpp @@ -165,7 +165,8 @@ int MusicXmlOctaveShiftList::octaveShift(const Fraction f) const void MusicXmlOctaveShiftList::addOctaveShift(const int shift, const Fraction f) { - Q_ASSERT(Fraction(0, 1) <= f); + if (Fraction(0, 1) > f) + return; //qDebug("addOctaveShift(shift %d f %s)", shift, qPrintable(f.print())); auto i = find(f); diff --git a/importexport/musicxml/musicxmlsupport.cpp b/importexport/musicxml/musicxmlsupport.cpp index 036addacd7f47..bc27db2e2f14a 100644 --- a/importexport/musicxml/musicxmlsupport.cpp +++ b/importexport/musicxml/musicxmlsupport.cpp @@ -252,6 +252,31 @@ void domNotImplemented(const QDomElement& e) } +//--------------------------------------------------------- +// xmlReaderLocation +//--------------------------------------------------------- + +QString xmlReaderLocation(const QXmlStreamReader& e) + { + return QObject::tr("line %1 column %2").arg(e.lineNumber()).arg(e.columnNumber()); + } + +//--------------------------------------------------------- +// checkAtEndElement +//--------------------------------------------------------- + +QString checkAtEndElement(const QXmlStreamReader& e, const QString& expName) + { + if (e.isEndElement() && e.name() == expName) + return ""; + + QString res = QObject::tr("expected token type and name 'EndElement %1', actual '%2 %3'") + .arg(expName) + .arg(e.tokenString()) + .arg(e.name().toString()); + return res; + } + //--------------------------------------------------------- // stringToInt //--------------------------------------------------------- diff --git a/importexport/musicxml/musicxmlsupport.h b/importexport/musicxml/musicxmlsupport.h index 1f13912108c9c..438626e614480 100644 --- a/importexport/musicxml/musicxmlsupport.h +++ b/importexport/musicxml/musicxmlsupport.h @@ -211,6 +211,7 @@ extern SymId mxmlString2accSymId(const QString mxmlName); extern AccidentalType microtonalGuess(double val); extern bool isLaissezVibrer(const SymId id); extern bool hasLaissezVibrer(const Chord* const chord); - +extern QString xmlReaderLocation(const QXmlStreamReader& e); +extern QString checkAtEndElement(const QXmlStreamReader& e, const QString& expName); } // namespace Ms #endif