From 7000b66f7e85e58d267b71322fbb3d5a0fc356b8 Mon Sep 17 00:00:00 2001 From: Laszlo Agocs Date: Sat, 28 Sep 2019 15:19:09 +0200 Subject: [PATCH] Remove QVector in the API of QRhiResource subclasses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Forcing users to go through a QVector, when in practice they almost always want to source the data from an initializer list, a QVarLengthArray, or a plain C array, is not ideal. Especially since we can reason about the maximum number of elements in the vast majority of use cases for all the affected lists. QRhiResource is also not copyable so we do not need the usual machinery offered by containers. So switch to a QVarLengthArray. Note that a resource is not a container. The only operations we are interested in is to be able to source data either via an initializer list or by iterating on something, and to be able to extract the data, in case a user wishes to set up another resource based on the existing one. In some cases a QVector overload is kept for source compatibility with other modules (Qt Quick). These may be removed in the future. Also do a similar QVector->QVarLengthArray change in the srb-related data in the backends. Change-Id: I6f5b2ebd8e75416ce0cca0817bb529446a4cb664 Reviewed-by: Christian Strømme --- src/gui/rhi/qrhi.cpp | 13 ++--- src/gui/rhi/qrhi_p.h | 51 +++++++++++++++---- src/gui/rhi/qrhid3d11.cpp | 2 +- src/gui/rhi/qrhid3d11_p_p.h | 4 +- src/gui/rhi/qrhimetal.mm | 4 +- src/gui/rhi/qrhimetal_p_p.h | 4 +- src/gui/rhi/qrhivulkan.cpp | 3 +- src/gui/rhi/qrhivulkan_p_p.h | 4 +- tests/manual/rhi/mrt/mrt.cpp | 10 ++-- tests/manual/rhi/msaatexture/msaatexture.cpp | 2 +- tests/manual/rhi/texuploads/texuploads.cpp | 25 +++++---- .../rhi/triquadcube/trianglerenderer.cpp | 4 +- 12 files changed, 77 insertions(+), 49 deletions(-) diff --git a/src/gui/rhi/qrhi.cpp b/src/gui/rhi/qrhi.cpp index 2bf8c3c7b67..fa0c5fadc0f 100644 --- a/src/gui/rhi/qrhi.cpp +++ b/src/gui/rhi/qrhi.cpp @@ -2534,16 +2534,8 @@ QRhiResource::Type QRhiTextureRenderTarget::resourceType() const be used with the same pipeline, assuming the pipeline was built with one of them in the first place. - Creating and then using a new \c srb2 that is very similar to \c srb with - the exception of referencing another texture could be implemented like the - following: - \badcode srb2 = rhi->newShaderResourceBindings(); - QVector bindings = srb->bindings(); - bindings[1] = QRhiShaderResourceBinding::sampledTexture(1, QRhiShaderResourceBinding::FragmentStage, anotherTexture, sampler); - srb2->setBindings(bindings); - srb2->build(); ... cb->setGraphicsPipeline(ps); cb->setShaderResources(srb2); // binds srb2 @@ -3064,6 +3056,11 @@ QDebug operator<<(QDebug dbg, const QRhiShaderResourceBinding &b) #endif #ifndef QT_NO_DEBUG_STREAM +QDebug operator<<(QDebug dbg, const QVarLengthArray &bindings) +{ + return QtPrivate::printSequentialContainer(dbg, "Bindings", bindings); +} + QDebug operator<<(QDebug dbg, const QRhiShaderResourceBindings &srb) { QDebugStateSaver saver(dbg); diff --git a/src/gui/rhi/qrhi_p.h b/src/gui/rhi/qrhi_p.h index fec69c607f8..f8f922cfdb9 100644 --- a/src/gui/rhi/qrhi_p.h +++ b/src/gui/rhi/qrhi_p.h @@ -52,6 +52,7 @@ #include #include #include +#include #include #include #include @@ -926,8 +927,22 @@ class Q_GUI_EXPORT QRhiShaderResourceBindings : public QRhiResource public: QRhiResource::Type resourceType() const override; - QVector bindings() const { return m_bindings; } - void setBindings(const QVector &b) { m_bindings = b; } + void setBindings(std::initializer_list list) { m_bindings = list; } + + template + void setBindings(InputIterator first, InputIterator last) + { + m_bindings.clear(); + std::copy(first, last, std::back_inserter(m_bindings)); + } + + void setBindings(const QVector &bindings) // compat., to be removed + { + setBindings(bindings.cbegin(), bindings.cend()); + } + + const QRhiShaderResourceBinding *cbeginBindings() const { return m_bindings.cbegin(); } + const QRhiShaderResourceBinding *cendBindings() const { return m_bindings.cend(); } bool isLayoutCompatible(const QRhiShaderResourceBindings *other) const; @@ -935,7 +950,7 @@ public: protected: QRhiShaderResourceBindings(QRhiImplementation *rhi); - QVector m_bindings; + QVarLengthArray m_bindings; #ifndef QT_NO_DEBUG_STREAM friend Q_GUI_EXPORT QDebug operator<<(QDebug, const QRhiShaderResourceBindings &); #endif @@ -1066,8 +1081,15 @@ public: FrontFace frontFace() const { return m_frontFace; } void setFrontFace(FrontFace f) { m_frontFace = f; } - QVector targetBlends() const { return m_targetBlends; } - void setTargetBlends(const QVector &blends) { m_targetBlends = blends; } + void setTargetBlends(std::initializer_list list) { m_targetBlends = list; } + template + void setTargetBlends(InputIterator first, InputIterator last) + { + m_targetBlends.clear(); + std::copy(first, last, std::back_inserter(m_targetBlends)); + } + const TargetBlend *cbeginTargetBlends() const { return m_targetBlends.cbegin(); } + const TargetBlend *cendTargetBlends() const { return m_targetBlends.cend(); } bool hasDepthTest() const { return m_depthTest; } void setDepthTest(bool enable) { m_depthTest = enable; } @@ -1099,8 +1121,19 @@ public: float lineWidth() const { return m_lineWidth; } void setLineWidth(float width) { m_lineWidth = width; } - QVector shaderStages() const { return m_shaderStages; } - void setShaderStages(const QVector &stages) { m_shaderStages = stages; } + void setShaderStages(std::initializer_list list) { m_shaderStages = list; } + template + void setShaderStages(InputIterator first, InputIterator last) + { + m_shaderStages.clear(); + std::copy(first, last, std::back_inserter(m_shaderStages)); + } + void setShaderStages(const QVector &stages) // compat., to be removed + { + setShaderStages(stages.cbegin(), stages.cend()); + } + const QRhiShaderStage *cbeginShaderStages() const { return m_shaderStages.cbegin(); } + const QRhiShaderStage *cendShaderStages() const { return m_shaderStages.cend(); } QRhiVertexInputLayout vertexInputLayout() const { return m_vertexInputLayout; } void setVertexInputLayout(const QRhiVertexInputLayout &layout) { m_vertexInputLayout = layout; } @@ -1119,7 +1152,7 @@ protected: Topology m_topology = Triangles; CullMode m_cullMode = None; FrontFace m_frontFace = CCW; - QVector m_targetBlends; + QVarLengthArray m_targetBlends; bool m_depthTest = false; bool m_depthWrite = false; CompareOp m_depthOp = Less; @@ -1130,7 +1163,7 @@ protected: quint32 m_stencilWriteMask = 0xFF; int m_sampleCount = 1; float m_lineWidth = 1.0f; - QVector m_shaderStages; + QVarLengthArray m_shaderStages; QRhiVertexInputLayout m_vertexInputLayout; QRhiShaderResourceBindings *m_shaderResourceBindings = nullptr; QRhiRenderPassDescriptor *m_renderPassDesc = nullptr; diff --git a/src/gui/rhi/qrhid3d11.cpp b/src/gui/rhi/qrhid3d11.cpp index a47f1ff86c1..b82a68f3dd2 100644 --- a/src/gui/rhi/qrhid3d11.cpp +++ b/src/gui/rhi/qrhid3d11.cpp @@ -3082,7 +3082,7 @@ bool QD3D11ShaderResourceBindings::build() if (!sortedBindings.isEmpty()) release(); - sortedBindings = m_bindings; + 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/qrhid3d11_p_p.h b/src/gui/rhi/qrhid3d11_p_p.h index cf4808510ca..6699e7ad641 100644 --- a/src/gui/rhi/qrhid3d11_p_p.h +++ b/src/gui/rhi/qrhid3d11_p_p.h @@ -199,7 +199,7 @@ struct QD3D11ShaderResourceBindings : public QRhiShaderResourceBindings void release() override; bool build() override; - QVector sortedBindings; + QVarLengthArray sortedBindings; uint generation = 0; // Keep track of the generation number of each referenced QRhi* to be able @@ -230,7 +230,7 @@ struct QD3D11ShaderResourceBindings : public QRhiShaderResourceBindings BoundStorageBufferData sbuf; }; }; - QVector boundResourceData; + QVarLengthArray boundResourceData; QRhiBatchedBindings vsubufs; QRhiBatchedBindings vsubufoffsets; diff --git a/src/gui/rhi/qrhimetal.mm b/src/gui/rhi/qrhimetal.mm index ca6f829acfe..4dc12f06911 100644 --- a/src/gui/rhi/qrhimetal.mm +++ b/src/gui/rhi/qrhimetal.mm @@ -2768,7 +2768,7 @@ bool QMetalShaderResourceBindings::build() if (!sortedBindings.isEmpty()) release(); - sortedBindings = m_bindings; + std::copy(m_bindings.cbegin(), m_bindings.cend(), std::back_inserter(sortedBindings)); std::sort(sortedBindings.begin(), sortedBindings.end(), [](const QRhiShaderResourceBinding &a, const QRhiShaderResourceBinding &b) { @@ -2782,7 +2782,7 @@ bool QMetalShaderResourceBindings::build() boundResourceData.resize(sortedBindings.count()); for (int i = 0, ie = sortedBindings.count(); i != ie; ++i) { - const QRhiShaderResourceBinding::Data *b = srbD->sortedBindings.at(i).data(); + const QRhiShaderResourceBinding::Data *b = sortedBindings.at(i).data(); QMetalShaderResourceBindings::BoundResourceData &bd(boundResourceData[i]); switch (b->type) { case QRhiShaderResourceBinding::UniformBuffer: diff --git a/src/gui/rhi/qrhimetal_p_p.h b/src/gui/rhi/qrhimetal_p_p.h index 7fd7aba923e..688fec8147e 100644 --- a/src/gui/rhi/qrhimetal_p_p.h +++ b/src/gui/rhi/qrhimetal_p_p.h @@ -188,7 +188,7 @@ struct QMetalShaderResourceBindings : public QRhiShaderResourceBindings void release() override; bool build() override; - QVector sortedBindings; + QVarLengthArray sortedBindings; int maxBinding = -1; struct BoundUniformBufferData { @@ -217,7 +217,7 @@ struct QMetalShaderResourceBindings : public QRhiShaderResourceBindings BoundStorageBufferData sbuf; }; }; - QVector boundResourceData; + QVarLengthArray boundResourceData; uint generation = 0; friend class QRhiMetal; diff --git a/src/gui/rhi/qrhivulkan.cpp b/src/gui/rhi/qrhivulkan.cpp index 36cbc06f5af..b7fc50d259b 100644 --- a/src/gui/rhi/qrhivulkan.cpp +++ b/src/gui/rhi/qrhivulkan.cpp @@ -5697,7 +5697,8 @@ bool QVkShaderResourceBindings::build() for (int i = 0; i < QVK_FRAMES_IN_FLIGHT; ++i) descSets[i] = VK_NULL_HANDLE; - sortedBindings = m_bindings; + sortedBindings.clear(); + 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/qrhivulkan_p_p.h b/src/gui/rhi/qrhivulkan_p_p.h index a390bc3707c..d83a338acda 100644 --- a/src/gui/rhi/qrhivulkan_p_p.h +++ b/src/gui/rhi/qrhivulkan_p_p.h @@ -232,7 +232,7 @@ struct QVkShaderResourceBindings : public QRhiShaderResourceBindings void release() override; bool build() override; - QVector sortedBindings; + QVarLengthArray sortedBindings; int poolIndex = -1; VkDescriptorSetLayout layout = VK_NULL_HANDLE; VkDescriptorSet descSets[QVK_FRAMES_IN_FLIGHT]; // multiple sets to support dynamic buffers @@ -268,7 +268,7 @@ struct QVkShaderResourceBindings : public QRhiShaderResourceBindings BoundStorageBufferData sbuf; }; }; - QVector boundResourceData[QVK_FRAMES_IN_FLIGHT]; + QVarLengthArray boundResourceData[QVK_FRAMES_IN_FLIGHT]; friend class QRhiVulkan; }; diff --git a/tests/manual/rhi/mrt/mrt.cpp b/tests/manual/rhi/mrt/mrt.cpp index 258871f9b3b..dc72c7d194d 100644 --- a/tests/manual/rhi/mrt/mrt.cpp +++ b/tests/manual/rhi/mrt/mrt.cpp @@ -200,12 +200,10 @@ void Window::customInit() { QRhiShaderStage::Vertex, getShader(QLatin1String(":/mrt.vert.qsb")) }, { QRhiShaderStage::Fragment, getShader(QLatin1String(":/mrt.frag.qsb")) } }); - QVector blends; - for (int i = 0; i < ATTCOUNT; ++i) { - QRhiGraphicsPipeline::TargetBlend blend; - blends.append(blend); - } - d.triPs->setTargetBlends(blends); + + QRhiGraphicsPipeline::TargetBlend blends[ATTCOUNT]; // defaults to blending == false + d.triPs->setTargetBlends(blends, blends + ATTCOUNT); + inputLayout.setBindings({ { 5 * sizeof(float) } }); diff --git a/tests/manual/rhi/msaatexture/msaatexture.cpp b/tests/manual/rhi/msaatexture/msaatexture.cpp index d23a4a8d475..2fb466c8d61 100644 --- a/tests/manual/rhi/msaatexture/msaatexture.cpp +++ b/tests/manual/rhi/msaatexture/msaatexture.cpp @@ -240,7 +240,7 @@ void Window::customInit() #else d.msaaTriPs->setSampleCount(1); #endif - d.msaaTriPs->setShaderStages(d.triPs->shaderStages()); + d.msaaTriPs->setShaderStages(d.triPs->cbeginShaderStages(), d.triPs->cendShaderStages()); d.msaaTriPs->setVertexInputLayout(d.triPs->vertexInputLayout()); d.msaaTriPs->setShaderResourceBindings(d.triSrb); d.msaaTriPs->setRenderPassDescriptor(d.msaaRtRp); diff --git a/tests/manual/rhi/texuploads/texuploads.cpp b/tests/manual/rhi/texuploads/texuploads.cpp index f29a9891872..4c10a6b9658 100644 --- a/tests/manual/rhi/texuploads/texuploads.cpp +++ b/tests/manual/rhi/texuploads/texuploads.cpp @@ -68,6 +68,8 @@ struct { QRhiTexture *newTex = nullptr; QRhiTexture *importedTex = nullptr; int testStage = 0; + + QRhiShaderResourceBinding bindings[2]; } d; void Window::customInit() @@ -100,10 +102,10 @@ void Window::customInit() d.srb = m_r->newShaderResourceBindings(); d.releasePool << d.srb; - d.srb->setBindings({ - QRhiShaderResourceBinding::uniformBuffer(0, QRhiShaderResourceBinding::VertexStage | QRhiShaderResourceBinding::FragmentStage, d.ubuf), - QRhiShaderResourceBinding::sampledTexture(1, QRhiShaderResourceBinding::FragmentStage, d.tex, d.sampler) - }); + + d.bindings[0] = QRhiShaderResourceBinding::uniformBuffer(0, QRhiShaderResourceBinding::VertexStage | QRhiShaderResourceBinding::FragmentStage, d.ubuf); + d.bindings[1] = QRhiShaderResourceBinding::sampledTexture(1, QRhiShaderResourceBinding::FragmentStage, d.tex, d.sampler); + d.srb->setBindings(d.bindings, d.bindings + 2); d.srb->build(); d.ps = m_r->newGraphicsPipeline(); @@ -211,9 +213,8 @@ void Window::customRender() u->copyTexture(d.newTex, d.tex, desc); // Now replace d.tex with d.newTex as the shader resource. - auto bindings = d.srb->bindings(); - bindings[1] = QRhiShaderResourceBinding::sampledTexture(1, QRhiShaderResourceBinding::FragmentStage, d.newTex, d.sampler); - d.srb->setBindings(bindings); + d.bindings[1] = QRhiShaderResourceBinding::sampledTexture(1, QRhiShaderResourceBinding::FragmentStage, d.newTex, d.sampler); + d.srb->setBindings(d.bindings, d.bindings + 2); // "rebuild", whatever that means for a given backend. This srb is // already live as the ps in the setGraphicsPipeline references it, // but that's fine. Changes will be picked up automatically. @@ -259,9 +260,8 @@ void Window::customRender() // underneath (owned by d.tex) // switch to showing d.importedTex - auto bindings = d.srb->bindings(); - bindings[1] = QRhiShaderResourceBinding::sampledTexture(1, QRhiShaderResourceBinding::FragmentStage, d.importedTex, d.sampler); - d.srb->setBindings(bindings); + d.bindings[1] = QRhiShaderResourceBinding::sampledTexture(1, QRhiShaderResourceBinding::FragmentStage, d.importedTex, d.sampler); + d.srb->setBindings(d.bindings, d.bindings + 2); d.srb->build(); } else { qWarning("Accessing native texture object is not supported"); @@ -270,9 +270,8 @@ void Window::customRender() // Exercise uploading uncompressed data without a QImage. if (d.testStage == 7) { - auto bindings = d.srb->bindings(); - bindings[1] = QRhiShaderResourceBinding::sampledTexture(1, QRhiShaderResourceBinding::FragmentStage, d.newTex, d.sampler); - d.srb->setBindings(bindings); + d.bindings[1] = QRhiShaderResourceBinding::sampledTexture(1, QRhiShaderResourceBinding::FragmentStage, d.newTex, d.sampler); + d.srb->setBindings(d.bindings, d.bindings + 2); d.srb->build(); const QSize sz(221, 139); diff --git a/tests/manual/rhi/triquadcube/trianglerenderer.cpp b/tests/manual/rhi/triquadcube/trianglerenderer.cpp index 0980acca490..5d932aea520 100644 --- a/tests/manual/rhi/triquadcube/trianglerenderer.cpp +++ b/tests/manual/rhi/triquadcube/trianglerenderer.cpp @@ -94,11 +94,11 @@ void TriangleRenderer::initResources(QRhiRenderPassDescriptor *rp) QRhiGraphicsPipeline::TargetBlend premulAlphaBlend; // convenient defaults... premulAlphaBlend.enable = true; - QVector rtblends; + QVarLengthArray rtblends; for (int i = 0; i < m_colorAttCount; ++i) rtblends << premulAlphaBlend; - m_ps->setTargetBlends(rtblends); + m_ps->setTargetBlends(rtblends.cbegin(), rtblends.cend()); m_ps->setSampleCount(m_sampleCount); if (m_depthWrite) { // TriangleOnCube may want to exercise this