From e23af8b4a6b852d820a5597f3c7a456087cf6518 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sun, 24 Jan 2021 19:26:40 +0100 Subject: [PATCH] Vk: provide comparison for MeshLayout. First step towards pipeline caches. Gah the pointer jungle is making this unnecessarily hard. --- src/Magnum/Vk/MeshLayout.cpp | 50 ++++++ src/Magnum/Vk/MeshLayout.h | 41 +++++ src/Magnum/Vk/Test/MeshLayoutTest.cpp | 235 ++++++++++++++++++++++++++ 3 files changed, 326 insertions(+) diff --git a/src/Magnum/Vk/MeshLayout.cpp b/src/Magnum/Vk/MeshLayout.cpp index 773b81474..5d49267ca 100644 --- a/src/Magnum/Vk/MeshLayout.cpp +++ b/src/Magnum/Vk/MeshLayout.cpp @@ -122,6 +122,56 @@ MeshLayout& MeshLayout::operator=(MeshLayout&& other) noexcept { return *this; } +#ifndef CORRADE_NO_ASSERT +bool MeshLayout::hasNoExternalPointers() const { + return + /* Vertex info should only point to the vertex divisor info */ + (!_vertexInfo.pNext || (_state && _vertexInfo.pNext == &_state->vertexDivisorInfo)) && + /* Vertex divisor info should not point anywhere, if it exists */ + (!_state || !_state->vertexDivisorInfo.pNext) && + /* Assembly info should not point anywhere */ + !_assemblyInfo.pNext && + + /* Vertex binding descriptions should point to our data, if any, + otherwise it should be null and the count zero */ + ((!_vertexInfo.vertexBindingDescriptionCount && !_vertexInfo.pVertexBindingDescriptions) || (_state && _vertexInfo.pVertexBindingDescriptions == _state->bindings)) && + /* Attribute descriptions should point to our data, if any, + otherwise it should be null and the count zero */ + ((!_vertexInfo.vertexAttributeDescriptionCount && !_vertexInfo.pVertexAttributeDescriptions) || (_state && _vertexInfo.pVertexAttributeDescriptions == _state->attributes)) && + /* Vertex divisor descriptions should point to our data, if any */ + (!_state || _state->vertexDivisorInfo.pVertexBindingDivisors == _state->bindingDivisors); +} +#endif + +bool MeshLayout::operator==(const MeshLayout& other) const { + CORRADE_ASSERT(hasNoExternalPointers() && other.hasNoExternalPointers(), + "Vk::MeshLayout: can't compare structures with external pointers", {}); + + #define _c(field) other.field == field + /* First compare the top-level fields */ + if(!(_c(_vertexInfo.flags) && + _c(_vertexInfo.vertexBindingDescriptionCount) && + _c(_vertexInfo.vertexAttributeDescriptionCount) && + _c(_assemblyInfo.flags) && + _c(_assemblyInfo.topology) && + _c(_assemblyInfo.primitiveRestartEnable))) return false; + + /* Then continue only if both have the state struct -- if only one has it, + it can be still equal if the counts are zero for both (which is verified + by the assert above) */ + if(!other._state || !_state) return true; + + return _c(_state->vertexDivisorInfo.vertexBindingDivisorCount) && + /* These assume the bindings and locations are sorted (as the asserts + enforce), otherwise this wouldn't be enough */ + /** @todo use Utility::equal() once it exists, this is way too + error-prone */ + std::memcmp(other._state->bindings, _state->bindings, _vertexInfo.vertexBindingDescriptionCount*sizeof(decltype(_state->bindings)::Type)) == 0 && + std::memcmp(other._state->bindingDivisors, _state->bindingDivisors, _state->vertexDivisorInfo.vertexBindingDivisorCount*sizeof(decltype(_state->bindingDivisors)::Type)) == 0 && + std::memcmp(other._state->attributes, _state->attributes, _vertexInfo.vertexAttributeDescriptionCount*sizeof(decltype(_state->attributes)::Type)) == 0; + #undef _c +} + MeshLayout& MeshLayout::addBinding(const UnsignedInt binding, const UnsignedInt stride) { if(!_state) _state.emplace(); diff --git a/src/Magnum/Vk/MeshLayout.h b/src/Magnum/Vk/MeshLayout.h index 566c956ff..ac6dfcede 100644 --- a/src/Magnum/Vk/MeshLayout.h +++ b/src/Magnum/Vk/MeshLayout.h @@ -192,6 +192,16 @@ could look like this: The `BufferBinding` is then subsequently used as a binding index for a concrete vertex buffer when drawing. + +@subsection Vk-MeshLayout-usage-comparison Layout comparison + +Because a pipeline is tied to a particular mesh layout (apart from certain +aspects that can be controlled via dynamic state), new pipelines should be +created only when the layout is actually different. For that, the class +provides a @ref operator==(), which returns @cpp true @ce when the two layouts +match. + +@todo provide a hash function */ class MAGNUM_VK_EXPORT MeshLayout { public: @@ -249,6 +259,33 @@ class MAGNUM_VK_EXPORT MeshLayout { /** @brief Move assignment */ MeshLayout& operator=(MeshLayout&& other) noexcept; + /** + * @brief Equality comparison + * + * Expects that neither instance has external `pNext` or other data + * pointers, as otherwise the implementation would have to be too + * complex and slow --- in short that means all bindings and attributes + * should be added via @ref addBinding(), @ref addInstancedBinding() + * and @ref addAttribute(), not by conversion from raw Vulkan + * structures or by direct editing of the underlying data. + * + * Given that @ref addBinding(), @ref addInstancedBinding() and + * @ref addAttribute() enforce monotonically increasing order, + * comparison is simple with a @f$ \mathcal{O}(n) @f$ complexity in the + * total number of bindings and attachments. + */ + bool operator==(const MeshLayout& other) const; + + /** + * @brief Non-equality comparison + * + * Inverse of @ref operator==(), see its documentation for more + * information. + */ + bool operator!=(const MeshLayout& other) const { + return !operator==(other); + } + /** * @brief Add a buffer binding * @param binding Binding index, to which a buffer subrange will be @@ -367,6 +404,10 @@ class MAGNUM_VK_EXPORT MeshLayout { } private: + #ifndef CORRADE_NO_ASSERT + MAGNUM_VK_LOCAL bool hasNoExternalPointers() const; + #endif + VkPipelineVertexInputStateCreateInfo _vertexInfo; VkPipelineInputAssemblyStateCreateInfo _assemblyInfo; diff --git a/src/Magnum/Vk/Test/MeshLayoutTest.cpp b/src/Magnum/Vk/Test/MeshLayoutTest.cpp index aba5f5328..d7634e9d1 100644 --- a/src/Magnum/Vk/Test/MeshLayoutTest.cpp +++ b/src/Magnum/Vk/Test/MeshLayoutTest.cpp @@ -57,6 +57,9 @@ struct MeshLayoutTest: TestSuite::Tester { template void addAttribute(); void addAttributeWrongOrder(); + void compare(); + void compareExternalPointers(); + void debugMeshPrimitive(); }; @@ -81,6 +84,9 @@ MeshLayoutTest::MeshLayoutTest() { &MeshLayoutTest::addAttribute, &MeshLayoutTest::addAttributeWrongOrder, + &MeshLayoutTest::compare, + &MeshLayoutTest::compareExternalPointers, + &MeshLayoutTest::debugMeshPrimitive}); } @@ -377,6 +383,235 @@ void MeshLayoutTest::addAttributeWrongOrder() { "Vk::MeshLayout::addAttribute(): location 5 can't be ordered after 5\n"); } +void MeshLayoutTest::compare() { + MeshLayout emptyTriangles1{MeshPrimitive::Triangles}; + MeshLayout emptyTriangles2{MeshPrimitive::Triangles}; + CORRADE_VERIFY(emptyTriangles1 == emptyTriangles1); + CORRADE_VERIFY(emptyTriangles1 == emptyTriangles2); + CORRADE_VERIFY(emptyTriangles2 == emptyTriangles1); + CORRADE_VERIFY(!(emptyTriangles1 != emptyTriangles2)); + + MeshLayout emptyLines{MeshPrimitive::Lines}; + CORRADE_VERIFY(emptyLines != emptyTriangles1); + CORRADE_VERIFY(emptyTriangles1 != emptyLines); + + MeshLayout basic1{MeshPrimitive::Triangles}; + basic1.addBinding(7, 25) + .addBinding(8, 3) + .addAttribute(0, 3, VertexFormat::Vector2h, 26) + .addAttribute(1, 7, VertexFormat::Vector2b, 27); + MeshLayout basic2{MeshPrimitive::Triangles}; + basic2.addBinding(7, 25) + .addBinding(8, 3) + .addAttribute(0, 3, VertexFormat::Vector2h, 26) + .addAttribute(1, 7, VertexFormat::Vector2b, 27); + CORRADE_VERIFY(basic1 == basic1); + CORRADE_VERIFY(basic1 == basic2); + CORRADE_VERIFY(basic2 == basic1); + + MeshLayout different1{MeshPrimitive::Triangles}; + different1.addBinding(7, 25) + .addBinding(8, 3) + .addAttribute(0, 3, VertexFormat::Vector2h, 26) + .addAttribute(1, 7, VertexFormat::Vector2h, 27); /* different format */ + CORRADE_VERIFY(basic1 != different1); + + MeshLayout different2{MeshPrimitive::Triangles}; + different2.addBinding(7, 25) + .addBinding(8, 4) /* different stride */ + .addAttribute(0, 3, VertexFormat::Vector2h, 26) + .addAttribute(1, 7, VertexFormat::Vector2b, 27); + CORRADE_VERIFY(basic1 != different2); + + MeshLayout larger1{MeshPrimitive::Triangles}; + larger1.addBinding(7, 25) + .addBinding(8, 3) + .addBinding(9, 27) /* new entry */ + .addAttribute(0, 3, VertexFormat::Vector2h, 26) + .addAttribute(1, 7, VertexFormat::Vector2b, 27); + CORRADE_VERIFY(basic1 != larger1); + + /* It should take this value into account, not the internal array size */ + larger1.vkPipelineVertexInputStateCreateInfo().vertexBindingDescriptionCount = 2; + CORRADE_VERIFY(basic1 == larger1); + + MeshLayout larger2{MeshPrimitive::Triangles}; + larger2.addBinding(7, 25) + .addBinding(8, 3) + .addAttribute(0, 3, VertexFormat::Vector2h, 26) + .addAttribute(1, 7, VertexFormat::Vector2b, 27) + .addAttribute(2, 15, VertexFormat::Vector2, 3); /* new entry */ + CORRADE_VERIFY(basic1 != larger2); + + /* It should take this value into account, not the internal array size */ + larger2.vkPipelineVertexInputStateCreateInfo().vertexAttributeDescriptionCount = 2; + CORRADE_VERIFY(basic1 == larger2); + + MeshLayout instanced1{MeshPrimitive::Triangles}; + instanced1.addInstancedBinding(15, 35) + .addBinding(17, 25); + MeshLayout instanced2{MeshPrimitive::Triangles}; + instanced2.addInstancedBinding(15, 35) + .addBinding(17, 25); + CORRADE_VERIFY(instanced1 == instanced1); + CORRADE_VERIFY(instanced1 == instanced2); + CORRADE_VERIFY(instanced2 == instanced1); + + MeshLayout nonInstanced{MeshPrimitive::Triangles}; + nonInstanced.addBinding(15, 35) /* not instanced, but same otherwise */ + .addBinding(17, 25); + CORRADE_VERIFY(instanced1 != nonInstanced); + + MeshLayout divisor1{MeshPrimitive::Triangles}; + divisor1.addInstancedBinding(15, 35) + .addInstancedBinding(16, 8, 75) + .addBinding(17, 25); + MeshLayout divisor2{MeshPrimitive::Triangles}; + divisor2.addInstancedBinding(15, 35) + .addInstancedBinding(16, 8, 75) + .addBinding(17, 25); + CORRADE_VERIFY(divisor1 == divisor1); + CORRADE_VERIFY(divisor1 == divisor2); + CORRADE_VERIFY(divisor2 == divisor1); + + MeshLayout larger3{MeshPrimitive::Triangles}; + larger3.addInstancedBinding(15, 35) + .addInstancedBinding(16, 8, 75) + .addBinding(17, 25) + .addInstancedBinding(18, 2, 11); /* new entry */ + CORRADE_VERIFY(divisor1 != larger3); + + /* It should take this value into account, not the internal array size */ + CORRADE_VERIFY(larger3.vkPipelineVertexInputStateCreateInfo().pNext); + static_cast(const_cast(larger3.vkPipelineVertexInputStateCreateInfo().pNext))->vertexBindingDivisorCount = 1; + larger3.vkPipelineVertexInputStateCreateInfo().vertexBindingDescriptionCount = 3; + CORRADE_VERIFY(divisor1 == larger3); + + MeshLayout divisor3{MeshPrimitive::Triangles}; + divisor3.addInstancedBinding(15, 35, 75) /* divisor here instead of 2nd */ + .addInstancedBinding(16, 8) + .addBinding(17, 25); + CORRADE_VERIFY(divisor1 != divisor3); +} + +void MeshLayoutTest::compareExternalPointers() { + #ifdef CORRADE_NO_ASSERT + CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions"); + #endif + + /* Disallowed pNext inside one struct */ + { + MeshLayout empty{MeshPrimitive::Lines}; + MeshLayout layout{MeshPrimitive::Lines}; + layout.vkPipelineVertexInputStateCreateInfo().pNext = &layout; + + /* Test both comparison directions to verify the check is done for + both */ + std::ostringstream out; + Error redirectError{&out}; + layout.operator==(empty); /* to avoid unused expression warnings */ + empty.operator==(layout); + CORRADE_COMPARE(out.str(), + "Vk::MeshLayout: can't compare structures with external pointers\n" + "Vk::MeshLayout: can't compare structures with external pointers\n"); + + /* Disallowed pNext inside the other */ + } { + MeshLayout layout{MeshPrimitive::Lines}; + layout.vkPipelineInputAssemblyStateCreateInfo().pNext = &layout; + + std::ostringstream out; + Error redirectError{&out}; + layout.operator==(layout); /* to avoid unused expression warnings */ + CORRADE_COMPARE(out.str(), "Vk::MeshLayout: can't compare structures with external pointers\n"); + + /* Disallowed pNext inside the divisor struct */ + } { + MeshLayout layout{MeshPrimitive::Lines}; + layout.addInstancedBinding(3, 5, 7); + /* At this point it should still work */ + CORRADE_VERIFY(layout == layout); + CORRADE_VERIFY(layout.vkPipelineVertexInputStateCreateInfo().pNext); + static_cast(const_cast(layout.vkPipelineVertexInputStateCreateInfo().pNext))->pNext = &layout; + + std::ostringstream out; + Error redirectError{&out}; + layout.operator==(layout); /* to avoid unused expression warnings */ + CORRADE_COMPARE(out.str(), "Vk::MeshLayout: can't compare structures with external pointers\n"); + + /* External vertex bindings */ + } { + VkVertexInputBindingDescription bindingData; + MeshLayout layout{MeshPrimitive::Lines}; + layout.vkPipelineVertexInputStateCreateInfo().pVertexBindingDescriptions = &bindingData; + + std::ostringstream out; + Error redirectError{&out}; + layout.operator==(layout); /* to avoid unused expression warnings */ + CORRADE_COMPARE(out.str(), "Vk::MeshLayout: can't compare structures with external pointers\n"); + + /* Null vertex bindings but non-zero size */ + } { + MeshLayout layout{MeshPrimitive::Lines}; + layout.vkPipelineVertexInputStateCreateInfo().vertexAttributeDescriptionCount = 3; + + std::ostringstream out; + Error redirectError{&out}; + layout.operator==(layout); /* to avoid unused expression warnings */ + CORRADE_COMPARE(out.str(), "Vk::MeshLayout: can't compare structures with external pointers\n"); + + /* External vertex divisors */ + } { + VkVertexInputBindingDivisorDescriptionEXT bindingDivisorData; + MeshLayout layout{MeshPrimitive::Lines}; + layout.addInstancedBinding(3, 5, 7); + /* At this point it should still work */ + CORRADE_VERIFY(layout == layout); + CORRADE_VERIFY(layout.vkPipelineVertexInputStateCreateInfo().pNext); + static_cast(const_cast(layout.vkPipelineVertexInputStateCreateInfo().pNext))->pVertexBindingDivisors = &bindingDivisorData; + + std::ostringstream out; + Error redirectError{&out}; + layout.operator==(layout); /* to avoid unused expression warnings */ + CORRADE_COMPARE(out.str(), "Vk::MeshLayout: can't compare structures with external pointers\n"); + + /* Null vertex divisors but non-zero size */ + } { + MeshLayout layout{MeshPrimitive::Lines}; + layout.addInstancedBinding(3, 5, 7); + /* At this point it should still work */ + CORRADE_VERIFY(layout == layout); + CORRADE_VERIFY(layout.vkPipelineVertexInputStateCreateInfo().pNext); + static_cast(const_cast(layout.vkPipelineVertexInputStateCreateInfo().pNext))->pVertexBindingDivisors = nullptr; + + std::ostringstream out; + Error redirectError{&out}; + layout.operator==(layout); /* to avoid unused expression warnings */ + CORRADE_COMPARE(out.str(), "Vk::MeshLayout: can't compare structures with external pointers\n"); + + /* External attributes */ + } { + VkVertexInputAttributeDescription attributeData; + MeshLayout layout{MeshPrimitive::Lines}; + layout.vkPipelineVertexInputStateCreateInfo().pVertexAttributeDescriptions = &attributeData; + + std::ostringstream out; + Error redirectError{&out}; + layout.operator==(layout); /* to avoid unused warnings */ + CORRADE_COMPARE(out.str(), "Vk::MeshLayout: can't compare structures with external pointers\n"); + + /* Null attributes but non-zero size */ + } { + MeshLayout layout{MeshPrimitive::Lines}; + layout.vkPipelineVertexInputStateCreateInfo().vertexAttributeDescriptionCount = 3; + + std::ostringstream out; + Error redirectError{&out}; + layout.operator==(layout); /* to avoid unused warnings */ + CORRADE_COMPARE(out.str(), "Vk::MeshLayout: can't compare structures with external pointers\n"); + } +} + void MeshLayoutTest::debugMeshPrimitive() { std::ostringstream out; Debug{&out} << MeshPrimitive::TriangleFan << MeshPrimitive(-10007655);