[gui] remember window size + better default sizes #3814
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3814 +/- ##
=======================================
Coverage 88.94% 88.95%
=======================================
Files 256 256
Lines 14584 14584
=======================================
+ Hits 12972 12973 +1
+ Misses 1612 1611 -1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
LGTM! Multipass now remembers my window size on KDE and resizes based on resolution.
Just checking: What happens if I'm on 1080p and I resize my window to almost fullscreen, and then I change my resolution to something smaller like 1280x720? Does Flutter automatically detect that its trying to open a window size larger than the current resolution or do we need to check for this?
|
Ah, good point. It does not do any of that. It just uses the last size exactly as it was. We could indeed clamp it by the screen size. |
There was a problem hiding this comment.
I'm getting behavior on my Framework laptop display (2256x1404) on KDE where if I open Multipass, resize it, and then close it, when I open it again it doesn't remember the window size properly anymore
framework_doesnt_remember_window_size_levkropp.webm
resetting to the previous commit has the GUI remembering the window size correctly
|
Interesting. For me it still works fine 🤔 I'll add some logging to help with debugging. |
|
I've tested on my desktop machine which is 1920x1080, Kubuntu 24.10, and I don't get the regression for remembering window size that I did on my Framework. However, the "clamping" does not appear to work: if I resize a window to 1680x1050 when on 1080p then set my display resolution to 1440x900, it opens up a window that is larger than my screen resolution. Additionally, the logs do not provide the actual numbers they just say "Instance of Size" etc, but I'm not sure if that's intended or not on your end |
|
Ugh, the logs do not print the actual values of |
|
The clamping behavior works on my 1080p desktop but the clamping resolution ends up being the resolution of the entire screen so the GUI is cut off vertically. We should apply the better default size logic when clamping as well in my opinion The logging revealed the reason why the GUI was not remembering the window size on my Framework laptop: for some reason, flutter thinks my laptop's resolution is half of what it actually is, so when I size it larger than half of the screen it clamps to half of the screen. It correctly remembers the screen size for sizes smaller than 1128x752. We could just have this as a known bug for the framework laptop, or put in unique code for when the screen size 1128x752 is detected.
|
|
Interesting! I see that in your second screenshot, the screen has a |
|
Alright, I looked through the plugin code and I can confirm that |
|
@levkropp, apparently on macos this does not seem to be an issue (at least in my case). My mac also has a scaleFactor of 2.0, but the returned screen size matches the one from my system settings. So perhaps this is a Linux only issue? Could you perhaps take over this PR and see if you can make it work on your scaleFactor 2.0 screen? I don't have one, so I can't test it 😅 |
|
@levkropp just to clarify, did you have access to a windows laptop with a HiDPI screen in order to test this? |
Sorry for not clarifying: I tested my commit on my Framework laptop which has a HiDPI screen running on Ubuntu. I haven't done testing on windows for this but plan on doing so today with my Framework |
|
Hey @andrei-toterman, here are the logs I get: This with a dual screen setup:
Notice that I have different scales on the two displays. The primary display is display 2 in that setup, which is where the GUI opened. |
|
Ugh, this is what I feared. In your case, the screen size retrieved by the flutter plugin matches the one you have in your system settings, so we shouldn't multiply by the scale factor here. So, @levkropp, in the best case, the disparity is present only on the framework laptop, or in the worst case, it just depends on some unknown stuff for different monitors. So, IMO, since we can't be sure that the screen size we get is the same one as specified in the system settings, we should take it as it is and hope that it's good. Again, on mac it also returns the right one without having to multiply by the scale factor. And apparently on Linux it can return the right size as well. And I have a hunch that on Windows it will also vary from monitor to monitor. Opinions? Also, thanks @ricab for checking! |
|
Could it be a KDE quirk? |
|
Well, the plugin calls gdk_monitor_get_geometry. So maybe 🤷 from PyQt5.QtWidgets import QApplication
from PyQt5.QtGui import QGuiApplication
import sys
app = QApplication(sys.argv)
screen = QGuiApplication.primaryScreen()
print(f"{screen.geometry()}") |
my resolution is 2256x1504 and I am using 125% scaling. Qt reports the height and width as 2256/1.25 and 1504/1.25 it seems I'll switch to GNOME and check if I'm having any similar behavior |
|
|
|
Well, the behavior we get is just too unpredictable to make any good decision. My suggestion is that we trust the window size that we get as it is. Apparently we can't really know for sure if it's the right one, but it sometimes is, so what else could we do? I've also been playing with some other apps (Dolphin on KDE) to see how they react. And what they do is they remember the last window size, per resolution. If I am on one resolution and set the window to be really wide, the app will remember that size when I open it again. If I go to another resolution and make the app really thin, it will remember that. But if I go back to the previous resolution, it will open the app very wide again, as it was before. So it remembers the last window size for each resolution, and if there is no last window size for a resolution, it opens it to a default size that fits. |
I agree with this. The screen size we get from the flutter plugin has been correct in all of our testing except specifically the Framework Laptop on specifically KDE, which appears to be from how Qt handles screen size and scaling
This would be useful for people who connect their laptops to external monitors e.g. in an office. If it is not too much work to implement/maintain then I am all for it |
andrei-toterman
force-pushed
the
better-window-size
branch
from
ff25418 to
8055fc1
Compare
There was a problem hiding this comment.
Testing it again and getting weird behavior both on my desktop and framework laptop, both on KDE :/
on my desktop (above) it was not remembering the window size even though it said "saving window size" and remembered a previous size
on my framework laptop it was not clamping when reading the resolution on the normal size screen resolution (when before it would glitch as shown in the video in this comment) and it seemed to forget the last window size entirely when changing the screen resolution in the display options. We've already discussed how we shouldn't cater to the unique situation of the framework laptop, and if these issues that I am seeing on my desktop aren't reproducible on your or @ricab's machines then it's probably safe to assume that it is just an issue with my setup or hardware or display drivers or configuration etc
|
@levkropp just to clarify, I implemented the behavior discussed above, which I observed that other apps use. This means that when you open the app on a given resolution, and there is no saved window size for that resolution, the app will compute a default window size using the algorithm we've had so far. After that, any change in the window while in that resolution size will be remembered. If I close the app and change my resolution, then open the app again, it will not use the size saved on the previous resolution, it will either use a saved size for that resolution or compute a default one for that resolution. So each individual screen resolution is treated separately, and it has its own associated window size. So in your fisrt screenshot, when you were on 1920x1080, it saved the window size 1847x942. You changed then the resolution to 1280x800, which saved a window size of 1024x640. And then you went back to 1920x1080, and the window remembered its size of 1847x942. In the second screenshot, there was a saved window size for the screen resolution of 1128x752, but not for 640x400, so it computed a default window size. |
There was a problem hiding this comment.
The code looks good, but there is some interesting behavior that I noticed that might be related but I have no clue. Some seem to be Flutter bugs.
It seems sometimes the window opens on one monitor, the size is calculated, and then the window moves to the other monitor (very quickly). This seems to be the case without these changes but it's hard to tell without the logging.
Which monitor the window opens on is extremely inconsistent. (not new)
The size is saved on window focus and unfocus, it seems the window losing/gaining a shadow triggers a resize event but the reported size is the same.
center seems to center the framebuffer (not the window), but doesn't work most of the time.
It seems resizing the window to the minimum will save the size as being smaller than the minimum.
Manually setting the saved width/height disregards minimumSize (unless maybe this is caused by above?).
It is possible to make the saved size much larger than the size of the window by having the window span multiple screens. (But maybe that's fine)
|
Hey, @Sploder12! Thanks for the in-depth feedback! I looked some more into the |
There was a problem hiding this comment.
Since the issues seem to be unrelated to these changes, LGTM!
Other apps on Ubuntu tend to open in random, but consistent per application, locations on the monitor I open them on. Sometimes it's top-left, sometimes it's center, sometimes they remember their last location. But fairly inconsistent overall.
Most Windows windows tend to save location and size. But on occasion an app will open centered or in some fixed random location.
Since the centering is terribly broken as is, it'd probably be better to let the OS handle it. Saving the position could be a nice change too though (for a later PR).
fix #3812
fix #3813
MULTI-1696
MULTI-1697