curScore returns always different values for the same score in MS3

I believe this is because MuseScore 3.x uses wrapper objects to provide access to internal objects. New wrappers are freely created when asked for even though they refer to the same internal objects. In v2.x the actual objects where exposed instead so you were comparing apples to apples--so to speak.

This will totally mess up object comparisons in QML.

To get around this I recently added a property called elementId to anything that subclasses ScoreElement in the plugin API. You can use it to compare the underlying elements:

https://musescore.org/en/node/293008
https://github.com/musescore/MuseScore/pull/5258

There are some test scripts at the above issue that illustrate its use.

Note that this pull request hasn't been accepted yet. My hope is it will make it into v3.3.

-Dale

I don't know if this will help you at all, but there is a built in score comparison tool in version 3. See https://musescore.org/en/handbook/3/score-comparison.

Everything at ScoreElement and to the right has the elementId property. That includes Scores.

I've been waiting for some feedback from the official MuseScore developers so this isn't a sure thing. But certainly some method is needed to do this.

Perhaps it should go even deeper and interpose a class between QObject and all plugin wrappers called something like PluginWrapperBase that exposes a more generic objectId.

Singleton wrapper mapping would also work but that's a more significant undertaking that impacts more code.

-Dale

I have used this code and found it useful to construct javascript maps of only the id's of objects I want to modify or consider for address- (EQ) equality. It would seem that doing this on the C++ side would have to create a duplicate of the entire score (every object at every level in it) because it doesn't know what you're goint to hold on to, and circular references exist in the structure. Did it really do that in MS2?

I wonder how it did that, and how it kept track of and destroyed all the objects it created ...

Or are you saying that in MS2 it handed out real addresses of C++ objects somehow yet had some other javascript mechanism for dereferencing their data members? Can someone explain what the MS2 mechanism was?

>Or are you saying that in MS2 it handed out real addresses of C++ objects somehow
>yet had some other javascript mechanism for dereferencing their data members?
>May be DLLarson knows exactly how it is/was exposed

All I can really tell you is that MuseScore 2.0 (1.x?) directly exposed to QML the inner objects of the C++ code. Qt/QML has all kinds of macros and templates to make it nearly effortless. So, if wrappers exist they would be something created by the QML engine on a one-to-one basis as a sort of proxy for the Javascript engine (for instance). All that would be implemented by the Qt/QML code. All you had to do was "mark up" so to speak your classes so that select things are available to QML. It's all built around Qt's QObject model.

In MuseScore 3.0 it looks like they wanted to factor out and isolate the plugin stuff. Personally I think this is a good idea. But apparently the wrapper system didn't account for the need compare if things are the same. You're always comparing the wrapper instances which of course, are always unique.

To be clear, all the above is just speculation on my part based on my travels through the code when working on fixing stuff that busted a really nice tin whistle tablature plugin. Necessity, being the mother of invention, I started fixing stuff. I had/have no part in the architecture as such.

I actually don't like using the elementId (or even more generally an objectId) property when the language has the intrinsic ability to compare things. It's a workaround that can get us pretty far but it's still a hack. It does give someone time to better integrate the wrappers though and as such is useful and not too much of a barnacle. ;)

-Dale

I suppose QML is designed with the intent of exposing C++ objects to javascript code in a sane manner, so I suspect that the Qt documentation contains the design expectations and formulae for building the Qt/QML interface for a Qt client subsystem. The non-EQ wrapper system adapts well to arbitrarily deep and circular list structure. How to do that with identity-preserving objects conveniently is not obvious. I will search out the Qt docs.

>so I suspect that the Qt documentation contains the design expectations and formulae
>for building the Qt/QML interface for a Qt client subsystem.

Your suspicions are well founded...

https://doc.qt.io/qt-5/qtqml-cppintegration-overview.html
https://doc.qt.io/qt-5/qtqml-cppintegration-topic.html

-Dale

Thanks, Dale. I'm becoming enlightened -- either you're willing to pay the per-object overhead of having QML-relevant storage reserved in each of your objects, or not. Basing all your returnable objects on the exportable Qt classes forces you to, and you get Javascript objects whose EQ-ness is guaranteed between calls; I don't see why calling them "actual C++ objects" is more accurate than that. That may be, or maybe was, too much overhead, considering that most users don't even invoke plugins. So either user code has to manage elementId (which, I admit, won't win any beauty prizes) or the C++ code had better make a large hash map of objects it has returned, and wrappers it has created for them, which gets cleared when the plugin exits. It's that (per-native-object overhead), that (C++/QML transactors maintain hashtable), or what we have now AFAICT. When circular and huge list-structure is involved, you can't chase down pointers except when asked. Somebody has to, literally, keep track of "what's what", and pay the storage cost; it is better that it be localized to the occasion of QML use and scaled to the number of objects asked for (voting for C++ hash table).

Rather than waiting for the perfect solution I think the elementId approach is fine moving forward so we can get functional 3.3 features like the tieX support etc...

When/If automatic language based comparisons are implemented the elementId property can be deprecated in the documentation but remain supported in that future.

Note that the elementId scheme isn't bullet proof because it uses the object's memory base address as the elementId. You can imagine a case where the object underpinning the wrapper object is deallocated and then that memory is reallocated to another object. If a wrapper still referenced the old object address you'd likely get a crash. I would be more concerned about this if plugin's had long life times but they appear to be one-run-and-done environments.

It's not clear to me what would happen in the older v2 direct object exposure approach when an internal object is destroyed but the Javascript still had a reference to it. I'll bet a signal is internally triggered to inform the JS engine of the deletion so it can take appropriate actions.

-Dale

Seems to me elementId being public is not the issue. If you retain a playElement in Javascript, then tell the note to clear its playElement array from JS, the retained element now has a dangling elementId. it'd better work whether elementId is exposed or not.

elementId has to be paired with a "time", an incrementing counter, and an std::set listing all such valid pairs be maintained, wherefrom stuff is removed when a call-in is known to delete underlying objects. It's probably easy to provoke a crash today by the formula I just gave. Keep in mind that any plugin can crash the app today by a mere infinite loop.

In the "everything is based on Qt classes" scheme, the destructor for the Qt base class can reach out to the JS avatar and punch its brains out; anyone retaining it will be sorely disappointed upon use. In the current wrapper scheme, the little guys are completely passive with a one-way admiration for their uhderlying object. If the latter goes away ... kaboom. The 3.0 scheme as is is not delete-safe.

We can reinvent the wheel and add to basic MS Element just one little tiny pointer, i.e., to the Official Wrapper, but someone has to keep track of 'em all to clear 'em when the plugin exits. And perhaps this storage expense is exactly what led (whomever) away from Qt-based scheme (although that introduces other overhead, like include files and namespaces all over the place).

Perhaps some of our salvation may exist in the Qt template called QPointer:

https://doc.qt.io/qt-5/qpointer.html

If the Ms::PluginAPI::ScoreElement::e member was a QPointer Qt would manage knowledge of destruction and notification of the underlying object.

In Ms::PluginAPI::ScoreElement change:

protected:
    Ms::ScoreElement* const e;

to

protected:
    QPointer< Ms::ScoreElement > e;

Then the pointer's existence can be tested before use:

if (e)
    e->method_on_wrapped_obj();
else
    qWarning("PluginAPI::ScoreElement: This object no longer exists.");

This should protect against program crashes.

It still doesn't address the object comparison issue but it would
make the QML/C++ code much safer.

-Dale

An Aside... How the heck can I put greater than and less than brackets in a code block on this forum? Is there a markdown guide for the forums?

put less-than-code-greater-than .....code less-than-/-code=greater-than around the code,... as in ...

code < 2;


If each of 10 different wrapper avatars for the same object are created, poor Qt has to bear the burden of the 10 QPointers they will carry, and call their owners all to disgorge, should the object be deleted. If chasing tie pointers, you might well get three or four avatars of the same note. Whatever a QElement has, it is likely more than 0 storage (perhaps it includes a pointer to an internal vector of all the QPointers naming it) and it is hard to believe that just one-more-little-old-pointer in the other direction would sink the ship. (of course, you have to keep a single vector<QElement*> to null them when the plugin exits, and that would give you object EQness.

I guess I'm just dense. I see that you get a great result but I don't understand the how-to. Here's what I'm "hearing"...

<>.....code

I'll send you email.

In that previous post many (most!) characters were removed!

Markdown manual anyone?

if this is what you mean:

use raw code between <xml> and </xml> tags?

<tag id="xyz_123">
value=1
</tag>

So far the XML block is the killer solution!

It doesn't eat space. It appears to be colorizing syntax so it parses it as XML.

I wonder if the site's forum/blog rendering has a C++ code bracketing available.

-Dale

Who knows if it works if the content isn't parseable as xml.

Back on topic...

The above QPointer suggestion wont work since Ms::ScoreElement is not a QObject subclass.

In MuseScore v2.x the class Ms::Element inherited from ScoreElement and QObject. This was removed in v3.0.

-Dale

Ms::Element or Ms::ScoreElement can implement a similar trick without help from Qt Olympus. The destructor for either can walk a "client list" of magic pointers.

sure...
use <cpp> code </cpp>

#include <iostream>
using namespace std;
int main() 
{
    cout << "Hello, World!";
    return 0;
}

Perfect!
Thx!

>Ms::Element or Ms::ScoreElement can implement a similar trick without help from Qt Olympus. The destructor for either can walk a "client list" of magic pointers.<

But some human would have create that solution and test it. And it would be done that way to simply because someone didn't want to add this:

class ScoreElement : public QObject {
      Score* _score;
      static ElementStyle const emptyStyle;

If you peek a little deeper the class Score actually inherits ScoreElement and QObject so it does insert itself into the hierarchy for some need:

//@@@@@@ class Score : public QObject, public ScoreElement {
class Score : public ScoreElement {
 

(I'm having fun with the newly discovered cpp blocks! ;) )

I say, just move it down a couple levels.

As a test I did just the above changes and replaced pointer as suggested above with the QPointer class:

   protected:
      //@@@@Ms::ScoreElement* const e;
      QPointer<Ms::ScoreElement> e;

It built and ran fine so the QPointer class works with the existing code before even using its features that detect object destruction.

-Dale

I'm a little confused here, because both the "real objects" and their wrappers are named the same in different namespaces. On which side of that divide do the QPointers live, and to which do they point? Do the "real elements" harbor QPointers to the avatars/wrappers, or vice versa? I would guess it would be the avatars that maintain QPointers to the "real elements", so that if a real element is deleted, the wrapper has no dangling pointer. But I thought the goal of the change was to remove the Qt-basing from the "real elements". It can't be so that the "real elements" contain a QPointer to the wrapper, because if that were so, it couldn't have more than one wrapper, because there is only one QPointer, and there wouldn't be an EQ problem. Could you clarify your report in this regard?

QPointer or not, "guarded pointers" (pointers which elicit a callback and nulling when their target is destructed) in both base classes (real elements and avatars) are the obvious solution; solving both problems (no dangling pointer after destruction and kooky multiple avatars for same real-object) in one stroke. If there is a 1:1/1:0 relationship, no lists or tables are needed.

I wish the person who decided on and implemented the change could speak to its goals here.

I have some theorems. Assume the MSO (base,"real" MuseScore object) and the (single) wrapper avatar maintain pointers to each other. If the MSO is deleted, it must not only revoke the avatar's pointer, but render the avatar itself "insensate" (easily tested -- MSO ptr == nullptr). The avatar can only be deleted in two ways: termination of the plugin, or Javascript garbage-collecting it because the QML script has no references to it (There is no "delete this object" in Javascript, Lisp, etc.) If the latter happens (JS GC's it), it is just fine to terminate the MSO's pointer (by hypothesis, the avatar and all its pointers is vanishing). If an avatar of that MSO be ever asked for again, you will get a new one, but that's ok because you can't tell the difference because by hypothesis, you retained no JS references to the old one. QED.

>On which side of that divide do the QPointers live, and to which do they point?
>:
>It can't be so that the "real elements" contain a QPointer to the wrapper, because
>if that were so, it couldn't have more than one wrapper, because there is only one
>QPointer, and there wouldn't be an EQ problem. Could you clarify your report in this regard?

The QPointer lives on the pluginAPI side of the equation and point to a "real" element. There
could be many QPointers pointing to the same single real element.

>I would guess it would be the avatars that maintain QPointers to the "real elements",
>so that if a real element is deleted, the wrapper has no dangling pointer.

Yes. Exactly true. This is the whole point of it. You then have a testable pointer that is in
sync with the actual object. If the object is deleted, then the QPointer's 'shadow'
of that pointer is also set to nullPtr. That really all there is to it. Of course
getting that to work is Qt's problem. It has one requirement: that the objects are
based on QObject.

>But I thought the goal of the change was to remove the Qt-basing from the "real elements".

That's my speculated goal. ;) I don't know the real goals for why it was factored out.
Like all things there are trade offs. Having the elements all subclassed from QObject adds
a memory hit to each object instance. That's one thing to balance. Having code that doesn't
crash is another. When only Score was subclassed the memory hit isn't too large because
there aren't many Score's.

If these subclasses (Like Ms::Chord, Ms::ChordRest, Ms::Note....) don't use the Q_OBJECT macro,
they shouldn't be picking any more memory footprint than the base class added. All the
Q_OBJECT stuff is in the pluginAPI side of the wrappers.

To explicitly see how my test code did this see the latest commit on this branch:

https://github.com/DLLarson/MuseScore/tree/plugin-crashproof-wrappers

This code compiles and runs fine with the QPointer interposed between the wrapper and "real"
object even though no code is actually testing the pointer for null-ness at this time.
I have to figure out how to actually test all this from a script to see if my understanding
matches reality.

>I wish the person who decided on and implemented the change could speak to its goals here.

I totally agree. They may just point to xyz and say it's covered! They are the experts on this
codebase after all. That includes tribal knowledge not obvious from the code.

-Dale

The way to test it would be to ask a note for its playEvents, save'em, and clear the note's play events array from QML. Then, prepared with a fire-extinguisher, try to use one of the saved events. Maybe if you did this to 50 notes you could provoke a crash (if your change didn't work).

I looked at your patch, it looks utterly like the Av-tracks-MSO side of the coin. One more pointer and some code in ~Element and api::Element destructors and factory, and the subject of the report, "multiple objects" is fixed.

Flaw in my previous lucid reasoning: If you ask for a QML avatar object, modify it in QML, and retain no reference to it, and there is a garbage collection, that object will be lost, and if asked-for again, will "reappear" as a doppelgänger not EQ to the old one, and thus without the modification. This is a serious problem, and I wonder if it happens in 2.x. You'd might as well keep a list of all the objects you would want to mark a certain way. This is an obscure problem which might be an acceptable tradeoff against "never garbage collect avatars", which would be unacceptable (imagine looping through an extensive score).

A workaround (and a recommended approach currently) is to use is() function of plugin objects:

if (oldScore.is(curScore))
    console.log("same score");