X11: Better support non-32bit visuals

This patch improves support for non-32bit screen configurations on X. The
patch mostly touches the xcb platform plugin but the changes to the glx
convenience functions do affect e.g. the offscreen plugin as well.

Since QWindow instances are now by default of type RasterGL instead of
Raster the majority of all windows are in fact instances of QXcbGlxWindow.
This means that the eventual QSurfaceFormat that we use is chosen based on
the available OpenGL configurations. Here the GLX config resolution code
did not do a very good job in trying to find the closest match relative to
the requested QSurfaceFormat, instead preferring higher bit depths.

This is an issue since many configurations support 32-bit windows even
if the screen itself has a root window with depth 16. In particular,
servers supporting both GLX and Render are very likely to have such
visuals. Particularly affected are remote X connections - even if the
application itself makes no use of OpenGL at all!

The changes introduced by this patch are as follows:
 1. Improve the GLX visual selection logic
 2. Improve the xcb visual selection logic
 3. Remove duplicated visual lookup for OpenGL-enabled windows
 4. Configure the default QSurfaceFormat to match the primary screen depth

Change-Id: Id1c176359e63a4581410e20350db5ac2c083e1cf
Reviewed-by: Lars Knoll <lars.knoll@theqtcompany.com>
Reviewed-by: Laszlo Agocs <laszlo.agocs@theqtcompany.com>

2 files changed, 201 insertions, 161 deletions

diff --git a/src/platformsupport/glxconvenience/qglxconvenience.cpp b/src/platformsupport/glxconvenience/qglxconvenience.cpp
index 870e746a53..7dc29fae6d 100644
--- a/src/platformsupport/glxconvenience/qglxconvenience.cpp
+++ b/src/platformsupport/glxconvenience/qglxconvenience.cpp
@@ -40,6 +40,7 @@
 // We have to include this before the X11 headers dragged in by
 // qglxconvenience_p.h.
 #include <QtCore/QByteArray>
+#include <QtCore/QScopedPointer>
 
 #include "qglxconvenience_p.h"
 
@@ -77,174 +78,182 @@ enum {
 
 QVector<int> qglx_buildSpec(const QSurfaceFormat &format, int drawableBit)
 {
-    QVector<int> spec(48);
-    int i = 0;
+    QVector<int> spec;
 
-    spec[i++] = GLX_LEVEL;
-    spec[i++] = 0;
-    spec[i++] = GLX_DRAWABLE_TYPE; spec[i++] = drawableBit;
+    spec << GLX_LEVEL
+         << 0
 
-    spec[i++] = GLX_RENDER_TYPE; spec[i++] = GLX_RGBA_BIT;
+         << GLX_RENDER_TYPE
+         << GLX_RGBA_BIT
 
-    spec[i++] = GLX_RED_SIZE; spec[i++] = (format.redBufferSize() == -1) ? 1 : format.redBufferSize();
-    spec[i++] = GLX_GREEN_SIZE; spec[i++] =  (format.greenBufferSize() == -1) ? 1 : format.greenBufferSize();
-    spec[i++] = GLX_BLUE_SIZE; spec[i++] = (format.blueBufferSize() == -1) ? 1 : format.blueBufferSize();
-    if (format.hasAlpha()) {
-        spec[i++] = GLX_ALPHA_SIZE; spec[i++] = format.alphaBufferSize();
-    }
+         << GLX_RED_SIZE
+         << qMax(1, format.redBufferSize())
 
-    spec[i++] = GLX_DOUBLEBUFFER; spec[i++] = format.swapBehavior() != QSurfaceFormat::SingleBuffer ? True : False;
+         << GLX_GREEN_SIZE
+         << qMax(1, format.greenBufferSize())
 
-    spec[i++] = GLX_STEREO; spec[i++] =  format.stereo() ? True : False;
+         << GLX_BLUE_SIZE
+         << qMax(1, format.blueBufferSize())
 
-    if (format.depthBufferSize() > 0) {
-        spec[i++] = GLX_DEPTH_SIZE; spec[i++] = format.depthBufferSize();
-    }
+         << GLX_ALPHA_SIZE
+         << qMax(0, format.alphaBufferSize());
 
-    if (format.stencilBufferSize() > 0) {
-        spec[i++] = GLX_STENCIL_SIZE; spec[i++] =  (format.stencilBufferSize() == -1) ? 1 : format.stencilBufferSize();
-    }
+    if (format.swapBehavior() != QSurfaceFormat::SingleBuffer)
+        spec << GLX_DOUBLEBUFFER
+             << True;
 
-    if (format.samples() > 1) {
-        spec[i++] = GLX_SAMPLE_BUFFERS_ARB;
-        spec[i++] = 1;
-        spec[i++] = GLX_SAMPLES_ARB;
-        spec[i++] = format.samples();
-    }
+    if (format.stereo())
+        spec << GLX_STEREO
+             << True;
+
+    if (format.depthBufferSize() != -1)
+        spec << GLX_DEPTH_SIZE
+             << format.depthBufferSize();
+
+    if (format.stencilBufferSize() != -1)
+        spec << GLX_STENCIL_SIZE
+             << format.stencilBufferSize();
+
+    if (format.samples() > 1)
+        spec << GLX_SAMPLE_BUFFERS_ARB
+             << 1
+             << GLX_SAMPLES_ARB
+             << format.samples();
+
+    spec << GLX_DRAWABLE_TYPE
+         << drawableBit
+
+         << XNone;
 
-    spec[i++] = XNone;
     return spec;
 }
 
-GLXFBConfig qglx_findConfig(Display *display, int screen , const QSurfaceFormat &format, int drawableBit)
-{
-    // Allow forcing LIBGL_ALWAYS_SOFTWARE for Qt 5 applications only.
-    // This is most useful with drivers that only support OpenGL 1.
-    // We need OpenGL 2, but the user probably doesn't want
-    // LIBGL_ALWAYS_SOFTWARE in OpenGL 1 apps.
-    static bool checkedForceSoftwareOpenGL = false;
-    static bool forceSoftwareOpenGL = false;
-    if (!checkedForceSoftwareOpenGL) {
-        // If LIBGL_ALWAYS_SOFTWARE is already set, don't mess with it.
-        // We want to unset LIBGL_ALWAYS_SOFTWARE at the end so it does not
-        // get inherited by other processes, of course only if it wasn't
-        // already set before.
-        if (!qEnvironmentVariableIsEmpty("QT_XCB_FORCE_SOFTWARE_OPENGL")
-            && !qEnvironmentVariableIsSet("LIBGL_ALWAYS_SOFTWARE"))
-            forceSoftwareOpenGL = true;
-
-        checkedForceSoftwareOpenGL = true;
-    }
+namespace  {
+struct QXcbSoftwareOpenGLEnforcer {
+    QXcbSoftwareOpenGLEnforcer() {
+        // Allow forcing LIBGL_ALWAYS_SOFTWARE for Qt 5 applications only.
+        // This is most useful with drivers that only support OpenGL 1.
+        // We need OpenGL 2, but the user probably doesn't want
+        // LIBGL_ALWAYS_SOFTWARE in OpenGL 1 apps.
+
+        if (!checkedForceSoftwareOpenGL) {
+            // If LIBGL_ALWAYS_SOFTWARE is already set, don't mess with it.
+            // We want to unset LIBGL_ALWAYS_SOFTWARE at the end so it does not
+            // get inherited by other processes, of course only if it wasn't
+            // already set before.
+            if (!qEnvironmentVariableIsEmpty("QT_XCB_FORCE_SOFTWARE_OPENGL")
+                && !qEnvironmentVariableIsSet("LIBGL_ALWAYS_SOFTWARE"))
+                forceSoftwareOpenGL = true;
+
+            checkedForceSoftwareOpenGL = true;
+        }
 
-    if (forceSoftwareOpenGL)
-        qputenv("LIBGL_ALWAYS_SOFTWARE", QByteArrayLiteral("1"));
+        if (forceSoftwareOpenGL)
+            qputenv("LIBGL_ALWAYS_SOFTWARE", QByteArrayLiteral("1"));
+    }
 
-    bool reduced = true;
-    GLXFBConfig chosenConfig = 0;
-    QSurfaceFormat reducedFormat = format;
-    while (!chosenConfig && reduced) {
-        QVector<int> spec = qglx_buildSpec(reducedFormat, drawableBit);
-        int confcount = 0;
-        GLXFBConfig *configs;
-        configs = glXChooseFBConfig(display, screen,spec.constData(),&confcount);
-        if (confcount)
-        {
-            for (int i = 0; i < confcount; i++) {
-                chosenConfig = configs[i];
-                // Make sure we try to get an ARGB visual if the format asked for an alpha:
-                if (reducedFormat.hasAlpha()) {
-                    int alphaSize;
-                    glXGetFBConfigAttrib(display,configs[i],GLX_ALPHA_SIZE,&alphaSize);
-                    if (alphaSize > 0) {
-                        XVisualInfo *visual = glXGetVisualFromFBConfig(display, chosenConfig);
-                        bool hasAlpha = false;
-
-#if !defined(QT_NO_XRENDER)
-                        XRenderPictFormat *pictFormat = XRenderFindVisualFormat(display, visual->visual);
-                        hasAlpha = pictFormat->direct.alphaMask > 0;
-#else
-                        hasAlpha = visual->depth == 32;
-#endif
+    ~QXcbSoftwareOpenGLEnforcer() {
+        // unset LIBGL_ALWAYS_SOFTWARE now so other processes don't inherit it
+        if (forceSoftwareOpenGL)
+            qunsetenv("LIBGL_ALWAYS_SOFTWARE");
+    }
 
-                        XFree(visual);
+    static bool checkedForceSoftwareOpenGL;
+    static bool forceSoftwareOpenGL;
+};
 
-                        if (hasAlpha)
-                            break;
-                    }
-                } else {
-                    break; // Just choose the first in the list if there's no alpha requested
-                }
-            }
+bool QXcbSoftwareOpenGLEnforcer::checkedForceSoftwareOpenGL = false;
+bool QXcbSoftwareOpenGLEnforcer::forceSoftwareOpenGL = false;
 
-            XFree(configs);
-        }
-        if (!chosenConfig)
-            reducedFormat = qglx_reduceSurfaceFormat(reducedFormat,&reduced);
+template <class T>
+struct QXlibScopedPointerDeleter {
+    static inline void cleanup(T *pointer) {
+        XFree(pointer);
     }
+};
 
-    // unset LIBGL_ALWAYS_SOFTWARE now so other processes don't inherit it
-    if (forceSoftwareOpenGL)
-        qunsetenv("LIBGL_ALWAYS_SOFTWARE");
+template <class T>
+using QXlibPointer = QScopedPointer<T, QXlibScopedPointerDeleter<T>>;
 
-    return chosenConfig;
+template <class T>
+using QXlibArrayPointer = QScopedArrayPointer<T, QXlibScopedPointerDeleter<T>>;
 }
 
-XVisualInfo *qglx_findVisualInfo(Display *display, int screen, QSurfaceFormat *format)
+GLXFBConfig qglx_findConfig(Display *display, int screen , QSurfaceFormat format, bool highestPixelFormat, int drawableBit)
 {
-    Q_ASSERT(format);
+    QXcbSoftwareOpenGLEnforcer softwareOpenGLEnforcer;
 
-    XVisualInfo *visualInfo = 0;
+    GLXFBConfig config = 0;
 
-    GLXFBConfig config = qglx_findConfig(display,screen,*format);
-    if (config) {
-        visualInfo = glXGetVisualFromFBConfig(display, config);
-        qglx_surfaceFormatFromGLXFBConfig(format, display, config);
-    }
+    do {
+        const QVector<int> spec = qglx_buildSpec(format, drawableBit);
 
-    // attempt to fall back to glXChooseVisual
-    bool reduced = true;
-    QSurfaceFormat reducedFormat = *format;
-    while (!visualInfo && reduced) {
-        QVarLengthArray<int, 13> attribs;
-        attribs.append(GLX_RGBA);
-
-        if (reducedFormat.redBufferSize() > 0) {
-            attribs.append(GLX_RED_SIZE);
-            attribs.append(reducedFormat.redBufferSize());
-        }
+        int confcount = 0;
+        QXlibArrayPointer<GLXFBConfig> configs(glXChooseFBConfig(display, screen, spec.constData(), &confcount));
 
-        if (reducedFormat.greenBufferSize() > 0) {
-            attribs.append(GLX_GREEN_SIZE);
-            attribs.append(reducedFormat.greenBufferSize());
+        if (!config && confcount > 0) {
+            config = configs[0];
+            if (highestPixelFormat && !format.hasAlpha())
+                break;
         }
 
-        if (reducedFormat.blueBufferSize() > 0) {
-            attribs.append(GLX_BLUE_SIZE);
-            attribs.append(reducedFormat.blueBufferSize());
-        }
+        const int requestedRed = qMax(0, format.redBufferSize());
+        const int requestedGreen = qMax(0, format.greenBufferSize());
+        const int requestedBlue = qMax(0, format.blueBufferSize());
+        const int requestedAlpha = qMax(0, format.alphaBufferSize());
 
-        if (reducedFormat.stencilBufferSize() > 0) {
-            attribs.append(GLX_STENCIL_SIZE);
-            attribs.append(reducedFormat.stencilBufferSize());
-        }
+        for (int i = 0; i < confcount; i++) {
+            GLXFBConfig candidate = configs[i];
+
+            QXlibPointer<XVisualInfo> visual(glXGetVisualFromFBConfig(display, candidate));
 
-        if (reducedFormat.depthBufferSize() > 0) {
-            attribs.append(GLX_DEPTH_SIZE);
-            attribs.append(reducedFormat.depthBufferSize());
+            const int actualRed = qPopulationCount(visual->red_mask);
+            const int actualGreen = qPopulationCount(visual->green_mask);
+            const int actualBlue = qPopulationCount(visual->blue_mask);
+            const int actualAlpha = visual->depth - actualRed - actualGreen - actualBlue;
+
+            if (requestedRed && actualRed != requestedRed)
+                continue;
+            if (requestedGreen && actualGreen != requestedGreen)
+                continue;
+            if (requestedBlue && actualBlue != requestedBlue)
+                continue;
+            if (requestedAlpha && actualAlpha != requestedAlpha)
+                continue;
+
+            return candidate;
         }
+    } while (qglx_reduceFormat(&format));
+
+    return config;
+}
 
-        if (reducedFormat.swapBehavior() != QSurfaceFormat::SingleBuffer)
-            attribs.append(GLX_DOUBLEBUFFER);
+XVisualInfo *qglx_findVisualInfo(Display *display, int screen, QSurfaceFormat *format, int drawableBit)
+{
+    Q_ASSERT(format);
 
-        attribs.append(XNone);
+    XVisualInfo *visualInfo = 0;
 
-        visualInfo = glXChooseVisual(display, screen, attribs.data());
-        if (visualInfo)
-            *format = reducedFormat;
+    GLXFBConfig config = qglx_findConfig(display, screen, *format, false, drawableBit);
+    if (config)
+        visualInfo = glXGetVisualFromFBConfig(display, config);
 
-        reducedFormat = qglx_reduceSurfaceFormat(reducedFormat, &reduced);
+    if (visualInfo) {
+        qglx_surfaceFormatFromGLXFBConfig(format, display, config);
+        return visualInfo;
     }
 
+    // attempt to fall back to glXChooseVisual
+    do {
+        QVector<int> attribs = qglx_buildSpec(*format, drawableBit);
+        visualInfo = glXChooseVisual(display, screen, attribs.data());
+
+        if (visualInfo) {
+            qglx_surfaceFormatFromVisualInfo(format, display, visualInfo);
+            return visualInfo;
+        }
+    } while (qglx_reduceFormat(format));
+
     return visualInfo;
 }
 
@@ -318,33 +327,64 @@ void qglx_surfaceFormatFromVisualInfo(QSurfaceFormat *format, Display *display,
     format->setStereo(stereo);
 }
 
-QSurfaceFormat qglx_reduceSurfaceFormat(const QSurfaceFormat &format, bool *reduced)
+bool qglx_reduceFormat(QSurfaceFormat *format)
 {
-    QSurfaceFormat retFormat = format;
-    *reduced = true;
-
-    if (retFormat.depthBufferSize() >= 32) {
-        retFormat.setDepthBufferSize(24);
-    } else if (retFormat.depthBufferSize() > 0) {
-        retFormat.setDepthBufferSize(0);
-    } else if (retFormat.redBufferSize() > 1) {
-        retFormat.setRedBufferSize(1);
-    } else if (retFormat.greenBufferSize() > 1) {
-        retFormat.setGreenBufferSize(1);
-    } else if (retFormat.blueBufferSize() > 1) {
-        retFormat.setBlueBufferSize(1);
-    } else if (retFormat.samples() > 1) {
-        retFormat.setSamples(qMin(retFormat.samples() / 2, 16));
-    } else if (retFormat.stereo()) {
-        retFormat.setStereo(false);
-    }else if (retFormat.stencilBufferSize() > 0) {
-        retFormat.setStencilBufferSize(0);
-    }else if (retFormat.hasAlpha()) {
-        retFormat.setAlphaBufferSize(0);
-    }else if (retFormat.swapBehavior() != QSurfaceFormat::SingleBuffer) {
-        retFormat.setSwapBehavior(QSurfaceFormat::SingleBuffer);
-    }else{
-        *reduced = false;
+    Q_ASSERT(format);
+
+    if (format->redBufferSize() > 1) {
+        format->setRedBufferSize(1);
+        return true;
+    }
+
+    if (format->greenBufferSize() > 1) {
+        format->setGreenBufferSize(1);
+        return true;
+    }
+
+    if (format->blueBufferSize() > 1) {
+        format->setBlueBufferSize(1);
+        return true;
     }
-    return retFormat;
+
+    if (format->swapBehavior() != QSurfaceFormat::SingleBuffer){
+        format->setSwapBehavior(QSurfaceFormat::SingleBuffer);
+        return true;
+    }
+
+    if (format->samples() > 1) {
+        format->setSamples(qMin(16, format->samples() / 2));
+        return true;
+    }
+
+    if (format->depthBufferSize() >= 32) {
+        format->setDepthBufferSize(24);
+        return true;
+    }
+
+    if (format->depthBufferSize() > 1) {
+        format->setDepthBufferSize(1);
+        return true;
+    }
+
+    if (format->depthBufferSize() > 0) {
+        format->setDepthBufferSize(0);
+        return true;
+    }
+
+    if (format->hasAlpha()) {
+        format->setAlphaBufferSize(0);
+        return true;
+    }
+
+    if (format->stencilBufferSize() > 1) {
+        format->setStencilBufferSize(1);
+        return true;
+    }
+
+    if (format->stencilBufferSize() > 0) {
+        format->setStencilBufferSize(0);
+        return true;
+    }
+
+    return false;
 }
diff --git a/src/platformsupport/glxconvenience/qglxconvenience_p.h b/src/platformsupport/glxconvenience/qglxconvenience_p.h
index 309974f357..d422668584 100644
--- a/src/platformsupport/glxconvenience/qglxconvenience_p.h
+++ b/src/platformsupport/glxconvenience/qglxconvenience_p.h
@@ -57,11 +57,11 @@
 #include <X11/Xlib.h>
 #include <GL/glx.h>
 
-XVisualInfo *qglx_findVisualInfo(Display *display, int screen, QSurfaceFormat *format);
-GLXFBConfig qglx_findConfig(Display *display, int screen, const QSurfaceFormat &format, int drawableBit = GLX_WINDOW_BIT);
+QVector<int> qglx_buildSpec(const QSurfaceFormat &format, int drawableBit = GLX_WINDOW_BIT);
+XVisualInfo *qglx_findVisualInfo(Display *display, int screen, QSurfaceFormat *format, int drawableBit = GLX_WINDOW_BIT);
+GLXFBConfig qglx_findConfig(Display *display, int screen, QSurfaceFormat format, bool highestPixelFormat = false, int drawableBit = GLX_WINDOW_BIT);
 void qglx_surfaceFormatFromGLXFBConfig(QSurfaceFormat *format, Display *display, GLXFBConfig config);
 void qglx_surfaceFormatFromVisualInfo(QSurfaceFormat *format, Display *display, XVisualInfo *visualInfo);
-QVector<int> qglx_buildSpec(const QSurfaceFormat &format, int drawableBit = GLX_WINDOW_BIT);
-QSurfaceFormat qglx_reduceSurfaceFormat(const QSurfaceFormat &format, bool *reduced);
+bool qglx_reduceFormat(QSurfaceFormat *format);
 
 #endif // QGLXCONVENIENCE_H