Bug 165200 - [GTK] GLXBadFBConfig error when creating an OpenGL context
Summary: [GTK] GLXBadFBConfig error when creating an OpenGL context
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Miguel Gomez
Depends on:
Reported: 2016-11-30 08:44 PST by Miguel Gomez
Modified: 2016-12-19 07:42 PST (History)
3 users (show)

See Also:

Patch (5.84 KB, patch)
2016-12-01 06:51 PST, Miguel Gomez
no flags Details | Formatted Diff | Diff
Patch (6.51 KB, patch)
2016-12-12 08:12 PST, Miguel Gomez
no flags Details | Formatted Diff | Diff
Patch (6.46 KB, patch)
2016-12-19 05:17 PST, Miguel Gomez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Miguel Gomez 2016-11-30 08:44:59 PST
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.
Comment 1 Miguel Gomez 2016-12-01 06:51:13 PST
Created attachment 295847 [details]
Comment 2 Carlos Garcia Campos 2016-12-01 23:21:17 PST
Comment on attachment 295847 [details]

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.
Comment 3 Miguel Gomez 2016-12-12 08:12:17 PST
Created attachment 296917 [details]
Comment 4 Carlos Garcia Campos 2016-12-19 02:05:52 PST
Comment on attachment 296917 [details]

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)));

Comment 5 Miguel Gomez 2016-12-19 05:17:03 PST
Created attachment 297456 [details]
Comment 6 WebKit Commit Bot 2016-12-19 07:42:09 PST
Comment on attachment 297456 [details]

Clearing flags on attachment: 297456

Committed r209982: <http://trac.webkit.org/changeset/209982>
Comment 7 WebKit Commit Bot 2016-12-19 07:42:13 PST
All reviewed patches have been landed.  Closing bug.