This bug report was migrated from our old Bugzilla tracker.

Reported in version: 2.0.9
Reported for operating system, platform: macOS 10.13, x86

Comments on the original bug report:

On 2018-12-27 21:27:01 +0000, Marcel wrote:

Hi!

I'm facing an issue in SDL2.0.9/Mojave. I'm using a single OpenGL context to draw into two windows. I've appended code below which reproduces the problem. This code used to work perfectly before Mojave w/ XCode 10, and I used a slightly more complicated setup to draw to many windows. One of the apps is sort of like a VST effect chain editor, which is now horribly broken.

Sadly, the issue isn't fixed with the black screen and vsync fixes that went in recently.

Expected behavior: Flickering red in the left window, flickering green in the right window.
Actual behavior: Flickering green in the right window.

Pressing 1 will only draw to window 1, and it gives flickering red in the left window (expected). Pressing 2 will only draw to window 2, and also gives the expected result. Pressing 3 will draw to both windows (the default), and fails with the actual behavior documented above.

I've tested against SDL 2.0.9 and head, and also 2.0.5 just for kicks. I've actually included SDL's source code directly into my project, and tried a lot of 'potential fixes' (hacks) to see what happens. I put a print in updateLayer and see updateLayer gets called for both windows at startup, but only for the 'working' window beyond that, which is suspicious.

If possible I'd like to cooperate with someone who knows more about the OSX opengl code to find a solution.

--

#include <SDL2/SDL.h>
#include <SDL2/SDL_opengl.h>

int main(int argc, char * argv[])
{
SDL_Init(SDL_INIT_VIDEO);

SDL_Window * window1 = SDL_CreateWindow("Hello World 1", SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED, 600, 400, SDL_WINDOW_RESIZABLE | SDL_WINDOW_OPENGL);
SDL_Window * window2 = SDL_CreateWindow("Hello World 2", SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED, 600, 400, SDL_WINDOW_RESIZABLE | SDL_WINDOW_OPENGL);

SDL_GLContext context = SDL_GL_CreateContext(window1);

SDL_SetWindowPosition(window1, 0, 0);
SDL_SetWindowPosition(window2, 600, 0);

bool stop = false;

int mode = 2;

while (stop == false)
{
SDL_Event e;

  while (SDL_PollEvent(&e))
  {
  	if (e.type == SDL_QUIT)
  		stop = true;
  	else if (e.type == SDL_MOUSEBUTTONDOWN)
  		printf("mouse down: %x\n", e.button.windowID);
  	else if (e.type == SDL_MOUSEBUTTONUP)
  		printf("mouse up: %x\n", e.button.windowID);
  	else if (e.type == SDL_MOUSEMOTION)
  		printf("mouse motion: %x: %d, %d\n", e.motion.windowID, e.motion.x, e.motion.y);
  	else if (e.type == SDL_KEYDOWN)
  	{
  		if (e.key.keysym.sym == SDLK_1) // only draw window 1
  			mode = 0;
  		if (e.key.keysym.sym == SDLK_2) // only draw window 2
  			mode = 1;
  		if (e.key.keysym.sym == SDLK_3) // draw both windows
  			mode = 2;
  	}
  }
  
  //
  
  if (mode == 2 || mode == 0)
  {
  	SDL_GL_MakeCurrent(window1, context);
  	
  	glClearColor((rand() % 100) / 99.f, 0.f, 0.f, 0.f);
  	glClear(GL_COLOR_BUFFER_BIT);
  	
  	SDL_GL_SwapWindow(window1);
  }
  
  //
  
  if (mode == 2 || mode == 1)
  {
  	SDL_GL_MakeCurrent(window2, context);
  	
  	glClearColor(0.f, (rand() % 100) / 99.f, 0.f, 0.f);
  	glClear(GL_COLOR_BUFFER_BIT);
  	
  	SDL_GL_SwapWindow(window2);
  }

}

SDL_GL_DeleteContext(context);

SDL_DestroyWindow(window1);
SDL_DestroyWindow(window2);

return 0;
}

On 2018-12-27 22:29:32 +0000, Ryan C. Gordon wrote:

I would say the absolute first thing to do is not share a context between windows. Make two separate contexts, use one for each window and use SDL_GL_MakeCurrent() to say "now gl*() calls apply to this different context" instead of "now this context applies to this different window."

I'm actually surprised that ever worked, Mojave or otherwise.

--ryan.

On 2018-12-27 23:10:09 +0000, Marcel wrote:

(In reply to Ryan C. Gordon from comment # 1)

I would say the absolute first thing to do is not share a context between
windows. Make two separate contexts, use one for each window and use
SDL_GL_MakeCurrent() to say "now gl*() calls apply to this different
context" instead of "now this context applies to this different window."

I'm actually surprised that ever worked, Mojave or otherwise.

--ryan.

Although possible, I would rather avoid using separate contexts, as it complicates sharing resources.

The documentation does not specifically mention the context has to be bound to the same window it was created on, so I assumed the api decoupled the rendering context from the drawing surfaces. Am I wrong? It worked well across Linux, Windows and OSX. Are you sure you are not supposed to do this? Window (wgl) and EGL explicitly decouple the target (the device or window on Windows, and read/draw surfaces for EGL) from the OpenGL context. The usual requirement is that the pixel formats are compatible and rendering occurs on the same device.

On 2018-12-28 18:55:07 +0000, Marcel wrote:

After a lot of digging around, I found a possible 'fix'.

The (undocumented?) behavior of NSOpenGLContext is to assign a layer to the NSView of type 'NSOpenGLViewBackingLayer'. Actually I see it cycle through a bunch of different instances when setting a breakpoint in the setLayer method of SDLView I added below. I think what it does is to allocate fresh backing storage for each NSView it comes across. It's a tricky bit of magical behavior (compared to manually allocating backing storage on the NSView with the help of the context perhaps) which I think broke in Mojave. What I see now is that it assigns a 'nil' layer to the previous view, when switching to a new view using [NSOpenGLContext setView:..] inside [SDLOpenGLContext setWindow:..]. This breaks the old behavior where a single NSOpenGLContext can be used to draw to and present multiple NSViews. I wonder if the change is intentional or some unintended breakage on Apples part. I suspect the latter, but will inform them and ask them what to expect later.

The change below filters out the nil-layer assignments, leaving the old backing layers intact when switching views. This restores the old behavior 100%. Just don't know if it's good practice or not. Could break in a future version of OSX or the SDK. I'd like to make some further inquiries about this at Apple.

B.t.w., Ryan, do you know a good place where to contact Apple? Their official bug reporting system is too impersonal for me. I'd prefer to avoid using it.

-(void)setLayer:(CALayer*)layer
{
if (layer == nil)
return;

[super setLayer:layer];

}

On 2018-12-28 20:38:15 +0000, Ryan C. Gordon wrote:

B.t.w., Ryan, do you know a good place where to contact Apple? Their
official bug reporting system is too impersonal for me. I'd prefer to avoid
using it.

My experience is they are no less impersonal when contacted directly, doubly so when it regards OpenGL in 2018.

This was good research, thank you. Mojave requires a proper layer for GL contexts, which was not true previously, so this fix is likely on the correct path. Ping me if I haven’t applied a fix to revision control in the next few days!

--ryan.

On 2018-12-30 10:31:51 +0000, Marcel wrote:

Thanks! I did some further testing to see if there were any side effects of this change. One possible issue I could imagine was a memory leak due to the retain count for a layer never reaching zero. Since this change effectively removes the possibility of setting the layer back to nil. I was able to confirm there is no memory leak however. The method I used is a stress test which creates and destroys a full screen window 1000x and draws to each window 10x. Memory goes up and down a little, but overall stays the same. I think the NSView cleans up after itself when the window is destroyed.

I will do some further testing still, perhaps I can find out exactly what's happening during the lifetime of a view and context.

On 2019-01-03 13:00:15 +0000, Marcel wrote:

I did some more testing. I learned NSView doesn't use setLayer to set the layer to nil and reduce the retain count. So that's good. It means the fix won't interfere with cleanup.

I still don't fully understand how NSOpenGLContext, NSView and the magically managed NSOpenGLViewBackingLayer all relate to each other. But I guess no one does except for someone at Apple..

Could you please apply the change (if you haven't done so already)?

Regards,
Marcel

On 2019-01-08 17:05:34 +0000, Steffen Hein wrote:

Just a quick note: I ran into the exact same problem and I'm glad that a potential fix exists. Thanks for all the effort you put into this!

(In reply to Ryan C. Gordon from comment # 1)

I'm actually surprised that ever worked, Mojave or otherwise.

I'm using the same technique and it works absolutely fine on Windows, Linux and (until 10.14.1) macOS - tested on nVidia, AMD and Intel GPUs.

In my experience, it works a lot more reliable, or maybe is just less prone to programmer errors, than a multi-context approach. It's been a few years since I tried multiple contexts, but I ran into some machine-/os-dependent problems back then, so I reverted to this method and it's been absolutely fine (until now).

On 2019-04-16 12:49:28 +0000, Marcel wrote:

(In reply to Ryan C. Gordon from comment # 4)

B.t.w., Ryan, do you know a good place where to contact Apple? Their
official bug reporting system is too impersonal for me. I'd prefer to avoid
using it.

My experience is they are no less impersonal when contacted directly, doubly
so when it regards OpenGL in 2018.

This was good research, thank you. Mojave requires a proper layer for GL
contexts, which was not true previously, so this fix is likely on the
correct path. Ping me if I haven’t applied a fix to revision control in the
next few days!

--ryan.

Hi Ryan,

You asked me to ping you when no commit has been made. I just checked the repo and the fix isn't in it yet. If there's something holding you back, please let me know!