From dd385741605df2dd0cd977ebd8cd49cbf5d90271 Mon Sep 17 00:00:00 2001 From: Laszlo Agocs Date: Fri, 17 Mar 2017 15:23:37 +0100 Subject: [PATCH] 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 --- src/gui/opengl/qopenglframebufferobject.cpp | 43 +++++---------------- tests/auto/opengl/qgl/tst_qgl.cpp | 4 +- 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")); + // 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) -#ifndef Q_OS_IOS - const char *renderer = reinterpret_cast(funcs->glGetString(GL_RENDERER)); - const char *ver = reinterpret_cast(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; - - 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); } {