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

Resolve #21494: Automatically set window scaling based on DPI #21907

Merged
merged 9 commits into from
Jul 21, 2024

Conversation

karstenvanf
Copy link
Contributor

I used a similar to solution to #6991, but modified it to include a config option that flags whether the DPI has been checked instead of setting an invalid scaling factor. If the DPI check returns an error, it does not change from the default of 1.

Check DPI at UiContext load if not previously checked, then set to reasonable division of 0.25.
@karstenvanf karstenvanf force-pushed the develop branch 2 times, most recently from 49ad833 to 565ed5e Compare April 27, 2024 02:32
@karstenvanf karstenvanf marked this pull request as ready for review April 27, 2024 03:12
@Gymnasiast
Copy link
Member

@karstenvanf Could you apply the requested changes?

Copy link

This pull request has been marked as stale and will be closed in 14 days if no action is taken. To keep it open, leave a comment or remove the stale-pr label. If you're awaiting feedback from a developer, please send us a reminder (either here or on Discord).

Copy link

This pull request has been marked as stale and will be closed in 14 days if no action is taken. To keep it open, leave a comment or remove the stale-pr label. If you're awaiting feedback from a developer, please send us a reminder (either here or on Discord).

@AaronVanGeffen AaronVanGeffen added changelog This issue/PR deserves a changelog entry. squash merge A PR that should be squashed on merge. labels Jul 16, 2024
Copy link
Member

@AaronVanGeffen AaronVanGeffen left a comment

Choose a reason for hiding this comment

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

I've rebased and updated this PR. The initial implementation used SDL_GetDisplayDPI, which was producing unreliable results on macOS, and apparently is also discouraged by SDL.

Instead, I've changed the implementation to use SDL_GetWindowSize and SDL_GetRendererOutputSize instead, as recommended by SDL. This infers the scaling ratio from the actual canvas size vs the one requested. This appears to work well for me.

Could someone test this works as expected on Windows as well?

@AaronVanGeffen AaronVanGeffen added this to the v0.4.12+ milestone Jul 16, 2024
@Basssiiie
Copy link
Member

Basssiiie commented Jul 16, 2024

Tested it on Windows 10 64-bit, regular DPI setup and I get this at launch:
image

crash.zip

Error: The thread tried to divide an integer value by an integer divisor of zero.

Top of the stack trace:

openrct2.com!OpenGLDrawingContext::CalculcateClipping(DrawPixelInfo & dpi) Line 1087
	at D:\a\OpenRCT2\OpenRCT2\src\openrct2-ui\drawing\engines\opengl\OpenGLDrawingEngine.cpp(1087)
openrct2.com!OpenGLDrawingContext::Clear(DrawPixelInfo & dpi, unsigned char paletteIndex) Line 516
	at D:\a\OpenRCT2\OpenRCT2\src\openrct2-ui\drawing\engines\opengl\OpenGLDrawingEngine.cpp(516)
openrct2.com!OpenRCT2::PreloaderScene::Load() Line 51
	at D:\a\OpenRCT2\OpenRCT2\src\openrct2\scenes\preloader\PreloaderScene.cpp(51)

@AaronVanGeffen
Copy link
Member

@Basssiiie Pushed a fix to take cases into account where SDL_GetRendererOutputSize returned an error code. Could you re-test?

@Basssiiie
Copy link
Member

@AaronVanGeffen Tested the newest build and no crashes anymore. The game looks the same as develop now. I wonder what the returned error code was. 😅

@AaronVanGeffen
Copy link
Member

I wonder what the returned error code was. 😅

In the current code, I can't be certain, but here's the enum from SDL_error.h:

typedef enum
{
    SDL_ENOMEM,
    SDL_EFREAD,
    SDL_EFWRITE,
    SDL_EFSEEK,
    SDL_UNSUPPORTED,
    SDL_LASTERROR
} SDL_errorcode;

I'm guessing it was returning SDL_UNSUPPORTED.

@AaronVanGeffen AaronVanGeffen merged commit 0c318a4 into OpenRCT2:develop Jul 21, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog This issue/PR deserves a changelog entry. squash merge A PR that should be squashed on merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants