Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.95%. Comparing base (a516c93) to head (7f40ac9).

@@           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.
📢 Have feedback on the report? Share it here.

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.

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

(this is my desktop even though the username is 'frame')

Ugh, the logs do not print the actual values of Size and Rect in release mode, only in debug mode 🙄 I have to update that.

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 scaleFactor: 2.0, so I think we need to take that into account when doing the calculation! It may be some fancy HiDPI stuff.
Also, yes, I agree that if the last window size is too big for the current screen, we should just apply the default size instead of this clamping. That makes more sense.

Alright, I looked through the plugin code and I can confirm that scaleFactor is related to fancy HiDPI stuff. On most normal monitors, it is 1, but on HiDPI ones it can be more, often 2. I think that also explain why the screen size was behaving weirdly on macs, since I think they have HiDPI screens. Now I need to figure out if we need to divide or multiply by that number 😆

@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?
@ricab since you mentioned you have a linux laptop with a hidpi screen, could you please open this version of the gui in the terminal and send us the logs, along with the screen resolution that is set in your system settings?

@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:

2024-12-12 16:57:54.480794 DEBUG Got Screen{frame: Rect.fromLTRB(0.0, 0.0, 3840.0, 2160.0), scaleFactor: 2.0, visibleFrame: Rect.fromLTRB(0.0, 0.0, 3840.0, 2160.0), scaledSize: Size(7680.0, 4320.0)}
2024-12-12 16:57:54.482362 DEBUG Got last window size: null
2024-12-12 16:57:54.482432 DEBUG Computed default window size: Size(1400.0, 822.0)
2024-12-12 16:57:54.482454 DEBUG Using clamped window size: Size(1400.0, 822.0)
2024-12-12 16:57:54.741920 INFO Sent FindRequest{showImages: true, showBlueprints: false}
2024-12-12 16:57:55.017374 INFO Sent VersionRequest{}
2024-12-12 16:57:55.018071 INFO FindRequest{showImages: true, showBlueprints: false} received FindReply{showImages: true, imagesInfo: [{os: Ubuntu, release: Core 16, version: 20200818, aliasesInfo: [{alias: core}, {alias: core16}], codename: Core 16}, {os: Ubuntu, release: Core 18, version: 20211124, aliasesInfo: [{alias: core18}], codename: Core 18}, {os: Ubuntu, release: Core 20, version: 20230119, aliasesInfo: [{alias: core20}], codename: Core 20}, {os: Ubuntu, release: Core 22, version: 20230717, aliasesInfo: [{alias: core22}], codename: Core 22}, {os: Ubuntu, release: Core 24, version: 20240603, aliasesInfo: [{alias: core24}], codename: Core 24}, {os: Ubuntu, release: 20.04 LTS, version: 20241112, aliasesInfo: [{alias: 20.04}, {alias: f}, {alias: focal}], codename: Focal Fossa}, {os: Ubuntu, release: 22.04 LTS, version: 20241206, aliasesInfo: [{alias: 22.04}, {alias: j}, {alias: jammy}], codename: Jammy Jellyfish}, {os: Ubuntu, release: 24.04 LTS, version: 20241119, aliasesInfo: [{alias: 24.04}, {alias: n}, {alias: noble}, {alias: lts}, {alias: default}], codename: Noble Numbat}, {os: Ubuntu, release: 24.10, version: 20241109, aliasesInfo: [{alias: 24.10}, {alias: o}, {alias: oracular}], codename: Oracular Oriole}, {os: Ubuntu, release: 25.04, version: 20241209, aliasesInfo: [{remoteName: daily, alias: 25.04}, {remoteName: daily, alias: p}, {remoteName: daily, alias: plucky}, {remoteName: daily, alias: devel}], codename: Plucky Puffin}, {os: Ubuntu, release: AdGuard Home Appliance, version: 20200812, aliasesInfo: [{remoteName: appliance, alias: adguard-home}], codename: Core 18}, {os: Ubuntu, release: Mosquitto Appliance, version: 20200812, aliasesInfo: [{remoteName: appliance, alias: mosquitto}], codename: Core 18}, {os: Ubuntu, release: Nextcloud Appliance, version: 20200812, aliasesInfo: [{remoteName: appliance, alias: nextcloud}], codename: Core 18}, {os: Ubuntu, release: openHAB Home Appliance, version: 20200812, aliasesInfo: [{remoteName: appliance, alias: openhab}], codename: Core 18}, {os: Ubuntu, release: Plex Media Server Appliance, version: 20200812, aliasesInfo: [{remoteName: appliance, alias: plexmediaserver}], codename: Core 18}]}
2024-12-12 16:57:55.018589 INFO FindRequest{showImages: true, showBlueprints: false} is done
2024-12-12 16:57:55.018960 INFO VersionRequest{} received VersionReply{version: 1.16.0-dev.42.pr3814+g600faa1c2, updateInfo: {}}
2024-12-12 16:57:55.019027 INFO VersionRequest{} is done
2024-12-12 16:57:55.558845 DEBUG Saving screen size: Size(1400.0, 822.0)
2024-12-12 16:58:04.346991 DEBUG Saving screen size: Size(1400.0, 822.0)
2024-12-12 16:58:15.732012 DEBUG Saving screen size: Size(1400.0, 822.0)
2024-12-12 17:03:16.020323 DEBUG Saving screen size: Size(1400.0, 822.0)

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 🤷
KDE is using Qt underneath, so @levkropp what does the following give you

from PyQt5.QtWidgets import QApplication
from PyQt5.QtGui import QGuiApplication
import sys

app = QApplication(sys.argv)
screen = QGuiApplication.primaryScreen()
print(f"{screen.geometry()}")

@levkropp what does the following give you

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

It looks like the issue might just be with KDE/Qt handling the screen on the Framework laptop. On GNOME, the GUI reports the proper resolution, but a scaleFactor of 1.0

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.
And perhaps we could add this for us as well. Opinions?

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.

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

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. And perhaps we could add this for us as well. Opinions?

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

@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.