From a4428d480b96d51f9979d45044f26c99fa82f465 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20R=C3=B8dal?= Date: Fri, 31 Oct 2014 16:56:25 +0100 Subject: 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 --- src/gui/kernel/qopenglcontext_p.h | 3 ++ src/gui/opengl/qopenglframebufferobject.cpp | 16 +++++- .../gl2paintengineex/qtextureglyphcache_gl.cpp | 2 + src/opengl/qgl.cpp | 29 ++++++++++ src/opengl/qgl_p.h | 2 + src/opengl/qglframebufferobject.cpp | 17 ++++-- src/opengl/qglpaintdevice.cpp | 13 +++-- src/opengl/qglpixelbuffer.cpp | 2 + tests/auto/opengl/qgl/tst_qgl.cpp | 63 ++++++++++++++++++++++ 9 files changed, 138 insertions(+), 9 deletions(-) diff --git a/src/gui/kernel/qopenglcontext_p.h b/src/gui/kernel/qopenglcontext_p.h index d5a3126176..975553e7cd 100644 --- a/src/gui/kernel/qopenglcontext_p.h +++ b/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); diff --git a/src/gui/opengl/qopenglframebufferobject.cpp b/src/gui/opengl/qopenglframebufferobject.cpp index b185e332e6..83cfaf8f93 100644 --- a/src/gui/opengl/qopenglframebufferobject.cpp +++ b/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::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); } diff --git a/src/opengl/gl2paintengineex/qtextureglyphcache_gl.cpp b/src/opengl/gl2paintengineex/qtextureglyphcache_gl.cpp index 211fad267f..8cd26f1ea4 100644 --- a/src/opengl/gl2paintengineex/qtextureglyphcache_gl.cpp +++ b/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; diff --git a/src/opengl/qgl.cpp b/src/opengl/qgl.cpp index 1e49d95087..35f08e0092 100644 --- a/src/opengl/qgl.cpp +++ b/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, ¤t); + + 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 diff --git a/src/opengl/qgl_p.h b/src/opengl/qgl_p.h index c4151a3d34..4cf656fd86 100644 --- a/src/opengl/qgl_p.h +++ b/src/opengl/qgl_p.h @@ -238,6 +238,8 @@ public: bool ownContext; void setupSharing(); + void refreshCurrentFbo(); + void setCurrentFbo(GLuint fbo); QGLFormat glFormat; QGLFormat reqFormat; diff --git a/src/opengl/qglframebufferobject.cpp b/src/opengl/qglframebufferobject.cpp index 4ef50e9334..49b28c36b9 100644 --- a/src/opengl/qglframebufferobject.cpp +++ b/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); diff --git a/src/opengl/qglpaintdevice.cpp b/src/opengl/qglpaintdevice.cpp index 40cc7bb71d..c07d0a761b 100644 --- a/src/opengl/qglpaintdevice.cpp +++ b/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); } diff --git a/src/opengl/qglpixelbuffer.cpp b/src/opengl/qglpixelbuffer.cpp index 56bfaebe4f..63b624aea2 100644 --- a/src/opengl/qglpixelbuffer.cpp +++ b/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()); diff --git a/tests/auto/opengl/qgl/tst_qgl.cpp b/tests/auto/opengl/qgl/tst_qgl.cpp index 0ba464a82c..d8ad3e1470 100644 --- a/tests/auto/opengl/qgl/tst_qgl.cpp +++ b/tests/auto/opengl/qgl/tst_qgl.cpp @@ -42,6 +42,8 @@ #include #include #include +#include +#include #include #include @@ -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 -- cgit v1.2.3