From 6b6475539815f73dcab5ccf94ea33fd54fcb23bf Mon Sep 17 00:00:00 2001 From: Laszlo Agocs Date: Mon, 6 Sep 2021 15:45:01 +0200 Subject: rhi: Make the serialized srb layout description accessible ...by the Qt Quick renderer, for example. A typical Qt Quick material binding set serializes to 8 uints. This would not demand a container like QVector. However, being implicitly shared is essential here due to the intended usage (query the serialized blob, put it into a cache key, hash it, compare it, all without any copying and new allocs; we can afford an extra alloc upon each srb construction, but don't want more afterwards in the rendering engines) Also make it clear in the pipeline docs that the optimization Qt Quick is (soon going to be) doing is legal. (the srb ref in the pipeline can be dead and dangling as long as every call to setShaderResources() specifies a layout-compatible alternative) Change-Id: I97efbea1fa3516b10c9832adbab0a21b7bc0845d Reviewed-by: Andy Nichols (cherry picked from commit b6b0c33058ba7f43661e316d9f27d4102f6a988f) Reviewed-by: Qt Cherry-pick Bot --- src/gui/rhi/qrhi.cpp | 8 ++++++++ src/gui/rhi/qrhi_p.h | 7 ++++++- tests/auto/gui/rhi/qrhi/tst_qrhi.cpp | 16 ++++++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/gui/rhi/qrhi.cpp b/src/gui/rhi/qrhi.cpp index e79dc1e822..1578bf0fd4 100644 --- a/src/gui/rhi/qrhi.cpp +++ b/src/gui/rhi/qrhi.cpp @@ -3002,6 +3002,7 @@ QRhiResource::Type QRhiTextureRenderTarget::resourceType() const QRhiShaderResourceBindings::QRhiShaderResourceBindings(QRhiImplementation *rhi) : QRhiResource(rhi) { + m_layoutDesc.reserve(BINDING_PREALLOC * LAYOUT_DESC_FIELD_COUNT); } /*! @@ -5620,6 +5621,13 @@ void QRhiCommandBuffer::setGraphicsPipeline(QRhiGraphicsPipeline *ps) srb. In this case setShaderResources() must be called even if \a srb is the same as in the last call. + When \a srb is not null, the QRhiShaderResourceBindings object the pipeline + was built with in create() is guaranteed to be not accessed in any form. In + fact, it does not need to be valid even at this point: destroying the + pipeline's associated srb after create() and instead explicitly specifying + another, \l{QRhiShaderResourceBindings::isLayoutCompatible()}{layout + compatible} one in every setShaderResources() call is valid. + \a dynamicOffsets allows specifying buffer offsets for uniform buffers that were associated with \a srb via QRhiShaderResourceBinding::uniformBufferWithDynamicOffset(). This is diff --git a/src/gui/rhi/qrhi_p.h b/src/gui/rhi/qrhi_p.h index 1e570e8818..8c3e0fdfb2 100644 --- a/src/gui/rhi/qrhi_p.h +++ b/src/gui/rhi/qrhi_p.h @@ -1037,6 +1037,8 @@ public: bool isLayoutCompatible(const QRhiShaderResourceBindings *other) const; + QVector serializedLayoutDescription() const { return m_layoutDesc; } + virtual bool create() = 0; enum UpdateFlag { @@ -1052,7 +1054,10 @@ protected: QRhiShaderResourceBindings(QRhiImplementation *rhi); QVarLengthArray m_bindings; uint m_layoutDescHash = 0; - QVarLengthArray m_layoutDesc; + // Intentionally not using QVLA for m_layoutDesc: clients like Qt Quick are much + // better served with an implicitly shared container here, because they will likely + // throw this directly into structs serving as cache keys. + QVector m_layoutDesc; friend class QRhiImplementation; #ifndef QT_NO_DEBUG_STREAM friend Q_GUI_EXPORT QDebug operator<<(QDebug, const QRhiShaderResourceBindings &); diff --git a/tests/auto/gui/rhi/qrhi/tst_qrhi.cpp b/tests/auto/gui/rhi/qrhi/tst_qrhi.cpp index be825c8ac2..61e19434e0 100644 --- a/tests/auto/gui/rhi/qrhi/tst_qrhi.cpp +++ b/tests/auto/gui/rhi/qrhi/tst_qrhi.cpp @@ -3271,6 +3271,8 @@ void tst_QRhi::srbLayoutCompatibility() QVERIFY(srb1->isLayoutCompatible(srb2.data())); QVERIFY(srb2->isLayoutCompatible(srb1.data())); + + QCOMPARE(srb1->serializedLayoutDescription(), srb2->serializedLayoutDescription()); } // different count (not compatible) @@ -3286,6 +3288,8 @@ void tst_QRhi::srbLayoutCompatibility() QVERIFY(!srb1->isLayoutCompatible(srb2.data())); QVERIFY(!srb2->isLayoutCompatible(srb1.data())); + + QVERIFY(srb1->serializedLayoutDescription() != srb2->serializedLayoutDescription()); } // full match (compatible) @@ -3306,6 +3310,10 @@ void tst_QRhi::srbLayoutCompatibility() QVERIFY(srb1->isLayoutCompatible(srb2.data())); QVERIFY(srb2->isLayoutCompatible(srb1.data())); + + QVERIFY(!srb1->serializedLayoutDescription().isEmpty()); + QVERIFY(!srb2->serializedLayoutDescription().isEmpty()); + QCOMPARE(srb1->serializedLayoutDescription(), srb2->serializedLayoutDescription()); } // different visibility (not compatible) @@ -3324,6 +3332,8 @@ void tst_QRhi::srbLayoutCompatibility() QVERIFY(!srb1->isLayoutCompatible(srb2.data())); QVERIFY(!srb2->isLayoutCompatible(srb1.data())); + + QVERIFY(srb1->serializedLayoutDescription() != srb2->serializedLayoutDescription()); } // different binding points (not compatible) @@ -3342,6 +3352,8 @@ void tst_QRhi::srbLayoutCompatibility() QVERIFY(!srb1->isLayoutCompatible(srb2.data())); QVERIFY(!srb2->isLayoutCompatible(srb1.data())); + + QVERIFY(srb1->serializedLayoutDescription() != srb2->serializedLayoutDescription()); } // different buffer region offset and size (compatible) @@ -3362,6 +3374,8 @@ void tst_QRhi::srbLayoutCompatibility() QVERIFY(srb1->isLayoutCompatible(srb2.data())); QVERIFY(srb2->isLayoutCompatible(srb1.data())); + + QCOMPARE(srb1->serializedLayoutDescription(), srb2->serializedLayoutDescription()); } // different resources (compatible) @@ -3382,6 +3396,8 @@ void tst_QRhi::srbLayoutCompatibility() QVERIFY(srb1->isLayoutCompatible(srb2.data())); QVERIFY(srb2->isLayoutCompatible(srb1.data())); + + QCOMPARE(srb1->serializedLayoutDescription(), srb2->serializedLayoutDescription()); } } -- cgit v1.2.1