Remove tracking of the current fbo

When binding an FBO directly via glBindFramebuffer, the QOpenGLContext's
internal current_fbo, that is maintained by QOpenGLFramebufferObject,
becomes out of sync. This will lead to QOpenGLFramebufferObjects thinking
they are still bound.

Such state tracking should be avoided since it is becoming increasingly
difficult to keep it consistent between the various OpenGL API wrappers
and will never be robust enough when the application changes the state by
directly calling OpenGL functions.

current_fbo is now removed in QtGui. QtOpenGL is not touched.

Change-Id: Id809aab1306c9486d1e2ba3bb5aa93593659e920
Reviewed-by: Gunnar Sletta <gunnar.sletta@jollamobile.com>

3 files changed, 20 insertions, 31 deletions

diff --git a/src/gui/kernel/qopenglcontext_p.h b/src/gui/kernel/qopenglcontext_p.h
index a7525c0e54..77e3605a4b 100644
--- a/src/gui/kernel/qopenglcontext_p.h
+++ b/src/gui/kernel/qopenglcontext_p.h
@@ -205,7 +205,6 @@ public:
         , surface(0)
         , functions(0)
         , textureFunctions(0)
-        , current_fbo(0)
         , max_texture_size(-1)
         , workaround_brokenFBOReadBack(false)
         , workaround_brokenTexSubImage(false)
@@ -236,7 +235,6 @@ public:
     mutable QSet<QByteArray> extensionNames;
     QOpenGLTextureHelper* textureFunctions;
 
-    GLuint current_fbo;
     GLint max_texture_size;
 
     bool workaround_brokenFBOReadBack;
diff --git a/src/gui/opengl/qopenglframebufferobject.cpp b/src/gui/opengl/qopenglframebufferobject.cpp
index 7fb6815120..9ba429e53a 100644
--- a/src/gui/opengl/qopenglframebufferobject.cpp
+++ b/src/gui/opengl/qopenglframebufferobject.cpp
@@ -475,7 +475,6 @@ void QOpenGLFramebufferObjectPrivate::init(QOpenGLFramebufferObject *, const QSi
 
     initAttachments(ctx, attachment);
 
-    funcs.glBindFramebuffer(GL_FRAMEBUFFER, ctx->d_func()->current_fbo);
     if (valid) {
         fbo_guard = new QOpenGLSharedResourceGuard(ctx, fbo, freeFramebufferFunc);
     } else {
@@ -959,8 +958,6 @@ bool QOpenGLFramebufferObject::bind()
         d->valid = d->checkFramebufferStatus(current);
     else
         d->initTexture(d->format.textureTarget(), d->format.internalTextureFormat(), d->size, d->format.mipmap());
-    if (d->valid && current)
-        current->d_func()->current_fbo = d->fbo();
     return d->valid;
 }
 
@@ -988,10 +985,8 @@ bool QOpenGLFramebufferObject::release()
         qWarning("QOpenGLFramebufferObject::release() called from incompatible context");
 #endif
 
-    if (current) {
-        current->d_func()->current_fbo = current->defaultFramebufferObject();
-        d->funcs.glBindFramebuffer(GL_FRAMEBUFFER, current->d_func()->current_fbo);
-    }
+    if (current)
+        d->funcs.glBindFramebuffer(GL_FRAMEBUFFER, current->defaultFramebufferObject());
 
     return true;
 }
@@ -1038,7 +1033,7 @@ GLuint QOpenGLFramebufferObject::takeTexture()
     GLuint id = 0;
     if (isValid() && d->texture_guard) {
         QOpenGLContext *current = QOpenGLContext::currentContext();
-        if (current && current->shareGroup() == d->fbo_guard->group() && current->d_func()->current_fbo == d->fbo())
+        if (current && current->shareGroup() == d->fbo_guard->group() && isBound())
             release();
         id = d->texture_guard->id();
         // Do not call free() on texture_guard, just null it out.
@@ -1154,16 +1149,13 @@ QImage QOpenGLFramebufferObject::toImage() const
 bool QOpenGLFramebufferObject::bindDefault()
 {
     QOpenGLContext *ctx = const_cast<QOpenGLContext *>(QOpenGLContext::currentContext());
-    QOpenGLFunctions functions(ctx);
 
-    if (ctx) {
-        ctx->d_func()->current_fbo = ctx->defaultFramebufferObject();
-        functions.glBindFramebuffer(GL_FRAMEBUFFER, ctx->d_func()->current_fbo);
+    if (ctx)
+        ctx->functions()->glBindFramebuffer(GL_FRAMEBUFFER, ctx->defaultFramebufferObject());
 #ifdef QT_DEBUG
-    } else {
+    else
         qWarning("QOpenGLFramebufferObject::bindDefault() called without current context.");
 #endif
-    }
 
     return ctx != 0;
 }
@@ -1176,7 +1168,7 @@ bool QOpenGLFramebufferObject::bindDefault()
 */
 bool QOpenGLFramebufferObject::hasOpenGLFramebufferObjects()
 {
-    return QOpenGLFunctions(QOpenGLContext::currentContext()).hasOpenGLFeature(QOpenGLFunctions::Framebuffers);
+    return QOpenGLContext::currentContext()->functions()->hasOpenGLFeature(QOpenGLFunctions::Framebuffers);
 }
 
 /*!
@@ -1227,20 +1219,21 @@ void QOpenGLFramebufferObject::setAttachment(QOpenGLFramebufferObject::Attachmen
 #endif
     d->funcs.glBindFramebuffer(GL_FRAMEBUFFER, d->fbo());
     d->initAttachments(current, attachment);
-    if (current->d_func()->current_fbo != d->fbo())
-        d->funcs.glBindFramebuffer(GL_FRAMEBUFFER, current->d_func()->current_fbo);
 }
 
 /*!
-    Returns \c true if the framebuffer object is currently bound to a context,
+    Returns \c true if the framebuffer object is currently bound to the current context,
     otherwise false is returned.
 */
-
 bool QOpenGLFramebufferObject::isBound() const
 {
     Q_D(const QOpenGLFramebufferObject);
-    QOpenGLContext *current = QOpenGLContext::currentContext();
-    return current ? current->d_func()->current_fbo == d->fbo() : false;
+    QOpenGLContext *ctx = QOpenGLContext::currentContext();
+    if (!ctx)
+        return false;
+    GLint fbo = 0;
+    ctx->functions()->glGetIntegerv(GL_FRAMEBUFFER_BINDING, &fbo);
+    return GLuint(fbo) == d->fbo();
 }
 
 /*!
@@ -1346,8 +1339,6 @@ void QOpenGLFramebufferObject::blitFramebuffer(QOpenGLFramebufferObject *target,
     extensions.glBlitFramebuffer(sx0, sy0, sx1, sy1,
                                  tx0, ty0, tx1, ty1,
                                  buffers, filter);
-
-    extensions.glBindFramebuffer(GL_FRAMEBUFFER, ctx->d_func()->current_fbo);
 }
 
 QT_END_NAMESPACE
diff --git a/src/opengl/qglpixelbuffer.cpp b/src/opengl/qglpixelbuffer.cpp
index 18dbf4bdbb..4205761472 100644
--- a/src/opengl/qglpixelbuffer.cpp
+++ b/src/opengl/qglpixelbuffer.cpp
@@ -338,7 +338,7 @@ void QGLPixelBuffer::updateDynamicTexture(GLuint texture_id) const
     if (d->invalid || !d->fbo)
         return;
 
-    QOpenGLContext *ctx = QOpenGLContext::currentContext();
+    const QGLContext *ctx = QGLContext::currentContext();
     if (!ctx)
         return;
 
@@ -352,19 +352,19 @@ void QGLPixelBuffer::updateDynamicTexture(GLuint texture_id) const
 #define GL_DRAW_FRAMEBUFFER 0x8CA9
 #endif
 
-    QOpenGLExtensions extensions(ctx);
+    QOpenGLExtensions extensions(ctx->contextHandle());
 
     if (d->blit_fbo) {
         QOpenGLFramebufferObject::blitFramebuffer(d->blit_fbo, d->fbo);
         extensions.glBindFramebuffer(GL_READ_FRAMEBUFFER, d->blit_fbo->handle());
     }
 
-    ctx->functions()->glBindTexture(GL_TEXTURE_2D, texture_id);
+    extensions.glBindTexture(GL_TEXTURE_2D, texture_id);
 #ifndef QT_OPENGL_ES
-    GLenum format = ctx->isOpenGLES() ? GL_RGBA : GL_RGBA8;
-    ctx->functions()->glCopyTexImage2D(GL_TEXTURE_2D, 0, format, 0, 0, d->req_size.width(), d->req_size.height(), 0);
+    GLenum format = ctx->contextHandle()->isOpenGLES() ? GL_RGBA : GL_RGBA8;
+    extensions.glCopyTexImage2D(GL_TEXTURE_2D, 0, format, 0, 0, d->req_size.width(), d->req_size.height(), 0);
 #else
-    ctx->functions()->glCopyTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 0, 0, d->req_size.width(), d->req_size.height(), 0);
+    extensions.glCopyTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 0, 0, d->req_size.width(), d->req_size.height(), 0);
 #endif
 
     if (d->blit_fbo)