From 868866cecd57e671d1b2826b59d3fa1c872b214c Mon Sep 17 00:00:00 2001 From: Laszlo Agocs Date: Mon, 28 Sep 2020 13:05:38 +0200 Subject: [PATCH] rhi: Improve layout compatibility test performance Also bump the non-heap buffer size in the binding list to 16, in order to accommodate complex Quick3D materials with many associated texture maps. Change-Id: Id190e5f8304f5941cffc41a2605fce45dfeb72f0 Reviewed-by: Andy Nichols --- src/gui/rhi/qrhi.cpp | 25 +++++++++++++++++-------- src/gui/rhi/qrhi_p.h | 5 ++++- src/gui/rhi/qrhi_p_p.h | 1 + src/gui/rhi/qrhid3d11.cpp | 2 ++ src/gui/rhi/qrhigles2.cpp | 2 ++ src/gui/rhi/qrhimetal.mm | 2 ++ src/gui/rhi/qrhinull.cpp | 6 ++++++ src/gui/rhi/qrhivulkan.cpp | 2 ++ 8 files changed, 36 insertions(+), 9 deletions(-) diff --git a/src/gui/rhi/qrhi.cpp b/src/gui/rhi/qrhi.cpp index a54d7ac405..21542930ac 100644 --- a/src/gui/rhi/qrhi.cpp +++ b/src/gui/rhi/qrhi.cpp @@ -2806,16 +2806,25 @@ bool QRhiShaderResourceBindings::isLayoutCompatible(const QRhiShaderResourceBind if (!other) return false; - const int count = m_bindings.count(); - if (count != other->m_bindings.count()) - return false; + // This can become a hot code path. Therefore we do not iterate and call + // isLayoutCompatible() on m_bindings, but rather check a pre-calculated + // hash code and then, if the hash matched, do a uint array comparison + // (that's still more cache friendly). - for (int i = 0; i < count; ++i) { - if (!m_bindings[i].isLayoutCompatible(other->m_bindings.at(i))) - return false; + return m_layoutDescHash == other->m_layoutDescHash + && m_layoutDesc == other->m_layoutDesc; +} + +void QRhiImplementation::updateLayoutDesc(QRhiShaderResourceBindings *srb) +{ + srb->m_layoutDescHash = 0; + srb->m_layoutDesc.clear(); + for (const QRhiShaderResourceBinding &b : qAsConst(srb->m_bindings)) { + const QRhiShaderResourceBinding::Data *d = b.data(); + // must match QRhiShaderResourceBinding::isLayoutCompatible() + srb->m_layoutDescHash ^= uint(d->binding) ^ uint(d->stage) ^ uint(d->type); + srb->m_layoutDesc << uint(d->binding) << uint(d->stage) << uint(d->type); } - - return true; } /*! diff --git a/src/gui/rhi/qrhi_p.h b/src/gui/rhi/qrhi_p.h index 1d0b6ebfab..d3e32f2723 100644 --- a/src/gui/rhi/qrhi_p.h +++ b/src/gui/rhi/qrhi_p.h @@ -1012,7 +1012,10 @@ public: protected: QRhiShaderResourceBindings(QRhiImplementation *rhi); - QVarLengthArray m_bindings; + QVarLengthArray m_bindings; + uint m_layoutDescHash = 0; + QVarLengthArray m_layoutDesc; + friend class QRhiImplementation; #ifndef QT_NO_DEBUG_STREAM friend Q_GUI_EXPORT QDebug operator<<(QDebug, const QRhiShaderResourceBindings &); #endif diff --git a/src/gui/rhi/qrhi_p_p.h b/src/gui/rhi/qrhi_p_p.h index 0128fd2d4e..51b3645b9d 100644 --- a/src/gui/rhi/qrhi_p_p.h +++ b/src/gui/rhi/qrhi_p_p.h @@ -211,6 +211,7 @@ public: bool sanityCheckGraphicsPipeline(QRhiGraphicsPipeline *ps); bool sanityCheckShaderResourceBindings(QRhiShaderResourceBindings *srb); + void updateLayoutDesc(QRhiShaderResourceBindings *srb); QRhi *q; diff --git a/src/gui/rhi/qrhid3d11.cpp b/src/gui/rhi/qrhid3d11.cpp index 2dc5763bd9..1c7f3cd17a 100644 --- a/src/gui/rhi/qrhid3d11.cpp +++ b/src/gui/rhi/qrhid3d11.cpp @@ -3456,6 +3456,8 @@ bool QD3D11ShaderResourceBindings::create() if (!rhiD->sanityCheckShaderResourceBindings(this)) return false; + rhiD->updateLayoutDesc(this); + std::copy(m_bindings.cbegin(), m_bindings.cend(), std::back_inserter(sortedBindings)); std::sort(sortedBindings.begin(), sortedBindings.end(), [](const QRhiShaderResourceBinding &a, const QRhiShaderResourceBinding &b) diff --git a/src/gui/rhi/qrhigles2.cpp b/src/gui/rhi/qrhigles2.cpp index 3405465d54..95f4e02976 100644 --- a/src/gui/rhi/qrhigles2.cpp +++ b/src/gui/rhi/qrhigles2.cpp @@ -4307,6 +4307,8 @@ bool QGles2ShaderResourceBindings::create() if (!rhiD->sanityCheckShaderResourceBindings(this)) return false; + rhiD->updateLayoutDesc(this); + generation += 1; return true; } diff --git a/src/gui/rhi/qrhimetal.mm b/src/gui/rhi/qrhimetal.mm index 7bcb6864d6..9058a7556a 100644 --- a/src/gui/rhi/qrhimetal.mm +++ b/src/gui/rhi/qrhimetal.mm @@ -3013,6 +3013,8 @@ bool QMetalShaderResourceBindings::create() if (!rhiD->sanityCheckShaderResourceBindings(this)) return false; + rhiD->updateLayoutDesc(this); + std::copy(m_bindings.cbegin(), m_bindings.cend(), std::back_inserter(sortedBindings)); std::sort(sortedBindings.begin(), sortedBindings.end(), [](const QRhiShaderResourceBinding &a, const QRhiShaderResourceBinding &b) diff --git a/src/gui/rhi/qrhinull.cpp b/src/gui/rhi/qrhinull.cpp index 087559728d..e0eb110c0d 100644 --- a/src/gui/rhi/qrhinull.cpp +++ b/src/gui/rhi/qrhinull.cpp @@ -800,6 +800,12 @@ void QNullShaderResourceBindings::destroy() bool QNullShaderResourceBindings::create() { + QRHI_RES_RHI(QRhiNull); + if (!rhiD->sanityCheckShaderResourceBindings(this)) + return false; + + rhiD->updateLayoutDesc(this); + return true; } diff --git a/src/gui/rhi/qrhivulkan.cpp b/src/gui/rhi/qrhivulkan.cpp index 7abfe37be1..73aa32f08b 100644 --- a/src/gui/rhi/qrhivulkan.cpp +++ b/src/gui/rhi/qrhivulkan.cpp @@ -6186,6 +6186,8 @@ bool QVkShaderResourceBindings::create() if (!rhiD->sanityCheckShaderResourceBindings(this)) return false; + rhiD->updateLayoutDesc(this); + for (int i = 0; i < QVK_FRAMES_IN_FLIGHT; ++i) descSets[i] = VK_NULL_HANDLE;