summaryrefslogtreecommitdiffstats
path: root/src/widgets/kernel
diff options
context:
space:
mode:
authorLaszlo Agocs <laszlo.agocs@qt.io>2022-11-11 08:54:37 +0100
committerLaszlo Agocs <laszlo.agocs@qt.io>2022-11-14 11:05:48 +0100
commit244daf4cfc44587c8c7c87e481688e840cc21c77 (patch)
treede974040d011604ff9a1a07f8d24a5c792017a0f /src/widgets/kernel
parent092bbc9ad30c6cd7389053dc4b332cc762693676 (diff)
Fix rhi flush eval perf. and native window problems
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. Pick-to: 6.4 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>
Diffstat (limited to 'src/widgets/kernel')
-rw-r--r--src/widgets/kernel/qwidget.cpp33
-rw-r--r--src/widgets/kernel/qwidgetrepaintmanager.cpp2
2 files changed, 19 insertions, 16 deletions
diff --git a/src/widgets/kernel/qwidget.cpp b/src/widgets/kernel/qwidget.cpp
index 2be71482e4..55bed86080 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<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) {
diff --git a/src/widgets/kernel/qwidgetrepaintmanager.cpp b/src/widgets/kernel/qwidgetrepaintmanager.cpp
index cf16d2a005..cdd8cee016 100644
--- a/src/widgets/kernel/qwidgetrepaintmanager.cpp
+++ b/src/widgets/kernel/qwidgetrepaintmanager.cpp
@@ -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