Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MU4 Issue] New Score creation: switching back and forth between instruments and templates #14342

Closed
Grim-O opened this issue Nov 5, 2022 · 17 comments · Fixed by #25019
Closed
Assignees
Labels
community Issues particularly suitable for community contributors to work on feature request Used to suggest improvements or new capabilities P2 Priority: Medium

Comments

@Grim-O
Copy link

Grim-O commented Nov 5, 2022

When creating a new score by adding individual instruments but then thinking "wouldn't a template exist for this" and clicking on the templates tab, the already entered individual instruments get forgotten. thus, when seeing no template exists for what you want and returning to the instruments tab you are greeted by an empty field and have to start over from scratch. It would be nice to see MU4 remembers what you already added and only delete that selection when you choose to go for a template.

@Tantacrul Tantacrul added P3 Priority: Low feature request Used to suggest improvements or new capabilities labels Nov 6, 2022
@Tantacrul Tantacrul added P2 Priority: Medium community Issues particularly suitable for community contributors to work on and removed P3 Priority: Low labels Nov 6, 2022
@L0uisc
Copy link
Contributor

L0uisc commented Jan 28, 2023

I'd like to take a crack at this.

@Tantacrul
Copy link
Contributor

@L0uisc - feel free. I'll add you to the project.

@Tantacrul
Copy link
Contributor

Let me know when you think you've cracked it and I'll add the fix to an upcoming release. Thanks!

@L0uisc
Copy link
Contributor

L0uisc commented Jan 30, 2023

This will take some cracking, mostly because I don't have a feeling for how QML and C++ models interact. Are there any good resources I can try to read/study to get up to speed?

@cbjeukendrup
Copy link
Contributor

There is only a handful of things you can do with C++ models in QML:

  • in QML, you can instantiate the model: MyModel { id: myModel }
  • in C++, you can expose properties to QML, using the syntax Q_PROPERTY(<type> <property name> READ <getter method name> WRITE <setter method name> NOTIFY <change notification signal>)
    For example: Q_PROPERTY(int someNumber READ someNumber WRITE setSomeNumber NOTIFY someNumberChanged)
    (the signal someNumberChanged needs to be declared too in the class definition, and must be emitted in C++ every time the property changes)
    The part WRITE <setter method name> is optional; for read-only properties you can leave it out.
    For constant properties, you can replace NOTIFY <change notification signal> with CONSTANT.
  • in QML, you can then read this property: Item { someProperty: myModel.someNumber * 3 }
    Note that you don't add () in QML, even though accessing that property will call the specified method in C++. When you emit the specified notification signal in C++, the property getter will be called again automatically and the value in QML will be updated.
  • in QML, writable properties can be modified by writing myModel.someNumber = 3. This will call the setSomeNumber method in C++.
    Additionally, you can set a property binding when creating the model:
    MyModel { 
        id: myModel 
        someNumber: 3 * root.width 
    }
    In this case, whenever the value of 3 * root.width changes, QML will update the model by calling setSomeNumber again.
  • in C++, you can expose methods to QML so that they can be called directly: Q_INVOKABLE void doSomething(int arg);
  • in QML, you can call such a Q_INVOKABLE method using myModel.doSomething(3).
  • in C++, you can declare additional signals, which can be listened for in QML. For example, if you have a signal called somethingHappened, then in QML you can do
    MyModel { 
        onSomethingHappened: { 
            console.log("Something happened, according to the model!") 
        } 
    }
    (this on… syntax is generated automatically by QML).

So you cannot access any QML things from within the C++ model; only the other way around. If you want to call a QML method from C++, then you should emit a signal instead, and listen for that signal in QML.
I recommend to just look at existing code, where you can see this in action.

@L0uisc
Copy link
Contributor

L0uisc commented Feb 4, 2023

Ok, so I now know the following:

When I put a breakpoint in the constructors of InstrumentListModel and InstrumentsOnScoreListModel I hit both each time I switch to the Choose Instruments page from the Create From Template page. The list is initialized each time the switch back happens.

The solution would be to somehow prevent destruction and recreation of whichever one of InstrumentListModel or InstrumentsOnScoreListModel required, possibly both. This is how far I got. Does this problem sound familiar to anybody, or am I on my own to find a way to keep QML components in a larger component alive longer?

@cbjeukendrup
Copy link
Contributor

@L0uisc That's a very useful investigation!

The views are destroyed because every time only one of them is instantiated by the Loader with id pageLoader in ChooseInstrumentsAndTemplatesPage.qml. A solution would be to keep both loaded at the same time by instantiating both of them in a StackLayout. This will require a small amount of refactoring inside ChooseInstrumentsAndTemplatesPage.qml.

However, this might have the disadvantage that both views are completely loaded in memory, even though only one is visible at a time. Therefore, an alternative solution would be to instantiate the InstrumentsOnScoreListModel outside the Loader (so that it will be preserved when the view is replaced) and pass it as a property to the ChooseInstrumentsPage which would pass it further down to the InstrumentsOnScoreView.

Hope this helps!

@L0uisc
Copy link
Contributor

L0uisc commented Feb 8, 2023

@cbjeukendrup I too hope it helps 😅 I'll see what I can figure out.

@L0uisc
Copy link
Contributor

L0uisc commented Feb 22, 2023

I finally got around to this again. I tried an evening to get the solution with the InstrumentsOnScoreListModel outside the Loader working, but I finally gave up. I have the solution of replacing the Loader with a StackLayout working.

Would that be OK for a PR, or do you want me to get the better option of only keeping the part we really want to persist between tab switches working? @cbjeukendrup (tagging you since you were the team member who was involved with this previously.)

@cbjeukendrup
Copy link
Contributor

@L0uisc Yes, feel free to open a PR for that solution! (It has the additional advantage that things like selected template or selected instrument category are persisted too.)

@L0uisc
Copy link
Contributor

L0uisc commented Feb 25, 2023

You are correct, it has more utility when both pages are kept around in their entirety. I also think memory use is not too much of an issue. The NewScore dialog is never open when e.g. playback or import is ongoing.

@Eism
Copy link
Contributor

Eism commented May 11, 2023

Actually, we replaced StackLayout with Loader in this PR #10513
So we cannot change the loader, as performance issues will return, see #8831.
We need to find another solution.

@L0uisc
Copy link
Contributor

L0uisc commented May 15, 2023

Hmm... I see. I'm not going to get round to this soon. If somebody else wants to work on it, they're welcome. I'll look into this if I get the time again and nobody else started working on it.

@rpatters1
Copy link
Contributor

rpatters1 commented Sep 27, 2024

If the New score dialog is ever refactored, I would like to request the opposite of this issue. That is, it ought to be possible to select a template, then add or edit the instrument list from there.

@chilkaditya
Copy link
Contributor

chilkaditya commented Oct 1, 2024

Hi, if this issue is still valid then I want to work on this issue. I need some help. @cbjeukendrup you mentioned the instantiate the InstrumentsOnScoreListModel outside the Loader. Can you please give some more hints. After searching online I found this approach ,we can use property string to store the selected instruments, when we switch to template page and then switch back to instruments the we can restore the instruments list from property string:
kind of like that: property string selectedInstrument: "".
I think your approach is more simple to implement. but I need some more help.

@cbjeukendrup
Copy link
Contributor

The specific Loader that I meant is the one in ChooseInstrumentsAndTemplatesPage.qml. That Loader loads the current tab, i.e. either instrumentsPageComp or templatePageComp. Only one of them can be loaded at a time; when switching to a different tab, the other tab is unloaded before the new tab is loaded, which means that any UI state inside the tab is lost.

So what we want to do, is making sure that the model of the selected instruments is stored outside the tab, so that it is not lost.
If you Ctrl+click ChooseInstrumentsPage, you see the definition of that component. Somewhere in there, you see InstrumentsOnScoreView; Ctrl+click it too, to go to its definition. There, you see

    InstrumentsOnScoreListModel {
        id: instrumentsOnScoreModel
    }

You basically want to cut this, and paste it in ChooseInstrumentsAndTemplatesPage.qml, i.e. outside the Loader / Components.
But now, you need to make sure that InstrumentsOnScoreView can still access the instrumentsOnScoreModel. To do this, you need to create a required property InstrumentsOnScoreListModel instrumentsOnScoreModel in InstrumentsOnScoreView.qml. And, also in ChooseInstrumentsPage.qml. Then pass the one from ChooseInstrumentsPage.qml to the InstrumentsOnScoreView view there (i.e. instrumentsOnScoreModel: root. instrumentsOnScoreModel). Finally, in ChooseInstrumentsAndTemplatesPage.qml, in the ChooseInstrumentsPage component there, add instrumentsOnScoreModel: instrumentsOnScoreModel, to assign the instantiated InstrumentsOnScoreListModel to the property.

Last but not least: ChooseInstrumentsPage is also used in one other place. I'll leave it to you to find that place, and make sure that it still works correctly.

@chilkaditya
Copy link
Contributor

chilkaditya commented Oct 3, 2024

@cbjeukendrup thank you for you detailed and precise instructions. It helped me a lot to understand qml properties. I open pull request for this issue but this pull request is not solving the issue. Musescore is running properly in my machine but NewScore creation is not working can you please check my code, where I am doing wrong. basically I getting this kind of errors in my application output window:

  1. Type ChooseInstrumentsAndTemplatesPage unavailable
  2. Type ChooseInstrumentsPage unavailable
  3. Type InstrumentsOnScoreView unavailable
  4. TypeError: Cannot read property 'objectId' of undefined.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Issues particularly suitable for community contributors to work on feature request Used to suggest improvements or new capabilities P2 Priority: Medium
Projects
Status: Done
Status: Done
8 participants