From a86df3b7cf64daf8a34b1aaa3f8b0427ee63baf1 Mon Sep 17 00:00:00 2001 From: Alex Szpakowski Date: Tue, 9 Jun 2015 21:08:24 -0300 Subject: [PATCH] iOS: Fixed some cases where SDL_DestroyWindow or SDL_GL_DeleteContext can cause crashes. --- src/video/uikit/SDL_uikitopengles.m | 46 ++++++++++++++++++++++----- src/video/uikit/SDL_uikitopenglview.h | 14 ++------ src/video/uikit/SDL_uikitopenglview.m | 23 +++----------- src/video/uikit/SDL_uikitview.m | 6 ++-- src/video/uikit/SDL_uikitwindow.m | 10 ++++++ 5 files changed, 57 insertions(+), 42 deletions(-) diff --git a/src/video/uikit/SDL_uikitopengles.m b/src/video/uikit/SDL_uikitopengles.m index 927fa9921fdc8..b8401f792fe74 100644 --- a/src/video/uikit/SDL_uikitopengles.m +++ b/src/video/uikit/SDL_uikitopengles.m @@ -34,6 +34,24 @@ #include "SDL_loadso.h" #include +@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 @@ void UIKit_GL_SwapWindow(_THIS, SDL_Window * window) 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 @@ void UIKit_GL_SwapWindow(_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 @@ void UIKit_GL_SwapWindow(_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_SwapWindow(_THIS, SDL_Window * window) 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 4e3dd6eaff58f..a9a0f42d175e8 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 abf4ab69facfe..c68f606e37c4e 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 @@ - (instancetype)initWithFrame:(CGRect)frame 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)setDebugLabels } } -- (void)setCurrentContext -{ - [EAGLContext setCurrentContext:context]; -} - - (void)swapBuffers { /* viewRenderbuffer should always be bound here. Code that binds something @@ -264,7 +249,7 @@ - (void)destroyFramebuffer - (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 8979839176fda..60cd1708e4f0a 100644 --- a/src/video/uikit/SDL_uikitview.m +++ b/src/video/uikit/SDL_uikitview.m @@ -62,6 +62,7 @@ - (void)setSDLWindow:(SDL_Window *)window return; } + /* Remove ourself from the old window. */ if (sdlwindow) { SDL_uikitview *view = nil; data = (__bridge SDL_WindowData *) sdlwindow->driverdata; @@ -71,9 +72,7 @@ - (void)setSDLWindow:(SDL_Window *)window [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 @@ - (void)setSDLWindow:(SDL_Window *)window [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 a3cb70e347eaf..782250bc6e47d 100644 --- a/src/video/uikit/SDL_uikitwindow.m +++ b/src/video/uikit/SDL_uikitwindow.m @@ -305,7 +305,17 @@ static int SetupWindowData(_THIS, SDL_Window *window, UIWindow *uiwindow, SDL_bo @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;