Fix GH#26593: Crash when importing MusicXML with multiple fermatas on a grace note

[Jojo-Schmitz]

Resolves: #26593

applies to master and 4.5.0

[cbjeukendrup]

I think it would be better to look for the root cause of the problem, rather than hiding it. When a Fermata has no Segment, it probably means that either the parent is nullptr (which is invalid anyway) or that the parent has only been passed to the constructor and there has been no setParent call to make explicitParent() return something.

But for fermatas in particular, there also seems to be a special situation where they are first temporarily added to a ChordRest (which becomes its parent), and then moved to a Segment. See ChordRest::addFermata. Perhaps that plays a role here. But that would be unexpected too, since that temporary situation is supposed to be resolved during the MusicXML import, so should not be seen during layout.

[Jojo-Schmitz]

Well, ChordRest::addFermata looks like this:

    //! TODO Look like a hack, see using
    void addFermata(Fermata* f) { addEl(f); }
    void removeFermata(Fermata* f) { removeEl(f); }
    //! --------------------------------

It is used only once in importmusicxmlpass2.cpp:

    Fermata* na = Factory::createFermata(cr);
...
    if (cr->segment() == nullptr && cr->isGrace()) {
        cr->addFermata(na);           // store for later move to segment
    } else {
        cr->segment()->add(na);
    }

When crashing, the FERMATA in question does have a(n explict) parent (that is not explicitly set ?!?) and that parent is a CHORD. That chord apparently is a grace note, as the call is comming from here:

void ChordLayout::layoutPitched(Chord* item, LayoutContext& ctx)
{
    for (Chord* c : item->graceNotes()) {
        layoutPitched(c, ctx);
    }

So it indeed must have gone through that code (the addFermata()) mentioned above.

addFermata() calls addEl(), which in turn is only used there and for STRECHED_BEND, CHORDLINE and FRET_CIRCLE ?!?

I do see that adding a fermata to a grace note really adds it to the parent note/chord, I don't see however where this move happens?

[Jojo-Schmitz]

The problem seems to be that there are multiple fermatas added to gracenote in the .musicxml file:

      <note default-x="367.51" default-y="15.00">
        <grace/>
        <pitch>
          <step>B</step>
          <octave>5</octave>
          </pitch>
        <voice>1</voice>
        <type>16th</type>
        <stem>up</stem>
        <staff>1</staff>
        <beam number="1">begin</beam>
        <beam number="2">begin</beam>
        <notations>
          <fermata type="upright"/>
          <fermata type="upright"/>
          <fermata type="upright"/>
          <fermata type="upright"/>
          </notations>
        </note>
 

Removing those duplicates apparently fixes the crash

The pretty bad part is that the exporter was MuseScore 3.4.2... The score stems from http://musescore.com/score/658031 BTW

Minimized (manually stripped down) sample score: foo.zip, 3 grace notes with 4 fermatas each. But 3.6.2 imports only 7 (!?) of them:

Even stranger: when removing the duplicates 3.6.2 still shows 7!
And 4.x doesn't crash anymore but shows 7 too, only superimposed on one another, so it at least looks right.

[cbjeukendrup]

See #26680 for an alternative solution, partially inspired by the comments here

[Jojo-Schmitz]

Looks good to me (but see my comment there), although mine might get used in addition, as an extra safety net? Checking for nullptr before dereferencing usn't wrong after all.
Or mine for 4.5 (being way smaller and less risky?), (mine and) your's for 4.6?