Fix incorrect FBO bindings with QOpenGLWidget

QOpenGLContext::defaultFramebufferObject() knows nothing about QOpenGLWidget
and QQuickWidget. The problem is that this function (and others that rely on it)
is expected to give the widget's backing FBO in paintGL() and friends.

To overcome this, we have to provide a way for such widgets that indicate what is
the expected "default fbo".

Task-number: QTBUG-43269
Change-Id: I43f439f8609382b9f7004707ab0ef9f091952b4f
Reviewed-by: Giuseppe D'Angelo <giuseppe.dangelo@kdab.com>
Reviewed-by: Gunnar Sletta <gunnar@sletta.org>

4 files changed, 54 insertions, 4 deletions

diff --git a/src/gui/kernel/qopenglcontext.cpp b/src/gui/kernel/qopenglcontext.cpp
index 7fc9cceb60..56a3729a4a 100644
--- a/src/gui/kernel/qopenglcontext.cpp
+++ b/src/gui/kernel/qopenglcontext.cpp
@@ -860,15 +860,26 @@ bool QOpenGLContext::hasExtension(const QByteArray &extension) const
 /*!
     Call this to get the default framebuffer object for the current surface.
 
-    On some platforms the default framebuffer object depends on the surface
-    being rendered to, and might be different from 0. Thus, instead of calling
-    glBindFramebuffer(0), you should call
+    On some platforms (for instance, iOS) the default framebuffer object depends
+    on the surface being rendered to, and might be different from 0. Thus,
+    instead of calling glBindFramebuffer(0), you should call
     glBindFramebuffer(ctx->defaultFramebufferObject()) if you want your
     application to work across different Qt platforms.
 
     If you use the glBindFramebuffer() in QOpenGLFunctions you do not have to
     worry about this, as it automatically binds the current context's
     defaultFramebufferObject() when 0 is passed.
+
+    \note Widgets that render via framebuffer objects, like QOpenGLWidget and
+    QQuickWidget, will override the value returned from this function when
+    painting is active, because at that time the correct "default" framebuffer
+    is the widget's associated backing framebuffer, not the platform-specific
+    one belonging to the top-level window's surface. This ensures the expected
+    behavior for this function and other classes relying on it (for example,
+    QOpenGLFramebufferObject::bindDefault() or
+    QOpenGLFramebufferObject::release()).
+
+    \sa QOpenGLFramebufferObject
 */
 GLuint QOpenGLContext::defaultFramebufferObject() const
 {
@@ -879,6 +890,9 @@ GLuint QOpenGLContext::defaultFramebufferObject() const
     if (!d->surface || !d->surface->surfaceHandle())
         return 0;
 
+    if (d->defaultFboRedirect)
+        return d->defaultFboRedirect;
+
     return d->platformGLContext->defaultFramebufferObject(d->surface->surfaceHandle());
 }
 
diff --git a/src/gui/kernel/qopenglcontext_p.h b/src/gui/kernel/qopenglcontext_p.h
index 5004ed6a8e..f9f3ce2c5f 100644
--- a/src/gui/kernel/qopenglcontext_p.h
+++ b/src/gui/kernel/qopenglcontext_p.h
@@ -204,6 +204,7 @@ public:
         , workaround_missingPrecisionQualifiers(false)
         , active_engine(0)
         , qgl_current_fbo_invalid(false)
+        , defaultFboRedirect(0)
     {
         requestedFormat = QSurfaceFormat::defaultFormat();
     }
@@ -242,6 +243,7 @@ public:
     bool qgl_current_fbo_invalid;
 
     QVariant nativeHandle;
+    GLuint defaultFboRedirect;
 
     static QOpenGLContext *setCurrentContext(QOpenGLContext *context);
 
diff --git a/src/widgets/kernel/qopenglwidget.cpp b/src/widgets/kernel/qopenglwidget.cpp
index c6c5e5bc80..d4d23604a3 100644
--- a/src/widgets/kernel/qopenglwidget.cpp
+++ b/src/widgets/kernel/qopenglwidget.cpp
@@ -46,6 +46,7 @@
 #include <QtGui/private/qopenglextensions_p.h>
 #include <QtGui/private/qfont_p.h>
 #include <QtGui/private/qopenglpaintdevice_p.h>
+#include <QtGui/private/qopenglcontext_p.h>
 #include <QtWidgets/private/qwidget_p.h>
 
 QT_BEGIN_NAMESPACE
@@ -792,11 +793,18 @@ void QOpenGLWidgetPrivate::resolveSamples()
 void QOpenGLWidgetPrivate::invokeUserPaint()
 {
     Q_Q(QOpenGLWidget);
-    QOpenGLFunctions *f = QOpenGLContext::currentContext()->functions();
+
+    QOpenGLContext *ctx = QOpenGLContext::currentContext();
+    Q_ASSERT(ctx && fbo);
+
+    QOpenGLFunctions *f = ctx->functions();
+    QOpenGLContextPrivate::get(ctx)->defaultFboRedirect = fbo->handle();
 
     f->glViewport(0, 0, q->width() * q->devicePixelRatio(), q->height() * q->devicePixelRatio());
     q->paintGL();
     flushPending = true;
+
+    QOpenGLContextPrivate::get(ctx)->defaultFboRedirect = 0;
 }
 
 void QOpenGLWidgetPrivate::render()
diff --git a/tests/auto/widgets/widgets/qopenglwidget/tst_qopenglwidget.cpp b/tests/auto/widgets/widgets/qopenglwidget/tst_qopenglwidget.cpp
index 0c0c50ac64..e9f9c67856 100644
--- a/tests/auto/widgets/widgets/qopenglwidget/tst_qopenglwidget.cpp
+++ b/tests/auto/widgets/widgets/qopenglwidget/tst_qopenglwidget.cpp
@@ -56,6 +56,7 @@ private slots:
     void reparentToNotYetCreated();
     void asViewport();
     void requestUpdate();
+    void fboRedirect();
 };
 
 void tst_QOpenGLWidget::create()
@@ -329,6 +330,31 @@ void tst_QOpenGLWidget::requestUpdate()
     QTRY_VERIFY(w.m_count > 0);
 }
 
+class FboCheckWidget : public QOpenGLWidget
+{
+public:
+    void paintGL() Q_DECL_OVERRIDE {
+        GLuint reportedDefaultFbo = QOpenGLContext::currentContext()->defaultFramebufferObject();
+        GLuint expectedDefaultFbo = defaultFramebufferObject();
+        QCOMPARE(reportedDefaultFbo, expectedDefaultFbo);
+    }
+};
+
+void tst_QOpenGLWidget::fboRedirect()
+{
+    FboCheckWidget w;
+    w.resize(640, 480);
+    w.show();
+    QTest::qWaitForWindowExposed(&w);
+
+    // Unlike in paintGL(), the default fbo reported by the context is not affected by the widget,
+    // so we get the real default fbo: either 0 or (on iOS) the fbo associated with the window.
+    w.makeCurrent();
+    GLuint reportedDefaultFbo = QOpenGLContext::currentContext()->defaultFramebufferObject();
+    GLuint widgetFbo = w.defaultFramebufferObject();
+    QVERIFY(reportedDefaultFbo != widgetFbo);
+}
+
 QTEST_MAIN(tst_QOpenGLWidget)
 
 #include "tst_qopenglwidget.moc"