Skip to content

Conversation

kikaitachi
Copy link
Contributor

Before:
Screenshot from 2023-10-13 13-49-02

After:
Screenshot from 2023-10-13 13-53-19

Layouts auto size components to reasonable defaults anyway, there is no need to specify constraints in this particular case.

@andrasfuchs
Copy link
Collaborator

Could you help me to find a way to test this (preferably on Windows)?

What operating system (and GUI framework) do you use?

I'm just a little hesitant to merge because I remember that we needed to put these setPreferredSize, setMinimumSize, setMaximumSize methods there for a reason (but I don't remember exactly why), so removing them might cause problems for others.

@andrasfuchs andrasfuchs added this to the Future milestone Oct 13, 2023
@kikaitachi
Copy link
Contributor Author

Could you help me to find a way to test this (preferably on Windows)?

What operating system (and GUI framework) do you use?

I'm just a little hesitant to merge because I remember that we needed to put these setPreferredSize, setMinimumSize, setMaximumSize methods there for a reason (but I don't remember exactly why), so removing them might cause problems for others.

I am using Ubuntu 22.04.03 LTS on 15 inch laptop screen with resolution 3840x2160.
Potentially we could at least double maximum size as 30 pixels is very small on HiDPI monitors. Too big is more usable than too small :)

@andrasfuchs
Copy link
Collaborator

I have just added details about the current screen resolution and DPI to the logs, because I would like to have a solution that works great for all the users.

Could you download the latest snapshot build, run it and send me the logs entries?

It would be a great help for me, and then I could implement a universal, DPI-dependent solution for the next release.

Thank you!

@andrasfuchs andrasfuchs self-assigned this Oct 16, 2023
@andrasfuchs andrasfuchs modified the milestones: Future, 1.9 Oct 16, 2023
@kikaitachi
Copy link
Contributor Author

I have just added details about the current screen resolution and DPI to the logs, because I would like to have a solution that works great for all the users.

Could you download the latest snapshot build, run it and send me the logs entries?

It would be a great help for me, and then I could implement a universal, DPI-dependent solution for the next release.

Thank you!

Screen: 3840x2160, 97 DPI

@andrasfuchs
Copy link
Collaborator

andrasfuchs commented Oct 16, 2023

Thanks!

I probably misunderstand something, but it looks to me that the reported DPI value is wrong either on my or on your machine.

You have a 15-inch display at 3840x2160 with 97 DPI.
I have a 24-inch display at 1920x1080 and I got 96 DPI.
(If I change my resolution above my native 1080p to 2048x1152 using NVIDIA super-resolution, I get 120 DPI. But if I lower it to 1280x720 I still get 96 DPI.)

But how? Shouldn't you have approximately 3 times as high DPI (because of the higher resolution and smaller size)? Wouldn't the DPI represent the pixel density?

@kikaitachi
Copy link
Contributor Author

Thanks!

I probably misunderstand something, but it looks to me that the reported DPI value is wrong either on my or on your machine.

You have a 15-inch display at 3840x2160 with 97 DPI. I have a 24-inch display at 1920x1080 and I got 96 DPI.

But how? Shouldn't you have approximately 3 times as high DPI (because of the higher resolution and smaller size)? Wouldn't the DPI represent the pixel density?

Potentially that value is affected by the fact that I use scaling factor 1.25 (to make everything on the screen bigger):
image
It is still interesting that after removing min/max constrains, fonts and component sizes become correct even if DPI value is suspicious.

@andrasfuchs
Copy link
Collaborator

Interesting!

We had a similar issue, but I'm not sure it was addressed properly.

I also found a commit that affected these Min/Max/Preferred sizes: 44767c17990589c0dc90a3a55ed21c541b0a31ba

All in all, I don't think we can rely on reported DPI, and the best solution would probably be yours: remove as many resolution specific constrains as possible.

I will still do some tests on my system in the hope of reproducing your situation.

@andrasfuchs andrasfuchs merged commit 5fd166e into freerouting:master Oct 16, 2023
@kikaitachi kikaitachi deleted the feature/fix_hidpi_monitor branch October 16, 2023 16:18
@andrasfuchs
Copy link
Collaborator

andrasfuchs commented Oct 16, 2023

Originally the window looks like this at 1080p:
image

When I switch to 3840x2160 (on my small monitor) I got these:

Before your PR:
image

After applying the PR:
image

Your PR is definitely an improvement. The curious thing is that the font size in the menu bar seems to be fine, all the others are wrong.

So, what I ended up doing is that I set the font size for all the UI components at startup manually and it seems to be working (at least on Windows with my resolutions).

The window at 3840x2160:
image

Could you try it as well?

@kikaitachi
Copy link
Contributor Author

Looks good for me now:
Screenshot from 2023-10-16 17-29-52
Thanks for merging!

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

Successfully merging this pull request may close these issues.

2 participants