From a4428d480b96d51f9979d45044f26c99fa82f465 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Samuel=20R=C3=B8dal?= <srodal@gmail.com>
Date: Fri, 31 Oct 2014 16:56:25 +0100
Subject: [PATCH] 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 | 3 +
src/gui/opengl/qopenglframebufferobject.cpp | 16 ++++-
.../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 d5a31261766..975553e7cda 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 b185e332e68..83cfaf8f933 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 *>(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 211fad267fb..8cd26f1ea4f 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 1e49d950879..35f08e00929 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 c4151a3d344..4cf656fd86f 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 4ef50e9334d..49b28c36b94 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 40cc7bb71d6..c07d0a761bb 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 56bfaebe4f9..63b624aea2b 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 0ba464a82c8..d8ad3e1470c 100644
--- a/tests/auto/opengl/qgl/tst_qgl.cpp
+++ b/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
--
GitLab