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

Qt 6.5+: menu bar and top-left toolbar are missing #24097

Open
4 tasks done
cbjeukendrup opened this issue Aug 19, 2024 · 17 comments · May be fixed by #24326
Open
4 tasks done

Qt 6.5+: menu bar and top-left toolbar are missing #24097

cbjeukendrup opened this issue Aug 19, 2024 · 17 comments · May be fixed by #24326
Assignees
Labels
community Issues particularly suitable for community contributors to work on dev Related to developer experience (compiling, code base, CI), rather than end user experience Qt-next Either broken or fixed by (updating to) newer Qt versions than we currently use tech debt

Comments

@cbjeukendrup
Copy link
Contributor

Issue type

Other type of issue

Description with steps to reproduce

In development builds made with Qt 6.5 or later, the top-left toolbar (Home/Score/Publish) is missing.
Additionally, on OSs without a system-wide menu bar, the menu bar at the top of the window appears as a blank strip.

Although we won't move away from Qt 6.2 in the near future because of macOS compatibility, it would be good to fix this problem, because

  • eventually, we'll have to update anyway
  • Linux packagers want to create unofficial distro-specific MuseScore packages against the latest version of Qt, i.e. 6.7.2 at the time of writing, so they suffer from this problem.

Supporting files, videos and screenshots

image
(from #23980 (comment))

What is the latest version of MuseScore Studio where this issue is present?

self-built from master with Qt 6.5+

Regression

No.

Operating system

*

Additional context

No response

Checklist

  • This report follows the guidelines for reporting bugs and issues
  • I have verified that this issue has not been logged before, by searching the issue tracker for similar issues
  • I have attached all requested files and information to this report
  • I have attempted to identify the root problem as concisely as possible, and have used minimal reproducible examples where possible
@cbjeukendrup cbjeukendrup added community Issues particularly suitable for community contributors to work on dev Related to developer experience (compiling, code base, CI), rather than end user experience labels Aug 19, 2024
@muse-bot muse-bot added the needs review The issue needs review to set priority, fix version or change status etc. label Aug 19, 2024
@cbjeukendrup cbjeukendrup added tech debt and removed needs review The issue needs review to set priority, fix version or change status etc. labels Aug 19, 2024
@kbloom
Copy link

kbloom commented Aug 21, 2024

I inspected this in GammaRay, and appMenuBar had a height of 0. Changing the height in GammaRay to 20 made it visible.

@kbloom
Copy link

kbloom commented Aug 21, 2024

I didn't see anything obvious in GammaRay when I inspected the top-left toolbar.

@darix
Copy link

darix commented Aug 28, 2024

will there be a patch fix without upgrading Qt to resolve this?

@kbloom
Copy link

kbloom commented Aug 28, 2024

Some more investigation in AppMenuBar: it appears that

Component.onCompleted: {
  appMenuModel.load()
}

doesn't get run until I change the height of the AppMenuBar to be non-zero, so the ListView in AppMenuBar.qml can't get a non-zero height from its children, because the children haven't been loaded. Chicken and egg.

@kbloom
Copy link

kbloom commented Aug 28, 2024

I wonder if this commit in Qt is related. qt/qtdeclarative@492dc98

@darix
Copy link

darix commented Aug 29, 2024

04:01:18.566 | ERROR | main_thread     | GuiApp::perform | error: qrc:/qml/Muse/UiComponents/internal/PopupContent.qml:81:5: QML QQuickItem: Binding loop detected for property "implicitWidth"

04:01:18.566 | WARN  | main_thread     | Qt              | qrc:/qml/Muse/UiComponents/internal/PopupContent.qml:81:5: QML QQuickItem: Binding loop detected for property "implicitWidth"

is that related maybe?

@darix
Copy link

darix commented Aug 29, 2024

is the mixer working for you?

@Vogtinator
Copy link

AppMenuBar.qml has

ListView {
    id: root

    height: contentItem.childrenRect.height
    width: contentWidth

And that's the only source for sizing, the instantiation using the Loader in Main.qml doesn't set one.

I don't see how this can work, because ListView explicitly documents (https://doc.qt.io/qt-6/qml-qtquick-listview.html#cacheBuffer-prop):

a horizontal ListView only calculates the contentWidth. The other dimension is set to -1.

which means it starts with only contentWidth set (initially "estimated" at 100px * model size, hardcoded at https://github.com/qt/qtdeclarative/blob/a208a60ad8e98d7ecd8795596341284a418650e9/src/quick/items/qquicklistview.cpp#L160) and (https://doc.qt.io/qt-6/qml-qtquick-listview.html#details):

Note: ListView will only load as many delegate items as needed to fill up the view. Items outside of the view will not be loaded unless a sufficient cacheBuffer has been set. Hence, a ListView with zero width or height might not load any delegate items at all.

Means that with no height set, there will not be any children, meaning that the height will never be set either.

To avoid this, it should be enough to start with a non-zero height and then when the delegates are instantiated, use the calculated height:

diff --git a/src/appshell/qml/platform/AppMenuBar.qml b/src/appshell/qml/platform/AppMenuBar.qml
index 1a57ffac7a74..ce7453515a68 100644
--- a/src/appshell/qml/platform/AppMenuBar.qml
+++ b/src/appshell/qml/platform/AppMenuBar.qml
@@ -28,7 +28,7 @@ import MuseScore.AppShell 1.0
 ListView {
     id: root
 
-    height: contentItem.childrenRect.height
+    height: Math.max(1, contentItem.childrenRect.height)
     width: contentWidth
 
     property alias appWindow: appMenuModel.appWindow

@darix
Copy link

darix commented Aug 29, 2024

Fix confirmed ... but I guess this also gets hit by https://blog.broulik.de/2024/08/a-fresh-perspective-on-things/

if you click "files" menu and then move the mouse cursor right the new menus open at the start of the menu bar. :)

so now I am wondering ... is there more places where a similar fix to this is needed? like e.g. the mixer isnt working here.

@kbloom
Copy link

kbloom commented Aug 29, 2024

You could probably grep the codebase to find other issues similar to this.

Looks like we should investigate all of the following:

$ grep -r 'height:.*contentItem' *                                                        
src/framework/extensions/qml/Muse/Extensions/ExtensionViewer.qml:    height: builder.contentItem ? builder.contentItem.implicitHeight : 600
src/appshell/qml/platform/AppMenuBar.qml:    height: contentItem.childrenRect.height
src/appshell/qml/MainToolBar.qml:        height: contentItem.childrenRect.height
$ grep -r 'width:.*contentItem' *
src/project/qml/MuseScore/Project/SaveToCloudDialog.qml:                    width: contentItem.width
src/framework/extensions/qml/Muse/Extensions/ExtensionViewer.qml:    width: builder.contentItem ? builder.contentItem.implicitWidth : 800
src/playback/qml/MuseScore/Playback/internal/MixerPanelSection.qml:            width: contentItem.childrenRect.width
src/appshell/qml/MainToolBar.qml:        width: contentItem.childrenRect.width
src/instrumentsscene/qml/MuseScore/InstrumentsScene/internal/InstrumentsTreeItemDelegate.qml:                width: treeView.contentItem.width

The mixer is definitely on the list.

I can try to write a PR for this tomorrow if nobody gets to it first. If you want to beat me to it, you sidestep the menu positioning problem by testing on X11. I don't think this is a Wayland-specific bug.

@hamkg
Copy link

hamkg commented Aug 29, 2024

Just confirming that Votginator's patch fixes the missing menu bar issue when building on slackware-current (and running X11).

kbloom added a commit to kbloom/MuseScore that referenced this issue Aug 30, 2024
The problem was that a ListView lazily creates delegates as needed to
actually display them. If the size of the ListView is 0, then it doesn't
need to display anything, so it may not create any of the delegates. If
it doesn't create any delegates, then we can't make it size itself to
fit its contents. By setting a minimum size of 1, we force the ListView
to create a delegate that we can then use to determine the ListView's
actual size.

Fixes: musescore#24097
@kbloom kbloom linked a pull request Aug 30, 2024 that will close this issue
8 tasks
@kbloom
Copy link

kbloom commented Aug 30, 2024

I've got PR #24326 for the menu bar and the main toolbar, but trying to make a similar fix in MixerPanelSection doesn't fix the mixer. I'm not sure what's wrong with the mixer.

@kbloom
Copy link

kbloom commented Aug 30, 2024

Regarding the mixer, I also couldn't get some other panels to open, like the piano keyboard.

@kbloom
Copy link

kbloom commented Aug 30, 2024

When I tried opening the Navigator, it didn't show any mini-pages until I also opened Braille, at which point Braille doesn't show anything, but the Navigator does.

When I close and reopen Palette, the re-opened palette is empty.

@kbloom
Copy link

kbloom commented Aug 30, 2024

In short, can you file a separate bug about all of the docks.

@cbjeukendrup
Copy link
Contributor Author

Speaking of docks: in combination with updating to the latest Qt, we should perhaps also update to the latest KDDockWidgets. That's going to be non-trivial, because we would have to update from 1.5-ish to 2.1 which is a major version update, and also because we're using some private KDDockWidgets headers in order to get the exact behaviour we want.

@cbjeukendrup cbjeukendrup added the Qt-next Either broken or fixed by (updating to) newer Qt versions than we currently use label Sep 14, 2024
@orivej
Copy link
Contributor

orivej commented Sep 22, 2024

Speaking of docks: in combination with updating to the latest Qt, we should perhaps also update to the latest KDDockWidgets. That's going to be non-trivial, because we would have to update from 1.5-ish to 2.1 which is a major version update, and also because we're using some private KDDockWidgets headers in order to get the exact behaviour we want.

EDIT: I've moved my comment to a separate bug report #24866

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 dev Related to developer experience (compiling, code base, CI), rather than end user experience Qt-next Either broken or fixed by (updating to) newer Qt versions than we currently use tech debt
Projects
Status: In progress
Status: In Progress
Development

Successfully merging a pull request may close this issue.

7 participants