We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 4714 - Mac: Calling SDL_GL_CreateContext on non-main thread produces Main Thread Checker warning.
Summary: Mac: Calling SDL_GL_CreateContext on non-main thread produces Main Thread Che...
Status: RESOLVED FIXED
Alias: None
Product: SDL
Classification: Unclassified
Component: video (show other bugs)
Version: 2.0.10
Hardware: All Mac OS X (All)
: P2 major
Assignee: Alex Szpakowski
QA Contact: Sam Lantinga
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-07-09 20:55 UTC by Tim McDaniel
Modified: 2019-07-13 21:36 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tim McDaniel 2019-07-09 20:55:03 UTC
Running with Xcode 11's Main Thread Checker, and calling SDL_GL_CreateContext on a non-main thread, results in the UI method [NSWindow contentView] being called and Main Thread Checker issues a warning.  This behavior has never triggered Main Thread Checker prior to Xcode 11, and has never seemed to cause any issues.  However, the warning can be eliminated with the following patch, which dispatches the contentView call to the main thread/queue.

--- src/video/cocoa/SDL_cocoaopengl#2.m	2019-07-09 15:27:59.000000000 
+++ src/video/cocoa/SDL_cocoaopengl.m	2019-07-09 15:39:18.000000000 
@@ -112,14 +112,20 @@
         /* Now sign up for scheduled updates for the new window. */
         NSMutableArray *contexts = windowdata->nscontexts;
         @synchronized (contexts) {
             [contexts addObject:self];
         }
 
-        if ([self view] != [windowdata->nswindow contentView]) {
-            [self setView:[windowdata->nswindow contentView]];
+        __block NSView* view;
+        if (NSThread.isMainThread) {
+            view = windowdata->nswindow.contentView;
+        } else {
+            dispatch_sync(dispatch_get_main_queue(), ^{ view = windowdata->nswindow.contentView; });
+        }
+        if (self.view != view) {
+            self.view = view;
             if (self == [NSOpenGLContext currentContext]) {
                 [self update];
             } else {
                 [self scheduleUpdate];
             }
         }
Comment 1 Sam Lantinga 2019-07-10 02:26:42 UTC
Alex, can you verify this?
Comment 2 Alex Szpakowski 2019-07-13 18:59:55 UTC
I can confirm the new warning message.

The code in question is also called by SDL_GL_MakeCurrent, which might be called every frame (or even multiple times per frame) in some games. So I think the dispatch_sync call would have an extremely significant performance impact in those cases (and might even cause deadlocks if the main thread's code waits for the worker thread's MakeCurrent call to complete before calling Poll/PumpEvents).

The other idea I had is to cache the content view's pointer in the SDL_WindowData structure during window creation. I think that's safe in all cases a window created from SDL_CreateWindowFrom is the situation I can think of where the cached pointer might get out of sync with the actual content view, but windows created via CreateWindowFrom don't have the SDL_WINDOW_OPENGL flag set so AFAIK they can't have GL contexts created using them in the first place.
Comment 3 Alex Szpakowski 2019-07-13 20:05:27 UTC
I've applied a fix here: https://hg.libsdl.org/SDL/rev/1e6980ce45c0
It works as expected in the small test I made.
Comment 4 Tim McDaniel 2019-07-13 21:36:07 UTC
Thanks Alex, I do agree, your solution is preferable to my proposed patch.