[GTK] GLXBadFBConfig error when creating an OpenGL context

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Miguel Gomez]

When the OpenGL extension GLX_ARB_create_context is available, we use it to request a gl context with version 3.2. If the returned context is null, we fall back to use glXCreateContext and get whatever version is available.

But when using the extension, if the OpenGL version is lower than 3.2, not only glXCreateContextAttribsARB returns null, but it also throws a GLXBadFBConfig X error that causes the app to crash instead falling back to the legacy code. We need to handle this GLXBadFBConfig error.

[Miguel Gomez]

Created attachment 295847 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 295847 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=295847&action=review

> Source/WebCore/platform/graphics/glx/GLContextGLX.cpp:81
> +    // There's no way to know whether glXCreateContextAttribsARB can provide an OpenGL version >= 3.2 until
> +    // we actually call it and check the return value.
> +    // To make things more fun, if a version >= 3.2 cannot be provided, glXCreateContextAttribsARB will throw
> +    // a GLXBadFBConfig X error, causing the app to crash.
> +    // So, the first time a context is requested, we set a X error trap to disable crashes with GLXBadFBConfig
> +    // and then check whether the return value is a context or not.

I think it's weird that even when we can use glXCreateContextAttribsARB we still fallback to legacy because the requested version is not supported. I think we should still use glXCreateContextAttribsARB but requesting a lower OpenGL version. Here we have two possibilities, always request 3.2 and fallback to not passing the version to the attribs, which means the driver will use the default (this should always work, but it seems Mesa doesn't do this). Another possibility is to try to create the highest version context like chromium does. It's only done for Mesa because context creation is is fast when failing and because it requires a version, for other drivers they only try to create a compatible context (passing no openg version). For mesa they try from 4.5 to 1.0.

> Source/WebCore/platform/graphics/glx/GLContextGLX.cpp:100
> +        // Set an X error trapper that ignores GLXBadFBConfig.
> +        // GLXBadFBConfig doesn't seem to be defined anywhere. From the source code it's defined as errorBase + 9.
> +        int GLXErrorBase = 0;
> +        glXQueryExtension(display, &GLXErrorBase, nullptr);
> +        XErrorTrapper trapper(display, XErrorTrapper::Policy::Crash, { (unsigned char)(GLXErrorBase + 9) });

I think it's safe to ignore all Xerrors here, it makes everything simpler.

> Source/WebCore/platform/graphics/glx/GLContextGLX.cpp:141
> +    if (hasGLXARBCreateContextExtension(display))
> +        context.reset(tryCreateGLXARBContext(display, config, sharingContext));

hasGLXARBCreateContextExtension could be checked by tryCreateGLXARBContext that could return a XUniqueGLXContext.

[Miguel Gomez]

Created attachment 296917 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 296917 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=296917&action=review

> Source/WebCore/platform/graphics/glx/GLContextGLX.cpp:144
> +        context.reset((createGLXARBContext(display, config, sharingContext)));

I think there are unneeded extra () there.

> Source/WebCore/platform/graphics/glx/GLContextGLX.cpp:184
> +        context.reset((createGLXARBContext(display, configs.get()[0], sharingContext)));

Ditto.

[Miguel Gomez]

Created attachment 297456 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 297456 [details]
Patch

Clearing flags on attachment: 297456

Committed r209982: <http://trac.webkit.org/changeset/209982>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.