1421129 - Unable to capture GL on Windows with RenderDoc

Status: NEW

(Core :: Graphics, enhancement, P3)

Description

[Dzmitry Malyshau [:kvark]]

Attachment: Patch to use ARB_create-context aggressively on WGL and avoid extra context being created

The WGL logic is checking for the robustness extension, and only when it's present the new `wglCreateContextAttribs` method of used. Otherwise, the old `wglCreateContext` is used.

RenderDoc can only work with `wglCreateContextAttribs`, so it fails to attach to the process on Windows systems not supporting ARB_robustness. AMD RX 480 falls into this bin.

Interestingly, the WGL path also happens to create the context again inside `CreateForWidget`. The attached patch removes this piece, and I'd be interested to know if this is acceptable.

Comment 1

[Dzmitry Malyshau [:kvark]]

Attachment: Use ARB_create_context and share the attributes with the widget

Ok, I don't want to remove that second context creation. We may need more than one widget, and it shouldn't be a big deal to have an extra context created initially.
A new patch is attached that is cleaner.

Comment 2

[Kelsey Gilbert [:jgilbert]]

To clarify, at the WGL level, AMD doesn't seem to be claiming support for WGL_ARB_create_context_robustness, though the contexts it's giving us back are all robust.

Comment 3

[Dzmitry Malyshau [:kvark]]

Filed an issue to RenderDoc: https://github.com/baldurk/renderdoc/issues/809

Comment 4

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8932307 [details] [diff] [review]
Use ARB_create_context and share the attributes with the widget

Review of attachment 8932307 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/gl/GLContextProviderWGL.cpp
@@ +244,4 @@
>      // reset back to the previous context, just in case
>      mSymbols.fMakeCurrent(curDC, curCtx);
>  
> +    if (mHasRobustness || mHasCreateContext) {

With a context creation helper, this would be:
mWindowGLContext = CreateContext()

@@ +262,4 @@
>      return true;
>  }
>  
> +const int* WGLLibrary::GetGLContextAttribs() const {

const int*
WGLLibrary::GetGLContextAttribs() const
{

@@ +270,5 @@
> +        LOCAL_WGL_CONTEXT_FLAGS_ARB, LOCAL_WGL_CONTEXT_ROBUST_ACCESS_BIT_ARB,
> +        LOCAL_WGL_CONTEXT_RESET_NOTIFICATION_STRATEGY_ARB, LOCAL_WGL_LOSE_CONTEXT_ON_RESET_ARB,
> +        0
> +    };
> +    return mHasRobustness ? attribs_robustness : mHasCreateContext ? attribs_basic : NULL;

Don't nest ternaries.
s/NULL/nullptr/
No snake_case in c++.

I think it'd be better to have the helper function do the whole context creation chain:
if (mHasRobustness)
  context = fCreateContextAttribs(kRobustnessAttribs)
if (!context && fCreateContextAttribs)
  context = fCreateContextAttribs(nullptr)
if (!context)
  context = fCreateContext()

We can skip the if-create-robust-fails-then-set-has-robustness-false part.

::: gfx/gl/WGLLibrary.h
@@ +83,5 @@
>  private:
>      bool mInitialized;
>      PRLibrary* mOGLLibrary;
>      bool mHasRobustness;
> +    bool mHasCreateContext;

Just test the pfn.