-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Working around missing screen dimensions for Steam Deck #4995
Conversation
And maybe more? Fixes fyne-io#4896
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some small things:
- consolidate scale fallback values
- explain “failure screen dimensions”
widthMm, _ := monitor.GetPhysicalSize() | ||
widthMm, heightMm := monitor.GetPhysicalSize() | ||
if widthMm == 60 && heightMm == 60 { // some sort of failure - mostly on Steam Deck | ||
return 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I think the return 1
statements should look all the same. Either with or without .0
. Personally, I would leave it out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I think the update should cover the feedback
@@ -314,7 +314,10 @@ func (w *window) detectScale() float32 { | |||
return 1.0 | |||
} | |||
|
|||
widthMm, _ := monitor.GetPhysicalSize() | |||
widthMm, heightMm := monitor.GetPhysicalSize() | |||
if widthMm == 60 && heightMm == 60 { // some sort of failure - mostly on Steam Deck |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where does these 60
come from? AFAIU the GLFW source, it would return 0
if the monitor did not provide a number.
If these values are based on experience, the comment should explain this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, this might result in a wrong scale for real-life 60mmx60mm displays with a huge DPI 😁.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is true. If there was ever a 6cm square screen that we had to support. If that ever happens I will update this for sure!
@@ -314,7 +314,10 @@ func (w *window) detectScale() float32 { | |||
return 1.0 | |||
} | |||
|
|||
widthMm, _ := monitor.GetPhysicalSize() | |||
widthMm, heightMm := monitor.GetPhysicalSize() | |||
if widthMm == 60 && heightMm == 60 { // some sort of failure - mostly on Steam Deck |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be not be protected by a runtime.GOOS == "linux"
check? Maybe even !build.IsWayland
from what I understand of the issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Wayland check is above and it will exit early.
Linux specific... I guess so, updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point :)
And maybe more?
Fixes #4896
Results in this (not the right DPI, but not unexpected given how other software looks)
Checklist: