From 20c8287efc82413b5d926143c19f564b3bbc6847 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rgen=20P=2E=20Tjern=C3=B8?= Date: Wed, 7 Aug 2013 16:29:21 -0700 Subject: [PATCH] Mac: Don't -[NSOpenGLContext update] on (potentially) the wrong thread. If the user is using their context from a non-main thread, we could be calling -[NSOpenGLContext update] on our thread, while they were accessing it on their thread. With this change, we schedule updates when the event comes in on the main thread, and act on them when the user calls SDL_GL_MakeCurrent or SDL_GL_SwapWindow. --- src/video/cocoa/SDL_cocoaopengl.h | 15 +++++++++ src/video/cocoa/SDL_cocoaopengl.m | 53 ++++++++++++++++++++++++++----- src/video/cocoa/SDL_cocoawindow.h | 4 ++- src/video/cocoa/SDL_cocoawindow.m | 33 +++++++------------ 4 files changed, 74 insertions(+), 31 deletions(-) diff --git a/src/video/cocoa/SDL_cocoaopengl.h b/src/video/cocoa/SDL_cocoaopengl.h index 77bdf4449..9a20b4a6a 100644 --- a/src/video/cocoa/SDL_cocoaopengl.h +++ b/src/video/cocoa/SDL_cocoaopengl.h @@ -25,11 +25,26 @@ #if SDL_VIDEO_OPENGL_CGL +#include "SDL_atomic.h" +#import + struct SDL_GLDriverData { int initialized; }; +@interface SDLOpenGLContext : NSOpenGLContext { + SDL_atomic_t dirty; +} + +- (id)initWithFormat:(NSOpenGLPixelFormat *)format + shareContext:(NSOpenGLContext *)share; +- (void)scheduleUpdate; +- (void)updateIfNeeded; + +@end + + /* OpenGL functions */ extern int Cocoa_GL_LoadLibrary(_THIS, const char *path); extern void *Cocoa_GL_GetProcAddress(_THIS, const char *proc); diff --git a/src/video/cocoa/SDL_cocoaopengl.m b/src/video/cocoa/SDL_cocoaopengl.m index 8289107e3..43af675f4 100644 --- a/src/video/cocoa/SDL_cocoaopengl.m +++ b/src/video/cocoa/SDL_cocoaopengl.m @@ -24,6 +24,7 @@ #if SDL_VIDEO_OPENGL_CGL #include "SDL_cocoavideo.h" +#include "SDL_cocoaopengl.h" #include #include @@ -45,6 +46,40 @@ #define kCGLOGLPVersion_3_2_Core 0x3200 #endif +@implementation SDLOpenGLContext : NSOpenGLContext + +- (id)initWithFormat:(NSOpenGLPixelFormat *)format + shareContext:(NSOpenGLContext *)share +{ + SDL_AtomicSet(&self->dirty, 0); + return [super initWithFormat:format shareContext:share]; +} + +- (void)scheduleUpdate +{ + SDL_AtomicAdd(&self->dirty, 1); +} + +/* This should only be called on the thread on which a user is using the context. */ +- (void)updateIfNeeded +{ + int value = SDL_AtomicSet(&self->dirty, 0); + if (value > 0) { + /* We call the real underlying update here, since -[SDLOpenGLContext update] just calls us. */ + [super update]; + } +} + +/* This should only be called on the thread on which a user is using the context. */ +- (void)update +{ + /* This ensures that regular 'update' calls clear the atomic dirty flag. */ + [self scheduleUpdate]; + [self updateIfNeeded]; +} + +@end + int Cocoa_GL_LoadLibrary(_THIS, const char *path) @@ -89,7 +124,7 @@ Cocoa_GL_CreateContext(_THIS, SDL_Window * window) SDL_DisplayData *displaydata = (SDL_DisplayData *)display->driverdata; NSOpenGLPixelFormatAttribute attr[32]; NSOpenGLPixelFormat *fmt; - NSOpenGLContext *context; + SDLOpenGLContext *context; NSOpenGLContext *share_context = nil; int i = 0; @@ -181,7 +216,7 @@ Cocoa_GL_CreateContext(_THIS, SDL_Window * window) share_context = (NSOpenGLContext*)SDL_GL_GetCurrentContext(); } - context = [[NSOpenGLContext alloc] initWithFormat:fmt shareContext:share_context]; + context = [[SDLOpenGLContext alloc] initWithFormat:fmt shareContext:share_context]; [fmt release]; @@ -210,13 +245,14 @@ Cocoa_GL_MakeCurrent(_THIS, SDL_Window * window, SDL_GLContext context) if (context) { SDL_WindowData *windowdata = (SDL_WindowData *)window->driverdata; - NSOpenGLContext *nscontext = (NSOpenGLContext *)context; - + SDLOpenGLContext *nscontext = (SDLOpenGLContext *)context; windowdata->nscontext = nscontext; if ([nscontext view] != [windowdata->nswindow contentView]) { [nscontext setView:[windowdata->nswindow contentView]]; - [nscontext update]; + [nscontext scheduleUpdate]; } + + [nscontext updateIfNeeded]; [nscontext makeCurrentContext]; } else { [NSOpenGLContext clearCurrentContext]; @@ -236,7 +272,7 @@ Cocoa_GL_SetSwapInterval(_THIS, int interval) pool = [[NSAutoreleasePool alloc] init]; - nscontext = [NSOpenGLContext currentContext]; + nscontext = (NSOpenGLContext*)SDL_GL_GetCurrentContext(); if (nscontext != nil) { value = interval; [nscontext setValues:&value forParameter:NSOpenGLCPSwapInterval]; @@ -259,7 +295,7 @@ Cocoa_GL_GetSwapInterval(_THIS) pool = [[NSAutoreleasePool alloc] init]; - nscontext = [NSOpenGLContext currentContext]; + nscontext = (NSOpenGLContext*)SDL_GL_GetCurrentContext(); if (nscontext != nil) { [nscontext getValues:&value forParameter:NSOpenGLCPSwapInterval]; status = (int)value; @@ -274,11 +310,12 @@ Cocoa_GL_SwapWindow(_THIS, SDL_Window * window) { NSAutoreleasePool *pool; SDL_WindowData *windowdata = (SDL_WindowData *)window->driverdata; - NSOpenGLContext *nscontext = windowdata->nscontext; + SDLOpenGLContext *nscontext = windowdata->nscontext; pool = [[NSAutoreleasePool alloc] init]; [nscontext flushBuffer]; + [nscontext updateIfNeeded]; [pool release]; } diff --git a/src/video/cocoa/SDL_cocoawindow.h b/src/video/cocoa/SDL_cocoawindow.h index 01ff63880..50a741de9 100644 --- a/src/video/cocoa/SDL_cocoawindow.h +++ b/src/video/cocoa/SDL_cocoawindow.h @@ -77,11 +77,13 @@ typedef enum { @end /* *INDENT-ON* */ +@class SDLOpenGLContext; + struct SDL_WindowData { SDL_Window *window; NSWindow *nswindow; - NSOpenGLContext *nscontext; + SDLOpenGLContext *nscontext; SDL_bool created; Cocoa_WindowListener *listener; struct SDL_VideoData *videodata; diff --git a/src/video/cocoa/SDL_cocoawindow.m b/src/video/cocoa/SDL_cocoawindow.m index 0c37f4de0..530d6bb11 100644 --- a/src/video/cocoa/SDL_cocoawindow.m +++ b/src/video/cocoa/SDL_cocoawindow.m @@ -32,6 +32,7 @@ #include "SDL_cocoavideo.h" #include "SDL_cocoashape.h" #include "SDL_cocoamouse.h" +#include "SDL_cocoaopengl.h" static Uint32 s_moveHack; @@ -187,7 +188,6 @@ static __inline__ void ConvertNSRect(NSRect *r) - (void)windowDidMove:(NSNotification *)aNotification { int x, y; - SDL_VideoDevice *device = SDL_GetVideoDevice(); SDL_Window *window = _data->window; NSWindow *nswindow = _data->nswindow; NSRect rect = [nswindow contentRectForFrameRect:[nswindow frame]]; @@ -211,16 +211,13 @@ static __inline__ void ConvertNSRect(NSRect *r) x = (int)rect.origin.x; y = (int)rect.origin.y; - if (window == device->current_glwin) { - [((NSOpenGLContext *) device->current_glctx) update]; - } + [_data->nscontext scheduleUpdate]; SDL_SendWindowEvent(window, SDL_WINDOWEVENT_MOVED, x, y); } - (void)windowDidResize:(NSNotification *)aNotification { - SDL_VideoDevice *device = SDL_GetVideoDevice(); int x, y, w, h; NSRect rect = [_data->nswindow contentRectForFrameRect:[_data->nswindow frame]]; ConvertNSRect(&rect); @@ -231,9 +228,7 @@ static __inline__ void ConvertNSRect(NSRect *r) if (SDL_IsShapedWindow(_data->window)) Cocoa_ResizeWindowShape(_data->window); - if (_data->window == device->current_glwin) { - [((NSOpenGLContext *) device->current_glctx) update]; - } + [_data->nscontext scheduleUpdate]; /* The window can move during a resize event, such as when maximizing or resizing from a corner */ @@ -788,7 +783,8 @@ void Cocoa_SetWindowPosition(_THIS, SDL_Window * window) { NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init]; - NSWindow *nswindow = ((SDL_WindowData *) window->driverdata)->nswindow; + SDL_WindowData *windata = (SDL_WindowData *) window->driverdata; + NSWindow *nswindow = windata->nswindow; NSRect rect; Uint32 moveHack; @@ -803,9 +799,7 @@ Cocoa_SetWindowPosition(_THIS, SDL_Window * window) [nswindow setFrameOrigin:rect.origin]; s_moveHack = moveHack; - if (window == _this->current_glwin) { - [((NSOpenGLContext *) _this->current_glctx) update]; - } + [windata->nscontext scheduleUpdate]; [pool release]; } @@ -822,9 +816,7 @@ Cocoa_SetWindowSize(_THIS, SDL_Window * window) size.height = window->h; [nswindow setContentSize:size]; - if (window == _this->current_glwin) { - [((NSOpenGLContext *) _this->current_glctx) update]; - } + [windata->nscontext scheduleUpdate]; [pool release]; } @@ -906,13 +898,12 @@ void Cocoa_MaximizeWindow(_THIS, SDL_Window * window) { NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init]; - NSWindow *nswindow = ((SDL_WindowData *) window->driverdata)->nswindow; + SDL_WindowData *windata = (SDL_WindowData *) window->driverdata; + NSWindow *nswindow = windata->nswindow; [nswindow zoom:nil]; - if (window == _this->current_glwin) { - [((NSOpenGLContext *) _this->current_glctx) update]; - } + [windata->nscontext scheduleUpdate]; [pool release]; } @@ -1049,9 +1040,7 @@ Cocoa_SetWindowFullscreen(_THIS, SDL_Window * window, SDL_VideoDisplay * display [nswindow makeKeyAndOrderFront:nil]; [data->listener resumeVisibleObservation]; - if (window == _this->current_glwin) { - [((NSOpenGLContext *) _this->current_glctx) update]; - } + [data->nscontext scheduleUpdate]; [pool release]; }