DisplayGLX::generateConfigs filter by MS and Accum buffers

src/libANGLE/renderer/gl/glx/DisplayGLX.cpp

@@ -45,6 +45,7 @@ class FunctionsGLGLX : public FunctionsGL
 DisplayGLX::DisplayGLX()
     : DisplayGL(),
       mFunctionsGL(nullptr),
+      mContextConfig(nullptr),
       mContext(nullptr),
       mDummyPbuffer(0),
       mUsesNewXDisplay(false),
@@ -95,7 +96,6 @@ egl::Error DisplayGLX::initialize(egl::Display *display)
         }
     }
 
-    glx::FBConfig contextConfig;
     // When glXMakeCurrent is called, the context and the surface must be
     // compatible which in glX-speak means that their config have the same
     // color buffer type, are both RGBA or ColorIndex, and their buffers have
@@ -133,11 +133,11 @@ egl::Error DisplayGLX::initialize(egl::Display *display)
             XFree(candidates);
             return egl::Error(EGL_NOT_INITIALIZED, "Could not find a decent GLX FBConfig to create the context.");
         }
-        contextConfig = candidates[0];
+        mContextConfig = candidates[0];
         XFree(candidates);
     }
 
-    mContext = mGLX.createContextAttribsARB(contextConfig, nullptr, True, nullptr);
+    mContext = mGLX.createContextAttribsARB(mContextConfig, nullptr, True, nullptr);
     if (!mContext)
     {
         return egl::Error(EGL_NOT_INITIALIZED, "Could not create GL context.");
@@ -150,7 +150,7 @@ egl::Error DisplayGLX::initialize(egl::Display *display)
 
     // to query things like limits. Ideally we would want to unset the current context
     // and destroy the pbuffer before going back to the application but this is TODO
-    mDummyPbuffer = mGLX.createPbuffer(contextConfig, nullptr);
+    mDummyPbuffer = mGLX.createPbuffer(mContextConfig, nullptr);
     if (!mDummyPbuffer)
     {
         return egl::Error(EGL_NOT_INITIALIZED, "Could not create the dummy pbuffer.");
@@ -242,6 +242,14 @@ egl::ConfigSet DisplayGLX::generateConfigs() const
 
     bool hasSwapControl = mGLX.hasExtension("GLX_EXT_swap_control");
 
+    int contextSamples = getGLXFBConfigAttrib(mContextConfig, GLX_SAMPLES);
+    int contextSampleBuffers = getGLXFBConfigAttrib(mContextConfig, GLX_SAMPLE_BUFFERS);
+
+    int contextAccumRedSize = getGLXFBConfigAttrib(mContextConfig, GLX_ACCUM_RED_SIZE);
+    int contextAccumGreenSize = getGLXFBConfigAttrib(mContextConfig, GLX_ACCUM_GREEN_SIZE);
+    int contextAccumBlueSize = getGLXFBConfigAttrib(mContextConfig, GLX_ACCUM_BLUE_SIZE);
+    int contextAccumAlphaSize = getGLXFBConfigAttrib(mContextConfig, GLX_ACCUM_ALPHA_SIZE);
+
     int attribList[] =
     {
         GLX_RENDER_TYPE, GLX_RGBA_BIT,
@@ -287,6 +295,25 @@ egl::ConfigSet DisplayGLX::generateConfigs() const
 
         config.bufferSize = config.redSize + config.greenSize + config.blueSize + config.alphaSize;
 
+        // Multisample and accumulation buffers
+        int samples = getGLXFBConfigAttrib(glxConfig, GLX_SAMPLES);
+        int sampleBuffers = getGLXFBConfigAttrib(glxConfig, GLX_SAMPLE_BUFFERS);
+
+        int accumRedSize = getGLXFBConfigAttrib(glxConfig, GLX_ACCUM_RED_SIZE);
+        int accumGreenSize = getGLXFBConfigAttrib(glxConfig, GLX_ACCUM_GREEN_SIZE);
+        int accumBlueSize = getGLXFBConfigAttrib(glxConfig, GLX_ACCUM_BLUE_SIZE);
+        int accumAlphaSize = getGLXFBConfigAttrib(glxConfig, GLX_ACCUM_ALPHA_SIZE);
+
+        if (samples != contextSamples ||
+            sampleBuffers != contextSampleBuffers ||
+            accumRedSize != contextAccumRedSize ||
+            accumGreenSize != contextAccumGreenSize ||
+            accumBlueSize != contextAccumBlueSize ||
+            accumAlphaSize != contextAccumAlphaSize)
+        {
+            continue;
+        }
+
         // Transparency
         if (getGLXFBConfigAttrib(glxConfig, GLX_TRANSPARENT_TYPE) == GLX_TRANSPARENT_RGB)
         {

src/libANGLE/renderer/gl/glx/DisplayGLX.h

@@ -72,6 +72,7 @@ class DisplayGLX : public DisplayGL
     //TODO(cwallez) yuck, change generateConfigs to be non-const or add a userdata member to egl::Config?
     mutable std::map<int, glx::FBConfig> configIdToGLXConfig;
 
+    glx::FBConfig mContextConfig;
     glx::Context mContext;
     // A pbuffer the context is current on during ANGLE initialization
     glx::Pbuffer mDummyPbuffer;

src/libANGLE/renderer/gl/glx/FBConfigCompatibility.md

@@ -40,7 +40,7 @@ the vast majority of applications use.
 use it for all types of EGL surfaces
  * It must have an associated visual ID so that we can use it with X, it seems
 like this would be strongly tied to it having the ```WINDOW_BIT``` set.
- * We would like a conformat context.
+ * We would like a conformant context.
 
 Study of compatible GLXFBConfigs
 --------------------------------
@@ -120,11 +120,50 @@ can find a trick to avoid that flicker, then we would be able to expose single
 and double-buffered surfaces at the EGL level. Not exposing them isn't too much
 of a problem though as the vast majority of application want double-buffering.
 
+AMD and extra buffers
+---------------------
+As can be seen above, NVIDIA does not expose conformant context with multisampled
+buffers or non RGBA16 accumulation buffers. The behavior is different on AMD that
+exposes them as conformant, which gives the following list after filtering as
+explained above:
+
+```
+    visual  x   bf lv rg d st  colorbuffer  sr ax dp st accumbuffer  ms  cav
+  id dep cl sp  sz l  ci b ro  r  g  b  a F gb bf th cl  r  g  b  a ns b eat
+----------------------------------------------------------------------------
+0x023 24 tc  0  32  0 r  y .   8  8  8  8 .  .  0 24  8 16 16 16 16  0 0 None
+0x027 24 tc  0  32  0 r  y .   8  8  8  8 .  .  0 24  8  0  0  0  0  0 0 None
+0x02b 24 tc  0  32  0 r  y .   8  8  8  8 .  .  0 24  8  0  0  0  0  2 1 None
+0x02f 24 tc  0  32  0 r  y .   8  8  8  8 .  .  0 24  8  0  0  0  0  4 1 None
+0x03b 24 dc  0  32  0 r  y .   8  8  8  8 .  .  0 24  8 16 16 16 16  0 0 None
+0x03f 24 dc  0  32  0 r  y .   8  8  8  8 .  .  0 24  8  0  0  0  0  0 0 None
+0x043 24 dc  0  32  0 r  y .   8  8  8  8 .  .  0 24  8  0  0  0  0  2 1 None
+0x047 24 dc  0  32  0 r  y .   8  8  8  8 .  .  0 24  8  0  0  0  0  4 1 None
+```
+
+ANGLE's context is created using 0x027 and experimentation shows it is only compatible
+with 0x03f which is the only other config lacking both an accumulation buffer and a
+multisample buffer. The GLX spec seems to hint it should still work ("should have the
+same size, if they exist") but it doesn't work in this case. Filtering the configs to
+have the same multisample and accumulation buffers gives the following:
+
+```
+    visual  x   bf lv rg d st  colorbuffer  sr ax dp st accumbuffer  ms  cav
+  id dep cl sp  sz l  ci b ro  r  g  b  a F gb bf th cl  r  g  b  a ns b eat
+----------------------------------------------------------------------------
+0x027 24 tc  0  32  0 r  y .   8  8  8  8 .  .  0 24  8  0  0  0  0  0 0 None
+0x03f 24 dc  0  32  0 r  y .   8  8  8  8 .  .  0 24  8  0  0  0  0  0 0 None
+```
+
 Future investigation
 --------------------
 All the non-conformant configs have a multisampled buffer, so it could be interesting
 to see if we can use them to expose another EGL extension.
 
-Finally all this document is written with respect to the proprietary NVIDIA driver,
-before using the GLX EGL implementation in the wild it would be good to test it on
-other drivers and hardware.
+Finally this document is written with respect to a small number of drivers, before
+using the GLX EGL implementation in the wild it would be good to test it on other
+drivers and hardware.
+
+The drivers tested were:
+ - the proprietary NVIDIA driver
+ - the proprietary AMD driver