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

[patch] macOS fix SDL_GetCurrentDisplayMode returning points in a scaled retina resolution #2500

Closed
SDLBugzilla opened this issue Feb 11, 2021 · 0 comments
Labels
wontfix This will not be worked on

Comments

@SDLBugzilla
Copy link
Collaborator

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

Reported in version: HG 2.1
Reported for operating system, platform: Other, x86

Comments on the original bug report:

On 2017-07-12 19:05:30 +0000, Eric Wasylishen wrote:

Created attachment 2796
patch for SDL_GetCurrentDisplayMode

from the discussion here: https://discourse.libsdl.org/t/scaled-resolutions-on-retina-screens-on-macos/22781

When the system has a scaled retina mode active, (e.g. system preferences -> display -> resolution (scaled) -> Larger Text), SDL_GetCurrentDisplayMode was returning width/height in points, not pixels.
Afaik, we want it to always return pixels (I also updated the comments in the SDL_DisplayMode struct to clarify that the units are pixels).

This patch is just using the CGDisplayModeGetPixelWidth/Height introduced in 10.8, instead of CGDisplayModeGetWidth/Height as before.
There is a runtime check as well as a preprocessor check so we can still compile on 10.7 (although, it would be safer to drop support for compiling on 10.7, since a build made on 10.7 would lack the bug fix if it were run on 10.8+.)

To reproduce the issue, set the "Larger Text" scaling mode in system preferences, on a retina display. Run testdraw2 with "--info all".
Without this patch:

INFO: Bounds: 1024x640 at 0,0
INFO: Usable bounds: 1003x618 at 0,0
INFO: DPI: 78.500000x78.500000
INFO: Current mode: 1024x640@60Hz, 32 bits-per-pixel (SDL_PIXELFORMAT_ARGB8888)

With this patch:

INFO: Bounds: 1024x640 at 0,0
INFO: Usable bounds: 1003x618 at 0,0
INFO: DPI: 78.500000x78.500000
INFO: Current mode: 2048x1280@60Hz, 32 bits-per-pixel (SDL_PIXELFORMAT_ARGB8888)

On 2017-07-12 20:24:30 +0000, Alex Szpakowski wrote:

(In reply to Eric Wasylishen from comment # 0)

When the system has a scaled retina mode active, (e.g. system preferences ->
display -> resolution (scaled) -> Larger Text), SDL_GetCurrentDisplayMode
was returning width/height in points, not pixels.
Afaik, we want it to always return pixels (I also updated the comments in
the SDL_DisplayMode struct to clarify that the units are pixels).

SDL display modes are intentionally in points rather than pixels. It would be handy to have additional fields in the struct for pixel dimensions, though

SDL window positions and dimensions are also in points rather than pixels (SDL_GL_GetDrawableSize and SDL_GetRendererOutputSize are the only two functions that explicitly return pixel dimensions, in SDL). So the coordinate space of display modes should match that of window sizes, right now.

On 2017-07-12 23:20:46 +0000, Eric Wasylishen wrote:

Ah, I see. The reason I was thinking the width/height of the SDL_DisplayMode struct had to be pixels, is the fact that the window is stretched to fill the screen means the points part of the high-DPI equation cancels out.

i.e. normally you have:

window_pixels = window_points * (screen_pixels / screen_points)

but if "window_points" is defined to be equal to "screen_points", as is the case in fullscreen, then you just have "window_pixels = screen_pixels".

The only part where this argument breaks is mouse cursor size (or if somehow other windows were dragged on top of a SDL fullscreen window).

On 2017-07-13 00:31:09 +0000, Alex Szpakowski wrote:

I committed a different change to fix the BZFlag issue: https://hg.libsdl.org/SDL/rev/1e26564c7288

@SDLBugzilla SDLBugzilla added bug wontfix This will not be worked on labels Feb 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

1 participant