Commit a86df3b7cf64daf8a34b1aaa3f8b0427ee63baf1

Alex Szpakowski 2015-06-09T21:08:24

iOS: Fixed some cases where SDL_DestroyWindow or SDL_GL_DeleteContext can cause crashes.

diff --git a/src/video/uikit/SDL_uikitopengles.m b/src/video/uikit/SDL_uikitopengles.m
index 927fa99..b8401f7 100644
--- a/src/video/uikit/SDL_uikitopengles.m
+++ b/src/video/uikit/SDL_uikitopengles.m
@@ -34,6 +34,24 @@
 #include "SDL_loadso.h"
 #include <dlfcn.h>
 
+@interface SDLEAGLContext : EAGLContext
+
+/* The OpenGL ES context owns a view / drawable. */
+@property (nonatomic, strong) SDL_uikitopenglview *sdlView;
+
+@end
+
+@implementation SDLEAGLContext
+
+- (void)dealloc
+{
+    /* When the context is deallocated, its view should be removed from any
+     * SDL window that it's attached to. */
+    [self.sdlView setSDLWindow:NULL];
+}
+
+@end
+
 static int UIKit_GL_Initialize(_THIS);
 
 void *
@@ -117,12 +135,17 @@ SDL_GLContext
 UIKit_GL_CreateContext(_THIS, SDL_Window * window)
 {
     @autoreleasepool {
+        SDLEAGLContext *context = nil;
         SDL_uikitopenglview *view;
         SDL_WindowData *data = (__bridge SDL_WindowData *) window->driverdata;
         CGRect frame = UIKit_ComputeViewFrame(window, data.uiwindow.screen);
         EAGLSharegroup *sharegroup = nil;
         CGFloat scale = 1.0;
 
+        /* The EAGLRenderingAPI enum values currently map 1:1 to major GLES
+         * versions. */
+        EAGLRenderingAPI api = _this->gl_config.major_version;
+
         if (_this->gl_config.share_with_current_context) {
             EAGLContext *context = (__bridge EAGLContext *) SDL_GL_GetCurrentContext();
             sharegroup = context.sharegroup;
@@ -142,6 +165,12 @@ UIKit_GL_CreateContext(_THIS, SDL_Window * window)
             }
         }
 
+        context = [[SDLEAGLContext alloc] initWithAPI:api sharegroup:sharegroup];
+        if (!context) {
+            SDL_SetError("OpenGL ES %d context could not be created", _this->gl_config.major_version);
+            return NULL;
+        }
+
         /* construct our view, passing in SDL's OpenGL configuration data */
         view = [[SDL_uikitopenglview alloc] initWithFrame:frame
                                                     scale:scale
@@ -153,13 +182,15 @@ UIKit_GL_CreateContext(_THIS, SDL_Window * window)
                                                 depthBits:_this->gl_config.depth_size
                                               stencilBits:_this->gl_config.stencil_size
                                                      sRGB:_this->gl_config.framebuffer_srgb_capable
-                                             majorVersion:_this->gl_config.major_version
-                                               shareGroup:sharegroup];
+                                                  context:context];
+
         if (!view) {
             return NULL;
         }
 
-        SDLEAGLContext *context = view.context;
+        /* The context owns the view / drawable. */
+        context.sdlView = view;
+
         if (UIKit_GL_MakeCurrent(_this, window, (__bridge SDL_GLContext) context) < 0) {
             UIKit_GL_DeleteContext(_this, (SDL_GLContext) CFBridgingRetain(context));
             return NULL;
@@ -175,11 +206,10 @@ void
 UIKit_GL_DeleteContext(_THIS, SDL_GLContext context)
 {
     @autoreleasepool {
-        /* Transfer ownership the +1'd context to ARC. */
-        SDLEAGLContext *eaglcontext = (SDLEAGLContext *) CFBridgingRelease(context);
-
-        /* Detach the context's view from its window. */
-        [eaglcontext.sdlView setSDLWindow:NULL];
+        /* The context was retained in SDL_GL_CreateContext, so we release it
+         * here. The context's view will be detached from its window when the
+         * context is deallocated. */
+        CFRelease(context);
     }
 }
 
diff --git a/src/video/uikit/SDL_uikitopenglview.h b/src/video/uikit/SDL_uikitopenglview.h
index 4e3dd6e..a9a0f42 100644
--- a/src/video/uikit/SDL_uikitopenglview.h
+++ b/src/video/uikit/SDL_uikitopenglview.h
@@ -26,14 +26,6 @@
 #import "SDL_uikitview.h"
 #include "SDL_uikitvideo.h"
 
-@class SDL_uikitopenglview;
-
-@interface SDLEAGLContext : EAGLContext
-
-@property (nonatomic, weak) SDL_uikitopenglview *sdlView;
-
-@end
-
 @interface SDL_uikitopenglview : SDL_uikitview
 
 - (instancetype)initWithFrame:(CGRect)frame
@@ -46,10 +38,9 @@
                     depthBits:(int)depthBits
                   stencilBits:(int)stencilBits
                          sRGB:(BOOL)sRGB
-                 majorVersion:(int)majorVersion
-                   shareGroup:(EAGLSharegroup*)shareGroup;
+                      context:(EAGLContext *)glcontext;
 
-@property (nonatomic, readonly, strong) SDLEAGLContext *context;
+@property (nonatomic, readonly, weak) EAGLContext *context;
 
 /* The width and height of the drawable in pixels (as opposed to points.) */
 @property (nonatomic, readonly) int backingWidth;
@@ -59,7 +50,6 @@
 @property (nonatomic, readonly) GLuint drawableFramebuffer;
 
 - (void)swapBuffers;
-- (void)setCurrentContext;
 
 - (void)updateFrame;
 
diff --git a/src/video/uikit/SDL_uikitopenglview.m b/src/video/uikit/SDL_uikitopenglview.m
index abf4ab6..c68f606 100644
--- a/src/video/uikit/SDL_uikitopenglview.m
+++ b/src/video/uikit/SDL_uikitopenglview.m
@@ -27,10 +27,6 @@
 #import "SDL_uikitopenglview.h"
 #include "SDL_uikitwindow.h"
 
-@implementation SDLEAGLContext
-
-@end
-
 @implementation SDL_uikitopenglview {
     /* The renderbuffer and framebuffer used to render to this layer. */
     GLuint viewRenderbuffer, viewFramebuffer;
@@ -61,26 +57,20 @@
                     depthBits:(int)depthBits
                   stencilBits:(int)stencilBits
                          sRGB:(BOOL)sRGB
-                 majorVersion:(int)majorVersion
-                   shareGroup:(EAGLSharegroup*)shareGroup
+                      context:(EAGLContext *)glcontext
 {
     if ((self = [super initWithFrame:frame])) {
         const BOOL useStencilBuffer = (stencilBits != 0);
         const BOOL useDepthBuffer = (depthBits != 0);
         NSString *colorFormat = nil;
 
-        /* The EAGLRenderingAPI enum values currently map 1:1 to major GLES
-         * versions, and this allows us to handle future OpenGL ES versions. */
-        EAGLRenderingAPI api = majorVersion;
+        context = glcontext;
 
-        context = [[SDLEAGLContext alloc] initWithAPI:api sharegroup:shareGroup];
         if (!context || ![EAGLContext setCurrentContext:context]) {
-            SDL_SetError("OpenGL ES %d not supported", majorVersion);
+            SDL_SetError("Could not create OpenGL ES drawable (could not make context current)");
             return nil;
         }
 
-        context.sdlView = self;
-
         if (sRGB) {
             /* sRGB EAGL drawable support was added in iOS 7. */
             if (UIKit_IsSystemVersionAtLeast(7.0)) {
@@ -209,11 +199,6 @@
     }
 }
 
-- (void)setCurrentContext
-{
-    [EAGLContext setCurrentContext:context];
-}
-
 - (void)swapBuffers
 {
     /* viewRenderbuffer should always be bound here. Code that binds something
@@ -264,7 +249,7 @@
 
 - (void)dealloc
 {
-    if ([EAGLContext currentContext] == context) {
+    if (context && context == [EAGLContext currentContext]) {
         [self destroyFramebuffer];
         [EAGLContext setCurrentContext:nil];
     }
diff --git a/src/video/uikit/SDL_uikitview.m b/src/video/uikit/SDL_uikitview.m
index 8979839..60cd170 100644
--- a/src/video/uikit/SDL_uikitview.m
+++ b/src/video/uikit/SDL_uikitview.m
@@ -62,6 +62,7 @@
         return;
     }
 
+    /* Remove ourself from the old window. */
     if (sdlwindow) {
         SDL_uikitview *view = nil;
         data = (__bridge SDL_WindowData *) sdlwindow->driverdata;
@@ -71,9 +72,7 @@
         [self removeFromSuperview];
 
         /* Restore the next-oldest view in the old window. */
-        if (data.views.count > 0) {
-            view = data.views[data.views.count - 1];
-        }
+        view = data.views.lastObject;
 
         data.viewcontroller.view = view;
 
@@ -83,6 +82,7 @@
         [data.uiwindow layoutIfNeeded];
     }
 
+    /* Add ourself to the new window. */
     if (window) {
         data = (__bridge SDL_WindowData *) window->driverdata;
 
diff --git a/src/video/uikit/SDL_uikitwindow.m b/src/video/uikit/SDL_uikitwindow.m
index a3cb70e..782250b 100644
--- a/src/video/uikit/SDL_uikitwindow.m
+++ b/src/video/uikit/SDL_uikitwindow.m
@@ -305,7 +305,17 @@ UIKit_DestroyWindow(_THIS, SDL_Window * window)
     @autoreleasepool {
         if (window->driverdata != NULL) {
             SDL_WindowData *data = (SDL_WindowData *) CFBridgingRelease(window->driverdata);
+            NSArray *views = nil;
+
             [data.viewcontroller stopAnimation];
+
+            /* Detach all views from this window. We use a copy of the array
+             * because setSDLWindow will remove the object from the original
+             * array, which would be undesirable if we were iterating over it. */
+            views = [data.views copy];
+            for (SDL_uikitview *view in views) {
+                [view setSDLWindow:NULL];
+            }
         }
     }
     window->driverdata = NULL;