1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix rhi flush eval perf. and native window problems

Browse Source 
This effectively reverts a4a51f6a641f4bf0a863251d6d3e026d81de6280
while solving the problem that change intended to fix by an
alternative approach: Swap the order of checks for rhi-based
flushing. Checking the widgets' wishes first was a mistake.  We should
first check what is forced, e.g. via the env.vars.  Then only move on
investigating the child widget hierarchy if there was nothing specific
requested.

This way having a QOpenGLWidget in a window and running with
QT_WIDGETS_RHI=1 QT_WIDGETS_RHI_BACKEND=vulkan will prioritize the
forced request (Vulkan) and so the QOpenGLWidget will gracefully not
render anything while printing the expected warning to tell what's
going on.

The expensive recursion plaguing the construction of larger widget
hierarchies is now avoided, that should no longer take seconds due to
walking the entire widget hierarchy of the top-level window every time
a new widget is added to it.

However, this then uncovered a set of problems concerning native child
widgets. The repaint manager seems to have an obvious mistake where
the usage of rhi (and so textures and whatnot) is decided based on
'widget' (which is either a top-level or a native child) instead of
'tlw' (which is the top-level with the backingstore). The usesRhiFlush
flag only really matters for a real top-level, not for native child
widgets.  The rhi-based flushing is tied to the backingstore, and the
backingstore comes from the top-level widget.

Finally, make the qopenglwidget autotest to actually exercise
something when it comes to QOpenGLWidgets (or their ancestors) turned
native.  The original code from a long time ago does not really test
native child widgets, because it turns the top-level into native which
is quite superfluous since the toplevel is backed by a native window
(and a backingstore) anyway.

Task-number: QTBUG-105017
Fixes: QTBUG-108344
Fixes: QTBUG-108277
Change-Id: I1785b0ca627cf151aad06a5489f63874d787f087
Reviewed-by: Andy Nichols <andy.nichols@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
(cherry picked from commit 244daf4cfc44587c8c7c87e481688e840cc21c77)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
Laszlo Agocs 2022-11-11 08:54:37 +01:00 committed by Qt Cherry-pick Bot
parent 852c40f290
commit d58ac206a8
4 changed files with 150 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
src/openglwidgets/qopenglwidget.cpp
View File

@ -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();

33
src/widgets/kernel/qwidget.cpp
View File

@ -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<const QWidget *>(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<const QWidget *>(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) {

2
src/widgets/kernel/qwidgetrepaintmanager.cpp
View File

@ -1032,7 +1032,7 @@ void QWidgetRepaintManager::flush(QWidget *widget, const QRegion &region, 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

151
tests/auto/widgets/widgets/qopenglwidget/tst_qopenglwidget.cpp
View File

@ -530,39 +530,132 @@ void tst_QOpenGLWidget::nativeWindow()
#ifdef Q_OS_ANDROID
QSKIP("Crashes on Android, figure out why (QTBUG-102043)");
#endif
QScopedPointer<ClearWidget> 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<ClearWidget> 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)