Fix for current_fbo getting out of sync in QtOpenGL

When using QGLWidget in combination with QOpenGLFramebufferObject from
QtGui, instead of QGLFramebufferObject from QtOpenGL, the current_fbo
variable doesn't get updated when framebuffer object bindings change.

To ensure that the QGLWidget correctly releases the currently bound
framebuffer object when using a QPainter, we keep track of whether
QOpenGLFramebufferObject has modified the current FBO binding, and if
that's the case we need to read the OpenGL state directly instead of
relying on a cached value.

Change-Id: If7e0bd936e202cad07365b5ce641ee01d2251930
Reviewed-by: Laszlo Agocs <laszlo.agocs@digia.com>

src/gui/kernel/qopenglcontext_p.h

@@ -203,6 +203,7 @@ public:
         , workaround_brokenTexSubImage(false)
         , workaround_missingPrecisionQualifiers(false)
         , active_engine(0)
+        , qgl_current_fbo_invalid(false)
     {
         requestedFormat = QSurfaceFormat::defaultFormat();
     }
@@ -237,6 +238,8 @@ public:
 
     QPaintEngineEx *active_engine;
 
+    bool qgl_current_fbo_invalid;
+
     QVariant nativeHandle;
 
     static QOpenGLContext *setCurrentContext(QOpenGLContext *context);

src/gui/opengl/qopenglframebufferobject.cpp

@@ -469,6 +469,8 @@ void QOpenGLFramebufferObjectPrivate::init(QOpenGLFramebufferObject *, const QSi
     funcs.glGenFramebuffers(1, &fbo);
     funcs.glBindFramebuffer(GL_FRAMEBUFFER, fbo);
 
+    QOpenGLContextPrivate::get(ctx)->qgl_current_fbo_invalid = true;
+
     GLuint color_buffer = 0;
 
     QT_CHECK_GLERROR();
@@ -997,7 +999,11 @@ bool QOpenGLFramebufferObject::bind()
     if (current->shareGroup() != d->fbo_guard->group())
         qWarning("QOpenGLFramebufferObject::bind() called from incompatible context");
 #endif
+
     d->funcs.glBindFramebuffer(GL_FRAMEBUFFER, d->fbo());
+
+    QOpenGLContextPrivate::get(current)->qgl_current_fbo_invalid = true;
+
     if (d->texture_guard || d->format.samples() != 0)
         d->valid = d->checkFramebufferStatus(current);
     else
@@ -1029,9 +1035,12 @@ bool QOpenGLFramebufferObject::release()
         qWarning("QOpenGLFramebufferObject::release() called from incompatible context");
 #endif
 
-    if (current)
+    if (current) {
         d->funcs.glBindFramebuffer(GL_FRAMEBUFFER, current->defaultFramebufferObject());
 
+        QOpenGLContextPrivate::get(current)->qgl_current_fbo_invalid = true;
+    }
+
     return true;
 }
 
@@ -1272,8 +1281,10 @@ bool QOpenGLFramebufferObject::bindDefault()
 {
     QOpenGLContext *ctx = const_cast<QOpenGLContext *>(QOpenGLContext::currentContext());
 
-    if (ctx)
+    if (ctx) {
         ctx->functions()->glBindFramebuffer(GL_FRAMEBUFFER, ctx->defaultFramebufferObject());
+        QOpenGLContextPrivate::get(ctx)->qgl_current_fbo_invalid = true;
+    }
 #ifdef QT_DEBUG
     else
         qWarning("QOpenGLFramebufferObject::bindDefault() called without current context.");
@@ -1342,6 +1353,7 @@ void QOpenGLFramebufferObject::setAttachment(QOpenGLFramebufferObject::Attachmen
         qWarning("QOpenGLFramebufferObject::setAttachment() called from incompatible context");
 #endif
     d->funcs.glBindFramebuffer(GL_FRAMEBUFFER, d->fbo());
+    QOpenGLContextPrivate::get(current)->qgl_current_fbo_invalid = true;
     d->initAttachments(current, attachment);
 }
 

src/opengl/gl2paintengineex/qtextureglyphcache_gl.cpp

@@ -167,6 +167,8 @@ void QGLTextureGlyphCache::resizeTextureData(int width, int height)
     // ### the QTextureGlyphCache API needs to be reworked to allow
     // ### resizeTextureData to fail
 
+    ctx->d_ptr->refreshCurrentFbo();
+
     funcs->glBindFramebuffer(GL_FRAMEBUFFER, m_textureResource->m_fbo);
 
     GLuint tmp_texture;

src/opengl/qgl.cpp

@@ -510,6 +510,35 @@ void QGLContextPrivate::setupSharing() {
     }
 }
 
+void QGLContextPrivate::refreshCurrentFbo()
+{
+    QOpenGLContextPrivate *guiGlContextPrivate =
+        guiGlContext ? QOpenGLContextPrivate::get(guiGlContext) : 0;
+
+    // if QOpenGLFramebufferObjects have been used in the mean-time, we've lost our cached value
+    if (guiGlContextPrivate && guiGlContextPrivate->qgl_current_fbo_invalid) {
+        GLint current;
+        QOpenGLFunctions *funcs = qgl_functions();
+        funcs->glGetIntegerv(GL_FRAMEBUFFER_BINDING, &current);
+
+        current_fbo = current;
+
+        guiGlContextPrivate->qgl_current_fbo_invalid = false;
+    }
+}
+
+void QGLContextPrivate::setCurrentFbo(GLuint fbo)
+{
+    current_fbo = fbo;
+
+    QOpenGLContextPrivate *guiGlContextPrivate =
+        guiGlContext ? QOpenGLContextPrivate::get(guiGlContext) : 0;
+
+    if (guiGlContextPrivate)
+        guiGlContextPrivate->qgl_current_fbo_invalid = false;
+}
+
+
 /*!
     \fn bool QGLFormat::doubleBuffer() const
 

src/opengl/qgl_p.h

@@ -238,6 +238,8 @@ public:
     bool ownContext;
 
     void setupSharing();
+    void refreshCurrentFbo();
+    void setCurrentFbo(GLuint fbo);
 
     QGLFormat glFormat;
     QGLFormat reqFormat;

src/opengl/qglframebufferobject.cpp

@@ -472,6 +472,8 @@ void QGLFramebufferObjectPrivate::init(QGLFramebufferObject *q, const QSize &sz,
     if (!funcs.hasOpenGLFeature(QOpenGLFunctions::Framebuffers))
         return;
 
+    ctx->d_ptr->refreshCurrentFbo();
+
     size = sz;
     target = texture_target;
     // texture dimensions
@@ -1027,7 +1029,7 @@ bool QGLFramebufferObject::bind()
     d->funcs.glBindFramebuffer(GL_FRAMEBUFFER, d->fbo());
     d->valid = d->checkFramebufferStatus();
     if (d->valid && current)
-        current->d_ptr->current_fbo = d->fbo();
+        current->d_ptr->setCurrentFbo(d->fbo());
     return d->valid;
 }
 
@@ -1060,7 +1062,7 @@ bool QGLFramebufferObject::release()
 #endif
 
     if (current) {
-        current->d_ptr->current_fbo = current->d_ptr->default_fbo;
+        current->d_ptr->setCurrentFbo(current->d_ptr->default_fbo);
         d->funcs.glBindFramebuffer(GL_FRAMEBUFFER, current->d_ptr->default_fbo);
     }
 
@@ -1173,7 +1175,7 @@ bool QGLFramebufferObject::bindDefault()
         if (!functions.hasOpenGLFeature(QOpenGLFunctions::Framebuffers))
             return false;
 
-        ctx->d_ptr->current_fbo = ctx->d_ptr->default_fbo;
+        ctx->d_ptr->setCurrentFbo(ctx->d_ptr->default_fbo);
         functions.glBindFramebuffer(GL_FRAMEBUFFER, ctx->d_ptr->default_fbo);
 #ifdef QT_DEBUG
     } else {
@@ -1320,7 +1322,12 @@ bool QGLFramebufferObject::isBound() const
 {
     Q_D(const QGLFramebufferObject);
     const QGLContext *current = QGLContext::currentContext();
-    return current ? current->d_ptr->current_fbo == d->fbo() : false;
+    if (current) {
+        current->d_ptr->refreshCurrentFbo();
+        return current->d_ptr->current_fbo == d->fbo();
+    }
+
+    return false;
 }
 
 /*!
@@ -1400,6 +1407,8 @@ void QGLFramebufferObject::blitFramebuffer(QGLFramebufferObject *target, const Q
     const int ty0 = th - (targetRect.top() + targetRect.height());
     const int ty1 = th - targetRect.top();
 
+    ctx->d_ptr->refreshCurrentFbo();
+
     functions.glBindFramebuffer(GL_READ_FRAMEBUFFER, source ? source->handle() : 0);
     functions.glBindFramebuffer(GL_DRAW_FRAMEBUFFER, target ? target->handle() : 0);
 

src/opengl/qglpaintdevice.cpp

@@ -74,6 +74,8 @@ void QGLPaintDevice::beginPaint()
     QGLContext *ctx = context();
     ctx->makeCurrent();
 
+    ctx->d_func()->refreshCurrentFbo();
+
     // Record the currently bound FBO so we can restore it again
     // in endPaint() and bind this device's FBO
     //
@@ -85,7 +87,7 @@ void QGLPaintDevice::beginPaint()
     m_previousFBO = ctx->d_func()->current_fbo;
 
     if (m_previousFBO != m_thisFBO) {
-        ctx->d_ptr->current_fbo = m_thisFBO;
+        ctx->d_func()->setCurrentFbo(m_thisFBO);
         ctx->contextHandle()->functions()->glBindFramebuffer(GL_FRAMEBUFFER, m_thisFBO);
     }
 
@@ -102,8 +104,10 @@ void QGLPaintDevice::ensureActiveTarget()
     if (ctx != QGLContext::currentContext())
         ctx->makeCurrent();
 
+    ctx->d_func()->refreshCurrentFbo();
+
     if (ctx->d_ptr->current_fbo != m_thisFBO) {
-        ctx->d_ptr->current_fbo = m_thisFBO;
+        ctx->d_func()->setCurrentFbo(m_thisFBO);
         ctx->contextHandle()->functions()->glBindFramebuffer(GL_FRAMEBUFFER, m_thisFBO);
     }
 
@@ -114,8 +118,11 @@ void QGLPaintDevice::endPaint()
 {
     // Make sure the FBO bound at beginPaint is re-bound again here:
     QGLContext *ctx = context();
+
+    ctx->d_func()->refreshCurrentFbo();
+
     if (m_previousFBO != ctx->d_func()->current_fbo) {
-        ctx->d_ptr->current_fbo = m_previousFBO;
+        ctx->d_func()->setCurrentFbo(m_previousFBO);
         ctx->contextHandle()->functions()->glBindFramebuffer(GL_FRAMEBUFFER, m_previousFBO);
     }
 

src/opengl/qglpixelbuffer.cpp

@@ -344,6 +344,8 @@ void QGLPixelBuffer::updateDynamicTexture(GLuint texture_id) const
 
     QOpenGLExtensions extensions(ctx->contextHandle());
 
+    ctx->d_ptr->refreshCurrentFbo();
+
     if (d->blit_fbo) {
         QOpenGLFramebufferObject::blitFramebuffer(d->blit_fbo, d->fbo);
         extensions.glBindFramebuffer(GL_READ_FRAMEBUFFER, d->blit_fbo->handle());

tests/auto/opengl/qgl/tst_qgl.cpp

@@ -42,6 +42,8 @@
 #include <qglcolormap.h>
 #include <qpaintengine.h>
 #include <qopenglfunctions.h>
+#include <qopenglframebufferobject.h>
+#include <qopenglpaintdevice.h>
 
 #include <QGraphicsView>
 #include <QGraphicsProxyWidget>
@@ -78,6 +80,7 @@ private slots:
     void glWidgetRendering();
     void glFBOSimpleRendering();
     void glFBORendering();
+    void currentFboSync();
     void multipleFBOInterleavedRendering();
     void glFBOUseInGLWidget();
     void glPBufferRendering();
@@ -1138,6 +1141,66 @@ void tst_QGL::glFBORendering()
     qt_opengl_check_test_pattern(fb);
 }
 
+class QOpenGLFramebufferObjectPaintDevice : public QOpenGLPaintDevice
+{
+public:
+    QOpenGLFramebufferObjectPaintDevice(int width, int height)
+        : QOpenGLPaintDevice(width, height)
+        , m_fbo(width, height, QOpenGLFramebufferObject::CombinedDepthStencil)
+    {
+    }
+
+    void ensureActiveTarget()
+    {
+        m_fbo.bind();
+    }
+
+    QImage toImage() const
+    {
+        return m_fbo.toImage();
+    }
+
+private:
+    QOpenGLFramebufferObject m_fbo;
+};
+
+void tst_QGL::currentFboSync()
+{
+    if (!QGLFramebufferObject::hasOpenGLFramebufferObjects())
+        QSKIP("QGLFramebufferObject not supported on this platform");
+
+#if defined(Q_OS_QNX)
+    QSKIP("Reading the QGLFramebufferObject is unsupported on this platform");
+#endif
+
+    QGLWidget glw;
+    glw.makeCurrent();
+
+    {
+        QGLFramebufferObject fbo1(256, 256, QGLFramebufferObject::CombinedDepthStencil);
+
+        QOpenGLFramebufferObjectPaintDevice fbo2(256, 256);
+
+        QImage sourceImage(256, 256, QImage::Format_ARGB32_Premultiplied);
+        QPainter sourcePainter(&sourceImage);
+        qt_opengl_draw_test_pattern(&sourcePainter, 256, 256);
+
+        QPainter fbo1Painter(&fbo1);
+
+        QPainter fbo2Painter(&fbo2);
+        fbo2Painter.drawImage(0, 0, sourceImage);
+        fbo2Painter.end();
+
+        QImage fbo2Image = fbo2.toImage();
+
+        fbo1Painter.drawImage(0, 0, sourceImage);
+        fbo1Painter.end();
+
+        QGLFramebufferObject::bindDefault();
+
+        QCOMPARE(fbo1.toImage(), fbo2Image);
+    }
+}
 
 // Tests multiple QPainters active on different FBOs at the same time, with
 // interleaving painting. Performance-wise, this is sub-optimal, but it still