[MusicXML export] invalid XML export: direction-type is missing child element

Came up again in https://musescore.org/en/node/312225
Relates to #270643: [EPIC] MusicXML import/export issues

Sorry for multiple reporting.
I should have searched on "direction-type" to see that the problem was already reported.

Could not get reproduced in current master (e3ec24f of 2021-07-30)

Isn't it then a duplicate of #326798: [MusicXML] Ties in cue notes?

It does happen in 4.0.0 too

With the XML from https://musescore.org/en/node/341231

Came up again in https://musescore.org/de/node/341863#comment-1191886 (along with mscz and mxl)
Has the issue with 3.6.2, 4.0.2 and the latest development builds

See https://www.w3.org/2021/06/musicxml40/musicxml-reference/elements/direc…

Content
Exactly one of the following
...

But here they are empty:

...
      <direction placement="above">
        <direction-type>
          </direction-type>
        <sound tempo="72"/>
        </direction>
      <direction placement="above">
        <direction-type>
          </direction-type>
        <sound tempo="72"/>
        </direction>
 ...

Here apparently lacking <words>...</words>?, but in other places it is just

      <direction>
        <direction-type>
          </direction-type>
        </direction>

Interestingly with the score from that report Mu4 crashes when exporting it to MusicXML with Multimeasure rests disabled

And the fix for that it to have the keysig (3 sharps) at the first measure, not at the first after all mm-rests, then also the exported musicxml is clean!

Yeah, I looked into that that method too and couldn't figure out where and why it goes wrong

There's two places where setLayoutInvalid(); is called, one in the context of multi-measure rests, in mmrestrange.cpp:

bool MMRestRange::setProperty(Pid id, const PropertyValue& val)
{
    switch (id) {
    case Pid::MMREST_RANGE_BRACKET_TYPE:
        setBracketType(MMRestRangeBracketType(val.toInt()));
        setLayoutInvalid();
        triggerLayout();
        return true;
    default:
        return MeasureNumberBase::setProperty(id, val);
    }
}

The other in measurenumberbase.cpp:

bool MeasureNumberBase::setProperty(Pid id, const PropertyValue& val)
{
    switch (id) {
    case Pid::HPLACEMENT:
        setHPlacement(val.value<PlacementH>());
        setLayoutInvalid();
        triggerLayout();
        return true;
    default:
        return TextBase::setProperty(id, val);
    }
}

And except for the initialisation the only other place where it is set to true is in textbase.cpp:

void TextBase::setXmlText(const String& s)
{
    _text = s;
    textInvalid = false;
    layoutInvalid = true;
}

Seems that it goes wrong in TextBase::fragementList() which returns an empty list !?!

We never get into the inner for loop here:

std::list<TextFragment> TextBase::fragmentList() const
{
    std::list<TextFragment> res;
    for (const TextBlock& block : _layout) {
        for (const TextFragment& f : block.fragments()) {
            /* TODO TBD
            if (f.text.empty())                     // skip empty fragments, not to
                  continue;                           // insert extra HTML formatting
             */
            res.push_back(f);
            if (block.eol()) {
                // simply append a newline
                res.back().text += u"\n";
            }
        }
    }
    return res;
}

And the apparent trigger for all this mess seems to lay in the missing key sig at the start of the sample score (no keygsig, not just C-Major/a-minor, just none at all). But why???

It may not be related to multi measure rests at all?

The initial issues seems fixed though in current master, only the score from https://musescore.org/en/node/341863 still fails

But even with an initial keysig the export still looses the <font size="10"/><font face="Edwin"/><b>Andantino quasi Andante</b> tempo text
And import does strange things to the rehearsal marks A, B and C (but not D): it places them below the top staff rather than above. Probably an entirely differtent issue though

Same thing, trigger is the apparently missing initial key signature.
Take Tempo_measure_rest_362.mscz, disable mm rests, add a C-Major keysig to the first measure, enable mm rests, export to MusicXML -> no problem

Hmm, strange, it isn't quite that, the saved closed and reopened score has the same issue, but when doing this in one session it works, guess it does cause a re-layout?

And the invalid tempo text isn't the only issue, there's at least one other issue with empty direction-types, with (multi) measure numbers, not with your small sample score though, but with that Agnus Dei

For the tempo text I have a (crude) fix:

diff --git a/src/importexport/musicxml/internal/musicxml/exportxml.cpp b/src/importexport/musicxml/internal/musicxml/exportxml.cpp
index 075c7f3aae..cb928457d1 100644
--- a/src/importexport/musicxml/internal/musicxml/exportxml.cpp
+++ b/src/importexport/musicxml/internal/musicxml/exportxml.cpp
@@ -4393,8 +4396,13 @@ static void wordsMetronome(XmlWriter& xml, Score* s, TextBase const* const text,
         }
         attr += ExportMusicXml::positioningAttributes(text);
         MScoreTextToMXML mttm("words", attr, defFmt, mtf);
-        //LOGD("words('%s')", qPrintable(text->text()));
-        mttm.writeTextFragments(text->fragmentList(), xml);
+        //LOGD("words('%s')", qPrintable(text->xmlText()));
+        if (list.size()) {
+            mttm.writeTextFragments(list, xml);
+        } else {
+            xml.tag("words", text->plainText());
+        }
+
         xml.endElement();
     }

So the fix to TextBase::plainText() in 3653f4b needs to get added to TextBase::fragmentList()?

Yes, I can confirm that it does fix it. I'll backport it to 3.x, see https://github.com/Jojo-Schmitz/MuseScore/pull/124

Adjusted mine (for 3.x)

Fixed in branch master, commit a658fca1f4

_Fix #293441: [MusicXML export] invalid XML export: direction-type is missing child element

TextBase::fragmentList() (last updated by commit 10ebbb9 on Aug 1, 2018) is missing a fix
introduced in the similar function TextBase::plainText() (added by commit 3653f4b on Nov 19, 2018).
The fix relayouts a clone of the TextBase when it encounters a TextBase with an invalid layout._

Fixed in branch master, commit ab0b70d907

_Merge pull request #18056 from Jojo-Schmitz/293441-direction-type-missing-child

Fix #293441: [MusicXML export] invalid XML export: direction-type is missing child element_

Automatically closed -- issue fixed for 2 weeks with no activity.