[MU4 Issue] Palette search box slow to open and close

[rgreen5]

Describe the bug

The palette search box is slow to open and close in MS4. In MS3, by comparison this happens instantaneously.

1. Click "Search palettes" icon.
2. RESULT: Takes 1-2 secs to open
3. Enter "par"; the parenthesis symbol should appear.
4. Click the search close button. RESULT: Takes 1-2 secs to close.

P.S. I also got a crash using this feature; not sure how to reproduce however.

Expected result

Opening and closing of search box should be instantaneous, like it was in MS3.

Platform information OS: Linux Mint 20.1, Arch.: x86_64, MuseScore version (64-bit): 4.0.0-2829968415, revision: github-musescore-musescore-2d21253

4MB / Pentium 2 x 2.4 GHz.

[Tantacrul]

Yeah, it's a bit slow, alright.

[MarcSabatella]

It also doesn't work at all if the palette window is not currently open. It should first open the palette and then transfer focus to the search box as per MU3, so that Ctrl+F9 is your "one stop shop" for accessing the palette search (especially important for accessibility). Should I open a separate issue for that?

[Tantacrul]

@MarcSabatella - yes, it seems like a separate issue alright.

[worldwideweary]

Just reiterating that with a faster PC than what was posted in OP description, I'm getting like a 3-4 second delay when activating palette search. In 3.6, activation was fairly instantaneous, but could get slightly jerky when typing/erasing text FWIW, the fastest it ever was was back before switching to QML where typing and updating and activating was all quite literally "instantaneous" feeling back in ... whenever that was. 3.3ish

[Tantacrul]

Feels like a potential for 4.2 - assuming it is a widespread problem

[worldwideweary]

P.S. also clearing with the X button is similarly slow. A screencast if it's of any value (showing circa 2.5 seconds or something along those lines):

https://github.com/musescore/MuseScore/assets/7139517/ad143739-3587-41f1-bb66-257a30e51b6a

[worldwideweary]

To whom it may concern, been checking a few things out and noticed that:

const Palette* PaletteTreeModel::findPalette(const QModelIndex& index) const

gets hit up at what appears to be an alarming rate (like thousands of times) during the "waiting period" of palette search activation, for what it's worth.

[ShibuyaCyana]

Same problem; it's more annoying than it seems, knowing this lag in advance encourages me to dig through the whole palette table, which is actually more time-consuming.

OS: Windows 10 Version 2009 or later, Arch.: x86_64, MuseScore version (64-bit): 4.1.1-232071203, revision: github-musescore-musescore-e4d1ddf

[worldwideweary]

Figured I'd check-in to see status/if this was resolved and looks like nothing has happened, so I figured I'd check some stuff out in my "free" time:

I notice that the third-party "deto_async" in channel.h has a function:

    void send(const T&... d)
    {
        NotifyData nd;
        nd.setArg<T...>(0, d ...);
        ptr()->invoke(Receive, nd);
    }

Now I ain't too brite, but this sucker gets called over two-thousand times (and it could be way more than that, but i put a 2k limit on there with GDB and it went beyond it) when pressing the search button between that and the actual "readiness" for searching. That just don't sound right to me. The ptr pointer is on line 32 invoked by

    void notify()
    {
        m_ch.send();
    }

through the navigation controller call:

DEFER {
        if (isChanged) {
            m_navigationChanged.notify();
        }
    };

seems crazy to me, but whatever.

[krasko78]

Several observations from me:

Palettes expanded after ending search Keep all or some of your palettes collapsed. Press CTRL+F9. Type something in the search box. Without clearing the search box, press ESC. The palettes return to the usual ones but notice they are expanded now. Issue happens because in PaletteTree.qml expanded is defined as: property bool expanded: filter.length || model.expanded I'd change that to: property bool expanded: (searchOpened && filter.length) || model.expanded or even property bool expanded: searchOpened || model.expanded This produces "Unable to assign [undefined] to bool" so maybe change to property bool expanded: searchOpened || model.expanded ? true : false or investigate why it ends up being undefined.

Palette search results inconsistent with text in search box Press CTRL+F9. Type something in the search box. Observe the search results. Without clearing the search box, press ESC. The palettes return to the usual ones. Press CTRL+F9 again. You are returned to your search, your search text has been retained in the search box but the search results no longer correspond to the search. Instead all palette items are displayed. Issue happens because PaletteProvider::setSearching clears the filter on the search results but nothing clears the search box text so they go out of sync. Either both should stay the same or both should be cleared. I think not clearing the filter in PaletteProvider::setSearching works well because this way MuseScore remembers the last search which might be useful and also the search text is selected so if the user wants to start a new search, they can start typing right away.

These were issues related to the search but not the slowness. :) Now about the slowness:

The main rule here is that the more palettes and the more palette elements are there in total in those palettes, the more time is needed to process them, i.e. the more the delay.

Search slow to open When the search is not active or the search box is empty, the PaletteProvider::m_searchFilterModel model is not filtered at all meaning it will contain all the palettes and all the palette icons that can be searched for. This is highly inefficient and in that situation the search results are hidden. We simply don't need them because once the user types the first search character, that model will be filtered. Try setting the filter on m_searchFilterModel to something that doesn't return any results 1) initially and 2) when the search text is empty:

In PaletteProvider.cpp:

In PaletteTree.qml:

Since no palette elements are returned, the rendering is super fast (even if the tree is not visible) and the search opens instantly. In this case notFoundHint's visible property in PalettesPanel.qml should become: visible: !searchHint.visible && !paletteTree.isResultFound

Search slow upon typing the first character or when deleting the last characters and one character remains This simply has to do with the fact that one-character searches return many matching elements which takes some time. We can live with it or maybe require at least 2-3 characters in order to initiate the search (does it make sense to search by one character only?)

Closing a search is slow and Returning to a search with one character having been search for previously This has to do with the fact that canceling a search returns the user to their usual palettes which normally I guess are many and contain many palette elements so again this takes time to process. I guess we should live with it or (from the top of my head) maybe use two separate trees and switch between them instead of having one tree and switching the models.

Further observations that may not be as important given the above but that I wanted to mention Aside from everything told, it also seems that colapsing a palette introduces some delay. Try this: in PaletteTree.qml for the "collapsed" state, change the visible property to true instead of false:

This will keep all the palettes open but will illustrate that the search opens noticeably faster, maybe the only delay in this case is the time needed to process and render the palettes. Revert "visible true" to "visible false" and change implicitHeight in PaletteGridView.qml to return for example 0 if parent.visible is false:

This breaks the expansion of the palettes but interestingly also removes the extra delay when opening the search. It seems to me something happens when the paletteContainer is invisible - possibly related to contentHeight being used in implicitHeight. A bit hard to tell but might be a lead.

There could be more room for optimization somewhere. Hope this helps.

[worldwideweary]

@krasko78 On the side, if you were to for example try searching in MuseScore 3.2.3 (2019), you'd notice no delay whatsoever while initializing and adding words to search, so to say the very reason is because of many palette items seems to miss the point about the underlying programming architecture:

https://github.com/musescore/MuseScore/assets/7139517/3248f39a-c39f-4f11-bacf-00a9154f164a

The switch to QML seemed to slow it down a bit into 2020ish times... and now's it's even slower than 3.4-3.6, yet there aren't really much added palette items. That's just how it is though. There's obviously room for optimization somewhere ;)

[krasko78]

@worldwideweary My point with "the more items to display, the longer the delay" was to suggest where to look for possible optimization further (at least for me) :) and that knowing this we can make at least the opening of the search fast (which IMHO is the most annoying delay). I had read somewhere (in one of the related issues) that typing in the search box in 3.x also exhibited a delay so I didn't know how MuseScore 4 and 3 compared exactly. Now I do know so yeah, there should be room for more optimization (QML permitting). :)

[worldwideweary]

Yeah man, I would think QML in and of itself would not necessitate a lagged user experience. I tried to use trace through with GDB in the past, but ended up crashing the damned thing when stepping into the programming. There's gotta be a way to fix it up.

[krasko78]

It seems that two of the main culprits are MoreElementsPopup in PaletteTree.qml and StyledMenuLoader in PaletteGridView.qml. Comment out both and it is much faster. Wondering if those could be one instance and created on demand?

[krasko78]

I moved MoreElementsPopup (in PaletteTree.qml) and StyledMenuLoader (in PaletteGridView.qml) outside of the models to end up with one instance of each to be bound on show. The results are very good. I'll do some more testing and will post my findings.

[krasko78]

Here are the changes I have made in my MuseScore fork:

https://github.com/krasko78/MuseScore/commit/e14e7b4787c8377c32fab83d57a81a0f6f9489a7

1. Made MuseScore remember the last palette search results (inconsistent with search texbox): Press CTRL+F9. Type something in the palette search text box. Observe the search results. Without clearing the search text box, press ESC. The palettes return to the usual ones. Press CTRL+F9 again. You are returned to your search, your search text has been retained in the search text box but the search results no longer correspond to the search. Instead all palette items are displayed. With this fix, the last search results will be preserved so when you return to the last search, it will be there and match the search text box. (this one is not related to the slow search but is a bug)

https://github.com/krasko78/MuseScore/commit/b81bc41cd1aa65d8c659d1de3864b0efd12fcc55

2. Fixed a bug where all palettes were expanded after a palette search. How to reproduce: keep all or some of your palettes collapsed. Press CTRL+F9. Type something in the palette search text box. Without clearing the search text box, press ESC. The palettes return to the usual ones but notice they are all expanded now regardless of their state before searching. (this one is not related to the slow search but is a bug)

https://github.com/krasko78/MuseScore/commit/b67a0ac09b881f6ec7ebb62156de524a3cd4ab09

3. Sped up the opening of the palette search. When opening the palette search and there was no search text, all of the palettes and palette items were returned (no filtering was performed). Although the search results were invisible, this was a time consuming operation and was causing the search to open with a noticeable delay. This changeset causes no results to be returned when there is no search text thus speeding things up significantly. As a side effect, slows down the search when the first search character is typed (but see the hack below that fixes this).

https://github.com/krasko78/MuseScore/commit/5c824c9c0127e9ed3a47849fd9b7530a67c64354

4. HACK to speed up palette search when removing search characters (going from fewer to more search results). This is a HACK discovered by accident from the previous changeset. Setting the source model to nullptr before applying the new filter seems to help tremendously when going from fewer to more search results (e.g. by having two search characters and the second one is deleted).

https://github.com/krasko78/MuseScore/commit/2c658d60a6eaf96392adf0984af451c6a3cdf94f

5. Sped up palette search and search close by having one instance of context menu. There was a StyledMenuLoader for every palette item. This changeset will make the PaletteGridView create one StyledMenuLoader instead and bind it to the target palette item when the palette item is right-clicked and before the menu is shown.

https://github.com/krasko78/MuseScore/commit/40aa88340ae60ed4320a2c70280900b3609ed7d3

6. Sped up palette search by having one right-click mouse area instance. This changeset creates one MouseArea for the entire palette GridView to listen for right-clicks. On reception of a right-click, the context menu will be bound to the grid view item under the mouse and displayed.

https://github.com/krasko78/MuseScore/commit/d078db53d61ea0c34ca4f681c21e8bf273fd17d8

7. Sped up palette search further by using a single instance loader for the More elements popup. The More Elements popup was instantiated in the delegate for every item in the model. Now there is one loader that loads the target More Elements popup when the More... button is clicked.

https://github.com/krasko78/MuseScore/commit/588904f76f5c794bbd328c9f2d2edbdbb384912c

8. Prevent a crash when a palette element's context menu is open and the user presses CTRL+F9 to start a palette search (since the gridview elements are destroyed and the menu too).

https://github.com/krasko78/MuseScore/commit/a2a9db41ca7dfabba1342fc948bd194c389a0593

9. Fixed a bug where the More button on the normal palettes would be absent if the palette search textbox contained text. How to reproduce: press CTRL+F9 to start a search. Type anything in the search box. Without clearing the text box, press ESC. You are returned to the usual palettes but their More buttons are now missing. You have to clear the search text to restore them. (this one is not related to the slow search)

After some testing, here are my observations:

1. There is still a noticeable delay (but not too bad) when you enter one character in the search box and then press ESC and CTRL+F9 again. However, this would be a very rare scenario (to search by just one character).

2. Opening the More elements popup of a custom palette always starts from the first page (whereas previously it was remembered).

3. There are occasional positioning issues with the More elements popup but without these changes MuseScore also has them.

Note: I am not a Qt/Qml/C++ developer, I've started looking at the code and getting familiar with it at the beginning of this year so it is possible that my changes are not ideal. I do hope, however, that something from them, at least the ideas, could be useful.

[RomanPudashkin]

@krasko78 could you please open a PR containing your optimization? Ideally, you can even open multiple PRs - for each bug fix / optimization. Thanks!

[krasko78]

@RomanPudashkin Sorry for the late reply, I was on vacation. Ummm, I'll give it a try, give me some time to read through my comments and commits, and refresh my memory. :)

[krasko78]

@RomanPudashkin I have created my first PR ever! Please bear with me if something is not quite right. I am still finding my way around PRs. I signed the CLA a few weeks ago, it looked like it did go through. Not sure what else I need to do. This PR is the first of a few changes I am going to make. Do I need to add reviewers? Can you look at it and let me know if it is OK? Once it is completed, I will go ahead and add some more optimizations. Thanks!

[cbjeukendrup]

@krasko78 Thanks for your great work on this! Are you planning to create a third PR related to this issue, or were all fixes included already?

I just realised that the bug from point 9 from your comment above is closely related to, or even the same as, https://github.com/musescore/MuseScore/issues/15449.

[krasko78]

@cbjeukendrup Yes, I'd like to implement all other fixes as well, but was thinking of opening a separate bug for them once we are done with this one since they are not related to the speed of the palette search. What do you guys prefer - a separate bug or add them here?

[RomanPudashkin]

@cbjeukendrup Yes, I'd like to implement all other fixes as well, but was thinking of opening a separate bug for them once we are done with this one since they are not related to the speed of the palette search. What do you guys prefer - a separate bug or add them here?

Yes, please open a separate issue. Thank you so much!

[krasko78]

Here it is: 24626: Miscellaneous palette search issues