diff --git a/src/openglwidgets/qopenglwidget.cpp b/src/openglwidgets/qopenglwidget.cpp index a3ffa0c0fa3..1e3f32e3eb6 100644 --- a/src/openglwidgets/qopenglwidget.cpp +++ b/src/openglwidgets/qopenglwidget.cpp @@ -719,7 +719,7 @@ void QOpenGLWidgetPrivate::ensureRhiDependentResources() rhi = repaintManager->rhi(); // If there is no rhi, because we are completely offscreen, then there's no wrapperTexture either - if (rhi) { + if (rhi && rhi->backend() == QRhi::OpenGLES2) { const QSize deviceSize = q->size() * q->devicePixelRatio(); if (!wrapperTexture || wrapperTexture->pixelSize() != deviceSize) { const uint textureId = resolvedFbo ? resolvedFbo->texture() : (fbo ? fbo->texture() : 0); @@ -854,7 +854,14 @@ void QOpenGLWidgetPrivate::render() q->makeCurrent(); QOpenGLContext *ctx = QOpenGLContext::currentContext(); - Q_ASSERT(ctx && fbo); + if (!ctx) { + qWarning("QOpenGLWidget: No current context, cannot render"); + return; + } + if (!fbo) { + qWarning("QOpenGLWidget: No fbo, cannot render"); + return; + } if (updateBehavior == QOpenGLWidget::NoPartialUpdate && hasBeenComposed) { invalidateFbo(); diff --git a/src/widgets/kernel/qwidget.cpp b/src/widgets/kernel/qwidget.cpp index b3d182c4a83..f89e6082c90 100644 --- a/src/widgets/kernel/qwidget.cpp +++ b/src/widgets/kernel/qwidget.cpp @@ -1083,30 +1083,30 @@ static bool q_evaluateRhiConfigRecursive(const QWidget *w, QPlatformBackingStore *outType = QBackingStoreRhiSupport::surfaceTypeForConfig(config); return true; } - QObjectList children = w->children(); - for (int i = 0; i < children.size(); i++) { - if (children.at(i)->isWidgetType()) { - const QWidget *childWidget = qobject_cast(children.at(i)); - if (childWidget) { - if (q_evaluateRhiConfigRecursive(childWidget, outConfig, outType)) - return true; - } + for (const QObject *child : w->children()) { + if (const QWidget *childWidget = qobject_cast(child)) { + if (q_evaluateRhiConfigRecursive(childWidget, outConfig, outType)) + return true; } } return false; } -// First tries q_evaluateRhiConfigRecursive, then if that did not indicate that rhi is wanted, -// then checks env.vars or something else to see if we need to force using rhi-based composition. bool q_evaluateRhiConfig(const QWidget *w, QPlatformBackingStoreRhiConfig *outConfig, QSurface::SurfaceType *outType) { - if (q_evaluateRhiConfigRecursive(w, outConfig, outType)) { - qCDebug(lcWidgetPainting) << "Tree with root" << w << "evaluates to flushing with QRhi"; + // First, check env.vars. or other means that force the usage of rhi-based + // flushing with a specific graphics API. This takes precedence over what + // the widgets themselves declare. This is global, applying to all + // top-levels. + if (QBackingStoreRhiSupport::checkForceRhi(outConfig, outType)) { + qCDebug(lcWidgetPainting) << "Tree with root" << w << "evaluated to forced flushing with QRhi"; return true; } - if (QBackingStoreRhiSupport::checkForceRhi(outConfig, outType)) { - qCDebug(lcWidgetPainting) << "Tree with root" << w << "evaluated to forced flushing with QRhi"; + // Otherwise, check the widget hierarchy to see if there is a child (or + // ourselves) that declare the need for rhi-based composition. + if (q_evaluateRhiConfigRecursive(w, outConfig, outType)) { + qCDebug(lcWidgetPainting) << "Tree with root" << w << "evaluates to flushing with QRhi"; return true; } @@ -10723,7 +10723,10 @@ void QWidget::setParent(QWidget *parent, Qt::WindowFlags f) QWidget *newtlw = window(); if (oldtlw != newtlw) { QSurface::SurfaceType surfaceType = QSurface::RasterSurface; - if (q_evaluateRhiConfig(newtlw, nullptr, &surfaceType)) { + // Only evaluate the reparented subtree. While it might be tempting to + // do it on newtlw instead, the performance implications of that are + // problematic when it comes to large widget trees. + if (q_evaluateRhiConfig(this, nullptr, &surfaceType)) { newtlw->d_func()->usesRhiFlush = true; if (QWindow *w = newtlw->windowHandle()) { if (w->surfaceType() != surfaceType) { diff --git a/src/widgets/kernel/qwidgetrepaintmanager.cpp b/src/widgets/kernel/qwidgetrepaintmanager.cpp index 19c94f22028..0ab09a26f2e 100644 --- a/src/widgets/kernel/qwidgetrepaintmanager.cpp +++ b/src/widgets/kernel/qwidgetrepaintmanager.cpp @@ -1032,7 +1032,7 @@ void QWidgetRepaintManager::flush(QWidget *widget, const QRegion ®ion, QPlatf if (widget != tlw) offset += widget->mapTo(tlw, QPoint()); - if (widget->d_func()->usesRhiFlush) { + if (tlw->d_func()->usesRhiFlush) { QRhi *rhi = store->handle()->rhi(); qCDebug(lcWidgetPainting) << "Flushing" << region << "of" << widget << "with QRhi" << rhi diff --git a/tests/auto/widgets/widgets/qopenglwidget/tst_qopenglwidget.cpp b/tests/auto/widgets/widgets/qopenglwidget/tst_qopenglwidget.cpp index 4ebc941f8b5..bc3e0b1a8f5 100644 --- a/tests/auto/widgets/widgets/qopenglwidget/tst_qopenglwidget.cpp +++ b/tests/auto/widgets/widgets/qopenglwidget/tst_qopenglwidget.cpp @@ -530,39 +530,132 @@ void tst_QOpenGLWidget::nativeWindow() #ifdef Q_OS_ANDROID QSKIP("Crashes on Android, figure out why (QTBUG-102043)"); #endif - QScopedPointer w(new ClearWidget(0, 800, 600)); - w->resize(800, 600); - w->show(); - w->winId(); - QVERIFY(QTest::qWaitForWindowExposed(w.data())); - QImage image = w->grabFramebuffer(); - QVERIFY(!image.isNull()); - QCOMPARE(image.width(), w->width()); - QCOMPARE(image.height(), w->height()); - QVERIFY(image.pixel(30, 40) == qRgb(255, 0, 0)); - QVERIFY(w->internalWinId()); + // NB these tests do not fully verify that native child widgets are fully + // functional since there is no guarantee that the content is composed and + // presented correctly as we can only do verification with + // grabFramebuffer() here which only exercises a part of the pipeline. - // Now as a native child. - QWidget nativeParent; - nativeParent.resize(800, 600); - nativeParent.setAttribute(Qt::WA_NativeWindow); - ClearWidget *child = new ClearWidget(0, 800, 600); - child->setClearColor(0, 1, 0); - child->setParent(&nativeParent); - child->resize(400, 400); - child->move(23, 34); - nativeParent.show(); - QVERIFY(QTest::qWaitForWindowExposed(&nativeParent)); + { + QScopedPointer w(new ClearWidget(nullptr, 800, 600)); + w->resize(800, 600); + w->show(); + w->winId(); + QVERIFY(QTest::qWaitForWindowExposed(w.data())); - QVERIFY(nativeParent.internalWinId()); - QVERIFY(!child->internalWinId()); + QImage image = w->grabFramebuffer(); + QVERIFY(!image.isNull()); + QCOMPARE(image.width(), w->width()); + QCOMPARE(image.height(), w->height()); + QVERIFY(image.pixel(30, 40) == qRgb(255, 0, 0)); + QVERIFY(w->internalWinId()); + } - image = child->grabFramebuffer(); - QVERIFY(!image.isNull()); - QCOMPARE(image.width(), child->width()); - QCOMPARE(image.height(), child->height()); - QVERIFY(image.pixel(30, 40) == qRgb(0, 255, 0)); + // Now as a native child + { + QWidget topLevel; + topLevel.resize(800, 600); + + ClearWidget *child = new ClearWidget(nullptr, 800, 600); + child->setParent(&topLevel); + + // make it a native child (native window, but not top-level -> no backingstore) + child->winId(); + + child->setClearColor(0, 1, 0); + child->resize(400, 400); + child->move(23, 34); + + topLevel.show(); + QVERIFY(QTest::qWaitForWindowExposed(&topLevel)); + + QVERIFY(topLevel.internalWinId()); + QVERIFY(child->internalWinId()); + + QImage image = child->grabFramebuffer(); + QVERIFY(!image.isNull()); + QCOMPARE(image.width(), child->width()); + QCOMPARE(image.height(), child->height()); + QVERIFY(image.pixel(30, 40) == qRgb(0, 255, 0)); + } + + // Now the same with WA_NativeWindow instead + { + QWidget topLevel; + topLevel.resize(800, 600); + + ClearWidget *child = new ClearWidget(nullptr, 800, 600); + child->setParent(&topLevel); + + // make it a native child (native window, but not top-level -> no backingstore) + child->setAttribute(Qt::WA_NativeWindow); + + child->setClearColor(0, 1, 0); + child->resize(400, 400); + child->move(23, 34); + + topLevel.show(); + QVERIFY(QTest::qWaitForWindowExposed(&topLevel)); + + QVERIFY(child->internalWinId()); + + QImage image = child->grabFramebuffer(); + QCOMPARE(image.width(), child->width()); + QCOMPARE(image.height(), child->height()); + QVERIFY(image.pixel(30, 40) == qRgb(0, 255, 0)); + } + + // Now as a child of a native child + { + QWidget topLevel; + topLevel.resize(800, 600); + + QWidget *container = new QWidget(&topLevel); + // make it a native child (native window, but not top-level -> no backingstore) + container->winId(); + + 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->resize(400, 400); + child->move(23, 34); + + topLevel.show(); + QVERIFY(QTest::qWaitForWindowExposed(&topLevel)); + + QVERIFY(topLevel.internalWinId()); + QVERIFY(container->internalWinId()); + QVERIFY(!child->internalWinId()); + + QImage image = child->grabFramebuffer(); + QCOMPARE(image.width(), child->width()); + QCOMPARE(image.height(), child->height()); + QVERIFY(image.pixel(30, 40) == qRgb(0, 255, 0)); + } + + // Again as a child of a native child, but this time specifying the parent + // upon construction, not with an explicit setParent() call afterwards. + { + QWidget topLevel; + topLevel.resize(800, 600); + QWidget *container = new QWidget(&topLevel); + container->winId(); + // parent it right away + ClearWidget *child = new ClearWidget(container, 800, 600); + child->setClearColor(0, 1, 0); + child->resize(400, 400); + child->move(23, 34); + topLevel.show(); + QVERIFY(QTest::qWaitForWindowExposed(&topLevel)); + QVERIFY(topLevel.internalWinId()); + QVERIFY(container->internalWinId()); + QVERIFY(!child->internalWinId()); + QImage image = child->grabFramebuffer(); + QCOMPARE(image.width(), child->width()); + QCOMPARE(image.height(), child->height()); + QVERIFY(image.pixel(30, 40) == qRgb(0, 255, 0)); + } } static inline QString msgRgbMismatch(unsigned actual, unsigned expected)