[GTK] GLXBadFBConfig error when creating an OpenGL context

https://bugs.webkit.org/show_bug.cgi?id=165200 Reviewed by Carlos Garcia Campos. glXCreateContextAttribsARB causes a GLXBadFBConfig X error when it's not able to provide the OpenGL version >= 3.2 we are requesting. Due to this, the app crashes instead of falling back to the legacy path. The patch modifies GLX context creation using a XErrorTrapper, so the first time a context is created we don't crash if OpenGL >= 3.2 is not available. If the gotten context is not valid, we fall back to whatever version glXCreateContextAttribsARB is able to provide. The legacy glXCreateContext is only used if the GLX_ARB_create_context extension is not available. Covered by existent tests. * platform/graphics/glx/GLContextGLX.cpp: (WebCore::tryCreateGLXARBContext): (WebCore::GLContextGLX::createWindowContext): (WebCore::GLContextGLX::createPbufferContext): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@209982 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Dec 19, 2016 · 41dc4cc · 41dc4cc
1 parent 0389b67
commit 41dc4cc
Show file tree

Hide file tree

Showing 2 changed files with 75 additions and 25 deletions.
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,26 @@
+2016-12-19  Miguel Gomez  <[email protected]>
+
+        [GTK] GLXBadFBConfig error when creating an OpenGL context
+        https://bugs.webkit.org/show_bug.cgi?id=165200
+
+        Reviewed by Carlos Garcia Campos.
+
+        glXCreateContextAttribsARB causes a GLXBadFBConfig X error when it's not able to provide the
+        OpenGL version >= 3.2 we are requesting. Due to this, the app crashes instead of falling back to
+        the legacy path.
+        The patch modifies GLX context creation using a XErrorTrapper, so the first time a context is created
+        we don't crash if OpenGL >= 3.2 is not available.
+        If the gotten context is not valid, we fall back to whatever version glXCreateContextAttribsARB is
+        able to provide.
+        The legacy glXCreateContext is only used if the GLX_ARB_create_context extension is not available.
+
+        Covered by existent tests.
+
+        * platform/graphics/glx/GLContextGLX.cpp:
+        (WebCore::tryCreateGLXARBContext):
+        (WebCore::GLContextGLX::createWindowContext):
+        (WebCore::GLContextGLX::createPbufferContext):
+
 2016-12-18  Brady Eidson  <[email protected]>
 
         IndexedDB 2.0: Prefetch cursor records in the server.

diff --git a/Source/WebCore/platform/graphics/glx/GLContextGLX.cpp b/Source/WebCore/platform/graphics/glx/GLContextGLX.cpp
@@ -23,6 +23,7 @@
 #include "GraphicsContext3D.h"
 #include "OpenGLShims.h"
 #include "PlatformDisplayX11.h"
+#include "XErrorTrapper.h"
 #include <GL/glx.h>
 #include <cairo.h>
 
@@ -70,6 +71,49 @@ static bool hasGLXARBCreateContextExtension(Display* display)
     return !!glXCreateContextAttribsARB;
 }
 
+static GLXContext createGLXARBContext(Display* display, GLXFBConfig config, GLXContext sharingContext)
+{
+    // We want to create a context with version >= 3.2 core profile, cause that ensures that the i965 driver won't
+    // use the software renderer. If that doesn't work, we will use whatever version available. Unfortunately,
+    // 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.
+
+    static bool canCreate320Context = false;
+    static bool canCreate320ContextInitialized = false;
+
+    static const int contextAttributes[] = {
+        GLX_CONTEXT_MAJOR_VERSION_ARB, 3,
+        GLX_CONTEXT_MINOR_VERSION_ARB, 2,
+        0
+    };
+
+    if (!canCreate320ContextInitialized) {
+        canCreate320ContextInitialized = true;
+
+        {
+            // Set an X error trapper that ignores errors to avoid crashing on GLXBadFBConfig. Use a scope
+            // here to limit the error trap to just this context creation call.
+            XErrorTrapper trapper(display, XErrorTrapper::Policy::Ignore);
+            GLXContext context = glXCreateContextAttribsARB(display, config, sharingContext, GL_TRUE, contextAttributes);
+            if (context) {
+                canCreate320Context = true;
+                return context;
+            }
+        }
+
+        // Creating the 3.2 context failed, so use whatever is available.
+        return glXCreateContextAttribsARB(display, config, sharingContext, GL_TRUE, nullptr);
+    }
+
+    if (canCreate320Context)
+        return glXCreateContextAttribsARB(display, config, sharingContext, GL_TRUE, contextAttributes);
+
+    return glXCreateContextAttribsARB(display, config, sharingContext, GL_TRUE, nullptr);
+}
+
 std::unique_ptr<GLContextGLX> GLContextGLX::createWindowContext(GLNativeWindowType window, PlatformDisplay& platformDisplay, GLXContext sharingContext)
 {
     Display* display = downcast<PlatformDisplayX11>(platformDisplay).native();
@@ -96,18 +140,10 @@ std::unique_ptr<GLContextGLX> GLContextGLX::createWindowContext(GLNativeWindowTy
     ASSERT(config);
 
     XUniqueGLXContext context;
-    if (hasGLXARBCreateContextExtension(display)) {
-        // Request OpenGL version 3.2 and core profile, which guarantees that the i965 driver doesn't use the software renderer.
-        static const int contextAttributes[] = {
-            GLX_CONTEXT_MAJOR_VERSION_ARB, 3,
-            GLX_CONTEXT_MINOR_VERSION_ARB, 2,
-            0
-        };
-        context.reset(glXCreateContextAttribsARB(display, config, sharingContext, GL_TRUE, contextAttributes));
-    }
-
-    if (!context) {
-        // Fallback to legacy OpenGL version.
+    if (hasGLXARBCreateContextExtension(display))
+        context.reset(createGLXARBContext(display, config, sharingContext));
+    else {
+        // Legacy OpenGL version.
         XUniquePtr<XVisualInfo> visualInfoList(glXGetVisualFromFBConfig(display, config));
         context.reset(glXCreateContext(display, visualInfoList.get(), sharingContext, True));
     }
@@ -144,19 +180,10 @@ std::unique_ptr<GLContextGLX> GLContextGLX::createPbufferContext(PlatformDisplay
         return nullptr;
 
     XUniqueGLXContext context;
-
-    if (hasGLXARBCreateContextExtension(display)) {
-        // Request OpenGL version 3.2 and core profile, which guarantees that the i965 driver doesn't use the software renderer.
-        static const int contextAttributes[] = {
-            GLX_CONTEXT_MAJOR_VERSION_ARB, 3,
-            GLX_CONTEXT_MINOR_VERSION_ARB, 2,
-            0
-        };
-        context.reset(glXCreateContextAttribsARB(display, configs.get()[0], sharingContext, GL_TRUE, contextAttributes));
-    }
-
-    if (!context) {
-        // Fallback to legacy OpenGL version.
+    if (hasGLXARBCreateContextExtension(display))
+        context.reset(createGLXARBContext(display, configs.get()[0], sharingContext));
+    else {
+        // Legacy OpenGL version.
         context.reset(glXCreateNewContext(display, configs.get()[0], GLX_RGBA_TYPE, sharingContext, GL_TRUE));
     }