From bc4570ed24d057be2a376aab8b73ca579331a08f Mon Sep 17 00:00:00 2001 From: Laszlo Agocs Date: Wed, 8 Dec 2021 19:40:25 +0100 Subject: [PATCH] rhi: Auto-rebuild rt by tracking attachment id and generation Unlike the shader resource binding lists that automatically recognize in setShaderResources() when a referenced QRhiResource has been rebuilt in the meantime (create() was called i.e. there may be completely different native objects underneath), QRhiTextureRenderTarget has no such thing. This leads to an asymmetric API and requires also rebuilding the rt whenever an attachment is rebuilt: rt = rhi->newTextureRenderTarget({ { texture } }) rt->create() cb->beginPass(rt, ...) texture->setPixelSize(...) texture->create() rt->create() // this should not be needed cb->beginPass(rt, ...) Avoid having to do that second rt->create(). Change-Id: If14eaa7aac3530950498bbdf834324d0741a7c4d Reviewed-by: Qt CI Bot Reviewed-by: Andy Nichols --- src/gui/rhi/qrhi.cpp | 36 ++++++++++++- src/gui/rhi/qrhi_p_p.h | 77 ++++++++++++++++++++++++++++ src/gui/rhi/qrhid3d11.cpp | 5 ++ src/gui/rhi/qrhid3d11_p_p.h | 3 ++ src/gui/rhi/qrhigles2.cpp | 10 ++++ src/gui/rhi/qrhigles2_p_p.h | 2 + src/gui/rhi/qrhimetal.mm | 6 +++ src/gui/rhi/qrhinull.cpp | 14 ++++- src/gui/rhi/qrhinull_p_p.h | 3 ++ src/gui/rhi/qrhivulkan.cpp | 6 +++ src/gui/rhi/qrhivulkan_p_p.h | 2 + tests/auto/gui/rhi/qrhi/tst_qrhi.cpp | 45 ++++++++++++++++ 12 files changed, 207 insertions(+), 2 deletions(-) diff --git a/src/gui/rhi/qrhi.cpp b/src/gui/rhi/qrhi.cpp index 952183f55e..70525689e9 100644 --- a/src/gui/rhi/qrhi.cpp +++ b/src/gui/rhi/qrhi.cpp @@ -330,9 +330,22 @@ Q_LOGGING_CATEGORY(QRHI_LOG_INFO, "qt.rhi.general") ubuf->setSize(512); ubuf->create(); // same as ubuf->destroy(); ubuf->create(); - // that's it, srb needs no changes whatsoever + // That's it, srb needs no changes whatsoever, any references in it to + // ubuf stay valid. When it comes to internal details, such as that + // ubuf may now be backed by a completely different native buffer + // resource, that is is recognized and handled automatically by the + // next setShaderResources(). \endcode + QRhiTextureRenderTarget offers the same contract: calling + QRhiCommandBuffer::beginPass() is safe even when one of the render target's + associated textures or renderbuffers has been rebuilt (by calling \c + create() on it) since the creation of the render target object. This allows + the application to resize a texture by setting a new pixel size on the + QRhiTexture and calling create(), thus creating a whole new native texture + resource underneath, without having to update the QRhiTextureRenderTarget + as that will be done implicitly in beginPass(). + \section3 Pooled objects In addition to resources, there are pooled objects as well, such as, @@ -5669,6 +5682,25 @@ void QRhiCommandBuffer::resourceUpdate(QRhiResourceUpdateBatch *resourceUpdates) called inside a pass. Also, with the exception of setGraphicsPipeline(), they expect to have a pipeline set already on the command buffer. Unspecified issues may arise otherwise, depending on the backend. + + If \a rt is a QRhiTextureRenderTarget, beginPass() performs a check to see + if the texture and renderbuffer objects referenced from the render target + are up-to-date. This is similar to what setShaderResources() does for + QRhiShaderResourceBindings. If any of the attachments had been rebuilt + since QRhiTextureRenderTarget::create(), an implicit call to create() is + made on \a rt. Therefore, if \a rt has a QRhiTexture color attachment \c + texture, and one needs to make the texture a different size, the following + is then valid: + \badcode + rt = rhi->newTextureRenderTarget({ { texture } }); + rt->create(); + ... + texture->setPixelSize(new_size); + texture->create(); + cb->beginPass(rt, ...); // this is ok, no explicit rt->create() is required before + \endcode + + \sa endPass() */ void QRhiCommandBuffer::beginPass(QRhiRenderTarget *rt, const QColor &colorClearValue, @@ -5684,6 +5716,8 @@ void QRhiCommandBuffer::beginPass(QRhiRenderTarget *rt, \a resourceUpdates, when not null, specifies a resource update batch that is to be committed and then released. + + \sa beginPass() */ void QRhiCommandBuffer::endPass(QRhiResourceUpdateBatch *resourceUpdates) { diff --git a/src/gui/rhi/qrhi_p_p.h b/src/gui/rhi/qrhi_p_p.h index 7a3f94846d..9e8a3a846f 100644 --- a/src/gui/rhi/qrhi_p_p.h +++ b/src/gui/rhi/qrhi_p_p.h @@ -706,6 +706,83 @@ private: int p = 0; }; +struct QRhiRenderTargetAttachmentTracker +{ + struct ResId { quint64 id; uint generation; }; + using ResIdList = QVarLengthArray; // color, resolve, ds + + template + static void updateResIdList(const QRhiTextureRenderTargetDescription &desc, ResIdList *dst); + + template + static bool isUpToDate(const QRhiTextureRenderTargetDescription &desc, const ResIdList ¤tResIdList); +}; + +inline bool operator==(const QRhiRenderTargetAttachmentTracker::ResId &a, const QRhiRenderTargetAttachmentTracker::ResId &b) +{ + return a.id == b.id && a.generation == b.generation; +} + +inline bool operator!=(const QRhiRenderTargetAttachmentTracker::ResId &a, const QRhiRenderTargetAttachmentTracker::ResId &b) +{ + return !(a == b); +} + +template +void QRhiRenderTargetAttachmentTracker::updateResIdList(const QRhiTextureRenderTargetDescription &desc, ResIdList *dst) +{ + const quintptr colorAttCount = desc.cendColorAttachments() - desc.cbeginColorAttachments(); + const bool hasDepthStencil = desc.depthStencilBuffer() || desc.depthTexture(); + dst->resize(colorAttCount * 2 + (hasDepthStencil ? 1 : 0)); + int n = 0; + for (auto it = desc.cbeginColorAttachments(), itEnd = desc.cendColorAttachments(); it != itEnd; ++it, ++n) { + const QRhiColorAttachment &colorAtt(*it); + if (colorAtt.texture()) { + TexType *texD = QRHI_RES(TexType, colorAtt.texture()); + (*dst)[n] = { texD->globalResourceId(), texD->generation }; + } else if (colorAtt.renderBuffer()) { + RenderBufferType *rbD = QRHI_RES(RenderBufferType, colorAtt.renderBuffer()); + (*dst)[n] = { rbD->globalResourceId(), rbD->generation }; + } else { + (*dst)[n] = { 0, 0 }; + } + ++n; + if (colorAtt.resolveTexture()) { + TexType *texD = QRHI_RES(TexType, colorAtt.resolveTexture()); + (*dst)[n] = { texD->globalResourceId(), texD->generation }; + } else { + (*dst)[n] = { 0, 0 }; + } + } + if (hasDepthStencil) { + if (desc.depthTexture()) { + TexType *depthTexD = QRHI_RES(TexType, desc.depthTexture()); + (*dst)[n] = { depthTexD->globalResourceId(), depthTexD->generation }; + } else if (desc.depthStencilBuffer()) { + RenderBufferType *depthRbD = QRHI_RES(RenderBufferType, desc.depthStencilBuffer()); + (*dst)[n] = { depthRbD->globalResourceId(), depthRbD->generation }; + } else { + (*dst)[n] = { 0, 0 }; + } + } +} + +template +bool QRhiRenderTargetAttachmentTracker::isUpToDate(const QRhiTextureRenderTargetDescription &desc, const ResIdList ¤tResIdList) +{ + // Just as setShaderResources() recognizes if an srb's referenced + // resources have been rebuilt (got a create() since the srb's + // create()), we should do the same for the textures and renderbuffers + // referenced from the rendertarget. It is not uncommon that a texture + // or ds buffer gets resized due to following a window size in some + // form, which involves a create() on them. It is then nice if the + // render target auto-rebuilds in beginPass(). + + ResIdList resIdList; + updateResIdList(desc, &resIdList); + return resIdList == currentResIdList; +} + QT_END_NAMESPACE #endif diff --git a/src/gui/rhi/qrhid3d11.cpp b/src/gui/rhi/qrhid3d11.cpp index f6076bc800..a8fd9a686f 100644 --- a/src/gui/rhi/qrhid3d11.cpp +++ b/src/gui/rhi/qrhid3d11.cpp @@ -1762,6 +1762,8 @@ void QRhiD3D11::beginPass(QRhiCommandBuffer *cb, QD3D11TextureRenderTarget *rtTex = QRHI_RES(QD3D11TextureRenderTarget, rt); wantsColorClear = !rtTex->m_flags.testFlag(QRhiTextureRenderTarget::PreserveColorContents); wantsDsClear = !rtTex->m_flags.testFlag(QRhiTextureRenderTarget::PreserveDepthStencilContents); + if (!QRhiRenderTargetAttachmentTracker::isUpToDate(rtTex->description(), rtD->currentResIdList)) + rtTex->create(); } cbD->commands.get().cmd = QD3D11CommandBuffer::Command::ResetShaderResources; @@ -2898,6 +2900,7 @@ bool QD3D11RenderBuffer::create() QRHI_PROF; QRHI_PROF_F(newRenderBuffer(this, false, false, int(sampleDesc.Count))); + generation += 1; rhiD->registerResource(this); return true; } @@ -3609,6 +3612,8 @@ bool QD3D11TextureRenderTarget::create() d.dsv = dsv; d.rp = QRHI_RES(QD3D11RenderPassDescriptor, m_renderPassDesc); + QRhiRenderTargetAttachmentTracker::updateResIdList(m_desc, &d.currentResIdList); + rhiD->registerResource(this); return true; } diff --git a/src/gui/rhi/qrhid3d11_p_p.h b/src/gui/rhi/qrhid3d11_p_p.h index 3a333fe8a7..70c7ad969b 100644 --- a/src/gui/rhi/qrhid3d11_p_p.h +++ b/src/gui/rhi/qrhid3d11_p_p.h @@ -96,6 +96,7 @@ struct QD3D11RenderBuffer : public QRhiRenderBuffer ID3D11RenderTargetView *rtv = nullptr; DXGI_FORMAT dxgiFormat; DXGI_SAMPLE_DESC sampleDesc; + uint generation = 0; friend class QRhiD3D11; }; @@ -172,6 +173,8 @@ struct QD3D11RenderTargetData static const int MAX_COLOR_ATTACHMENTS = 8; ID3D11RenderTargetView *rtv[MAX_COLOR_ATTACHMENTS]; ID3D11DepthStencilView *dsv = nullptr; + + QRhiRenderTargetAttachmentTracker::ResIdList currentResIdList; }; struct QD3D11ReferenceRenderTarget : public QRhiRenderTarget diff --git a/src/gui/rhi/qrhigles2.cpp b/src/gui/rhi/qrhigles2.cpp index f0b97ce36a..67d08d3042 100644 --- a/src/gui/rhi/qrhigles2.cpp +++ b/src/gui/rhi/qrhigles2.cpp @@ -3793,6 +3793,12 @@ void QRhiGles2::beginPass(QRhiCommandBuffer *cb, // glMemoryBarrier() calls based on that tracker when submitted. enqueueBarriersForPass(cbD); + if (rt->resourceType() == QRhiRenderTarget::TextureRenderTarget) { + QGles2TextureRenderTarget *rtTex = QRHI_RES(QGles2TextureRenderTarget, rt); + if (!QRhiRenderTargetAttachmentTracker::isUpToDate(rtTex->description(), rtTex->d.currentResIdList)) + rtTex->create(); + } + bool wantsColorClear, wantsDsClear; QGles2RenderTargetData *rtD = enqueueBindFramebuffer(rt, cbD, &wantsColorClear, &wantsDsClear); @@ -4626,6 +4632,7 @@ bool QGles2RenderBuffer::create() } owns = true; + generation += 1; rhiD->registerResource(this); return true; } @@ -4653,6 +4660,7 @@ bool QGles2RenderBuffer::createFrom(NativeRenderBuffer src) QRHI_PROF_F(newRenderBuffer(this, false, false, samples)); owns = false; + generation += 1; rhiD->registerResource(this); return true; } @@ -5114,6 +5122,8 @@ bool QGles2TextureRenderTarget::create() return false; } + QRhiRenderTargetAttachmentTracker::updateResIdList(m_desc, &d.currentResIdList); + rhiD->registerResource(this); return true; } diff --git a/src/gui/rhi/qrhigles2_p_p.h b/src/gui/rhi/qrhigles2_p_p.h index 6ee6af8fc5..c0f3883002 100644 --- a/src/gui/rhi/qrhigles2_p_p.h +++ b/src/gui/rhi/qrhigles2_p_p.h @@ -108,6 +108,7 @@ struct QGles2RenderBuffer : public QRhiRenderBuffer GLuint stencilRenderbuffer = 0; // when packed depth-stencil not supported int samples; bool owns = true; + uint generation = 0; friend class QRhiGles2; }; @@ -213,6 +214,7 @@ struct QGles2RenderTargetData int colorAttCount = 0; int dsAttCount = 0; bool srgbUpdateAndBlend = false; + QRhiRenderTargetAttachmentTracker::ResIdList currentResIdList; }; struct QGles2ReferenceRenderTarget : public QRhiRenderTarget diff --git a/src/gui/rhi/qrhimetal.mm b/src/gui/rhi/qrhimetal.mm index 5219b5c71d..626eac433b 100644 --- a/src/gui/rhi/qrhimetal.mm +++ b/src/gui/rhi/qrhimetal.mm @@ -296,6 +296,8 @@ struct QMetalRenderTargetData bool hasStencil = false; bool depthNeedsStore = false; } fb; + + QRhiRenderTargetAttachmentTracker::ResIdList currentResIdList; }; struct QMetalGraphicsPipelineData @@ -2030,6 +2032,8 @@ void QRhiMetal::beginPass(QRhiCommandBuffer *cb, { QMetalTextureRenderTarget *rtTex = QRHI_RES(QMetalTextureRenderTarget, rt); rtD = rtTex->d; + if (!QRhiRenderTargetAttachmentTracker::isUpToDate(rtTex->description(), rtD->currentResIdList)) + rtTex->create(); cbD->d->currentPassRpDesc = d->createDefaultRenderPass(rtD->dsAttCount, colorClearValue, depthStencilClearValue, rtD->colorAttCount); if (rtTex->m_flags.testFlag(QRhiTextureRenderTarget::PreserveColorContents)) { for (uint i = 0; i < uint(rtD->colorAttCount); ++i) @@ -3181,6 +3185,8 @@ bool QMetalTextureRenderTarget::create() d->dsAttCount = 0; } + QRhiRenderTargetAttachmentTracker::updateResIdList(m_desc, &d->currentResIdList); + return true; } diff --git a/src/gui/rhi/qrhinull.cpp b/src/gui/rhi/qrhinull.cpp index b23af75bd9..6e85e02cc2 100644 --- a/src/gui/rhi/qrhinull.cpp +++ b/src/gui/rhi/qrhinull.cpp @@ -553,12 +553,18 @@ void QRhiNull::beginPass(QRhiCommandBuffer *cb, QRhiResourceUpdateBatch *resourceUpdates, QRhiCommandBuffer::BeginPassFlags flags) { - Q_UNUSED(rt); Q_UNUSED(colorClearValue); Q_UNUSED(depthStencilClearValue); Q_UNUSED(flags); + if (resourceUpdates) resourceUpdate(cb, resourceUpdates); + + if (rt->resourceType() == QRhiRenderTarget::TextureRenderTarget) { + QNullTextureRenderTarget *rtTex = QRHI_RES(QNullTextureRenderTarget, rt); + if (!QRhiRenderTargetAttachmentTracker::isUpToDate(rtTex->description(), rtTex->d.currentResIdList)) + rtTex->create(); + } } void QRhiNull::endPass(QRhiCommandBuffer *cb, QRhiResourceUpdateBatch *resourceUpdates) @@ -660,6 +666,7 @@ bool QNullRenderBuffer::create() destroy(); valid = true; + generation += 1; QRHI_PROF; QRHI_PROF_F(newRenderBuffer(this, false, false, 1)); @@ -726,6 +733,8 @@ bool QNullTexture::create() } } + generation += 1; + QRHI_PROF; QRHI_PROF_F(newTexture(this, true, mipLevelCount, layerCount, 1)); rhiD->registerResource(this); @@ -748,6 +757,8 @@ bool QNullTexture::createFrom(QRhiTexture::NativeTexture src) QSize size = m_pixelSize.isEmpty() ? QSize(1, 1) : m_pixelSize; const int mipLevelCount = hasMipMaps ? rhiD->q->mipLevelsForSize(size) : 1; + generation += 1; + QRHI_PROF; QRHI_PROF_F(newTexture(this, false, mipLevelCount, isCube ? 6 : (isArray ? m_arraySize : 1), 1)); rhiD->registerResource(this); @@ -871,6 +882,7 @@ bool QNullTextureRenderTarget::create() } else if (m_desc.depthTexture()) { d.pixelSize = m_desc.depthTexture()->pixelSize(); } + QRhiRenderTargetAttachmentTracker::updateResIdList(m_desc, &d.currentResIdList); return true; } diff --git a/src/gui/rhi/qrhinull_p_p.h b/src/gui/rhi/qrhinull_p_p.h index 4d3c508044..217ec8ef39 100644 --- a/src/gui/rhi/qrhinull_p_p.h +++ b/src/gui/rhi/qrhinull_p_p.h @@ -78,6 +78,7 @@ struct QNullRenderBuffer : public QRhiRenderBuffer QRhiTexture::Format backingFormat() const override; bool valid = false; + uint generation = 0; }; struct QNullTexture : public QRhiTexture @@ -91,6 +92,7 @@ struct QNullTexture : public QRhiTexture bool valid = false; QVarLengthArray, 6> image; + uint generation = 0; }; struct QNullSampler : public QRhiSampler @@ -119,6 +121,7 @@ struct QNullRenderTargetData QNullRenderPassDescriptor *rp = nullptr; QSize pixelSize; float dpr = 1; + QRhiRenderTargetAttachmentTracker::ResIdList currentResIdList; }; struct QNullReferenceRenderTarget : public QRhiRenderTarget diff --git a/src/gui/rhi/qrhivulkan.cpp b/src/gui/rhi/qrhivulkan.cpp index 03f3fb4ad8..e23900069c 100644 --- a/src/gui/rhi/qrhivulkan.cpp +++ b/src/gui/rhi/qrhivulkan.cpp @@ -2175,6 +2175,9 @@ static inline QRhiPassResourceTracker::UsageState toPassTrackerUsageState(const void QRhiVulkan::activateTextureRenderTarget(QVkCommandBuffer *cbD, QVkTextureRenderTarget *rtD) { + if (!QRhiRenderTargetAttachmentTracker::isUpToDate(rtD->description(), rtD->d.currentResIdList)) + rtD->create(); + rtD->lastActiveFrameSlot = currentFrameSlot; rtD->d.rp->lastActiveFrameSlot = currentFrameSlot; QRhiPassResourceTracker &passResTracker(cbD->passResTrackers[cbD->currentPassResTrackerIndex]); @@ -5759,6 +5762,7 @@ bool QVkRenderBuffer::create() } lastActiveFrameSlot = -1; + generation += 1; rhiD->registerResource(this); return true; } @@ -6572,6 +6576,8 @@ bool QVkTextureRenderTarget::create() return false; } + QRhiRenderTargetAttachmentTracker::updateResIdList(m_desc, &d.currentResIdList); + lastActiveFrameSlot = -1; rhiD->registerResource(this); return true; diff --git a/src/gui/rhi/qrhivulkan_p_p.h b/src/gui/rhi/qrhivulkan_p_p.h index 42ae9aef3b..da69a0d70d 100644 --- a/src/gui/rhi/qrhivulkan_p_p.h +++ b/src/gui/rhi/qrhivulkan_p_p.h @@ -123,6 +123,7 @@ struct QVkRenderBuffer : public QRhiRenderBuffer QVkTexture *backingTexture = nullptr; VkFormat vkformat; int lastActiveFrameSlot = -1; + uint generation = 0; friend class QRhiVulkan; }; @@ -214,6 +215,7 @@ struct QVkRenderTargetData int colorAttCount = 0; int dsAttCount = 0; int resolveAttCount = 0; + QRhiRenderTargetAttachmentTracker::ResIdList currentResIdList; static const int MAX_COLOR_ATTACHMENTS = 8; }; diff --git a/tests/auto/gui/rhi/qrhi/tst_qrhi.cpp b/tests/auto/gui/rhi/qrhi/tst_qrhi.cpp index 5142d5566f..89c02322dd 100644 --- a/tests/auto/gui/rhi/qrhi/tst_qrhi.cpp +++ b/tests/auto/gui/rhi/qrhi/tst_qrhi.cpp @@ -137,6 +137,8 @@ private slots: void finishWithinSwapchainFrame(); void resourceUpdateBatchBufferTextureWithSwapchainFrames_data(); void resourceUpdateBatchBufferTextureWithSwapchainFrames(); + void textureRenderTargetAutoRebuild_data(); + void textureRenderTargetAutoRebuild(); void pipelineCache_data(); void pipelineCache(); @@ -3767,6 +3769,49 @@ void tst_QRhi::resourceUpdateBatchBufferTextureWithSwapchainFrames() } } +void tst_QRhi::textureRenderTargetAutoRebuild_data() +{ + rhiTestData(); +} + +void tst_QRhi::textureRenderTargetAutoRebuild() +{ + QFETCH(QRhi::Implementation, impl); + QFETCH(QRhiInitParams *, initParams); + + QScopedPointer rhi(QRhi::create(impl, initParams, QRhi::Flags(), nullptr)); + 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()); + + 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)); + + 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(); +} + void tst_QRhi::srbLayoutCompatibility_data() { rhiTestData();