Skip to content

Commit

Permalink
Merge pull request qt#100 from doupp/doupp/rhi-fxes - Imported fixes …
Browse files Browse the repository at this point in the history
…from Qt to remediate issues with RHI.

From Qt:

widgets: Use per-surface-format RHI support and compositor

The RHI support and compositor in QPlatformBackingStore were tied to the surface format of the top level window owning the backing store.

This meant that inserting an RHI-enabled widget (QRhiWidget, QOpenGLWidget, QQuickWidget, QWebEngineView) into the widget hierarchy required recreating the top level window with a matching surface format that could support the RHI composition.

It also meant that we could not have two RHI enabled widgets with different surface format requirements (Metal and OpenGL for example) in the same top level widget hierarchy.

The recreation of the window had various visual side effects, such as temporarily switching out of full screen state, or the widget rendering a frame of black, as well as more serious problems such as not correctly restoring the window geometry.

In addition, if client code had pulled out the winId() of the window, and did not invalidate these references on window destruction via QEvent::WinIdChange or QEvent::PlatformSurface, the client would reference stale window handles. Although this is a programming error (QWidget::winId() specifically mentions this requirement), we should avoid recreation if we can.

We were already supporting flushing the backingstore to individual native child widgets, but always did so via a single RHI managed by the platform backingstore. By expanding QPlatformBackingStore to keep one set of RHI support and compositor per surface format, we can refine the logic in QWidget and QWidgetRepaintManager to not require recreating the top level. Native child widgets are then flushed independently, including any RHI textures and raster content that overlaps with the widget.

We still assume that a single RHI support and compositor can be be used for multiple windows, as long as those windows have the same surface format. In the future, if needed, we can refine this to use one set per surface format e.g.

Fixes: QTBUG-119221
Fixes: QTBUG-121181
Fixes: QTBUG-120096
Task-number: QTBUG-115652
Task-number: QTBUG-108344
Task-number: QTBUG-113557
Task-number: QTBUG-119309
  • Loading branch information
jpek42 authored and GitHub Enterprise committed Mar 25, 2024
2 parents 0519e86 + 6d86ab7 commit 544655d
Show file tree
Hide file tree
Showing 12 changed files with 390 additions and 157 deletions.
7 changes: 7 additions & 0 deletions src/gui/painting/qbackingstorerhisupport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
#include <QtGui/private/qvulkandefaultinstance_p.h>
#endif

#include <QtGui/private/qrhinull_p.h>

QT_BEGIN_NAMESPACE

Q_DECLARE_LOGGING_CATEGORY(lcQpaBackingStore)
Expand Down Expand Up @@ -66,6 +68,11 @@ bool QBackingStoreRhiSupport::create()
QOffscreenSurface *surface = nullptr;
QRhi::Flags flags;

if (m_config.api() == QPlatformBackingStoreRhiConfig::Null) {
QRhiNullInitParams params;
rhi = QRhi::create(QRhi::Null, &params, flags);
}

#if QT_CONFIG(opengl)
if (!rhi && m_config.api() == QPlatformBackingStoreRhiConfig::OpenGL) {
surface = QRhiGles2InitParams::newFallbackSurface(m_format);
Expand Down
84 changes: 60 additions & 24 deletions src/gui/painting/qplatformbackingstore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

#include <QtCore/private/qobject_p.h>

#include <unordered_map>

QT_BEGIN_NAMESPACE

Q_LOGGING_CATEGORY(lcQpaBackingStore, "qt.qpa.backingstore", QtWarningMsg);
Expand All @@ -26,11 +28,14 @@ class QPlatformBackingStorePrivate
QWindow *window;
QBackingStore *backingStore;

// The order matters. if it needs to be rearranged in the future, call
// reset() explicitly from the dtor in the correct order.
// (first the compositor, then the rhiSupport)
QBackingStoreRhiSupport rhiSupport;
QBackingStoreDefaultCompositor compositor;
struct SurfaceSupport {
// The order matters. if it needs to be rearranged in the future, call
// reset() explicitly from the dtor in the correct order.
// (first the compositor, then the rhiSupport)
QBackingStoreRhiSupport rhiSupport;
QBackingStoreDefaultCompositor compositor;
};
std::unordered_map<QSurface::SurfaceType, SurfaceSupport> surfaceSupport;
};

struct QBackingstoreTextureInfo
Expand Down Expand Up @@ -210,8 +215,12 @@ QPlatformBackingStore::FlushResult QPlatformBackingStore::rhiFlush(QWindow *wind
QPlatformTextureList *textures,
bool translucentBackground)
{
return d_ptr->compositor.flush(this, d_ptr->rhiSupport.rhi(), d_ptr->rhiSupport.swapChainForWindow(window),
window, sourceDevicePixelRatio, region, offset, textures, translucentBackground);
auto &surfaceSupport = d_ptr->surfaceSupport[window->surfaceType()];
return surfaceSupport.compositor.flush(this,
surfaceSupport.rhiSupport.rhi(),
surfaceSupport.rhiSupport.swapChainForWindow(window),
window, sourceDevicePixelRatio, region, offset, textures,
translucentBackground);
}

/*!
Expand Down Expand Up @@ -261,7 +270,10 @@ QRhiTexture *QPlatformBackingStore::toTexture(QRhiResourceUpdateBatch *resourceU
const QRegion &dirtyRegion,
TextureFlags *flags) const
{
return d_ptr->compositor.toTexture(this, d_ptr->rhiSupport.rhi(), resourceUpdates, dirtyRegion, flags);
auto &surfaceSupport = d_ptr->surfaceSupport[window()->surfaceType()];
return surfaceSupport.compositor.toTexture(this,
surfaceSupport.rhiSupport.rhi(), resourceUpdates,
dirtyRegion, flags);
}

/*!
Expand Down Expand Up @@ -356,32 +368,56 @@ bool QPlatformBackingStore::scroll(const QRegion &area, int dx, int dy)
return false;
}

void QPlatformBackingStore::setRhiConfig(const QPlatformBackingStoreRhiConfig &config)
{
if (!config.isEnabled())
return;

d_ptr->rhiSupport.setConfig(config);
d_ptr->rhiSupport.setWindow(d_ptr->window);
d_ptr->rhiSupport.setFormat(d_ptr->window->format());
d_ptr->rhiSupport.create();
void QPlatformBackingStore::createRhi(QWindow *window, QPlatformBackingStoreRhiConfig config)
{
auto &support = d_ptr->surfaceSupport[window->surfaceType()];
if (!support.rhiSupport.rhi()) {
qCDebug(lcQpaBackingStore) << "Setting up RHI support in" << this
<< "for" << window << "with" << window->surfaceType()
<< "and requested API" << config.api();
if (config.api() == QPlatformBackingStoreRhiConfig::Null) {
// Auto detect based on window's surface type
switch (window->surfaceType()) {
case QSurface::OpenGLSurface:
config.setApi(QPlatformBackingStoreRhiConfig::OpenGL);
break;
case QSurface::MetalSurface:
config.setApi(QPlatformBackingStoreRhiConfig::Metal);
break;
case QSurface::Direct3DSurface:
config.setApi(QPlatformBackingStoreRhiConfig::D3D11);
break;
case QSurface::VulkanSurface:
config.setApi(QPlatformBackingStoreRhiConfig::Vulkan);
break;
default:
; // Default null-configuration
}
}
support.rhiSupport.setConfig(config);
support.rhiSupport.setWindow(window);
support.rhiSupport.setFormat(window->format());
support.rhiSupport.create();
}
}

QRhi *QPlatformBackingStore::rhi() const
QRhi *QPlatformBackingStore::rhi(QWindow *window) const
{
// Returning null is valid, and means this is not a QRhi-capable backingstore.
return d_ptr->rhiSupport.rhi();
return d_ptr->surfaceSupport[window->surfaceType()].rhiSupport.rhi();
}

void QPlatformBackingStore::graphicsDeviceReportedLost()
void QPlatformBackingStore::graphicsDeviceReportedLost(QWindow *window)
{
if (!d_ptr->rhiSupport.rhi())
auto &surfaceSupport = d_ptr->surfaceSupport[window->surfaceType()];
if (!surfaceSupport.rhiSupport.rhi())
return;

qWarning("Rhi backingstore: graphics device lost, attempting to reinitialize");
d_ptr->rhiSupport.reset();
d_ptr->rhiSupport.create();
if (!d_ptr->rhiSupport.rhi())
surfaceSupport.compositor.reset();
surfaceSupport.rhiSupport.reset();
surfaceSupport.rhiSupport.create();
if (!surfaceSupport.rhiSupport.rhi())
qWarning("Rhi backingstore: failed to reinitialize after losing the device");
}

Expand Down
11 changes: 7 additions & 4 deletions src/gui/painting/qplatformbackingstore.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,16 @@ class QRhiSwapChain;

struct Q_GUI_EXPORT QPlatformBackingStoreRhiConfig
{
Q_GADGET
public:
enum Api {
OpenGL,
Metal,
Vulkan,
D3D11,
Null
};
Q_ENUM(Api)

QPlatformBackingStoreRhiConfig()
: m_enable(false)
Expand Down Expand Up @@ -170,11 +173,11 @@ class Q_GUI_EXPORT QPlatformBackingStore
virtual void beginPaint(const QRegion &);
virtual void endPaint();

void setRhiConfig(const QPlatformBackingStoreRhiConfig &config);
QRhi *rhi() const;
QRhiSwapChain *rhiSwapChain() const;
void createRhi(QWindow *window, QPlatformBackingStoreRhiConfig config = {});
QRhi *rhi(QWindow *window) const;

void surfaceAboutToBeDestroyed();
void graphicsDeviceReportedLost();
void graphicsDeviceReportedLost(QWindow *window);

private:
QPlatformBackingStorePrivate *d_ptr;
Expand Down
20 changes: 2 additions & 18 deletions src/gui/painting/qrhibackingstore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,24 +19,8 @@ void QRhiBackingStore::flush(QWindow *window, const QRegion &region, const QPoin
Q_UNUSED(region);
Q_UNUSED(offset);

if (window != this->window())
return;

if (!rhi()) {
QPlatformBackingStoreRhiConfig rhiConfig;
switch (window->surfaceType()) {
case QSurface::OpenGLSurface:
rhiConfig.setApi(QPlatformBackingStoreRhiConfig::OpenGL);
break;
case QSurface::MetalSurface:
rhiConfig.setApi(QPlatformBackingStoreRhiConfig::Metal);
break;
default:
Q_UNREACHABLE();
}
rhiConfig.setEnabled(true);
setRhiConfig(rhiConfig);
}
if (!rhi(window))
createRhi(window);

static QPlatformTextureList emptyTextureList;
bool translucentBackground = m_image.hasAlphaChannel();
Expand Down
12 changes: 2 additions & 10 deletions src/opengl/qopenglcompositorbackingstore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -141,11 +141,7 @@ void QOpenGLCompositorBackingStore::flush(QWindow *window, const QRegion &region
Q_UNUSED(region);
Q_UNUSED(offset);

m_rhi = rhi();
if (!m_rhi) {
setRhiConfig(QPlatformBackingStoreRhiConfig(QPlatformBackingStoreRhiConfig::OpenGL));
m_rhi = rhi();
}
m_rhi = rhi(window);
Q_ASSERT(m_rhi);

QOpenGLCompositor *compositor = QOpenGLCompositor::instance();
Expand Down Expand Up @@ -181,11 +177,7 @@ QPlatformBackingStore::FlushResult QOpenGLCompositorBackingStore::rhiFlush(QWind
Q_UNUSED(translucentBackground);
Q_UNUSED(sourceDevicePixelRatio);

m_rhi = rhi();
if (!m_rhi) {
setRhiConfig(QPlatformBackingStoreRhiConfig(QPlatformBackingStoreRhiConfig::OpenGL));
m_rhi = rhi();
}
m_rhi = rhi(window);
Q_ASSERT(m_rhi);

QOpenGLCompositor *compositor = QOpenGLCompositor::instance();
Expand Down
13 changes: 4 additions & 9 deletions src/openglwidgets/qopenglwidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -780,9 +780,7 @@ void QOpenGLWidgetPrivate::ensureRhiDependentResources()
{
Q_Q(QOpenGLWidget);

QRhi *rhi = nullptr;
if (QWidgetRepaintManager *repaintManager = QWidgetPrivate::get(q->window())->maybeRepaintManager())
rhi = repaintManager->rhi();
QRhi *rhi = QWidgetPrivate::rhi();

// If there is no rhi, because we are completely offscreen, then there's no wrapperTexture either
if (rhi && rhi->backend() == QRhi::OpenGLES2) {
Expand Down Expand Up @@ -828,7 +826,6 @@ void QOpenGLWidgetPrivate::initialize()
// If no global shared context get our toplevel's context with which we
// will share in order to make the texture usable by the underlying window's backingstore.
QWidget *tlw = q->window();
QWidgetPrivate *tlwd = get(tlw);

// Do not include the sample count. Requesting a multisampled context is not necessary
// since we render into an FBO, never to an actual surface. What's more, attempting to
Expand All @@ -837,9 +834,7 @@ void QOpenGLWidgetPrivate::initialize()
requestedSamples = requestedFormat.samples();
requestedFormat.setSamples(0);

QRhi *rhi = nullptr;
if (QWidgetRepaintManager *repaintManager = tlwd->maybeRepaintManager())
rhi = repaintManager->rhi();
QRhi *rhi = QWidgetPrivate::rhi();

// Could be that something else already initialized the window with some
// other graphics API for the QRhi, that's not good.
Expand Down Expand Up @@ -1687,8 +1682,8 @@ bool QOpenGLWidget::event(QEvent *e)
if (!QCoreApplication::testAttribute(Qt::AA_ShareOpenGLContexts))
d->reset();
}
if (QWidgetRepaintManager *repaintManager = QWidgetPrivate::get(window())->maybeRepaintManager()) {
if (!d->initialized && !size().isEmpty() && repaintManager->rhi()) {
if (d->rhi()) {
if (!d->initialized && !size().isEmpty()) {
d->initialize();
if (d->initialized) {
d->recreateFbos();
Expand Down
Loading

0 comments on commit 544655d

Please sign in to comment.