Using shared context on Windows fails in PlatformWGL::makeCurrent() and question about createSwapChain()

[roxlu]

As I'm trying to get shared contexts to work on Windows, especially with GLFW and Filament (but any other windowing lib will probably have similar issues) I've read up on OpenGL context creation on Windows to figure out what is causing the issue described in below. As I've mostly used GLFW I'm probably still lacking some know-how about this. Please bare with me when I'm talking sh**t here :-)

The problem I have, is that PlatformWGL::makeCurrent() triggers the error reason: wglMakeCurrent() failed. hdc = 0000000014010D59 Windows error code: 2000. (null). In this code:

void PlatformWGL::makeCurrent(Platform::SwapChain* drawSwapChain,
                              Platform::SwapChain* readSwapChain) noexcept {
    ASSERT_PRECONDITION_NON_FATAL(drawSwapChain == readSwapChain,
                                  "PlatformWGL does not support distinct draw/read swap chains.");
    HDC hdc = (HDC)(drawSwapChain);
    if (hdc != NULL) {
        BOOL success = wglMakeCurrent(hdc, mContext);
        if (!ASSERT_POSTCONDITION_NON_FATAL(success, "wglMakeCurrent() failed. hdc = %p", hdc)) {
            reportLastWindowsError();
            wglMakeCurrent(0, NULL);
        }
    }
}

It's unclear what is causing this error, but while looking into this I ran into a couple of questions which someone can hopefully clarify. The call to PlatformWGL::createDriver() is executed form a different thread then the main thread which is important to understand the code there. Because when the code was executed on the same thread as where the sharedGLContext was created (when given), one could have created a new context by making that context current, retrieving wglCreateContextAttribsARB() and create a new one (we would have to find a way to get a HDC for the curent window because we need this to pass this into the wglCreateContextAttribsARB() function, but Windows provides several solutions for this).

Because we are creating a context in a different thread, we have to create a dummy window, use that to create a context and retrieve a pointer to wglCreateContextAttribsARB(). Once we have access to wglCreateContextAttribsARB() we can create the GL 4.1 context. This is basically my understanding of what's going on in PlatformWGL::createDriver().

But when it comes to the pixel format it's a bit unclear to me. From my understanding, the pixel format is used when creating the OpenGL context and might have influence on the compatibility between different contexts. Someone told me this is not the case and as long as the resulting GL_VENDOR and GL_RENDERER are the same as the shared context and the created one, they are compatible and things can be shared.

As a test I tried to create a context that uses the same pixel format as the shared context to see if that solved the issue as described above. Currently I'm querying the current window that was created by GLFW, retrieving the HWND, the HDC and then the pixel format index using GetPixelFormat(). Then I fill a pixel format descriptor using the found pixel format index and set this found pixel format on the dummy context. This seems to solve the error that is triggered in PlatformWGL::makeCurrent(). Maybe the pixel formats do have an effect on the compatibility between contexts (?).

Another question I have while looking into PlatformWGL is what the purpose of createSwapChain() is? Why are we setting the pixel format on the given nativeWindow ? Is this function supposed to be used as a helper function for the users of Filament, to make sure that the nativeWindow shares the same pixel format? and if so why?

In any case, my understanding is that you are only allowed to call SetPixelFormat() just once for a hdc as described in the documentation: Setting the pixel format of a window more than once can lead to significant complications for the Window Manager and for multithread applications, so it is not allowed.

[romainguy]

The format of both contexts have to be compatible. The GL spec is very light on what this means but in practice your best bet is to match bit depth for color and depth/stencil. We recently added a depth buffer to our main buffer.

[roxlu]

Is it allowed to create a context with a shared context as argument to wglCreateContextAttribsARB() where the shared context has been created in another thread? I've done a couple of experiments which makes me wonder if that might be the cause of the failure where wglCreateContextAttribsARB() returns NULL.

I wrote an experiment that creates another context that is shared with my GLFW based one; both created on the main thread. Then I pass over this context to PlatformWGL::createDriver() and use that as mContext instead. This seems to be working well. I do need to test this more thoroughly but if it's not allowed to call wglCreateContextAttribsARB() with a shared context that was created in another thread that might clear up a lot.

[romainguy]

The point of shared contexts is to be able to have multiple threads so I hope both contexts don't have to be created on the same thread. It's probably a compatibility issue with the attributs/pixel format.

[roxlu]

@romainguy yeah I have the same thoughts but if it's an issue with the attributes/pixelformat then I would expect that the code that I wrote that creates a GL context would either both fail or succeeed independently from where it was executed (main thread or partly main / partly filaments thread). The code works fine when executed in the main thread but fails when it's executed in the thead where PlatformWGL::createDriver() is executed.

I've used GetPixelFormat() in PlatformWGL::createDriver() to retrieve the identical pixelformat that is used by the HDC that GLFW creates. Then I used that pixel format with ChoosePixelFormat() to fill a PIXELFORMATDESCRIPTOR struct and use SetPixelFormat() with the same pixel format to make sure they were identical. I'll revisit that code tomorrow, but if I recall correctly this also resulted in the same error.

[roxlu]

@romainguy, @bejado I created this repository with some tests that create GL contexts using the (what I've read) the preferred steps. These tests confirm what I found earlier and it seems like it's not possible to create a context in a thread that shares with one that was created in another thread. As long as the contexts are created on the same thread things are fine (and you can use them in any thread). I know this sounds wierd and counter-intuitive and I would be happy if I made a mistake somewhere 😊.

But I might be wrong as I could have overlooked something as there are quite some things that can screw up the GL context creation process. But if you can't find anything wrong with the tests then we can maybe have a chat about how to properly fix this in PlatformWGL.cpp

[roxlu]

Any thoughts on this @bejado or @romainguy? I've been in touch with several people who told me that you should indeed create the contexts that you want to share on the same thread. After creating them you can share them freely.

I've created a feature branch where I added a BackendConfig struct which gets passed as void* instead of the sharedGLContext. In general it would be a better approach to pass a struct that gives us the opportunity to make changes/updates without the need to alter the API. But more importantly, this approach works.

[romainguy]

How does the new struct address the threading problem?

[roxlu]

What I'm proposing, is that instead of letting Filament create the context we allow the user to create the context him/herself. This is only necessary when you want to use a shared context. To allow the driver to use bluegl::bind() it has to make the context current (as is the case now). Maybe we can skip this call but I don't think so. If we don't have to make the given context current we don't need a new struct (although I think having an struct prepares us for the future and therefore would be better).

To make the context current, we need a HDC handle. Because atm, we can only pass the sharedGLContext we don't have a HDC. This is fixed by this new struct.

A bit related: why do we need bluegl? Is this to make the GL symbols available when we build a static lib?

[bejado]

bluegl is our OpenGL loader library. I took a look at your repo, and seems like there is indeed an issue with sharing contexts created on separate threads. Thanks for the thorough research efforts. I haven't been able to find any Microsoft documentation on this, which is unfortunate. I'm not opposed to your workaround of requiring the user to create the context if they want to share it, though this would be an API-breaking change and I need to think through the implications.

[roxlu]

@bejado thanks for looking into this! Yep, it's an API-breaking change, but a good one and I think it nicely fits into the current code where we use Builders to construct instances. As a fix I created this BackendConfig, but we could also create a Builder{SomeName} that sets up the driver/backend.

I asked around about the GL issue which seems to be Windows specific and might be related to Windows thread affinity. I haven't researched this in detail.

I would recommend removing the bluegl loader. Because of the proxies/stubs that are created it results in some linker issues (replaced symbols; I can dive into this more but I would need to understand more about bluegl first). In my opinion it's a better approach to allow the user to load the GL functions. See this article from the Handmade Network, especially the paragraph Don’t load OpenGL in the library if you depend on it.

[roxlu]

https://github.com/baldurk pointed me to what seems to be the causse of the issue. See https://github.com/roxlu/windows-opengl-context#solution- for a possible solution regarding the error I got when creating the shared context. I still have to test this with Filament. Even though this might solve the issue, I still think that the suggestions regarding bluegl and BackendConfig above need to be considered.

[roxlu]

I just realised what is causing the issue although I haven't found any specs/documentation that describes why this fails (although it kinda makes sense now). The issue here is with the way you create swapchain and use a shared GL context.

There are two approaches to create a swapchain. One where we pass a native window handle and another one where we pass the width, height and some flags. The latter is called a headless swapchain in Filament.

When you want to create your own GL context and share resources with the GL context that Filament creates, you have to make sure to create a headless swapchain. When you don''t create a headless swapchain, Filament will use the Device Context (on Windows) when it makes its own GL context current. When you make your GL context current using the same Device Context things go wrong it seems.

I've created a test repository which shows how to use GLFW + Filament using a shared GL context. It also demonstrates the use of my feature-get-texture-id branch to get the native texture ID from a filament::Texture to be able to use the shared resource in with your own GL context; which is kind of the point of creating shared contexts.

You can find the repository I created here: https://github.com/roxlu/filament-with-glfw

[devshgraphicsprogramming]

When you make your GL context current using the same Device Context things go wrong it seems.

I think its stated somewhere that there can be only one current HGLRC to one HDC.