fix #303983 : MuseScore takes very long on opening some 2.x scores; w…

[AntonioBL]

…ithout breaking #285434

See https://musescore.org/en/node/303983

This rewrites text property reading for 2.x scores, so that "style" tag can be read before the others.
Refactoring of the helper class was done with the precious help of dmitrio95.

• I signed CLA
• I made sure the code in the PR follows the coding rules
• I made sure the code compiles on my machine
• I made sure there are no unnecessary changes in the code
• I made sure the title of the PR reflects the core meaning of the issue you are solving
• I made sure the commit message(s) contain a description and answer the question "Why do those changes fix that particular issue?" or "Why are those changes really necessary as improvements?"
• I made sure the commit message title starts with "fix #424242:" if there is a related issue
• [N/A] I created the test (mtest, vtest, script test) to verify the changes I made

[AntonioBL]

I think I managed to solve point 1 and 2.
Please review this PR.
I wanted to condense the code in one function, but XmlReaders can't be copied. How can I write the PR in a more elegant way?

[dmitrio95]

There are many options to avoid code duplication but one option I can think of is to create a helper class for this instead of just a function, something like this (my comments are marked with NOTE, they are certainly not suggested to be in the code):

class TextReaderContext { // or, perhaps, TextReaderContext206?
      XmlReader& origReader;
      XmlReader tagReader;
      QString xmlTag;

   public:
      TextReaderContext(XmlReader& e)
         : origReader(e), tagReader(QString())
            {
            // NOTE: The code initializing text reading-related variables:
            QString name = origReader.name().toString();
            qint64 additionalLines = origReader.lineNumber() - 2; // Subtracting the 2 new lines that will be added
            QString xmlTag = origReader.readXml();
            xmlTag.prepend("<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<" + name + ">");
            xmlTag.append("</" + name + ">\n");
            tagReader.addData(xmlTag); // NOTE: The different part: adding data to existing XmlReader 
            tagReader.setOffsetLines(additionalLines);
            copyProperties(origReader, tagReader);
            tagReader.readNextStartElement(); // read up to the first "name" tag
            }

      // NOTE: just to be cautious, disable copying the context
      TextReaderContext(const TextReaderContext&) = delete;
      TextReaderContext& operator=(const TextReaderContext&) = delete;

      ~TextReaderContext()
            {
            // NOTE: Copying properties in context destructor to
            // ensure we don't forget to return them back
            copyProperties(tagReader, origReader);
            }

      XmlReader& reader() { return tagReader; }
      const QString& tag() { return xmlTag; }
      };

and using it like

void readSomething(Something* s, XmlReader& e)
      {
      TextReaderContext ctx(e);
      readTextPropertyStyle206(e.xmlTag(), e, d, d);
      while (ctx.reader().readNextStartElement())
            // use ctx.reader() where needed
      // no need for explicit copyProperties() call:
      // it is done in TextReaderContext destructor.
      }

Initializing an deinitializing these text reading-related variables can certainly also be done by standalone functions but perhaps such a helper class will make them more convenient to use.

[dmitrio95]

A minor suggestion: to ensure that this XmlReader is not actually used for reading it might be good to also declare the e parameter as const XmlReader& e (this will also require to mark XmlReader::lookupUserTextStyle() function with const modifier which should logically be there anyway).

[AntonioBL]

@dmitrio95 : thank you very much for your help.
I updated the PR following your advice.
Now the code looks a lot less messy. And if changes are needed, we can act only on the helper class instead of searching and modifying all repeated instances inside the code.
I checked that the result for the score posted in the issue tracker (Symphony n. 41 by Mozart) is the same as that given by the current code, even when all parts are created.
(I noticed that there is still a bug about user styles for parts; but that does not depend on this PR. I will post later in the issue tracker)