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

Restore layout for window doesn't work if it's currently maximized #477

Closed
bbc131 opened this issue Mar 14, 2024 · 7 comments
Closed

Restore layout for window doesn't work if it's currently maximized #477

bbc131 opened this issue Mar 14, 2024 · 7 comments
Assignees

Comments

@bbc131
Copy link
Contributor

bbc131 commented Mar 14, 2024

The problem can be reproduced with the example qtwidgets_dockwidgets:

  1. Turn your mainwindow in 'normal' state, i.e. not 'maximized' etc.
  2. Save the current layout
  3. Maximize your mainwindow
  4. Restore the saved layout

Now, on Windows, the mainwindow looks like correctly restored but isn't.
For example, you cannot change its size by click'n'drag on its edges, you first need to move it a bit by click'n'drag at the windows title bar.
(On Ubuntu, the restoring also doesn't work, but behaves differently. The window just stays maximized.)

To fix this, I think there are two things in LayoutSaver::restoreLayout(..), that need to be changed.

  1. the window-state has to be set, if the desired state differs from the current state, not only if the desired state is not 'normal'/WindowState::None.
  2. deserializeWindowGeometry cannot set the geometry correctly, if the window is currently maximized, so the window-state should be changed beforehand.

I checked this by playing a bit and this hack seems to solve the problem:

         if (!(d->m_restoreOptions & InternalRestoreOption::SkipMainWindowGeometry)) {
             Window::Ptr window = mainWindow->view()->window();
+            if (window->windowState() != WindowState::None) {
+                if (auto w = mainWindow->view()->window()) {
+                    w->setWindowState(WindowState::None);
+                }
+            }
             d->deserializeWindowGeometry(mw, window);
-            if (mw.windowState != WindowState::None) {
+            if (mw.windowState != window->windowState()) {
                 if (auto w = mainWindow->view()->window()) {
                     w->setWindowState(mw.windowState);
                 }
@iamsergio iamsergio self-assigned this Mar 18, 2024
@iamsergio
Copy link
Contributor

You might be right, but I would like to reproduce this as well, to maybe write a test.

Can't repro on Win11 / Qt 6.6.2, stock qtwidgets_dockwidgets without flags.

Which versions are you on ?

@bbc131
Copy link
Contributor Author

bbc131 commented Mar 18, 2024

Sure, my code above should just be a hint where / what the problems are...
I use Win 10 and Qt 5.15.8
Can reproduce it with the default example qtwidgets_dockwidgets without any flags.

@iamsergio
Copy link
Contributor

Thanks, reproduced with Qt 5

iamsergio added a commit that referenced this issue Mar 18, 2024
We'll be having some tests that we want to run in the real
window manager, not offscreen.

For issue #477
iamsergio added a commit that referenced this issue Mar 18, 2024
Tests maximize vs restore in the real QPA, not offscreen.

For issue #477.
iamsergio added a commit that referenced this issue Mar 18, 2024
By black listing all failing platforms.
We'll be fixing one by one.

For issue #477
iamsergio added a commit that referenced this issue Mar 18, 2024
Tested on Linux/KDE and offscreen platforms.
Will whitelist the others as soon as tested there as well.

Patch suggested by bbc131 in issue #477
@iamsergio
Copy link
Contributor

Works for me now, thanks for the investigation

@bbc131
Copy link
Contributor Author

bbc131 commented Mar 20, 2024

I think there is a regression.
The case, where you first maximize the window, save this (maximized) layout and try to restore this later, doesn't work anymore.
This worked before.

@iamsergio iamsergio reopened this Mar 22, 2024
iamsergio added a commit that referenced this issue Mar 22, 2024
Works on Windows/QtWidgets
Fails on Windows/QtQuick.

Will test other platforms as well.
For issue #477
iamsergio added a commit that referenced this issue Mar 22, 2024
We need to set geometry before maximizing, otherwise it doesn't
get set as "normal geometry".

However, when going from maximized to normal, we need to set state
first.

If CI stays green then it means issue #477 can be closed.
@iamsergio
Copy link
Contributor

I've added tests for that case as well now, and fixed it.

Windows and KDE pass the test.
Ubuntu and macOS pass a manual test, but not the automatic one, need to investigate further.

@iamsergio
Copy link
Contributor

Tests are passing on mac and ubuntu now

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

No branches or pull requests

2 participants