From 406bb6ae20471cf9bba6d910256b416792c99322 Mon Sep 17 00:00:00 2001 From: Laszlo Agocs Date: Mon, 24 Jan 2022 15:28:20 +0100 Subject: [PATCH] rhi: Make sure pixelSize() to a texture rt is always up to date MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is an issue for QQuickWindow in practice, although it is not hit by our current tests. Pick-to: 6.3 Change-Id: Ia73704c1af6a82b2689ce7b844d3b0eb9a17ec18 Reviewed-by: Christian Strømme --- src/gui/rhi/qrhi.cpp | 12 +++++ src/gui/rhi/qrhid3d11.cpp | 3 ++ src/gui/rhi/qrhigles2.cpp | 3 ++ src/gui/rhi/qrhimetal.mm | 3 ++ src/gui/rhi/qrhinull.cpp | 3 ++ src/gui/rhi/qrhivulkan.cpp | 3 ++ tests/auto/gui/rhi/qrhi/tst_qrhi.cpp | 66 ++++++++++++++++++---------- 7 files changed, 69 insertions(+), 24 deletions(-) diff --git a/src/gui/rhi/qrhi.cpp b/src/gui/rhi/qrhi.cpp index 452a39642f6..8196d107fe6 100644 --- a/src/gui/rhi/qrhi.cpp +++ b/src/gui/rhi/qrhi.cpp @@ -2929,6 +2929,18 @@ QRhiResource::Type QRhiRenderTarget::resourceType() const \fn QSize QRhiRenderTarget::pixelSize() const \return the size in pixels. + + Valid only after create() has been called successfully. Until then the + result is a default-constructed QSize. + + With QRhiTextureRenderTarget the returned size is the size of the + associated attachments at the time of create(), in practice the size of the + first color attachment, or the depth/stencil buffer if there are no color + attachments. If the associated textures or renderbuffers are resized and + rebuilt afterwards, then pixelSize() performs an implicit call to create() + in order to rebuild the underlying data structures. This implicit check is + similar to what QRhiCommandBuffer::beginPass() does, and ensures that the + returned size is always up-to-date. */ /*! diff --git a/src/gui/rhi/qrhid3d11.cpp b/src/gui/rhi/qrhid3d11.cpp index d029cd2fbf9..fc6fd59f4f5 100644 --- a/src/gui/rhi/qrhid3d11.cpp +++ b/src/gui/rhi/qrhid3d11.cpp @@ -3631,6 +3631,9 @@ bool QD3D11TextureRenderTarget::create() QSize QD3D11TextureRenderTarget::pixelSize() const { + if (!QRhiRenderTargetAttachmentTracker::isUpToDate(m_desc, d.currentResIdList)) + const_cast(this)->create(); + return d.pixelSize; } diff --git a/src/gui/rhi/qrhigles2.cpp b/src/gui/rhi/qrhigles2.cpp index 0c2225f194e..21dd8da2290 100644 --- a/src/gui/rhi/qrhigles2.cpp +++ b/src/gui/rhi/qrhigles2.cpp @@ -5317,6 +5317,9 @@ bool QGles2TextureRenderTarget::create() QSize QGles2TextureRenderTarget::pixelSize() const { + if (!QRhiRenderTargetAttachmentTracker::isUpToDate(m_desc, d.currentResIdList)) + const_cast(this)->create(); + return d.pixelSize; } diff --git a/src/gui/rhi/qrhimetal.mm b/src/gui/rhi/qrhimetal.mm index dd79c77dc9b..4198306ffcd 100644 --- a/src/gui/rhi/qrhimetal.mm +++ b/src/gui/rhi/qrhimetal.mm @@ -3183,6 +3183,9 @@ bool QMetalTextureRenderTarget::create() QSize QMetalTextureRenderTarget::pixelSize() const { + if (!QRhiRenderTargetAttachmentTracker::isUpToDate(m_desc, d->currentResIdList)) + const_cast(this)->create(); + return d->pixelSize; } diff --git a/src/gui/rhi/qrhinull.cpp b/src/gui/rhi/qrhinull.cpp index 95448783504..2790b19ebcd 100644 --- a/src/gui/rhi/qrhinull.cpp +++ b/src/gui/rhi/qrhinull.cpp @@ -860,6 +860,9 @@ bool QNullTextureRenderTarget::create() QSize QNullTextureRenderTarget::pixelSize() const { + if (!QRhiRenderTargetAttachmentTracker::isUpToDate(m_desc, d.currentResIdList)) + const_cast(this)->create(); + return d.pixelSize; } diff --git a/src/gui/rhi/qrhivulkan.cpp b/src/gui/rhi/qrhivulkan.cpp index 025419d0457..c27b55f8858 100644 --- a/src/gui/rhi/qrhivulkan.cpp +++ b/src/gui/rhi/qrhivulkan.cpp @@ -6637,6 +6637,9 @@ bool QVkTextureRenderTarget::create() QSize QVkTextureRenderTarget::pixelSize() const { + if (!QRhiRenderTargetAttachmentTracker::isUpToDate(m_desc, d.currentResIdList)) + const_cast(this)->create(); + return d.pixelSize; } diff --git a/tests/auto/gui/rhi/qrhi/tst_qrhi.cpp b/tests/auto/gui/rhi/qrhi/tst_qrhi.cpp index e71e7a9a2f8..db2ee58aaef 100644 --- a/tests/auto/gui/rhi/qrhi/tst_qrhi.cpp +++ b/tests/auto/gui/rhi/qrhi/tst_qrhi.cpp @@ -3828,33 +3828,51 @@ void tst_QRhi::textureRenderTargetAutoRebuild() if (!rhi) QSKIP("QRhi could not be created, skipping testing rendering"); - QScopedPointer texture(rhi->newTexture(QRhiTexture::RGBA8, QSize(512, 512), 1, QRhiTexture::RenderTarget)); - QVERIFY(texture->create()); - QScopedPointer rt(rhi->newTextureRenderTarget({ { texture.data() } })); - QScopedPointer rp(rt->newCompatibleRenderPassDescriptor()); - rt->setRenderPassDescriptor(rp.data()); - QVERIFY(rt->create()); + // case 1: beginPass's implicit create() + { + QScopedPointer texture(rhi->newTexture(QRhiTexture::RGBA8, QSize(512, 512), 1, QRhiTexture::RenderTarget)); + QVERIFY(texture->create()); + QScopedPointer rt(rhi->newTextureRenderTarget({ { texture.data() } })); + QScopedPointer rp(rt->newCompatibleRenderPassDescriptor()); + rt->setRenderPassDescriptor(rp.data()); + QVERIFY(rt->create()); - QRhiCommandBuffer *cb = nullptr; - QVERIFY(rhi->beginOffscreenFrame(&cb) == QRhi::FrameOpSuccess); - QVERIFY(cb); - cb->beginPass(rt.data(), Qt::red, { 1.0f, 0 }); - cb->endPass(); - rhi->endOffscreenFrame(); + QRhiCommandBuffer *cb = nullptr; + QVERIFY(rhi->beginOffscreenFrame(&cb) == QRhi::FrameOpSuccess); + QVERIFY(cb); + cb->beginPass(rt.data(), Qt::red, { 1.0f, 0 }); + cb->endPass(); + rhi->endOffscreenFrame(); - texture->setPixelSize(QSize(256, 256)); - QVERIFY(texture->create()); - QCOMPARE(texture->pixelSize(), QSize(256, 256)); - // rt still has the old size and knows nothing about texture's underlying native texture resource possibly changing - QCOMPARE(rt->pixelSize(), QSize(512, 512)); + texture->setPixelSize(QSize(256, 256)); + QVERIFY(texture->create()); + QCOMPARE(texture->pixelSize(), QSize(256, 256)); - QVERIFY(rhi->beginOffscreenFrame(&cb) == QRhi::FrameOpSuccess); - QVERIFY(cb); - // no rt->create() but beginPass() does it implicitly for us - cb->beginPass(rt.data(), Qt::red, { 1.0f, 0 }); - QCOMPARE(rt->pixelSize(), QSize(256, 256)); - cb->endPass(); - rhi->endOffscreenFrame(); + QVERIFY(rhi->beginOffscreenFrame(&cb) == QRhi::FrameOpSuccess); + QVERIFY(cb); + // no rt->create() but beginPass() does it implicitly for us + cb->beginPass(rt.data(), Qt::red, { 1.0f, 0 }); + QCOMPARE(rt->pixelSize(), QSize(256, 256)); + cb->endPass(); + rhi->endOffscreenFrame(); + } + + // case 2: pixelSize's implicit create() + { + QSize sz(512, 512); + QScopedPointer texture(rhi->newTexture(QRhiTexture::RGBA8, sz, 1, QRhiTexture::RenderTarget)); + QVERIFY(texture->create()); + QScopedPointer rt(rhi->newTextureRenderTarget({ { texture.data() } })); + QScopedPointer rp(rt->newCompatibleRenderPassDescriptor()); + rt->setRenderPassDescriptor(rp.data()); + QVERIFY(rt->create()); + QCOMPARE(rt->pixelSize(), sz); + + sz = QSize(256, 256); + texture->setPixelSize(sz); + QVERIFY(texture->create()); + QCOMPARE(rt->pixelSize(), sz); + } } void tst_QRhi::srbLayoutCompatibility_data()