From dd9d7738961587045e80101f47602cdd06b469fa Mon Sep 17 00:00:00 2001 From: AntonioBL Date: Mon, 27 Apr 2020 20:02:21 +0200 Subject: [PATCH] fix #303983 : MuseScore takes very long on opening some 2.x scores; without breaking #285434 --- libmscore/read206.cpp | 195 ++++++++++++++++++++++++++-------------- libmscore/scorefile.cpp | 3 - libmscore/xml.h | 17 ++-- libmscore/xmlreader.cpp | 6 +- 4 files changed, 140 insertions(+), 81 deletions(-) diff --git a/libmscore/read206.cpp b/libmscore/read206.cpp index a793e6489171d..627b99be2edd7 100644 --- a/libmscore/read206.cpp +++ b/libmscore/read206.cpp @@ -1525,46 +1525,18 @@ bool readNoteProperties206(Note* note, XmlReader& e) //--------------------------------------------------------- // ReadStyleName206 -// For 2.x files, the style tag could be in a different -// position with respect to 3.x files. Since seek -// position is not reliable for readline in QIODevices (for -// example because of non-single-byte characters in at least -// one of the fields; some two-byte characters are counted as -// two single-byte characters and thus the reading could -// start at the wrong position) +// Retrieve the content of the "style" tag from the +// QString with the content of the whole text tag //--------------------------------------------------------- -static QString ReadStyleName206(XmlReader& e) +static QString ReadStyleName206(QString xmlTag) { QString s; - QIODevice* device = e.getDevice(); - if (!device || device->isSequential()) - return s; - if (!device->isOpen()) - device->open(QIODevice::ReadOnly); - const auto pos = device->pos(); - const auto pos1 = e.characterOffset(); - const QString tagName = e.name().toString(); - device->seek(0); - XmlReader streamReader(device); - QString xmltag; - QString name; - for (;;) { - streamReader.readNextStartElement(); - name = streamReader.name().toString(); - if ((name == tagName) && (streamReader.characterOffset() == pos1)) { - xmltag = streamReader.readXml(); - if (xmltag.contains(""); - if (re.indexIn(xmltag) > -1) - s = re.cap(1); - } - break; - } - if (streamReader.atEnd()) - break; + if (xmlTag.contains(""); + if (re.indexIn(xmlTag) > -1) + s = re.cap(1); } - device->seek(pos); return s; } @@ -1574,9 +1546,9 @@ static QString ReadStyleName206(XmlReader& e) // before setting anything else. //--------------------------------------------------------- -static bool readTextPropertyStyle206(XmlReader& e, TextBase* t, Element* be) +static bool readTextPropertyStyle206(QString xmlTag, const XmlReader& e, TextBase* t, Element* be) { - QString s = ReadStyleName206(e); + QString s = ReadStyleName206(xmlTag); if (s.isEmpty()) return false; @@ -1682,16 +1654,99 @@ static bool readTextProperties206(XmlReader& e, TextBase* t) return true; } +//--------------------------------------------------------- +// TextReaderContext206 +// For 2.x files, the style tag could be in a different +// position with respect to 3.x files. Since seek +// position is not reliable for readline in QIODevices (for +// example because of non-single-byte characters in at least +// one of the fields; some two-byte characters are counted as +// two single-byte characters and thus the reading could +// start at the wrong position), a copy of the text tag +// is created and read in a separate XmlReader, while +// the text style is extracted from a QString containing +// the whole text xml tag. +// TextReaderContext206 takes care of this process +//--------------------------------------------------------- + +class TextReaderContext206 { + XmlReader& origReader; + XmlReader tagReader; + QString xmlTag; + + public: + TextReaderContext206(XmlReader& e) + : origReader(e), tagReader(QString()) + { + // Create a new xml document containing only the (text) xml chunk + QString name = origReader.name().toString(); + qint64 additionalLines = origReader.lineNumber() - 2; // Subtracting the 2 new lines that will be added + xmlTag = origReader.readXml(); + xmlTag.prepend("\n<" + name + ">"); + xmlTag.append("\n"); + tagReader.addData(xmlTag); // Add the xml data to the XmlReader + // the additional lines are needed to output the correct line number + // of the original file in case of error + tagReader.setOffsetLines(additionalLines); + copyProperties(origReader, tagReader); + tagReader.readNextStartElement(); // read up to the first "name" tag + } + + // Disable copying the TextReaderContext206 + TextReaderContext206(const TextReaderContext206&) = delete; + TextReaderContext206& operator=(const TextReaderContext206&) = delete; + + ~TextReaderContext206() + { + // Ensure to copy back the potentially changed properties + // to the original XmlReader before destruction + copyProperties(tagReader, origReader); + } + + XmlReader& reader() { return tagReader; } + const QString& tag() { return xmlTag; } + private: + void copyProperties(XmlReader& original, XmlReader& derived); + }; + +//--------------------------------------------------------- +// copyProperties +// Copy some of the XmlReader properties of an original +// XmlReader object to a "derived" XmlReader object. +// This is used for example when using the derived XmlReader +// to read the element properties of a 2.x text, or to +// update the base XmlReader object (for example, its +// link list) after reading the properties of a 2.x text +//--------------------------------------------------------- + +void TextReaderContext206::copyProperties(XmlReader& original, XmlReader& derived) + { + derived.setDocName(original.getDocName()); + derived.setTrackOffset(original.trackOffset()); + derived.setTrack(original.track() - original.trackOffset()); + + derived.setTickOffset(original.tickOffset()); + derived.setTick(original.tick() - original.tickOffset()); + + derived.setLastMeasure(original.lastMeasure()); + derived.setCurrentMeasure(original.currentMeasure()); + derived.setCurrentMeasureIndex(original.currentMeasureIndex()); + + derived.linkIds() = original.linkIds(); + derived.staffLinkedElements() = original.staffLinkedElements(); + } + //--------------------------------------------------------- // readText206 //--------------------------------------------------------- static void readText206(XmlReader& e, TextBase* t, Element* be) { - readTextPropertyStyle206(e, t, be); - while (e.readNextStartElement()) { - if (!readTextProperties206(e, t)) - e.unknown(); + TextReaderContext206 ctx(e); + readTextPropertyStyle206(ctx.tag(), e, t, be); + while (ctx.reader().readNextStartElement()) { + if (!readTextProperties206(ctx.reader(), t)) + ctx.reader().unknown(); } } @@ -1701,15 +1756,16 @@ static void readText206(XmlReader& e, TextBase* t, Element* be) static void readTempoText(TempoText* t, XmlReader& e) { - readTextPropertyStyle206(e, t, t); - while (e.readNextStartElement()) { - const QStringRef& tag(e.name()); + TextReaderContext206 ctx(e); + readTextPropertyStyle206(ctx.tag(), e, t, t); + while (ctx.reader().readNextStartElement()) { + const QStringRef& tag(ctx.reader().name()); if (tag == "tempo") - t->setTempo(e.readDouble()); + t->setTempo(ctx.reader().readDouble()); else if (tag == "followText") - t->setFollowText(e.readInt()); - else if (!readTextProperties206(e, t)) - e.unknown(); + t->setFollowText(ctx.reader().readInt()); + else if (!readTextProperties206(ctx.reader(), t)) + ctx.reader().unknown(); } // check sanity if (t->xmlText().isEmpty()) { @@ -1726,18 +1782,19 @@ static void readTempoText(TempoText* t, XmlReader& e) static void readMarker(Marker* m, XmlReader& e) { - readTextPropertyStyle206(e, m, m); + TextReaderContext206 ctx(e); + readTextPropertyStyle206(ctx.tag(), e, m, m); Marker::Type mt = Marker::Type::SEGNO; - while (e.readNextStartElement()) { - const QStringRef& tag(e.name()); + while (ctx.reader().readNextStartElement()) { + const QStringRef& tag(ctx.reader().name()); if (tag == "label") { - QString s(e.readElementText()); + QString s(ctx.reader().readElementText()); m->setLabel(s); mt = m->markerType(s); } - else if (!readTextProperties206(e, m)) - e.unknown(); + else if (!readTextProperties206(ctx.reader(), m)) + ctx.reader().unknown(); } m->setMarkerType(mt); } @@ -1748,17 +1805,18 @@ static void readMarker(Marker* m, XmlReader& e) static void readDynamic(Dynamic* d, XmlReader& e) { - readTextPropertyStyle206(e, d, d); - while (e.readNextStartElement()) { - const QStringRef& tag = e.name(); + TextReaderContext206 ctx(e); + readTextPropertyStyle206(ctx.tag(), e, d, d); + while (ctx.reader().readNextStartElement()) { + const QStringRef& tag = ctx.reader().name(); if (tag == "subtype") - d->setDynamicType(e.readElementText()); + d->setDynamicType(ctx.reader().readElementText()); else if (tag == "velocity") - d->setVelocity(e.readInt()); + d->setVelocity(ctx.reader().readInt()); else if (tag == "dynType") - d->setDynRange(Dynamic::Range(e.readInt())); - else if (!readTextProperties206(e, d)) - e.unknown(); + d->setDynRange(Dynamic::Range(ctx.reader().readInt())); + else if (!readTextProperties206(ctx.reader(), d)) + ctx.reader().unknown(); } } @@ -3153,7 +3211,8 @@ static void readMeasure(Measure* m, int staffIdx, XmlReader& e) // MuseScore 3 has different types for system text and // staff text while MuseScore 2 didn't. // We need to decide first which one we should create. - QString styleName = ReadStyleName206(e); + TextReaderContext206 ctx(e); + QString styleName = ReadStyleName206(ctx.tag()); StaffTextBase* t; if (styleName == "System" || styleName == "Tempo" || styleName == "Marker" || styleName == "Jump" @@ -3162,12 +3221,16 @@ static void readMeasure(Measure* m, int staffIdx, XmlReader& e) else t = new StaffText(score); t->setTrack(e.track()); - readText206(e, t, t); + readTextPropertyStyle206(ctx.tag(), e, t, t); + while (ctx.reader().readNextStartElement()) { + if (!readTextProperties206(ctx.reader(), t)) + ctx.reader().unknown(); + } if (t->empty()) { if (t->links()) { if (t->links()->size() == 1) { qDebug("reading empty text: deleted lid = %d", t->links()->lid()); - e.linkIds().remove(t->links()->lid()); + ctx.reader().linkIds().remove(t->links()->lid()); delete t; } } @@ -3188,7 +3251,7 @@ static void readMeasure(Measure* m, int staffIdx, XmlReader& e) t->ryoffset() = yo; } #endif - segment = m->getSegment(SegmentType::ChordRest, e.tick()); + segment = m->getSegment(SegmentType::ChordRest, ctx.reader().tick()); segment->add(t); } } diff --git a/libmscore/scorefile.cpp b/libmscore/scorefile.cpp index c75606288b99b..2280f2cf75714 100644 --- a/libmscore/scorefile.cpp +++ b/libmscore/scorefile.cpp @@ -853,8 +853,6 @@ Score::FileError MasterScore::loadCompressedMsc(QIODevice* io, bool ignoreVersio } } XmlReader e(dbuf); - QBuffer readBuf(&dbuf); - e.setDevice(&readBuf); e.setDocName(masterScore()->fileInfo()->completeBaseName()); FileError retval = read1(e, ignoreVersionError); @@ -912,7 +910,6 @@ Score::FileError MasterScore::loadMsc(QString name, QIODevice* io, bool ignoreVe return loadCompressedMsc(io, ignoreVersionError); else { XmlReader r(io); - r.setDevice(io); return read1(r, ignoreVersionError); } } diff --git a/libmscore/xml.h b/libmscore/xml.h index bf2eb35351428..db876a4d295d4 100644 --- a/libmscore/xml.h +++ b/libmscore/xml.h @@ -69,10 +69,6 @@ class LinksIndexer { class XmlReader : public QXmlStreamReader { QString docName; // used for error reporting - // For readahead possibility. - // If needed, must be explicitly set by setDevice. - QIODevice* _readDevice = nullptr; - // Score read context (for read optimizations): Fraction _tick { Fraction(0, 1) }; Fraction _tickOffset { Fraction(0, 1) }; @@ -106,6 +102,8 @@ class XmlReader : public QXmlStreamReader { void addConnectorInfo(std::unique_ptr); void removeConnector(const ConnectorInfoReader*); // Removes the whole ConnectorInfo chain from the connectors list. + qint64 _offsetLines { 0 }; + public: XmlReader(QFile* f) : QXmlStreamReader(f), docName(f->fileName()) {} XmlReader(const QByteArray& d, const QString& st = QString()) : QXmlStreamReader(d), docName(st) {} @@ -148,6 +146,7 @@ class XmlReader : public QXmlStreamReader { Fraction tick() const { return _tick + _tickOffset; } Fraction rtick() const ; + Fraction tickOffset() const { return _tickOffset; } void setTick(const Fraction& f); void incTick(const Fraction& f); void setTickOffset(const Fraction& val) { _tickOffset = val; } @@ -203,13 +202,13 @@ class XmlReader : public QXmlStreamReader { void checkTuplets(); Tid addUserTextStyle(const QString& name); - Tid lookupUserTextStyle(const QString& name); - - // Ownership on read device is NOT transferred to XmlReader. - void setDevice(QIODevice* dev) { if (!dev->isSequential()) _readDevice = dev; } - QIODevice* getDevice() { return _readDevice; } + Tid lookupUserTextStyle(const QString& name) const; QList>& fixOffsets() { return _fixOffsets; } + + // for reading old files (< 3.01) + QMap>>& staffLinkedElements() { return _staffLinkedElements; } + void setOffsetLines(qint64 val) { _offsetLines = val; } }; //--------------------------------------------------------- diff --git a/libmscore/xmlreader.cpp b/libmscore/xmlreader.cpp index 1f3690330960c..637e915956124 100644 --- a/libmscore/xmlreader.cpp +++ b/libmscore/xmlreader.cpp @@ -207,9 +207,9 @@ void XmlReader::unknown() qDebug("%s ", qPrintable(errorString())); if (!docName.isEmpty()) qDebug("tag in <%s> line %lld col %lld: %s", - qPrintable(docName), lineNumber(), columnNumber(), name().toUtf8().data()); + qPrintable(docName), lineNumber() + _offsetLines, columnNumber(), name().toUtf8().data()); else - qDebug("line %lld col %lld: %s", lineNumber(), columnNumber(), name().toUtf8().data()); + qDebug("line %lld col %lld: %s", lineNumber() + _offsetLines, columnNumber(), name().toUtf8().data()); skipCurrentElement(); } @@ -538,7 +538,7 @@ Tid XmlReader::addUserTextStyle(const QString& name) // lookupUserTextStyle //--------------------------------------------------------- -Tid XmlReader::lookupUserTextStyle(const QString& name) +Tid XmlReader::lookupUserTextStyle(const QString& name) const { for (const auto& i : userTextStyles) { if (i.name == name)