WinRT: fixed a crash in SDL_Quit

SDL was expected that each SDL_DisplayMode had a driverdata field that was SDL_malloc'ed, and was calling SDL_free on them. This change moves WinRT's driverdata content into a SDL_malloc'ed field.
libsdl-org · Mar 1, 2014 · f4a5a0f · f4a5a0f
1 parent abfbed9
commit f4a5a0f
Show file tree

Hide file tree

Showing 3 changed files with 88 additions and 26 deletions.
diff --git a/src/core/winrt/SDL_winrtapp_direct3d.cpp b/src/core/winrt/SDL_winrtapp_direct3d.cpp
@@ -157,17 +157,30 @@ WINRT_ProcessWindowSizeChange()
     // window-resize event as it appeared the SDL window didn't change
     // size, and the Direct3D 11.1 renderer wouldn't resize its swap
     // chain.
-    SDL_DisplayMode resizedDisplayMode = WINRT_CalcDisplayModeUsingNativeWindow();
+    SDL_DisplayMode resizedDisplayMode;
+    if (WINRT_CalcDisplayModeUsingNativeWindow(&resizedDisplayMode) != 0) {
+        return;
+    }
+
     if (resizedDisplayMode.w == 0 || resizedDisplayMode.h == 0) {
+        if (resizedDisplayMode.driverdata) {
+            SDL_free(resizedDisplayMode.driverdata);
+        }
         return;
     }
 
     SDL_DisplayMode oldDisplayMode;
     SDL_zero(oldDisplayMode);
     if (WINRT_GlobalSDLVideoDevice) {
         oldDisplayMode = WINRT_GlobalSDLVideoDevice->displays[0].desktop_mode;
+        if (WINRT_DuplicateDisplayMode(&(WINRT_GlobalSDLVideoDevice->displays[0].desktop_mode), &resizedDisplayMode) != 0) {
+            SDL_free(resizedDisplayMode.driverdata);
+            return;
+        }
         WINRT_GlobalSDLVideoDevice->displays[0].current_mode = resizedDisplayMode;
-        WINRT_GlobalSDLVideoDevice->displays[0].desktop_mode = resizedDisplayMode;
+        if (WINRT_GlobalSDLVideoDevice->displays[0].display_modes[0].driverdata) {
+            SDL_free(WINRT_GlobalSDLVideoDevice->displays[0].display_modes[0].driverdata);
+        }
         WINRT_GlobalSDLVideoDevice->displays[0].display_modes[0] = resizedDisplayMode;
     }
 
@@ -184,8 +197,8 @@ WINRT_ProcessWindowSizeChange()
         // Landscape to LandscapeFlipped, Portrait to PortraitFlipped,
         // or vice-versa on either of those two, lead to the Direct3D renderer
         // getting updated.
-        const DisplayOrientations oldOrientation = (DisplayOrientations) (unsigned int) oldDisplayMode.driverdata;
-        const DisplayOrientations newOrientation = (DisplayOrientations) (unsigned int) resizedDisplayMode.driverdata;
+        const DisplayOrientations oldOrientation = ((SDL_DisplayModeData *)oldDisplayMode.driverdata)->currentOrientation;
+        const DisplayOrientations newOrientation = ((SDL_DisplayModeData *)resizedDisplayMode.driverdata)->currentOrientation;
 
         if ((oldOrientation == DisplayOrientations::Landscape && newOrientation == DisplayOrientations::LandscapeFlipped) ||
             (oldOrientation == DisplayOrientations::LandscapeFlipped && newOrientation == DisplayOrientations::Landscape) ||
@@ -212,6 +225,10 @@ WINRT_ProcessWindowSizeChange()
         }
 #endif
     }
+
+    if (oldDisplayMode.driverdata) {
+        SDL_free(oldDisplayMode.driverdata);
+    }
 }
 
 SDL_WinRTApp::SDL_WinRTApp() :

diff --git a/src/video/winrt/SDL_winrtvideo.cpp b/src/video/winrt/SDL_winrtvideo.cpp
@@ -147,33 +147,42 @@ WINRT_VideoInit(_THIS)
     return 0;
 }
 
-SDL_DisplayMode
-WINRT_CalcDisplayModeUsingNativeWindow()
+int
+WINRT_CalcDisplayModeUsingNativeWindow(SDL_DisplayMode * mode)
 {
+    SDL_DisplayModeData * driverdata;
+
     using namespace Windows::Graphics::Display;
 
-    // Create an empty, zeroed-out display mode:
-    SDL_DisplayMode mode;
-    SDL_zero(mode);
+    // Initialize the mode to all zeros:
+    SDL_zerop(mode);
 
     // Go no further if a native window cannot be accessed.  This can happen,
     // for example, if this function is called from certain threads, such as
     // the SDL/XAML thread.
     if (!CoreWindow::GetForCurrentThread()) {
-        return mode;
+        return SDL_SetError("SDL/WinRT display modes cannot be calculated outside of the main thread, such as in SDL's XAML thread");
+    }
+
+    // Create a driverdata field:
+    driverdata = (SDL_DisplayModeData *) SDL_malloc(sizeof(*driverdata));
+    if (!driverdata) {
+        return SDL_OutOfMemory();
     }
+    SDL_zerop(driverdata);
 
     // Fill in most fields:
-    mode.format = SDL_PIXELFORMAT_RGB888;
-    mode.refresh_rate = 0;  // TODO, WinRT: see if refresh rate data is available, or relevant (for WinRT apps)
-    mode.driverdata = (void *) DisplayProperties::CurrentOrientation;
+    mode->format = SDL_PIXELFORMAT_RGB888;
+    mode->refresh_rate = 0;  // TODO, WinRT: see if refresh rate data is available, or relevant (for WinRT apps)
+    mode->driverdata = driverdata;
+    driverdata->currentOrientation = DisplayProperties::CurrentOrientation;
 
     // Calculate the display size given the window size, taking into account
     // the current display's DPI:
     const float currentDPI = Windows::Graphics::Display::DisplayProperties::LogicalDpi; 
     const float dipsPerInch = 96.0f;
-    mode.w = (int) ((CoreWindow::GetForCurrentThread()->Bounds.Width * currentDPI) / dipsPerInch);
-    mode.h = (int) ((CoreWindow::GetForCurrentThread()->Bounds.Height * currentDPI) / dipsPerInch);
+    mode->w = (int) ((CoreWindow::GetForCurrentThread()->Bounds.Width * currentDPI) / dipsPerInch);
+    mode->h = (int) ((CoreWindow::GetForCurrentThread()->Bounds.Height * currentDPI) / dipsPerInch);
 
 #if WINAPI_FAMILY == WINAPI_FAMILY_PHONE_APP
     // On Windows Phone, the native window's size is always in portrait,
@@ -186,32 +195,52 @@ WINRT_CalcDisplayModeUsingNativeWindow()
         case DisplayOrientations::Landscape:
         case DisplayOrientations::LandscapeFlipped:
         {
-            const int tmp = mode.h;
-            mode.h = mode.w;
-            mode.w = tmp;
+            const int tmp = mode->h;
+            mode->h = mode->w;
+            mode->w = tmp;
             break;
         }
 
         default:
             break;
     }
-
-    // Attach the mode to te
 #endif
 
-    return mode;
+    return 0;
+}
+
+int
+WINRT_DuplicateDisplayMode(SDL_DisplayMode * dest, const SDL_DisplayMode * src)
+{
+    SDL_DisplayModeData * driverdata;
+    driverdata = (SDL_DisplayModeData *) SDL_malloc(sizeof(*driverdata));
+    if (!driverdata) {
+        return SDL_OutOfMemory();
+    }
+    SDL_memcpy(driverdata, src->driverdata, sizeof(SDL_DisplayModeData));
+    SDL_memcpy(dest, src, sizeof(SDL_DisplayMode));
+    dest->driverdata = driverdata;
+    return 0;
 }
 
 int
 WINRT_InitModes(_THIS)
 {
     // Retrieve the display mode:
-    SDL_DisplayMode mode = WINRT_CalcDisplayModeUsingNativeWindow();
+    SDL_DisplayMode mode, desktop_mode;
+    if (WINRT_CalcDisplayModeUsingNativeWindow(&mode) != 0) {
+        return -1;	// If WINRT_CalcDisplayModeUsingNativeWindow fails, it'll already have set the SDL error
+    }
+
     if (mode.w == 0 || mode.h == 0) {
+        SDL_free(mode.driverdata);
         return SDL_SetError("Unable to calculate the WinRT window/display's size");
     }
 
-    if (SDL_AddBasicVideoDisplay(&mode) < 0) {
+    if (WINRT_DuplicateDisplayMode(&desktop_mode, &mode) != 0) {
+        return -1;
+    }
+    if (SDL_AddBasicVideoDisplay(&desktop_mode) < 0) {
         return -1;
     }
 

diff --git a/src/video/winrt/SDL_winrtvideo_cpp.h b/src/video/winrt/SDL_winrtvideo_cpp.h
@@ -44,9 +44,25 @@ extern SDL_Window * WINRT_GlobalSDLWindow;
 /* The global, WinRT, video device. */
 extern SDL_VideoDevice * WINRT_GlobalSDLVideoDevice;
 
-/* Computes the current display mode for Plain Direct3D (non-XAML) apps */
-extern SDL_DisplayMode WINRT_CalcDisplayModeUsingNativeWindow();
-
+/* Creates a display mode for Plain Direct3D (non-XAML) apps, using the lone, native window's settings.
+
+   Pass in an allocated SDL_DisplayMode field to store the data in.
+
+   This function will return 0 on success, -1 on failure.
+
+   If this function succeeds, be sure to call SDL_free on the
+   SDL_DisplayMode's driverdata field.
+*/
+extern int WINRT_CalcDisplayModeUsingNativeWindow(SDL_DisplayMode * mode);
+
+/* Duplicates a display mode, copying over driverdata as necessary */
+extern int WINRT_DuplicateDisplayMode(SDL_DisplayMode * dest, const SDL_DisplayMode * src);
+
+/* Display mode internals */
+typedef struct
+{
+    Windows::Graphics::Display::DisplayOrientations currentOrientation;
+} SDL_DisplayModeData;
 
 #ifdef __cplusplus_winrt