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

Use StelMainView as the parent for all message boxes and file dialogs #3448

Merged
merged 1 commit into from
Oct 7, 2023

Conversation

10110111
Copy link
Contributor

@10110111 10110111 commented Oct 5, 2023

Description

In many cases the separate-window dialogs like message boxes and file open/save/etc. dialogs are given no parent in Stellarium. This leads to them looking very different in style and color scheme from the rest of the Stellarium GUI. The most striking difference is the light color scheme of the dialogs contrasting with the dark colors of Stellarium. Example:

Screenshot_2023-10-05_22-17-08

This patch changes almost all places that use QMessageBox and QFileDialog to pass &StelMainView::getInstance() as the parent, following the example of other places (and sometimes even the same files in other lines!). Only the message boxes used to tell about OpenGL errors are not parented since in this case this might be useless. The result looks like this:

Screenshot_2023-10-05_22-16-24

Only those used to tell about OpenGL errors are not parented since in
this case this might be useless.
This makes the styling of these dialogs match that of the main window.
@github-actions
Copy link

github-actions bot commented Oct 5, 2023

Great PR! Please pay attention to the following items before merging:

Files matching src/**/*.cpp:

  • Are possibly unused includes removed?

This is an automatically generated QA checklist based on modified files.

@gzotti
Copy link
Member

gzotti commented Oct 5, 2023

Looks great! I hope to have a closer look on Saturday.

@alex-w alex-w added this to the 23.4 milestone Oct 6, 2023
@alex-w
Copy link
Member

alex-w commented Oct 6, 2023

I see native dialogs in Windows/macOS. As far as I remember, the non-native style of dialogs was caused problems in Linux Mint.

@10110111
Copy link
Contributor Author

10110111 commented Oct 6, 2023

I see native dialogs in Windows/macOS. As far as I remember, the non-native style of dialogs was caused problems in Linux Mint.

What do you mean by native style? X11/Wayland-based desktops don't have any native style, each GUI toolkit uses its own. And what problems did you have? I'm using Ubuntu 20.04 and there are no problems here.

Moreover, some dialogs are already parented to StelMainView, so apparently they didn't cause any problems.

@alex-w
Copy link
Member

alex-w commented Oct 6, 2023

I see native dialogs in Windows/macOS. As far as I remember, the non-native style of dialogs was caused problems in Linux Mint.

What do you mean by native style? X11/Wayland-based desktops don't have any native style, each GUI toolkit uses its own. And what problems did you have? I'm using Ubuntu 20.04 and there are no problems here.

Moreover, some dialogs are already parented to StelMainView, so apparently they didn't cause any problems.

Native look for operating system UI. The issue (crash?) was happened around Qt 5.5-5.6 and it was caused from Qt/GTK+ bridge. Probably this is not actual anymore.

@gzotti
Copy link
Member

gzotti commented Oct 7, 2023

Tried on Windows: No visual change.
Tried WSL: seems to work, but...

some observations (maybe unrelated): when first running, I had no wayland plugin installed and a warning. The fullscreen was black, only black. I could not F11 to window mode. Edited the config.ini, then it seemed to work. In exit, there was a Segfault.

Then I installed qt6-wayland. It works windowed, but now the window is not resizable ( I have read about decoration issues with wayland a few days ago, so maybe that's expected now?) But no Segfault at exit.

If those observations are unrelated, I think it's OK. I hope window resizing on wayland (if that is what's used on WSL now) will be re-established. Or some alternative Qt plugin recommended for WSL.

@10110111
Copy link
Contributor Author

10110111 commented Oct 7, 2023

wayland plugin installed

Is Wayland the default window system in WSL? I thought it would be X11... I think Wayland support still has a long road towards stability even on "real" Unices.

In exit, there was a Segfault.

This may be #3427.

If those observations are unrelated, I think it's OK.

Shouldn't be related. In any case, we've already had some dialogs with non-null parent, I've just made this set more complete in this PR.

@gzotti
Copy link
Member

gzotti commented Oct 7, 2023

wayland plugin installed

Is Wayland the default window system in WSL? I thought it would be X11... I think Wayland support still has a long road towards stability even on "real" Unices.

I did nothing to configure a particular GUI plugin, and never bothered about the syntax to try other plugins. Just running WSL/U22.04.3 LTS.

In exit, there was a Segfault.

This may be #3427.

Maybe, but no other error message.

@10110111
Copy link
Contributor Author

10110111 commented Oct 7, 2023

In any case, you can just try this commit and the preceding one, and if no regression is noticed, this should be OK.

@gzotti
Copy link
Member

gzotti commented Oct 7, 2023

Yes, unrelated, more observations of -platform wayland (apparently now a new default) vs -platform xcb: Switching from windowed to fullscreen on wayland shows a black screen, then (on alt-tabbing away),
qt.qpa.wayland: Wayland does not support QWindow::requestActivate()
I can resize the window now, although the mouse cursor does not change.

xcb: just works.

Uhm, now I just had a Segfault on exit with xcb :-( :-(. During use, I did not call any system panels (color picking, file dialog, ...)

StelGLWidget destroyed
Segmentation fault

And now Segfault on exiting the wayland run.

Going back one commit: No segfault. Going to this commit: the segfault seems no longer reproducible, and I also have one on exiting emacs when launched into background. Weird.

So, I think this spurious segfault may also be unrelated. OK for me.

@10110111 10110111 merged commit 2d680de into Stellarium:master Oct 7, 2023
12 checks passed
@10110111 10110111 deleted the give-parent-to-dialogs branch October 7, 2023 22:21
@alex-w alex-w added the state: published The fix has been published for testing in weekly binary package label Oct 23, 2023
@github-actions
Copy link

Hello @10110111!

Please check the fresh version (development snapshot) of Stellarium:
https://github.com/Stellarium/stellarium-data/releases/tag/weekly-snapshot

@alex-w alex-w removed the state: published The fix has been published for testing in weekly binary package label Dec 23, 2023
Copy link

Hello @10110111!

Please check the latest stable version of Stellarium:
https://github.com/Stellarium/stellarium/releases/latest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants