Switch to RGB(A|X)8888[_Premultiplied] for QOpenGLFBO readbacks on GLES

Instead of the never-ending blacklisting of "broken" drivers, simply switch
to always choosing a byte ordered QImage format with OpenGL ES, and keep
on using the (one and only) spec-mandated GL_RGBA/GL_UNSIGNED_BYTE combo.

There is nothing broken with not supporting BGRA for glReadPixels even when
GL_EXT_read_format_bgra (an out of date, pre-2.0 extension that got folded
into the spec to begin with) is present. We do not have a good way to tell
if BGRA_EXT is supported for glReadPixels or not, so just skip the whole
problem altogether.

Task-number: QTBUG-59283
Task-number: QTBUG-59303
Change-Id: I9f0605380923bd3b3ffdeb80f5c172d3e4cc7927
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

2 files changed, 12 insertions, 35 deletions

diff --git a/src/gui/opengl/qopenglframebufferobject.cpp b/src/gui/opengl/qopenglframebufferobject.cpp
index 98ff49ea31..b56bcd0866 100644
--- a/src/gui/opengl/qopenglframebufferobject.cpp
+++ b/src/gui/opengl/qopenglframebufferobject.cpp
@@ -1282,35 +1282,11 @@ static inline QImage qt_gl_read_framebuffer_rgba8(const QSize &size, bool includ
         return img;
     }
 
-#if Q_BYTE_ORDER == Q_LITTLE_ENDIAN
-    // Without GL_UNSIGNED_INT_8_8_8_8_REV, GL_BGRA only makes sense on little endian.
-    const bool has_bgra_ext = context->isOpenGLES()
-                              ? context->hasExtension(QByteArrayLiteral("GL_EXT_read_format_bgra"))
-                              : context->hasExtension(QByteArrayLiteral("GL_EXT_bgra"));
-
-#ifndef Q_OS_IOS
-    const char *renderer = reinterpret_cast<const char *>(funcs->glGetString(GL_RENDERER));
-    const char *ver = reinterpret_cast<const char *>(funcs->glGetString(GL_VERSION));
-
-    // Blacklist GPU chipsets that have problems with their BGRA support.
-    const bool blackListed = (qstrcmp(renderer, "PowerVR Rogue G6200") == 0
-                             && ::strstr(ver, "1.3") != 0) ||
-                             (qstrcmp(renderer, "Mali-T760") == 0
-                             && ::strstr(ver, "3.1") != 0) ||
-                             (qstrcmp(renderer, "Mali-T720") == 0
-                             && ::strstr(ver, "3.1") != 0) ||
-                             qstrcmp(renderer, "PowerVR SGX 554") == 0;
-#else
-    const bool blackListed = true;
-#endif
-    const bool supports_bgra = has_bgra_ext && !blackListed;
+    // For OpenGL ES stick with the byte ordered format / RGBA readback format
+    // since that is the only spec mandated way. (also, skip the
+    // GL_IMPLEMENTATION_COLOR_READ_FORMAT mess since there is nothing saying a
+    // BGRA capable impl would return BGRA from there)
 
-    if (supports_bgra) {
-        QImage img(size, include_alpha ? QImage::Format_ARGB32_Premultiplied : QImage::Format_RGB32);
-        funcs->glReadPixels(0, 0, w, h, GL_BGRA, GL_UNSIGNED_BYTE, img.bits());
-        return img;
-    }
-#endif
     QImage rgbaImage(size, include_alpha ? QImage::Format_RGBA8888_Premultiplied : QImage::Format_RGBX8888);
     funcs->glReadPixels(0, 0, w, h, GL_RGBA, GL_UNSIGNED_BYTE, rgbaImage.bits());
     return rgbaImage;
@@ -1362,8 +1338,11 @@ Q_GUI_EXPORT QImage qt_gl_read_framebuffer(const QSize &size, bool alpha_format,
     If used together with QOpenGLPaintDevice, \a flipped should be the opposite of the value
     of QOpenGLPaintDevice::paintFlipped().
 
-    The returned image has a format of premultiplied ARGB32 or RGB32. The latter is used
-    only when internalTextureFormat() is set to \c GL_RGB.
+    The returned image has a format of premultiplied ARGB32 or RGB32. The latter
+    is used only when internalTextureFormat() is set to \c GL_RGB. Since Qt 5.2
+    the function will fall back to premultiplied RGBA8888 or RGBx8888 when
+    reading to (A)RGB32 is not supported, and this includes OpenGL ES. Since Qt
+    5.4 an A2BGR30 image is returned if the internal format is RGB10_A2.
 
     If the rendering in the framebuffer was not done with premultiplied alpha in mind,
     create a wrapper QImage with a non-premultiplied format. This is necessary before
@@ -1376,10 +1355,6 @@ Q_GUI_EXPORT QImage qt_gl_read_framebuffer(const QSize &size, bool alpha_format,
     QImage image(fboImage.constBits(), fboImage.width(), fboImage.height(), QImage::Format_ARGB32);
     \endcode
 
-    Since Qt 5.2 the function will fall back to premultiplied RGBA8888 or RGBx8888 when
-    reading to (A)RGB32 is not supported. Since 5.4 an A2BGR30 image is returned if the
-    internal format is RGB10_A2.
-
     For multisampled framebuffer objects the samples are resolved using the
     \c{GL_EXT_framebuffer_blit} extension. If the extension is not available, the contents
     of the returned image is undefined.
diff --git a/tests/auto/opengl/qgl/tst_qgl.cpp b/tests/auto/opengl/qgl/tst_qgl.cpp
index cf92c9fab6..7dfa8e4e22 100644
--- a/tests/auto/opengl/qgl/tst_qgl.cpp
+++ b/tests/auto/opengl/qgl/tst_qgl.cpp
@@ -1195,7 +1195,9 @@ void tst_QGL::currentFboSync()
 
         QGLFramebufferObject::bindDefault();
 
-        QCOMPARE(fbo1.toImage(), fbo2Image);
+        // Convert the QGLFBO's result since QOpenGLFBO uses a wider
+        // variety of possible return formats.
+        QCOMPARE(fbo1.toImage().convertToFormat(fbo2Image.format()), fbo2Image);
     }
 
     {