Fix #27201 - PLUGINS: methods returning pntrs to QObject lead to crash.

[mgavioli]

Fix #27201 - PLUGINS: methods returning pntrs to QObject lead to crash.

For details, including an analysis of the problem, see http://musescore.org/en/node/27201

Fixes the following Q_INVOKABLE methods returning a QObject* by turning them into a property:

• Measure:
  • firstSegment
  • lastSegment
• MeasureBase:
  • nextMeasure
  • nextMeasureMM (new)
  • prevMeasure
  • prevMeasureMM (new)
• Note:
  • accidental
  • dots[]
  • tieFor
  • tieBack
• Score:
  • firstMeasure
  • firstMeasureMM (new)
  • (for firstSegment(), see special cases below)
  • lastMeasure
  • lastMeasureMM (new)
  • lastSegment
• Segment:
  • next (renamed from next1)
  • nextInMeasure (renamed from next)
  • prev (renamed from prev1)
  • prevInMeasure (renamed from prev)

Special cases:

• Cursor: The prototype of the Q_INVOKABLE Ms::Note* Cursor::addNote(int pitch) was wrong: corrected in Q_INVOKABLE void Cursor::addNote(int pitch).
• QmlPlugin: Q_INVOKABLE Score* QmlPlugin::readScore() and Q_INVOKABLE Score* QmlPlugin::newScore() has been kept, as they are intended to be called from QML; code has been added to ensure the C++ ownership of the returned object.
• Score: Q_INVOKABLE Segment* Score::firstSegment(Segment::Type segType) is kept (as it needs a parameters), but code is added to ensure C++ ownership of the returned Segment*.
• Segment: Ms::Element* Segment::element(int track) has been made NOT Q_INVOKABLE; a variant Q_INVOKABLE Ms::Element* elementAt(int track) has been added specifically for QML with code to ensure the C++ ownership of the returned Element* (this was the cause for the crash of the Walk plug-in).
• FiguredBass: Q_INVOKABLE Ms::FiguredBassItem* FiguredBass::addItem() has been removed; plugin interface for FiguredBass needs to be redesigned anyway.

The few occurrences in the supplied plug-ins of the methods whose names did change have been updated.

[lasconic]

Typo here, should be qmlDots.
Except this, it's a wonderful analysis! Thank you for doing it!

[mgavioli]

Thanks for spotting the typos.

This is a potentially exhaustive list of Q_INVOKABLE methods returning pointers to objects derived from QObject in the various classes. It has been made by searching for "Q_INVOKABLE Ms::" in .h files:

• libmscore/cursor.h

Q_INVOKABLE Ms::Note* addNote(int pitch)

• libmscore/figuredbass.h:

Q_INVOKABLE Ms::FiguredBassItem* addItem()

• libmscore/measure.h

Q_INVOKABLE Ms::Segment* first()
Q_INVOKABLE Ms::Segment* last()

• libmscore/measurebase.h:

Q_INVOKABLE Ms::Measure* nextMeasure()
Q_INVOKABLE Ms::Measure* prevMeasure()

• libmscore/score.h:

Q_INVOKABLE Ms::Measure* firstMeasure()
Q_INVOKABLE Ms::Measure* lastMeasure()
Q_INVOKABLE Ms::Segment* firstSegment(Segment::Type s = Segment::Type::All)
Q_INVOKABLE Ms::Segment* lastSegment()

• libmscore/segment.h:

Q_INVOKABLE Ms::Segment* next()
Q_INVOKABLE Ms::Segment* prev()
Q_INVOKABLE Ms::Segment* next1()
Q_INVOKABLE Ms::Segment* prev1()
Q_INVOKABLE Ms::Element* element(int track)

I think all of them should be converted into properties or, in some way, given a C++ ownership, as the returned element should not be garbage collected. Probably, Cursor::addNote() and FiguredBass::addItem() should be removed and replaced by some usage of QmlPlugin::newElement() (the last one is a temporary hack anyway...).

I am not sure what to do about the following in mscore/qmlplugin.h:
Q_INVOKABLE Ms::Score* newScore(...)
Q_INVOKABLE Ms::Element* newElement(int)
Q_INVOKABLE Ms::MsProcess* newQProcess()
Q_INVOKABLE Ms::Score* readScore(const QString& name)
as the first two seem to work as they are (it never occurred to me to loose to g.c. a newly created element in a plugin; but some more focused tests might be in order). I do not have enough familiarity with the last two to have an opinion.

Lastly, in libmscore/score.h:
Q_INVOKABLE Ms::Cursor* newCursor()
should remain as it is: the QML ownership of the returned cursor is correct, I presume.

Shall I go on and change at least the first group?

[lasconic]

Sure, go ahead and fix the more obvious. While converting them, I would give next1 and next proper names... Also prev/nextMeasure should probably call their MM counterpart from a plugin perspective... or we could expose both.

[mgavioli]

About the documentation in @P comments: as far I understand them, they are for JS documentation; shouldn't they reflect the code as used in JS?

1. Is it useful to put the Ms:: prefix? It is not used in actual JS code (Element, not Ms.Element and even less Ms::Element of course!) and having it in the docs may be misleading.

2. Same for enums (again!): for instance, element type is accessed with if (type == Element.CHORD), not with if (type == Element.Type.CHORD). So, for instance, the glissando property:

@P glissandoType Ms::Glissando::Type (STRAIGHT, WAVY)

should probably read:

A) @P glissandoType Glissando.STRAIGHT, Glissando.WAVY

or maybe with pattern:

B) @P glissandoType Glissando.[STRAIGHT|WAVY]

for properties with a large number of retuned values, which would result in over-long lines with the A) pattern.

Am I missing something? Comments?

[Jojo-Schmitz]

See http://musescore.org/en/node/26796, it does request e better method to document enums for plugins

[Jojo-Schmitz]

I put in the Ms:: in the hope of this making the hyperlinks in the documentation work