Skip to content

Commit

Permalink
fix #303983 : MuseScore takes very long on opening some 2.x scores; w…
Browse files Browse the repository at this point in the history
…ithout breaking #285434
  • Loading branch information
AntonioBL committed May 11, 2020
1 parent 5eaa04f commit dd9d773
Show file tree
Hide file tree
Showing 4 changed files with 140 additions and 81 deletions.
195 changes: 129 additions & 66 deletions libmscore/read206.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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("<style>")) {
QRegExp re("<style>([^<]+)</style>");
if (re.indexIn(xmltag) > -1)
s = re.cap(1);
}
break;
}
if (streamReader.atEnd())
break;
if (xmlTag.contains("<style>")) {
QRegExp re("<style>([^<]+)</style>");
if (re.indexIn(xmlTag) > -1)
s = re.cap(1);
}
device->seek(pos);
return s;
}

Expand All @@ -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;
Expand Down Expand Up @@ -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("<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<" + name + ">");
xmlTag.append("</" + name + ">\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();
}
}

Expand All @@ -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()) {
Expand All @@ -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);
}
Expand All @@ -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();
}
}

Expand Down Expand Up @@ -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"
Expand All @@ -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;
}
}
Expand All @@ -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);
}
}
Expand Down
3 changes: 0 additions & 3 deletions libmscore/scorefile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
}
Expand Down
17 changes: 8 additions & 9 deletions libmscore/xml.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) };
Expand Down Expand Up @@ -106,6 +102,8 @@ class XmlReader : public QXmlStreamReader {
void addConnectorInfo(std::unique_ptr<ConnectorInfoReader>);
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) {}
Expand Down Expand Up @@ -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; }
Expand Down Expand Up @@ -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<std::pair<Element*, QPointF>>& fixOffsets() { return _fixOffsets; }

// for reading old files (< 3.01)
QMap<int, QList<QPair<LinkedElements*, Location>>>& staffLinkedElements() { return _staffLinkedElements; }
void setOffsetLines(qint64 val) { _offsetLines = val; }
};

//---------------------------------------------------------
Expand Down
6 changes: 3 additions & 3 deletions libmscore/xmlreader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit dd9d773

Please sign in to comment.