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

Remember window size #449

Open
Tiagoquix opened this issue Aug 30, 2024 · 9 comments
Open

Remember window size #449

Tiagoquix opened this issue Aug 30, 2024 · 9 comments
Labels
enhancement New feature or request

Comments

@Tiagoquix
Copy link

When the app is closed, it does not remember neither previous window size nor maximized state. Would be cool if the app remembered it.

@Tiagoquix Tiagoquix added the enhancement New feature or request label Aug 30, 2024
@sonic2kk
Copy link
Contributor

Been looking into this recently for KDE applications, they typically store geometry. However on Wayland restoring window position is a bit funky with Qr and instead of defaulting to centered it can default to what appears to be 0,0.

It would be possible to store geometry though. We'd just have to be careful. If we maximise a window and store that geometry, then un-maximizing on load would set that maximised geometry. But not storing geometry on close when the window is maximised results in a problem where if you resize, maximise, close, then re-open, the resized geometry is not remembered.

And storing on each resize event is a bit costly with performance. One option is to use a timer, I think that's what KDE's new KConfig.WindowStateSaver does.

And we may also want to store if the window was maximised, maybe not applying geometry if a window was opened maximised (we could store a flag for this).

@DavidoTek
Copy link
Owner

And storing on each resize event is a bit costly with performance. One option is to use a timer, I think that's what KDE's new KConfig.WindowStateSaver does.

It makes the most sense to store the geometry once the applications exits. The signal QtCoreApplication::aboutToQuit seems like a reasonable choice for that.

instead of defaulting to centered it can default to what appears to be 0,0.
If we maximise a window and store that geometry, then un-maximizing on load would set that maximised geometry

I think storing whether a window was maximized or not is the most interesting part. We could store whether it was maximized before closing and ignore the size on startup if that is the case and just maximize the window.
Otherwise, the window size is set and opened in the center. We can store the location on X11 if that is worth the effort.

@Tiagoquix
Copy link
Author

Hey, many thanks for the interest!

If you're going to implement this feature, would be good to target Wayland first, as X11 will be deprecated at some point. Implementing for both would be the ideal solution, but if we have to choose between one of them then Wayland would be the priority and future-proof.

@sonic2kk
Copy link
Contributor

sonic2kk commented Sep 3, 2024

There shouldn't be any need to prioritize either or, Qt should handle that. However, Wayland itself does not offer a protocol for setting window positions to my understanding, positioning is handled by the compositor. If Wayland itself doesn't offer a protocol for it, then toolkits like Qt can't offer it, and so ProtonUp-Qt can't do it.

@DavidoTek If you're interested, this is how KDE handle it on their side:

We wouldn't need to separate it like this per-se, some KDE apps before this like PlasmaTube had a WindowManager type class which handled saving and restoring window sizes. We could essentially do what KDE do in KFrameworks and take it as one class that accepts a window, and copy their logic to skip window positioning on Wayland for now.

@DavidoTek
Copy link
Owner

We can use QWindow::setGeometry(x, y, width, height) for resizing on X11/Wayland and positioning on X11.

KDE handle it on their side:

I wonder how much effort it is to implement it for common compositors like KDE KWin and GNOME Mutter. At least in the Flatpak, all KDE libraries should be included, I'm not sure about GNOME stuff.

(I think if we decide to implement it, the code should go into a separate file like windowutil.py. This way, we can "easily" replace it once Qt adds support and other projects may also be interested in "stealing" this windowutil.py file 😄. Maybe there is already something similar out there?)


I guess the first steps are implementing storing the width, height, and maximized state for Wayland/X11 and the position for X11. We can then concentrate on (re-)storing the position on Wayland.

@sonic2kk
Copy link
Contributor

sonic2kk commented Sep 4, 2024

I think if we decide to implement it, the code should go into a separate file like windowutil.py

I agree, we can use this to write the window information out to a config file, one standard seems to be to end the filename with rc and no extension, so we could make a file like pupguirc in our config folder. This could hold information about the window geometry and the maximized state as a boolean / store the Qt window "visibility" information and parse out whether this corresponds to maximized or not based on Qt flags.

Before it recently switched to using WindowStateSaver, PlasmaTube used a WindowController, we could essentially implement this but in Python (and with a regular QtWidgets window and not a QQuickWindow, although I would imagine the methods are very similar), and we would implement the respective KWindowConfig methods. It seems KFrameworks stores position and geometry separately, so we could do something similar (or they just have it split out into different methods for convenience, but under-the-hood they write to the same key, unsure).

PlasmaTube had a QTimer set up in QML to save the sizing after a fixed time interval. This avoids writing to a file constantly, as the comment in the linked file explains. We would have to set this up ourselves. We could use KConfig::WIndowStateSaver's C++ implementation as a reference.

We can then concentrate on (re-)storing the position on Wayland.

I think currently it is not possible at all for applications to specify their positioning to a compositor. To my understanding it is the compositor alone that makes this decision (which I assume is how KWin/Mutter can center windows, place them on the display under the cursor, etc, but why applications cannot restore their positions/last displays on re-open).

If this is correct then I assume if support for this is ever accepted into Wayland, Qt would implement support for it under setGeometry.

@DavidoTek
Copy link
Owner

one standard seems to be to end the filename with rc and no extension, so we could make a file like pupguirc in our config folder. This could hold information about the window geometry and the maximized state as a boolean / store the Qt window "visibility" information and parse out whether this corresponds to maximized or not based on Qt flags.

I wasn't aware of this, is there a standard of how the content of this file would look? Which program does use that "rc" file?

Yeah, I think we would need posx: int, posy: int, width: int, height: int and maximized: bool. If something is not applicable, we can set it to 0 or -1 if it is an int.

Before it recently switched to using WindowStateSaver, PlasmaTube used a WindowController, we could essentially implement this but in Python (and with a regular QtWidgets window and not a QQuickWindow, although I would imagine the methods are very similar), and we would implement the respective KWindowConfig methods

Ah, I see. We could add something like KWindowConfig::restoreWindowSize and KWindowConfig::restoreWindowPosition. Something like this:

# windowutil.py - PSEUDO CODE
def restore_window_geometry(window: QWindow):
    window_name: str = window.getSomeUuidOrClassName()  # some unique value to identify the window class
    # window_name alternative: supply window_name as function parameter of restore_window_geometry()
    cfg: dict = some_config_load_fn(CONFIG_FILE_LOCATION)  # or parameter with config file ?
    if win_cfg := cfg.get(window_name):
        window.setGeometry(
            win_cfg.get("posx", 0), win_cfg.get("posy", 0),
            win_cfg.get("width", 400), win_cfg.get("height", 500)
            )

PlasmaTube had a QTimer set up in QML to save the sizing after a fixed time interval. This avoids writing to a file constantly, as the comment in the linked file explains. We would have to set this up ourselves. We could use KConfig::WIndowStateSaver's C++ implementation as a reference.

If feel like the QtCoreApplication::aboutToQuit signal solution would have the least overhead.

BUT: I'm not sure if it possible to determine the geometry of the window after the user clicked the close button. If that is not possible, we can do the timer approach as you suggested.

I think currently it is not possible at all for applications to specify their positioning to a compositor. To my understanding it is the compositor alone that makes this decision (which I assume is how KWin/Mutter can center windows, place them on the display under the cursor, etc, but why applications cannot restore their positions/last displays on re-open).

I see. Apparently there are a few drafts/discussions, but nothing production ready yet.

@sonic2kk
Copy link
Contributor

sonic2kk commented Sep 5, 2024

I wasn't aware of this, is there a standard of how the content of this file would look? Which program does use that "rc" file?

I am not sure if there is a standard, but I checked about ten (there are almost 300 in my ~/.config directory) and their structure is similar to INI files.

Most Qt-based applications seem to use it, KDE applications which are Qt-based will use it, and the Strawberry Music Player also stores an rc file. The files are typically named i.e. strawberryrc. An old Minecraft launcher from many years ago called MultiMC also has a MultiMCrc file.

But on further inspection, these may be auto-generated by Plasma, as there is also a ProtonUp-Qtrc that denotes the screen that ProtonUp-Qt was last opened on, and the window size. This file has not been written to since 18th May this year (maybe it's for the AppImage? Not sure, it doesn't get updated when I interact when using python3 -m pupgui2), though. But on the other hand, this may be related to the key used. Inspecting other files shows that some seemingly auto-generated information is inserted here, too, like a list of Recent Files that I presume show up when you right click on supported applications in an Application Launcher and can view the "Recent Files" and so on.

For ProtonUp-Qt, the file looks like this on my system. Note that the key is [FileDialogSize].

[FileDialogSize]
4 screens: Height=582
4 screens: Width=1116

Perhaps there is a way we can write into these files, and set our own key? Plasma applications typically use Window. To my understanding the configGroupName property for WindowStateSaver corresponds to this, so if this was named AwesomeWindow, then the section (or "group" I guess?) in the file would be called [AwesomeWindow].

BUT: I'm not sure if it possible to determine the geometry of the window after the user clicked the close button. If that is not possible, we can do the timer approach as you suggested.

You should be able to, at least in C++ this is possible. Discover used to do this, when it got the QEvent::Close event, it could query m_mainWindow->geometry(). This is also possible in Python, on event.type() == QEvent.Close.

import sys

from PySide6.QtCore import Qt, QEvent
from PySide6.QtWidgets import QApplication, QLabel, QWidget, QVBoxLayout

class HelloWorld(QWidget):
    def __init__(self):
        super().__init__()

        self.setup_ui()

    def setup_ui(self):
        self.setWindowTitle('Hello, World!')
        self.show()

    def event(self, event):
        if event.type() == QEvent.Close:
            print(f'Geometry is: {self.geometry()}')


app = QApplication(sys.argv)
ex = HelloWorld()
sys.exit(app.exec())

But this alone is not enough, we still need the timer approach. Otherwise we end up with the problem Discover had, where if a window is maximized and we query the geometry on close, we store the maximized geometry. If we don't store on close if maximized, we will lose window geometry. Therefore we have to store geometry on close if not maximized, and on resize (with a QTimer). At least that's how I understand it, and how I understand what WindowStateSaver and KWindowConfig to be doing.

I think you mentioned this earlier, it might be good to see if something like this is already written for PySide6 that we could use as a library, or a code snippet we could drop-in.

@DavidoTek
Copy link
Owner

But on further inspection, these may be auto-generated by Plasma

I wonder if that file is created by Plasma itself or by the application (Qt/KDE framework in the case of [FileDialogSize]).

QSettings supports an ini-like format. We can use that to read/write the *rc file.

Perhaps there is a way we can write into these files, and set our own key? Plasma applications typically use Window. To my understanding the configGroupName property for WindowStateSaver corresponds to this, so if this was named AwesomeWindow, then the section (or "group" I guess?) in the file would be called [AwesomeWindow].

The restoring is also handled by the WindowStateSaver, right? If the rc file would be read by Plasma itself, it would be easy to set a position on Wayland...

But this alone is not enough, we still need the timer approach. Otherwise we end up with the problem Discover had, where if a window is maximized and we query the geometry on close, we store the maximized geometry. If we don't store on close if maximized, we will lose window geometry.

Aah, right. What we could do is to only set the maximized property to True and leave the geometry untouched when the window is maximized on close. It will use the geometry from the last time the window was closed not-maximized.

something like this is already written for PySide6 that we could use as a library, or a code snippet we could drop-in.

There is this example: https://doc.qt.io/qtforpython-6/overviews/restoring-geometry.html
That should work with QMainWindow as it inherits from QWidget, not sure about QWindow because it inherits from QObject...

If we create our own code, we could do something like this and handle the type checks using isinstance():
windowutil.py#save_geometry(window: QObject, window_name: str)
windowutil.py#restore_geometry(window: QObject, window_name: str)

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

No branches or pull requests

3 participants