21148: Undocked panels change their sizes between MuseScore sessions

[krasko78]

Resolves: #21148 (partially, the other half should be covered by PR27695

Symptoms:
Undocked panels change their width and height with each launch of MuseScore. They can increase or decrease them with decrease being the more common case. The Mixer auto-calculates its height so the height is not problematic, but the width is: it will usually shrink significantly upon every launch eventually ending up as wide as a pen (as described by many users). A vertical panel, e.g. the Layout panel, when undocked will also significantly reduce its height with every launch ending up as high as a pen. The vertical panels are constrained to a width of 300px so it is not as problematic.

Increase or decrease? By how much?
The next time MuseScore is launched:

• all floating panels will have their widths multiplied by 1150 / W where W is the width of the MuseScore window it had when it was last closed. Consequently, if W is less than 1150px, all widths will increase; if W is more than 1150px, all widths will be reduced. The farther W is from 1150, the more pronounced the width change.
• all floating panels will have their heights multiplied by 768 / H where H is the height of the MuseScore window it had when it was last closed. Consequently, if H is less than 768px, all heights will increase; if H is more than 768px, all heights will be reduced. The farther H is from 768, the more pronounced the height change.

The numbers 1150 and 768 are approximate and may vary from system to system a bit. I suppose most users have MuseScore maximized to something close to 1920x1080px on their laptops. This is the reason why the panels mostly shrink.

Cause:
The LayoutSaver scales all the widget sizes relative to the the main window. In LayoutSaver::restoreLayout() there is a call layout.scaleSizes(d->m_restoreOptions); (LayoutSaver.cpp:220) which calls scaleSizes() on each main window (line 528). This is where the saved geometry of the window when it was last closed is compared against its current size and then everything is scaled according to that ratio. I believe the idea is that if the main window would end up outside of the screen, its size and position will be adjusted but at the same time all other floating windows should be adjusted by the same amount so as to scale the whole layout evently. The problem is this does not seem to work quite correctly.

I see a couple of issues here:

1. LayoutSaver::Private::deserializeWindowGeometry() is where the size of the main window is restored. If the window was not maximized, the method will use its saved size. If the window was maximized, the method will use the normal (restored) size and the window will be maximized later. This is fine. The problem is that later when LayoutSaver::MainWindow::scaleSizes() is calculating the scaling coefficients, it always takes the saved size of the window. So if the window was maximized, it will be created with its normal size and this normal size will be used together with the saved (maximized) size for the scaling. Does not make sense to me. There is no way for the coefficients to be 1.0 so some scaling will take place. It feels like comparing apples to pears.

2. The second problem is that in LayoutSaver::ScalingInfo::ScalingInfo the saved size of the window is not compared to the actual size of the window set by LayoutSaver::Private::deserializeWindowGeometry(). The latter uses setTopLevelGeometry which sets the size of the main window correctly but the former uses mainWindow->window()->geometry(); instead. This returns a width like 1150px and height like 768px which are very close to the width and height set on the ApplicationWindow in AppWindow.qml (probably the height excludes the titlebar). So the scaling coefficients always dependent on: a) the size of main window of MuseScore when it was last closed - and - b) the fixed width and height set in AppWindow.qml. This does not make sense to me either.

My fix:
I propose that in LayoutSaver::MainWindow::scaleSizes() we use the same logic as in deserializeWindowGeometry(): use either the save or normal size of the window as the denominator and make the nominator the same size but run through FloatingWindow::ensureRectIsOnScreen. In almost 100% of the time the two will be equal except when the size goes out of the screen and needs adjustment. Due to the unknowns as to why KDDockWidgets does all that, my fix may not be optimal. Shout out if you have better ideas.

• I signed the CLA
• The title of the PR describes the problem it addresses
• Each commit's message describes its purpose and effects, and references the issue it resolves
• If changes are extensive, there is a sequence of easily reviewable commits
• The code in the PR follows the coding rules
• There are no unnecessary changes
• The code compiles and runs on my machine, preferably after each commit individually
• I created a unit test or vtest to verify the changes I made (if applicable)

[mathesoncalum]

Hey @krasko78 - just investigating/discussing a couple of things internally related to this. In the meantime if you could address the warning that would be ideal:

QRect geometry = !isNormalWindowState(windowState) ? normalGeometry : geometry;
declaration of 'geometry' hides class member

Many thanks!

[mathesoncalum]

Thanks for your patience @krasko78! Your other PRs are on my radar as well, I'm just trying to balance a few things at the moment :)

Something I find quite curious about this bug is that it doesn't occur when you load from recent files (HomePage). I could be wrong, but the fact that it only occurs when loading straight to the NotationPage would seemingly imply there's something we can do about this on the MuseScore Studio side of things (as opposed to tweaking the KDDockWidgets framework directly, which we try to avoid where possible).

Would be interested to hear your thoughts on that!

[krasko78]

@mathesoncalum Thank you for looking at my PRs. Take your time! :) Let me do a bit more research on this one.

[krasko78]

Cheers @mathesoncalum! Everything I said previously holds true but I have some updates regarding the fix. First let me sum up the two problems that we have:

1. The saved geometry of the window is properly restored in LayoutSaver::Private::deserializeWindowGeometry() but later for the scaling in LayoutSaver::ScalingInfo::ScalingInfo some other width and height are used - 1150x768px. Those are actually the size of the KDDockWidgets::MainWindowQuick instance that we create in DockWindow::componentComplete(). The parent is the DockWindow (WindowContent.qml) which is a child of the application window (Main.qml). Looking at the QML files we see that the DockWindow and KDDockWidgets::MainWindowQuick (the latter fills its parent so both are the same) get the width of the application window - 1150px (AppWindow.qml) and the height they receive is that of the application window (800px in AppWindow.qml) minus the titlebar height (32px on my system) = 768px.

Yesterday I looked at the source code of KDDockWidgets on GitHub and discovered that this was indeed a bug in KDDockWidgets fixed a long time ago but we didn't fix it. Our KDDockWidgets::MainWindowQuick instance is obviously not a top-level window and the bug is present.

So I propose we fix LayoutSaver::Private::deserializeWindowGeometry() to use mainWindow->windowGeometry() like it was fixed by KDDockWidgets.

2. The second problem is when the window of MuseScore was maximized when MuseScore was closed. When restoring the layout, first the normal geometry is set on the main window and then its state (maximized, minimized, etc.) is set. This is so that when the user then unmaximizes the window, it will be restored to its previous normal position and size. This is correct.

Before I didn't understand how this was supposed to work for the calculation of the scaling coefficients where the saved geometry is matched against the current (normal) geometry. For a maximized window the saved geometry is the maximized size. I now do have a very good explanation - KDDockWidgets must be expecting the main window to be visible when its layout is getting restored. If it is visible, setting the normal geometry and then the state to maximized maximizes the window immediately causing it to resize to its maximized size. So later LayoutSaver::Private::deserializeWindowGeometry() is able to correctly see the maximized size and scale according to it. Since the saved geometry will reflect the maximized state of the window from when MuseScore was closed, the scaling will be the ratio of the maximized state from the previous session and the maximized size from the new session that is starting. These will match almost 100% of the time unless the screen resolution has changed. So in this case we divide apples by apples and not apples by steel. :) And if the resolution is different, everything will scale up or down - which is exactly the purpose of the scaling that KDDockWidgets does. I've looked at couple of examples - example 1 and example 2 - they both set the main window to visible early on.

However, if the main window is not visible, as is our case, then setting the normal geometry works fine but setting the state to maximized is where things go wrong: the window is not maximized (resized) until it is actually made visible. This causes LayoutSaver::Private::deserializeWindowGeometry() to see the normal geometry instead of the maximized and therefore calculates the scaling as the ratio of the maximized size of the main window from the last session and the normal size of the main window from the new session (which is the same as the normal size from the last session).

So to fix this we need to make sure that the main window of MuseScore is visible when the layout is getting restored. I've added a line for that in DockWindow. This works and indeed fixes the scaling, but the caveat is that it now causes wose user experience when MuseScore is starting: on my system, after the splash screen disappears, I see a blank maximized window with a big rectangle that does not repaint itself, then I see the window getting filled with the tollbars, panels, etc. and finally the score.

I have updated the PR with these two fixes so that you can see for yourself. I am not sure how to avoid the empty window on startup now that the window is made visible much early in the startup cycle. Would be nice to hear your thoughts. The only thing that passes my mind is to make the main window transparent and remove the transparency when everything has loaded...

P.S. The reason why the bug does not appear when opening from the Recent file list is simply because at that moment the Score page is not open. It is open and restored when opening a score and at that moment the main window of MuseScore is already visible and maximized. So the problem only appears when starting MuseScore with a score (either double-clicking on a file or in my case having MuseScore set to restore the last session). And as I mentioned in the story, it looks like the bug is not present in this case but the heights are slightly reduced: this is because of the first part of the bug. If the maximized window has a size of 1920 x 1050px for example, our KDDockWidgets::MainWindowQuick instance will be 1920 x 1018px (the height excludes the titlebar of MuseScore). :)

[krasko78]

Update: I figured we don't actually need to show the main window as early as I thought. The scaling happens when the pages are open (Home, Score, etc.) which does not happen until startupScenario()->runAfterSplashScreen(); in guiapp.cpp. So showing the main window just after hiding the splash screen and before startupScenario()->runAfterSplashScreen(); works great.

I noticed something though that bothers me a bit: WindowContent.qml has an InteractiveProvider and InteractiveProvider has a very dark-red colored Rectangle of size (100,100). I have absolutely no idea what its purpose is but it does not make any sense to me. What bothers me is that if you add qApp->processEvents() after the line that shows the main window, this rectangle is clearly visible for some time when you are launching MuseScore. It appears that only the lack of qApp->processEvents() in the startup logic and the late display of the main window help to "cover up" the presense of this rectangle. What do you think? Should we do something about it?

Have a great weekend.

[mathesoncalum]

Excellent stuff @krasko78! Just one requested change from me.

Yeah I'm not sure what that Rectangle is for actually (maybe @igorkorsukov knows). Is calling processEvents immediatedly after setProperty necessary for this fix?

[igorkorsukov]

Yeah I'm not sure what that Rectangle is for actually (maybe @igorkorsukov knows)

This is just for debugging, to understand what is happening, if we see this rectangle, it means that the main window itself has loaded, but the page does not load, there is probably some kind of error there.

[krasko78]

Fortunately processEvents is not required here. I was experimenting with it to see if it would lead to improved user experience and this is how I saw the black rectangle. But without it, the rectangle is not visible.

I did see the warning about obj being null in Qt Creator now. I use Qt Creator only for QML and Visual Studio for C++. I am putting a note to myself to inspect all C++ files for such warnings in Qt Creator before I check them in. Recently I installed uncrustify locally and got it to work in Qt Creator (but not in Visual Studio) so I'll be having another reason to run all changed C++ through Qt.

I've also shortened the comment a bit as it includes a link to the issue with all the info.

[zacjansheski]

Tested on MacOS 15, Windows 11, Ubuntu 22.04.3. Approved
#21148 FIXED