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

Qt/RenderWidget: Account for devicePixelRatio when auto-adjusting window size #8366

Merged
merged 1 commit into from Jan 24, 2020

Conversation

Techjar
Copy link
Contributor

@Techjar Techjar commented Sep 21, 2019

We want the game displayed at its actual resolution, not a scaled resolution. Fixes https://bugs.dolphin-emu.org/issues/11861

@BhaaLseN
Copy link
Member

Do we handle moving the window between screens anywhere? If so, that place needs to be updated as well (assuming we declare PerMonitorDPI in our manifest)

@Techjar
Copy link
Contributor Author

Techjar commented Sep 22, 2019

There doesn't appear to be anything handling movement between screens. Qt doesn't provide an elegant way to do it either (QWindow::screenChanged doesn't do what you think it does), so we'd have to handle the movement event and check if the screen ID changed ourselves.

@MayImilae
Copy link
Contributor

Tested! So this fixes it on the primary display, 1:1 exactly as it should be! However, if I move the window to another display or even open dolphin on another display, the ui is always scaled based on the primary display. So my primary display is 200% scaling, but my side monitors are 150% scaling. On the primary monitor 2x native is correctly set to 1280 pixels wide, but on the secondary displays, it is incorrectly 960 pixels wide. No matter what I did, changing multiple resolutions to auto-adjust multiple times, moving the render window between displays, or opening Dolphin on those screens, it was always 960 pixels wide.

Well, it's still an improvement, but, hmm.

Copy link
Contributor

@spycrab spycrab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment above. Merging it as is would break building with CMake on Windows.

@Techjar
Copy link
Contributor Author

Techjar commented Nov 1, 2019

Pinging @MayImilae to check the behavior of this PR again, since it's now using a different function to get the QScreen.

@Techjar
Copy link
Contributor Author

Techjar commented Nov 30, 2019

We've verified that per-monitor DPI works now, so this should be good to merge! We noticed some other odd bugs in testing but they aren't in any way related to this PR, and also occur on master.

Copy link
Contributor

@spycrab spycrab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to go now as far as I am concerned.

Copy link
Member

@jordan-woyak jordan-woyak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@stenzek stenzek merged commit a0b7c1b into dolphin-emu:master Jan 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
7 participants