Refine the rhi-based flush logic

Amends 244daf4cfc44587c8c7c87e481688e840cc21c77

Fixes: QTBUG-113557
Fixes: QTBUG-113652
Task-number: QTBUG-108277
Change-Id: I9e369b0e1261ea37eb2dedd80083f82f5df97b30
Reviewed-by: Andy Nichols <andy.nichols@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
(cherry picked from commit 8c0b657c9a119cf60d96d36351ccf553e21ff3fc)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

src/widgets/kernel/qwidgetrepaintmanager.cpp

@@ -1033,7 +1033,36 @@ void QWidgetRepaintManager::flush(QWidget *widget, const QRegion &region, QPlatf
     if (widget != tlw)
         offset += widget->mapTo(tlw, QPoint());
 
-    if (tlw->d_func()->usesRhiFlush) {
+    // Use a condition that tries to keep both QTBUG-108344 and QTBUG-113557
+    // happy, i.e. support both (A) "native rhi-based child in a rhi-based
+    // toplevel" and (B) "native raster child in a rhi-based toplevel".
+    //
+    // If the tlw and the backingstore are RHI-based, then there are two cases
+    // to consider:
+    //
+    // (1) widget is not a native child, i.e. the QWindow for widget and tlw are
+    // the same,
+    //
+    // (2) widget is a native child which we now attempt to flush with tlw's
+    // backingstore to widget's native window. This is the interesting one.
+    //
+    // Using the condition tlw->usesRhiFlush on its own is insufficient since
+    // it fails to capture the case of a raster-based native child widget
+    // within tlw. (which must hit the non-rhi flush path)
+    //
+    // Extending the condition with tlw->windowHandle() == widget->windowHandle()
+    // would be logical but wrong, when it comes to (A) since flushing a
+    // RHI-based native child with a given 3D API using a RHI-based
+    // tlw/backingstore with the same 3D API needs to be supported still. (this
+    // happens when e.g. someone calls winId() on a QOpenGLWidget)
+    //
+    // Different 3D APIs do not need to be supported since we do not allow to
+    // do things like having a QQuickWidget with Vulkan and a QOpenGLWidget in
+    // the same toplevel, regardless of the widgets being native children or
+    // not. Hence comparing the surfaceType() instead. This satisfies both (A)
+    // and (B) given that an RHI-based toplevel cannot be RasterSurface.
+    //
+    if (tlw->d_func()->usesRhiFlush && tlw->windowHandle()->surfaceType() == widget->windowHandle()->surfaceType()) {
         QRhi *rhi = store->handle()->rhi();
         qCDebug(lcWidgetPainting) << "Flushing" << region << "of" << widget
                                   << "with QRhi" << rhi

tests/auto/widgets/widgets/qopenglwidget/tst_qopenglwidget.cpp

@@ -528,6 +528,21 @@ void tst_QOpenGLWidget::showHide()
     QVERIFY(image.pixel(30, 40) == qRgb(0, 0, 255));
 }
 
+QtMessageHandler oldHandler = nullptr;
+
+void nativeWindowMessageHandler(QtMsgType type, const QMessageLogContext &context, const QString &msg)
+{
+    if (oldHandler)
+        oldHandler(type, context, msg);
+
+    if (type == QtWarningMsg
+        && (msg.contains("QOpenGLContext::makeCurrent() called with non-opengl surface")
+            || msg.contains("Failed to make context current")))
+    {
+        QFAIL("Unexpected warning got printed");
+    }
+}
+
 void tst_QOpenGLWidget::nativeWindow()
 {
 #ifdef Q_OS_ANDROID
@@ -539,6 +554,10 @@ void tst_QOpenGLWidget::nativeWindow()
     // presented correctly as we can only do verification with
     // grabFramebuffer() here which only exercises a part of the pipeline.
 
+    // Install a message handler that looks for some typical warnings from
+    // QRhi/QOpenGLConext that occur when the RHI-related logic in widgets goes wrong.
+    oldHandler = qInstallMessageHandler(nativeWindowMessageHandler);
+
     {
         QScopedPointer<ClearWidget> w(new ClearWidget(nullptr, 800, 600));
         w->resize(800, 600);
@@ -554,7 +573,33 @@ void tst_QOpenGLWidget::nativeWindow()
         QVERIFY(w->internalWinId());
     }
 
-    // Now as a native child
+    // QTBUG-113557: a plain _raster_ QWidget that is a _native_ child in a toplevel
+    // combined with a RHI-based (non-native) widget (QOpenGLWidget in this case)
+    // in the same toplevel.
+    {
+        QWidget topLevel;
+        topLevel.resize(800, 600);
+
+        ClearWidget *child = new ClearWidget(&topLevel, 800, 600);
+        child->setClearColor(1, 0, 0);
+        child->resize(400, 400);
+        child->move(23, 34);
+
+        QWidget *raster = new QWidget(&topLevel);
+        raster->setGeometry(23, 240, 120, 120);
+        raster->setStyleSheet("QWidget { background-color: yellow; }");
+
+        raster->winId();
+
+        topLevel.show();
+        QVERIFY(QTest::qWaitForWindowExposed(&topLevel));
+
+        // Do not bother checking the output, i.e. if the yellow raster native child
+        // shows up as it should, but rather rely on the message handler catching the
+        // qWarnings if they occur.
+    }
+
+    // Now with the QOpenGLWidget being a native child
     {
         QWidget topLevel;
         topLevel.resize(800, 600);
@@ -620,7 +665,7 @@ void tst_QOpenGLWidget::nativeWindow()
         ClearWidget *child = new ClearWidget(nullptr, 800, 600);
         // set the parent separately, this is important, see next test case
         child->setParent(container);
-        child->setClearColor(0, 1, 0);
+        child->setClearColor(0, 0, 1);
         child->resize(400, 400);
         child->move(23, 34);
 
@@ -634,7 +679,7 @@ void tst_QOpenGLWidget::nativeWindow()
         QImage image = child->grabFramebuffer();
         QCOMPARE(image.width(), child->width());
         QCOMPARE(image.height(), child->height());
-        QVERIFY(image.pixel(30, 40) == qRgb(0, 255, 0));
+        QVERIFY(image.pixel(30, 40) == qRgb(0, 0, 255));
     }
 
     // Again as a child of a native child, but this time specifying the parent
@@ -646,7 +691,7 @@ void tst_QOpenGLWidget::nativeWindow()
         container->winId();
         // parent it right away
         ClearWidget *child = new ClearWidget(container, 800, 600);
-        child->setClearColor(0, 1, 0);
+        child->setClearColor(0, 0, 1);
         child->resize(400, 400);
         child->move(23, 34);
         topLevel.show();
@@ -657,7 +702,12 @@ void tst_QOpenGLWidget::nativeWindow()
         QImage image = child->grabFramebuffer();
         QCOMPARE(image.width(), child->width());
         QCOMPARE(image.height(), child->height());
-        QVERIFY(image.pixel(30, 40) == qRgb(0, 255, 0));
+        QVERIFY(image.pixel(30, 40) == qRgb(0, 0, 255));
+    }
+
+    if (oldHandler) {
+        qInstallMessageHandler(oldHandler);
+        oldHandler = nullptr;
     }
 }