Fix QOpenGLWidget on Cocoa when used as viewport

Having a QOpenGLWidget as a graphics view viewport was not functioning
on OS X: it was showing incomplete content due to accessing the texture
attached to the framebuffer object before the rendering is complete.

On the normal path, when rendering is done via paintGL(), the flush was
there. When used as a viewport however, this path is not used. The missing
flush is now added for the other case too. For performance reasons, we will
not flush on every paint engine end(). Instead, the flush is deferred until
composition starts.

QGLWidget also featured a weird on-by-default autoFillBackground concept. To
maintain compatibility with apps that used QGLWidget as the viewport for
QGraphicsView, we will now do the same for QOpenGLWidget, but only when it
is used as a viewport. For regular QOpenGLWidgets autoFillBackground defaults
to false, like for any other widget. The docs are extended with a small section
about differences between QGLWidget and QOpenGLWidget.

Task-number: QTBUG-41046
Change-Id: I42c2033fdd2ef5815783fd640fe11373761061e0
Reviewed-by: Jørgen Lind <jorgen.lind@digia.com>

6 files changed, 103 insertions, 5 deletions

diff --git a/src/gui/opengl/qopenglpaintdevice.cpp b/src/gui/opengl/qopenglpaintdevice.cpp
index 59bca6efdf..c2f3295bc3 100644
--- a/src/gui/opengl/qopenglpaintdevice.cpp
+++ b/src/gui/opengl/qopenglpaintdevice.cpp
@@ -351,14 +351,39 @@ bool QOpenGLPaintDevice::paintFlipped() const
 }
 
 /*!
+  This virtual method is called when starting to paint.
+
+  The default implementation does nothing.
+
+  \sa endPaint()
+ */
+void QOpenGLPaintDevice::beginPaint()
+{
+}
+
+/*!
     This virtual method is provided as a callback to allow re-binding a target
     frame buffer object or context when different QOpenGLPaintDevice instances
     are issuing draw calls alternately.
 
-    QPainter::beginNativePainting will also trigger this method.
+    \l{QPainter::beginNativePainting()}{beginNativePainting()} will also trigger
+    this method.
+
+    The default implementation does nothing.
 */
 void QOpenGLPaintDevice::ensureActiveTarget()
 {
 }
 
+/*!
+  This virtual method is called when the painting has finished.
+
+  The default implementation does nothing.
+
+  \sa beginPaint()
+*/
+void QOpenGLPaintDevice::endPaint()
+{
+}
+
 QT_END_NAMESPACE
diff --git a/src/gui/opengl/qopenglpaintdevice.h b/src/gui/opengl/qopenglpaintdevice.h
index e1be9b525d..d88992d6db 100644
--- a/src/gui/opengl/qopenglpaintdevice.h
+++ b/src/gui/opengl/qopenglpaintdevice.h
@@ -73,7 +73,9 @@ public:
     void setPaintFlipped(bool flipped);
     bool paintFlipped() const;
 
+    virtual void beginPaint();
     virtual void ensureActiveTarget();
+    virtual void endPaint();
 
 protected:
     int metric(QPaintDevice::PaintDeviceMetric metric) const;
diff --git a/src/gui/opengl/qopenglpaintengine.cpp b/src/gui/opengl/qopenglpaintengine.cpp
index 21bc4a95e8..40c836b2bb 100644
--- a/src/gui/opengl/qopenglpaintengine.cpp
+++ b/src/gui/opengl/qopenglpaintengine.cpp
@@ -1994,6 +1994,8 @@ bool QOpenGL2PaintEngineEx::begin(QPaintDevice *pdev)
     d->ctx = QOpenGLContext::currentContext();
     d->ctx->d_func()->active_engine = this;
 
+    d->device->beginPaint();
+
     d->funcs.initializeOpenGLFunctions();
 
     for (int i = 0; i < QT_GL_VERTEX_ARRAY_TRACKED_COUNT; ++i)
@@ -2044,6 +2046,8 @@ bool QOpenGL2PaintEngineEx::end()
 {
     Q_D(QOpenGL2PaintEngineEx);
 
+    d->device->endPaint();
+
     QOpenGLContext *ctx = d->ctx;
     d->funcs.glUseProgram(0);
     d->transferMode(BrushDrawingMode);
diff --git a/src/widgets/kernel/qopenglwidget.cpp b/src/widgets/kernel/qopenglwidget.cpp
index 7782c4c1d4..dd8dd64470 100644
--- a/src/widgets/kernel/qopenglwidget.cpp
+++ b/src/widgets/kernel/qopenglwidget.cpp
@@ -239,6 +239,28 @@ QT_BEGIN_NAMESPACE
   \note Avoid calling winId() on a QOpenGLWidget. This function triggers the creation of
   a native window, resulting in reduced performance and possibly rendering glitches.
 
+  \section1 Differences to QGLWidget
+
+  Besides the main conceptual difference of being backed by a framebuffer object, there
+  are a number of smaller, internal differences between QOpenGLWidget and the older
+  QGLWidget:
+
+  \list
+
+  \li OpenGL state when invoking paintGL(). QOpenGLWidget sets up the viewport via
+  glViewport(). It does not perform any clearing.
+
+  \li Clearing when starting to paint via QPainter. Unlike regular widgets, QGLWidget
+  defaulted to a value of \c true for
+  \l{QWidget::autoFillBackground()}{autoFillBackground}. It then performed clearing to the
+  palette's background color every time QPainter::begin() was used. QOpenGLWidget does not
+  follow this: \l{QWidget::autoFillBackground()}{autoFillBackground} defaults to false,
+  like for any other widget. The only exception is when being used as a viewport for other
+  widgets like QGraphicsView. In such a case autoFillBackground will be automatically set
+  to true to ensure compatibility with QGLWidget-based viewports.
+
+  \endlist
+
   \section1 Multisampling
 
   To enable multisampling, set the number of requested samples on the
@@ -436,6 +458,7 @@ class QOpenGLWidgetPaintDevice : public QOpenGLPaintDevice
 {
 public:
     QOpenGLWidgetPaintDevice(QOpenGLWidget *widget) : w(widget) { }
+    void beginPaint() Q_DECL_OVERRIDE;
     void ensureActiveTarget() Q_DECL_OVERRIDE;
 
 private:
@@ -454,7 +477,8 @@ public:
           initialized(false),
           fakeHidden(false),
           paintDevice(0),
-          inBackingStorePaint(false)
+          inBackingStorePaint(false),
+          flushPending(false)
     {
         requestedFormat = QSurfaceFormat::defaultFormat();
     }
@@ -478,6 +502,7 @@ public:
     void endBackingStorePainting() Q_DECL_OVERRIDE { inBackingStorePaint = false; }
     void beginCompose() Q_DECL_OVERRIDE;
     void endCompose() Q_DECL_OVERRIDE;
+    void initializeViewportFramebuffer() Q_DECL_OVERRIDE;
     void resizeViewportFramebuffer() Q_DECL_OVERRIDE;
     void resolveSamples() Q_DECL_OVERRIDE;
 
@@ -490,8 +515,28 @@ public:
     QOpenGLPaintDevice *paintDevice;
     bool inBackingStorePaint;
     QSurfaceFormat requestedFormat;
+    bool flushPending;
 };
 
+void QOpenGLWidgetPaintDevice::beginPaint()
+{
+    // NB! autoFillBackground is and must be false by default. Otherwise we would clear on
+    // every QPainter begin() which is not desirable. This is only for legacy use cases,
+    // like using QOpenGLWidget as the viewport of a graphics view, that expect clearing
+    // with the palette's background color.
+    if (w->autoFillBackground()) {
+        QOpenGLFunctions *f = QOpenGLContext::currentContext()->functions();
+        if (w->testAttribute(Qt::WA_TranslucentBackground)) {
+            f->glClearColor(0, 0, 0, 0);
+        } else {
+            QColor c = w->palette().brush(w->backgroundRole()).color();
+            float alpha = c.alphaF();
+            f->glClearColor(c.redF() * alpha, c.greenF() * alpha, c.blueF() * alpha, alpha);
+        }
+        f->glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT | GL_STENCIL_BUFFER_BIT);
+    }
+}
+
 void QOpenGLWidgetPaintDevice::ensureActiveTarget()
 {
     QOpenGLWidgetPrivate *d = static_cast<QOpenGLWidgetPrivate *>(QWidgetPrivate::get(w));
@@ -502,6 +547,11 @@ void QOpenGLWidgetPaintDevice::ensureActiveTarget()
         w->makeCurrent();
     else
         d->fbo->bind();
+
+    // When used as a viewport, drawing is done via opening a QPainter on the widget
+    // without going through paintEvent(). We will have to make sure a glFlush() is done
+    // before the texture is accessed also in this case.
+    d->flushPending = true;
 }
 
 GLuint QOpenGLWidgetPrivate::textureId() const
@@ -572,6 +622,11 @@ void QOpenGLWidgetPrivate::recreateFbo()
 void QOpenGLWidgetPrivate::beginCompose()
 {
     Q_Q(QOpenGLWidget);
+    if (flushPending) {
+        flushPending = false;
+        q->makeCurrent();
+        context->functions()->glFlush();
+    }
     emit q->aboutToCompose();
 }
 
@@ -653,9 +708,10 @@ void QOpenGLWidgetPrivate::invokeUserPaint()
 {
     Q_Q(QOpenGLWidget);
     QOpenGLFunctions *f = QOpenGLContext::currentContext()->functions();
-    f->glViewport(0, 0, q->width() * q->devicePixelRatio(), q->height() * q->devicePixelRatio());
 
+    f->glViewport(0, 0, q->width() * q->devicePixelRatio(), q->height() * q->devicePixelRatio());
     q->paintGL();
+    f->glFlush();
 }
 
 void QOpenGLWidgetPrivate::render()
@@ -667,7 +723,6 @@ void QOpenGLWidgetPrivate::render()
 
     q->makeCurrent();
     invokeUserPaint();
-    context->functions()->glFlush();
 }
 
 extern Q_GUI_EXPORT QImage qt_gl_read_framebuffer(const QSize &size, bool alpha_format, bool include_alpha);
@@ -686,6 +741,14 @@ QImage QOpenGLWidgetPrivate::grabFramebuffer()
     return res;
 }
 
+void QOpenGLWidgetPrivate::initializeViewportFramebuffer()
+{
+    Q_Q(QOpenGLWidget);
+    // Legacy behavior for compatibility with QGLWidget when used as a graphics view
+    // viewport: enable clearing on each painter begin.
+    q->setAutoFillBackground(true);
+}
+
 void QOpenGLWidgetPrivate::resizeViewportFramebuffer()
 {
     Q_Q(QOpenGLWidget);
@@ -929,7 +992,6 @@ void QOpenGLWidget::resizeEvent(QResizeEvent *e)
     d->recreateFbo();
     resizeGL(width(), height());
     d->invokeUserPaint();
-    d->context->functions()->glFlush();
     d->resolveSamples();
 }
 
diff --git a/src/widgets/kernel/qwidget_p.h b/src/widgets/kernel/qwidget_p.h
index e80447c477..bbdbabc14b 100644
--- a/src/widgets/kernel/qwidget_p.h
+++ b/src/widgets/kernel/qwidget_p.h
@@ -641,6 +641,8 @@ public:
         }
     }
     static void sendComposeStatus(QWidget *w, bool end);
+    // Called on setViewport().
+    virtual void initializeViewportFramebuffer() { }
     // When using a QOpenGLWidget as viewport with QAbstractScrollArea, resize events are
     // filtered away from the widget. This is fine for QGLWidget but bad for QOpenGLWidget
     // since the fbo must be resized. We need an alternative way to notify.
diff --git a/src/widgets/widgets/qabstractscrollarea.cpp b/src/widgets/widgets/qabstractscrollarea.cpp
index 3b75591998..4cafeafcec 100644
--- a/src/widgets/widgets/qabstractscrollarea.cpp
+++ b/src/widgets/widgets/qabstractscrollarea.cpp
@@ -609,6 +609,9 @@ void QAbstractScrollArea::setViewport(QWidget *widget)
 #endif
 #endif
         d->layoutChildren();
+#ifndef QT_NO_OPENGL
+        QWidgetPrivate::get(d->viewport)->initializeViewportFramebuffer();
+#endif
         if (isVisible())
             d->viewport->show();
         setupViewport(widget);