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

UIKit SDL_GetDisplayMode implementation does not match macOS / Wayland high-dpi behavior #5290

Closed
TheSpydog opened this issue Feb 1, 2022 · 10 comments
Milestone

Comments

@TheSpydog
Copy link
Contributor

For both the macOS and Wayland video subsystems, the display modes returned by SDL_GetDisplayMode return the full pixel resolution of the display. On iOS, however, the display modes are in unscaled "points".

Looking at the implementation it seems this was a deliberate choice. However, it's not clear why points are used instead of pixels, since this makes life harder for applications that want to target a high-dpi fullscreen display mode.

This inconsistency requires special handling for the iOS platform on the application side, since the "resolution" returned for the display mode needs to be scaled by UIScreen.nativeScale, which is not trivial to access, especially when using a language binding for SDL.

Should this be changed to match Mac and Wayland behavior? If that would break applications, could we at least add an SDL_Hint to request the full retina resolution when getting display modes?

@slime73
Copy link
Contributor

slime73 commented Feb 1, 2022

macOS returns 'points' as well. There are just some display modes in macOS that have the same resolution in points and pixels, which is probably what you're noticing. Those resolutions typically aren't meant for normal use, on high-dpi displays.

I believe the APIs are meant to use the same coordinate system as window positions and sizes - which are in points. It would be nice to have an extra API to query the DPI scale of a display mode, but in my experience it hasn't been strictly needed to make high DPI work well (the DPI scale comes from the window's surface, plus in a typical fullscreen-desktop situation the display mode is already high-dpi). What sort of problems are you running into, in particular?

@slime73
Copy link
Contributor

slime73 commented Feb 1, 2022

It would be nice to have an extra API to query the DPI scale of a display mode

Re the above, I think it was mentioned here #2119 (and I brought it up again in #3519 (comment)) – I actually had a patch I made to implement something similar, but it was on Bitbucket so it's gone now... maybe I still have it on my computer somewhere.

Here's what I wrote about that old patch:

Generally I think there should only be a couple specific cases where APIs use pixels instead of points (and the OS APIs agree for the most part here).

Creating render targets and loading content suitable for the actual pixel resolution of the window is one case, and SDL_GL_GetDrawableSize (or SDL_GetRendererOutputSize) exposes that to developers.

Determining the pixel density scale factor of the current display mode and the system’s other display mode is the other case (e.g. if I have a game with a list of display modes and the user wants to pick an appropriate one, they’d want to know the size in points for content sizing purposes, and the size in pixels for performance purposes) and that’s what the new functionality in my patch is for.

These are some of the resolutions my MBP exposes to SDL in macOS:

3360x2100 @1x (pixel size 3360x2100) -> not my desktop resolution
2880x1800 @1x (pixel size 2880x1800) -> not my desktop resolution
2560x1600 @1x (pixel size 2560x1600)
2048x1280 @1x (pixel size 2048x1280)
1920x1200 @2x (pixel size 3840x2400)
1680x1050 @2x (pixel size 3360x2100)
1650x1050 @1x (pixel size 1650x1050)
1440x900 @2x (pixel size 2880x1800) -> my desktop resolution, which SDL_WINDOW_FULLSCREEN_DESKTOP uses as well
1280x800 @2x (pixel size 2560x1600)
1152x720 @1x (pixel size 1152x720)

Without the new functionality, users and developers would have no way of knowing that the 1440x900 mode is high dpi-capable without the app switching to it and creating a window. Similarly, if display modes were always in pixels, users who want content sized according to a 1440x900 screen but using a high pixel density (which is the default for my laptop) would have no good way to choose that in a resolution picker.

@flibitijibibo
Copy link
Collaborator

To elaborate: The way we do high-DPI in FNA is by exploiting the API's resize function, which defines the width and height as "BackBufferWidth" and "BackBufferHeight". There isn't a well-defined window size query, so this actually lets us support high-DPI by treating size changes as drawable size changes, and then we scale the window size to match. As an example, for a 1920x1080 drawable the window size will be 960x540.

The trouble starts with fullscreen windows. For Cocoa/Wayland this system works out perfectly, because we can simply ask for the max display resolution, then make the size change with those values - the window size will just happen to be the desktop res, and so the math conveniently fits.

UIKit complicates this a bit by only returning the simulated resolution. Because of this we can't get a good guess as to what the drawable size is, because it could actually be a scale we don't expect (i.e. how do we know it's not 300% rather than 200?).

@slime73
Copy link
Contributor

slime73 commented Feb 1, 2022

The trouble starts with fullscreen windows. For Cocoa/Wayland this system works out perfectly, because we can simply ask for the max display resolution, then make the size change with those values - the window size will just happen to be the desktop res, and so the math conveniently fits.

Based on my above list I don't think that'll happen on my computer, unless I'm misunderstanding.

@flibitijibibo
Copy link
Collaborator

I think it comes down to how precise the OS scaling value is. If it's integer-only and the desktop somehow has fractional scaling it will start to get weird, but if you can get the correct multiplier out of GetDrawableSize / GetWindowSize then it should always end up working out to be the correct scaling ratio.

@slime73
Copy link
Contributor

slime73 commented Feb 1, 2022

Yeah, Apple likes to expose effectively supersampled "display mode" resolutions sometimes. My screen's native resolution in pixels is 2880x1800, and the typical DPI scale from that resolution is 2x (which I chose for my desktop resolution) - making desktop sizing appear at 1440x900. But 3360x2100 @1x is also exposed on my system, which doesn't correlate to 1440x900@2x at all and isn't pixel-perfect.

In any case, I think the core issue here is the SDL display mode API is missing information that'd be useful (i.e. a DPI scale or resolution in pixels, for the display mode). Then you'd just be able to query the values from the actual desktop display mode.

@slime73
Copy link
Contributor

slime73 commented Feb 2, 2022

I actually had a patch I made to implement something similar, but it was on Bitbucket so it's gone now... maybe I still have it on my computer somewhere.

Here it is (from 2017). However I'm not positive if it's the best approach (would a DPI scale query be better? And does every major operating system that uses DPI scaling have a concept of per-display-mode DPI scales?)

# HG changeset patch
# User slime73 <slime73@gmail.com>
# Date 1500173132 10800
#      Sat Jul 15 23:45:32 2017 -0300
# Branch DisplayMode-Pixels
# Node ID 30dbc2f82680a47de7a17e00c8458e4488631d68
# Parent  5b2841f73a3363376de36493c24ae146f84fd8f3
Add SDL_GetDisplayModePixelSize, with backend implementations on macOS and iOS.

diff --git a/include/SDL_video.h b/include/SDL_video.h
--- a/include/SDL_video.h
+++ b/include/SDL_video.h
@@ -394,6 +394,27 @@
 extern DECLSPEC SDL_DisplayMode * SDLCALL SDL_GetClosestDisplayMode(int displayIndex, const SDL_DisplayMode * mode, SDL_DisplayMode * closest);
 
 /**
+ *  \brief Get the size in pixels of a display mode.
+ *
+ *  The 'w' and 'h' fields of the SDL_DisplayMode struct are in DPI-scaled
+ *  coordinates (screen coordinates / points), which are different from raw
+ *  pixels in high-dpi or dpi-scaled situations, when the app is dpi-aware.
+ *
+ *  \param mode The display mode to query. This *must* be obtained from another
+ *              SDL function such as SDL_GetDisplayMode().
+ *  \param w    Pointer to variable for storing the pixel width of the mode.
+ *  \param h    Pointer to variable for storing the pixel height of the mode.
+ *
+ *  \return 0 on success, or -1 if the pixel dimensions could not be retrieved.
+ *
+ *  \sa SDL_GetDisplayMode()
+ *  \sa SDL_GetClosestDisplayMode()
+ *  \sa SDL_GetCurrentDisplayMode()
+ *  \sa SDL_GetDesktopDisplayMode()
+ */
+extern DECLSPEC int SDLCALL SDL_GetDisplayModePixelSize(const SDL_DisplayMode * mode, int *w, int *h);
+
+/**
  *  \brief Get the display index associated with a window.
  *
  *  \return the display index of the display containing the center of the
diff --git a/src/dynapi/SDL_dynapi_overrides.h b/src/dynapi/SDL_dynapi_overrides.h
--- a/src/dynapi/SDL_dynapi_overrides.h
+++ b/src/dynapi/SDL_dynapi_overrides.h
@@ -625,3 +625,4 @@
 #define SDL_MemoryBarrierAcquireFunction SDL_MemoryBarrierAcquireFunction_REAL
 #define SDL_JoystickGetDeviceInstanceID SDL_JoystickGetDeviceInstanceID_REAL
 #define SDL_utf8strlen SDL_utf8strlen_REAL
+#define SDL_GetDisplayModePixelSize SDL_GetDisplayModePixelSize_REAL
diff --git a/src/dynapi/SDL_dynapi_procs.h b/src/dynapi/SDL_dynapi_procs.h
--- a/src/dynapi/SDL_dynapi_procs.h
+++ b/src/dynapi/SDL_dynapi_procs.h
@@ -657,3 +657,4 @@
 SDL_DYNAPI_PROC(void,SDL_MemoryBarrierAcquireFunction,(void),(),)
 SDL_DYNAPI_PROC(SDL_JoystickID,SDL_JoystickGetDeviceInstanceID,(int a),(a),return)
 SDL_DYNAPI_PROC(size_t,SDL_utf8strlen,(const char *a),(a),return)
+SDL_DYNAPI_PROC(int,SDL_GetDisplayModePixelSize,(const SDL_DisplayMode *a, int *b, int *c),(a,b,c),return)
diff --git a/src/test/SDL_test_common.c b/src/test/SDL_test_common.c
--- a/src/test/SDL_test_common.c
+++ b/src/test/SDL_test_common.c
@@ -710,6 +710,8 @@
             float hdpi = 0;
             float vdpi = 0;
             SDL_DisplayMode mode;
+            int modepw = 0;
+            int modeph = 0;
             int bpp;
             Uint32 Rmask, Gmask, Bmask, Amask;
 #if SDL_VIDEO_DRIVER_WINDOWS
@@ -734,10 +736,11 @@
                 SDL_Log("DPI: %fx%f\n", hdpi, vdpi);
 
                 SDL_GetDesktopDisplayMode(i, &mode);
+                SDL_GetDisplayModePixelSize(&mode, &modepw, &modeph);
                 SDL_PixelFormatEnumToMasks(mode.format, &bpp, &Rmask, &Gmask,
                                            &Bmask, &Amask);
-                SDL_Log("  Current mode: %dx%d@%dHz, %d bits-per-pixel (%s)\n",
-                        mode.w, mode.h, mode.refresh_rate, bpp,
+                SDL_Log("  Current mode: %dx%d@%dHz (pixel size %dx%d), %d bits-per-pixel (%s)\n",
+                        mode.w, mode.h, mode.refresh_rate, modepw, modeph, bpp,
                         SDL_GetPixelFormatName(mode.format));
                 if (Rmask || Gmask || Bmask) {
                     SDL_Log("      Red Mask   = 0x%.8x\n", Rmask);
@@ -755,10 +758,11 @@
                     SDL_Log("  Fullscreen video modes:\n");
                     for (j = 0; j < m; ++j) {
                         SDL_GetDisplayMode(i, j, &mode);
+                        SDL_GetDisplayModePixelSize(&mode, &modepw, &modeph);
                         SDL_PixelFormatEnumToMasks(mode.format, &bpp, &Rmask,
                                                    &Gmask, &Bmask, &Amask);
-						SDL_Log("    Mode %d: %dx%d@%dHz, %d bits-per-pixel (%s)\n",
-                                j, mode.w, mode.h, mode.refresh_rate, bpp,
+						SDL_Log("    Mode %d: %dx%d@%dHz (pixel size %dx%d), %d bits-per-pixel (%s)\n",
+                                j, mode.w, mode.h, mode.refresh_rate, modepw, modeph, bpp,
                                 SDL_GetPixelFormatName(mode.format));
                         if (Rmask || Gmask || Bmask) {
                             SDL_Log("        Red Mask   = 0x%.8x\n",
diff --git a/src/video/SDL_sysvideo.h b/src/video/SDL_sysvideo.h
--- a/src/video/SDL_sysvideo.h
+++ b/src/video/SDL_sysvideo.h
@@ -201,6 +201,11 @@
      */
     int (*SetDisplayMode) (_THIS, SDL_VideoDisplay * display, SDL_DisplayMode * mode);
 
+    /*
+     * Get the pixel dimensions of a display mode.
+     */
+    int (*GetDisplayModePixelSize) (_THIS, const SDL_DisplayMode * mode, int *w, int *h);
+
     /* * * */
     /*
      * Window functions
diff --git a/src/video/SDL_video.c b/src/video/SDL_video.c
--- a/src/video/SDL_video.c
+++ b/src/video/SDL_video.c
@@ -960,6 +960,35 @@
     return SDL_GetClosestDisplayModeForDisplay(display, mode, closest);
 }
 
+int
+SDL_GetDisplayModePixelSize(const SDL_DisplayMode * mode, int *w, int *h)
+{
+    int pw = 0;
+    int ph = 0;
+
+    if (!mode) {
+        return SDL_SetError("Missing mode parameter");
+    } else if (!mode->driverdata) {
+        return SDL_SetError("Display mode must be obtained from SDL's APIs");
+    } else if (_this->GetDisplayModePixelSize) {
+        if (_this->GetDisplayModePixelSize(_this, mode, &pw, &ph) < 0) {
+            return -1;
+        }
+    } else {
+        pw = mode->w;
+        ph = mode->h;
+    }
+
+    if (w) {
+        *w = pw;
+    }
+    if (h) {
+        *h = ph;
+    }
+
+    return 0;
+}
+
 static int
 SDL_SetDisplayModeForDisplay(SDL_VideoDisplay * display, const SDL_DisplayMode * mode)
 {
diff --git a/src/video/cocoa/SDL_cocoamodes.h b/src/video/cocoa/SDL_cocoamodes.h
--- a/src/video/cocoa/SDL_cocoamodes.h
+++ b/src/video/cocoa/SDL_cocoamodes.h
@@ -39,6 +39,7 @@
 extern void Cocoa_GetDisplayModes(_THIS, SDL_VideoDisplay * display);
 extern int Cocoa_GetDisplayDPI(_THIS, SDL_VideoDisplay * display, float * ddpi, float * hpdi, float * vdpi);
 extern int Cocoa_SetDisplayMode(_THIS, SDL_VideoDisplay * display, SDL_DisplayMode * mode);
+extern int Cocoa_GetDisplayModePixelSize(_THIS, const SDL_DisplayMode * mode, int *w, int *h);
 extern void Cocoa_QuitModes(_THIS);
 
 #endif /* _SDL_cocoamodes_h */
diff --git a/src/video/cocoa/SDL_cocoamodes.m b/src/video/cocoa/SDL_cocoamodes.m
--- a/src/video/cocoa/SDL_cocoamodes.m
+++ b/src/video/cocoa/SDL_cocoamodes.m
@@ -489,6 +489,25 @@
     return -1;
 }
 
+int
+Cocoa_GetDisplayModePixelSize(_THIS, const SDL_DisplayMode * mode, int *w, int *h)
+{
+    SDL_DisplayModeData *data = (SDL_DisplayModeData *)mode->driverdata;
+
+#ifdef MAC_OS_X_VERSION_10_8
+    if (floor(NSAppKitVersionNumber) > NSAppKitVersionNumber10_7) {
+        *w = (int)CGDisplayModeGetPixelWidth(data->moderef);
+        *h = (int)CGDisplayModeGetPixelHeight(data->moderef);
+    } else
+#endif
+    {
+        *w = (int)CGDisplayModeGetWidth(data->moderef);
+        *h = (int)CGDisplayModeGetHeight(data->moderef);
+    }
+
+    return 0;
+}
+
 void
 Cocoa_QuitModes(_THIS)
 {
diff --git a/src/video/cocoa/SDL_cocoavideo.m b/src/video/cocoa/SDL_cocoavideo.m
--- a/src/video/cocoa/SDL_cocoavideo.m
+++ b/src/video/cocoa/SDL_cocoavideo.m
@@ -77,6 +77,7 @@
     device->GetDisplayDPI = Cocoa_GetDisplayDPI;
     device->GetDisplayModes = Cocoa_GetDisplayModes;
     device->SetDisplayMode = Cocoa_SetDisplayMode;
+    device->GetDisplayModePixelSize = Cocoa_GetDisplayModePixelSize;
     device->PumpEvents = Cocoa_PumpEvents;
     device->SuspendScreenSaver = Cocoa_SuspendScreenSaver;
 
diff --git a/src/video/uikit/SDL_uikitmodes.h b/src/video/uikit/SDL_uikitmodes.h
--- a/src/video/uikit/SDL_uikitmodes.h
+++ b/src/video/uikit/SDL_uikitmodes.h
@@ -42,6 +42,7 @@
 extern int UIKit_InitModes(_THIS);
 extern void UIKit_GetDisplayModes(_THIS, SDL_VideoDisplay * display);
 extern int UIKit_SetDisplayMode(_THIS, SDL_VideoDisplay * display, SDL_DisplayMode * mode);
+extern int UIKit_GetDisplayModePixelSize(_THIS, const SDL_DisplayMode * mode, int *w, int *h);
 extern void UIKit_QuitModes(_THIS);
 extern int UIKit_GetDisplayUsableBounds(_THIS, SDL_VideoDisplay * display, SDL_Rect * rect);
 
diff --git a/src/video/uikit/SDL_uikitmodes.m b/src/video/uikit/SDL_uikitmodes.m
--- a/src/video/uikit/SDL_uikitmodes.m
+++ b/src/video/uikit/SDL_uikitmodes.m
@@ -268,6 +268,18 @@
 }
 
 int
+UIKit_GetDisplayModePixelSize(_THIS, const SDL_DisplayMode * mode, int *w, int *h)
+{
+    @autoreleasepool {
+        SDL_DisplayModeData *modedata = (__bridge SDL_DisplayModeData *)mode->driverdata;
+        *w = (int)modedata.uiscreenmode.size.width;
+        *h = (int)modedata.uiscreenmode.size.height;
+    }
+
+    return 0;
+}
+
+int
 UIKit_GetDisplayUsableBounds(_THIS, SDL_VideoDisplay * display, SDL_Rect * rect)
 {
     @autoreleasepool {
diff --git a/src/video/uikit/SDL_uikitvideo.m b/src/video/uikit/SDL_uikitvideo.m
--- a/src/video/uikit/SDL_uikitvideo.m
+++ b/src/video/uikit/SDL_uikitvideo.m
@@ -88,6 +88,7 @@
         device->VideoQuit = UIKit_VideoQuit;
         device->GetDisplayModes = UIKit_GetDisplayModes;
         device->SetDisplayMode = UIKit_SetDisplayMode;
+        device->GetDisplayModePixelSize = UIKit_GetDisplayModePixelSize;
         device->PumpEvents = UIKit_PumpEvents;
         device->SuspendScreenSaver = UIKit_SuspendScreenSaver;
         device->CreateWindow = UIKit_CreateWindow;

@flibitijibibo
Copy link
Collaborator

I believe Windows 10 and Wayland have per-display DPI, but I think Windows requires the stuff I was thinking of in #4908.

Since we do sizes for windows it makes sense to do it for displays as well - having sizes for one and scales for another might get weird.

@vkedwardli
Copy link
Contributor

But 3360x2100 @1x is also exposed on my system

kDisplayModeNativeFlag is required to fetch the native resolution, or else macOS could return something larger unfortunately
https://github.com/flibitijibibo/SDL/blob/main/src/video/cocoa/SDL_cocoamodes.m#L466-L480

@slime73
Copy link
Contributor

slime73 commented Jan 27, 2023

This is fixed in SDL3's recent commits.

@slouken slouken closed this as completed Jan 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants