xcb: Unify visual to QImage::Format logic

Make a common function to replace the two existing ones that convert
from XCB visuals to QImage format.

Change-Id: I2ae08ef4df96df950910a45e71c9d9cd98375b2e
Reviewed-by: Gatis Paeglis <gatis.paeglis@qt.io>

src/plugins/platforms/xcb/qxcbconnection.cpp

@@ -2070,7 +2070,8 @@ const xcb_format_t *QXcbConnection::formatForDepth(uint8_t depth) const
         xcb_format_next(&iterator);
     }
 
-    return 0;
+    qWarning() << "XCB failed to find an xcb_format_t for depth:" << depth;
+    return nullptr;
 }
 
 void QXcbConnection::sync()

src/plugins/platforms/xcb/qxcbimage.cpp

@@ -53,39 +53,108 @@ extern "C" {
 #undef template
 #endif
 
+#include "qxcbconnection.h"
+#include "qxcbintegration.h"
+
+namespace {
+
+QImage::Format imageFormatForMasks(int depth, int bits_per_pixel, int red_mask, int blue_mask)
+{
+    if (bits_per_pixel == 32) {
+        switch (depth) {
+        case 32:
+            if (red_mask == 0xff0000 && blue_mask == 0xff)
+                return QImage::Format_ARGB32_Premultiplied;
+#if Q_BYTE_ORDER == Q_LITTLE_ENDIAN
+            if (red_mask == 0xff && blue_mask == 0xff0000)
+                return QImage::Format_RGBA8888_Premultiplied;
+#else
+            if (red_mask == 0xff000000 && blue_mask == 0xff00)
+                return QImage::Format_RGBA8888_Premultiplied;
+#endif
+            if (red_mask == 0x3ff && blue_mask == 0x3ff00000)
+                return QImage::Format_A2BGR30_Premultiplied;
+            if (red_mask == 0x3ff00000 && blue_mask == 0x3ff)
+                return QImage::Format_A2RGB30_Premultiplied;
+            break;
+        case 30:
+            if (red_mask == 0x3ff && blue_mask == 0x3ff00000)
+                return QImage::Format_BGR30;
+            if (blue_mask == 0x3ff && red_mask == 0x3ff00000)
+                return QImage::Format_RGB30;
+            break;
+        case 24:
+            if (red_mask == 0xff0000 && blue_mask == 0xff)
+                return QImage::Format_RGB32;
+#if Q_BYTE_ORDER == Q_LITTLE_ENDIAN
+            if (red_mask == 0xff && blue_mask == 0xff0000)
+                return QImage::Format_RGBX8888;
+#else
+            if (red_mask == 0xff000000 && blue_mask == 0xff00)
+                return QImage::Format_RGBX8888;
+#endif
+            break;
+        }
+    } else if (bits_per_pixel == 16) {
+        if (depth == 16 && red_mask == 0xf800 && blue_mask == 0x1f)
+            return QImage::Format_RGB16;
+        if (depth == 15 && red_mask == 0x7c00 && blue_mask == 0x1f)
+            return QImage::Format_RGB555;
+    }
+    return QImage::Format_Invalid;
+}
+
+} // namespace
+
 QT_BEGIN_NAMESPACE
 
-// TODO: Merge with imageFormatForVisual in qxcbwindow.cpp
+bool qt_xcb_imageFormatForVisual(QXcbConnection *connection, uint8_t depth, const xcb_visualtype_t *visual,
-QImage::Format qt_xcb_imageFormatForVisual(QXcbConnection *connection, uint8_t depth,
+                                 QImage::Format *imageFormat, bool *needsRgbSwap)
-                                           const xcb_visualtype_t *visual)
 {
+    Q_ASSERT(connection && visual && imageFormat);
+
+    if (needsRgbSwap)
+        *needsRgbSwap = false;
+    *imageFormat = QImage::Format_Invalid;
+
+    if (depth == 8) {
+        if (visual->_class == XCB_VISUAL_CLASS_GRAY_SCALE) {
+            *imageFormat = QImage::Format_Grayscale8;
+            return true;
+        }
+#if QT_CONFIG(xcb_native_painting)
+        if (QXcbIntegration::instance() && QXcbIntegration::instance()->nativePaintingEnabled()) {
+            *imageFormat = QImage::Format_Indexed8;
+            return true;
+        }
+#endif
+        return false;
+    }
+
     const xcb_format_t *format = connection->formatForDepth(depth);
+    if (!format)
+        return false;
 
-    if (!visual || !format)
+    const bool connectionEndianSwap = connection->imageNeedsEndianSwap();
-        return QImage::Format_Invalid;
+    // We swap the masks and see if we can recognize it as a host format
+    const quint32 red_mask = connectionEndianSwap ? qbswap(visual->red_mask) : visual->red_mask;
+    const quint32 blue_mask = connectionEndianSwap ? qbswap(visual->blue_mask) : visual->blue_mask;
 
-    if (depth == 32 && format->bits_per_pixel == 32 && visual->red_mask == 0xff0000
+    *imageFormat = imageFormatForMasks(depth, format->bits_per_pixel, red_mask, blue_mask);
-        && visual->green_mask == 0xff00 && visual->blue_mask == 0xff)
+    if (*imageFormat != QImage::Format_Invalid)
-        return QImage::Format_ARGB32_Premultiplied;
+        return true;
 
-    if (depth == 30 && format->bits_per_pixel == 32 && visual->red_mask == 0x3ff
+    if (needsRgbSwap) {
-        && visual->green_mask == 0x0ffc00 && visual->blue_mask == 0x3ff00000)
+        *imageFormat = imageFormatForMasks(depth, format->bits_per_pixel, blue_mask, red_mask);
-        return QImage::Format_BGR30;
+        if (*imageFormat != QImage::Format_Invalid) {
+            *needsRgbSwap = true;
+            return true;
+        }
+    }
 
-    if (depth == 30 && format->bits_per_pixel == 32 && visual->blue_mask == 0x3ff
+    qWarning("Unsupported screen format: depth: %d, bits_per_pixel: %d, red_mask: %x, blue_mask: %x", depth, format->bits_per_pixel, red_mask, blue_mask);
-        && visual->green_mask == 0x0ffc00 && visual->red_mask == 0x3ff00000)
-        return QImage::Format_RGB30;
 
-    if (depth == 24 && format->bits_per_pixel == 32 && visual->red_mask == 0xff0000
+    return false;
-        && visual->green_mask == 0xff00 && visual->blue_mask == 0xff)
-        return QImage::Format_RGB32;
-
-    if (depth == 16 && format->bits_per_pixel == 16 && visual->red_mask == 0xf800
-        && visual->green_mask == 0x7e0 && visual->blue_mask == 0x1f)
-        return QImage::Format_RGB16;
-
-    qWarning("qt_xcb_imageFormatForVisual did not recognize format");
-    return QImage::Format_Invalid;
 }
 
 QPixmap qt_xcb_pixmapFromXPixmap(QXcbConnection *connection, xcb_pixmap_t pixmap,
@@ -105,33 +174,14 @@ QPixmap qt_xcb_pixmapFromXPixmap(QXcbConnection *connection, xcb_pixmap_t pixmap
 
     QPixmap result;
 
-    QImage::Format format = qt_xcb_imageFormatForVisual(connection, depth, visual);
+    QImage::Format format;
-    if (format != QImage::Format_Invalid) {
+    bool needsRgbSwap;
+    if (qt_xcb_imageFormatForVisual(connection, depth, visual, &format, &needsRgbSwap)) {
         uint32_t bytes_per_line = length / height;
         QImage image(const_cast<uint8_t *>(data), width, height, bytes_per_line, format);
 
-        // we may have to swap the byte order
+        if (needsRgbSwap)
-        if (connection->imageNeedsEndianSwap()) {
+            image = std::move(image).rgbSwapped();
-            if (image.depth() == 16) {
-                for (int i = 0; i < image.height(); ++i) {
-                    ushort *p = reinterpret_cast<ushort *>(image.scanLine(i));
-                    ushort *end = p + image.width();
-                    while (p < end) {
-                        *p = qbswap(*p);
-                        p++;
-                    }
-                }
-            } else if (image.depth() == 32) {
-                for (int i = 0; i < image.height(); ++i) {
-                    uint *p = reinterpret_cast<uint *>(image.scanLine(i));
-                    uint *end = p + image.width();
-                    while (p < end) {
-                        *p = qbswap(*p);
-                        p++;
-                    }
-                }
-            }
-        }
 
         // fix-up alpha channel
         if (format == QImage::Format_RGB32) {

src/plugins/platforms/xcb/qxcbimage.h

@@ -48,8 +48,8 @@
 
 QT_BEGIN_NAMESPACE
 
-QImage::Format qt_xcb_imageFormatForVisual(QXcbConnection *connection,
+bool qt_xcb_imageFormatForVisual(QXcbConnection *connection, uint8_t depth, const xcb_visualtype_t *visual,
-                                           uint8_t depth, const xcb_visualtype_t *visual);
+                                 QImage::Format *imageFormat, bool *needsRgbSwap = nullptr);
 QPixmap qt_xcb_pixmapFromXPixmap(QXcbConnection *connection, xcb_pixmap_t pixmap,
                                  int width, int height, int depth,
                                  const xcb_visualtype_t *visual);

src/plugins/platforms/xcb/qxcbscreen.cpp

@@ -583,7 +583,9 @@ QRect QXcbScreen::availableGeometry() const
 
 QImage::Format QXcbScreen::format() const
 {
-    return qt_xcb_imageFormatForVisual(connection(), screen()->root_depth, visualForId(screen()->root_visual));
+    QImage::Format format;
+    qt_xcb_imageFormatForVisual(connection(), screen()->root_depth, visualForId(screen()->root_visual), &format);
+    return format;
 }
 
 QDpi QXcbScreen::virtualDpi() const

src/plugins/platforms/xcb/qxcbwindow.cpp

@@ -42,7 +42,6 @@
 #include <QtDebug>
 #include <QMetaEnum>
 #include <QScreen>
-#include <QtCore/qendian.h>
 #include <QtGui/QIcon>
 #include <QtGui/QRegion>
 #include <QtGui/private/qhighdpiscaling_p.h>
@@ -52,6 +51,7 @@
 #include "qxcbscreen.h"
 #include "qxcbdrag.h"
 #include "qxcbkeyboard.h"
+#include "qxcbimage.h"
 #include "qxcbwmsupport.h"
 #include "qxcbimage.h"
 #include "qxcbnativeinterface.h"
@@ -177,108 +177,23 @@ static inline bool isTransient(const QWindow *w)
            || w->type() == Qt::Popup;
 }
 
-// TODO: Merge with qt_xcb_imageFormatForVisual in qxcbimage.cpp
+void QXcbWindow::setImageFormatForVisual(const xcb_visualtype_t *visual)
-QImage::Format QXcbWindow::imageFormatForVisual(const xcb_visualtype_t *visual, bool *rgbSwap) const
 {
-    const bool connectionEndianSwap = connection()->imageNeedsEndianSwap();
+    if (qt_xcb_imageFormatForVisual(connection(), m_depth, visual, &m_imageFormat, &m_imageRgbSwap))
-    // We swap the masks and see if we can recognize it as a host format
+        return;
-    const quint32 red_mask = connectionEndianSwap ? qbswap(visual->red_mask) : visual->red_mask;
-    const quint32 blue_mask = connectionEndianSwap ? qbswap(visual->blue_mask) : visual->blue_mask;
-
-    if (rgbSwap)
-        *rgbSwap = false;
-    switch (m_depth) {
-    case 32:
-        if (blue_mask == 0xff)
-            return QImage::Format_ARGB32_Premultiplied;
-        if (red_mask == 0x3ff)
-            return QImage::Format_A2BGR30_Premultiplied;
-        if (blue_mask == 0x3ff)
-            return QImage::Format_A2RGB30_Premultiplied;
-        if (red_mask == 0xff) {
-#if Q_BYTE_ORDER == Q_LITTLE_ENDIAN
-            return QImage::Format_RGBA8888_Premultiplied;
-#else
-            if (rgbSwap)
-                *rgbSwap = true;
-            return QImage::Format_ARGB32_Premultiplied;
-#endif
-        }
-#if Q_BYTE_ORDER == Q_BIG_ENDIAN
-        if (red_mask == 0xff00 && blue_mask == 0xff000000) {
-            if (rgbSwap)
-                *rgbSwap = true;
-            return QImage::Format_RGBA8888_Premultiplied;
-        }
-#endif
-        break;
-    case 30:
-        if (red_mask == 0x3ff)
-            return QImage::Format_BGR30;
-        if (blue_mask == 0x3ff)
-            return QImage::Format_RGB30;
-        break;
-    case 24:
-        if (blue_mask == 0xff)
-            return QImage::Format_RGB32;
-        if (red_mask == 0xff) {
-#if Q_BYTE_ORDER == Q_LITTLE_ENDIAN
-            return QImage::Format_RGBX8888;
-#else
-            if (rgbSwap)
-                *rgbSwap = true;
-            return QImage::Format_RGB32;
-#endif
-        }
-#if Q_BYTE_ORDER == Q_BIG_ENDIAN
-        if (red_mask == 0xff00 && blue_mask == 0xff000000) {
-            *rgbSwap = true;
-            return QImage::Format_RGBX8888;
-        }
-#endif
-        break;
-    case 16:
-        if (blue_mask == 0x1f)
-            return QImage::Format_RGB16;
-        if (red_mask == 0x1f) {
-            if (rgbSwap)
-                *rgbSwap = true;
-            return QImage::Format_RGB16;
-        }
-        break;
-    case 15:
-        if (blue_mask == 0x1f)
-            return QImage::Format_RGB555;
-        if (red_mask == 0x1f) {
-            if (rgbSwap)
-                *rgbSwap = true;
-            return QImage::Format_RGB555;
-        }
-        break;
-#if QT_CONFIG(xcb_native_painting)
-    case 8:
-        if (QXcbIntegration::instance() && QXcbIntegration::instance()->nativePaintingEnabled())
-            return QImage::Format_Indexed8;
-        break;
-#endif
-    default:
-        break;
-    }
-    qWarning("Unsupported screen format: depth: %d, red_mask: %x, blue_mask: %x", m_depth, red_mask, blue_mask);
 
     switch (m_depth) {
     case 32:
     case 24:
         qWarning("Using RGB32 fallback, if this works your X11 server is reporting a bad screen format.");
-        return QImage::Format_RGB32;
+        m_imageFormat = QImage::Format_RGB32;
+        break;
     case 16:
         qWarning("Using RGB16 fallback, if this works your X11 server is reporting a bad screen format.");
-        return QImage::Format_RGB16;
+        m_imageFormat = QImage::Format_RGB16;
     default:
         break;
     }
-
-    return QImage::Format_Invalid;
 }
 
 static inline bool positionIncludesFrame(QWindow *w)
@@ -410,7 +325,7 @@ void QXcbWindow::create()
         }
         if (!visual)
             visual = platformScreen->visualForId(m_visualId);
-        m_imageFormat = imageFormatForVisual(visual, &m_imageRgbSwap);
+        setImageFormatForVisual(visual);
         connection()->addWindowEventListener(m_window, this);
         return;
     }
@@ -480,7 +395,7 @@ void QXcbWindow::create()
 
     m_visualId = visual->visual_id;
     m_depth = platformScreen->depthOfVisual(m_visualId);
-    m_imageFormat = imageFormatForVisual(visual, &m_imageRgbSwap);
+    setImageFormatForVisual(visual);
 
     quint32 mask = XCB_CW_BACK_PIXMAP
                  | XCB_CW_BORDER_PIXEL

src/plugins/platforms/xcb/qxcbwindow.h

@@ -188,7 +188,7 @@ public Q_SLOTS:
 protected:
     virtual void resolveFormat(const QSurfaceFormat &format) { m_format = format; }
     virtual const xcb_visualtype_t *createVisual();
-    QImage::Format imageFormatForVisual(const xcb_visualtype_t *visual, bool *rgbSwap) const;
+    void setImageFormatForVisual(const xcb_visualtype_t *visual);
 
     QXcbScreen *parentScreen();