From 1f89710b350c2a638c4e6f124a503d46251b82dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sun, 24 Jan 2021 17:36:11 +0100 Subject: [PATCH] Vk: enforce binding and location ordering in MeshLayout. In order to make them easier to compare. --- src/Magnum/Vk/MeshLayout.cpp | 12 +++++++++ src/Magnum/Vk/MeshLayout.h | 12 ++++++--- src/Magnum/Vk/Test/MeshLayoutTest.cpp | 36 +++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 3 deletions(-) diff --git a/src/Magnum/Vk/MeshLayout.cpp b/src/Magnum/Vk/MeshLayout.cpp index d24ba5c55..773b81474 100644 --- a/src/Magnum/Vk/MeshLayout.cpp +++ b/src/Magnum/Vk/MeshLayout.cpp @@ -125,6 +125,10 @@ MeshLayout& MeshLayout::operator=(MeshLayout&& other) noexcept { MeshLayout& MeshLayout::addBinding(const UnsignedInt binding, const UnsignedInt stride) { if(!_state) _state.emplace(); + /* Ensure order for efficient comparisons */ + CORRADE_ASSERT(_state->bindings.empty() || _state->bindings.back().binding < binding, + "Vk::MeshLayout::addBinding(): binding" << binding << "can't be ordered after" << _state->bindings.back().binding, *this); + VkVertexInputBindingDescription description{}; description.binding = binding; description.stride = stride; @@ -139,6 +143,10 @@ MeshLayout& MeshLayout::addBinding(const UnsignedInt binding, const UnsignedInt MeshLayout& MeshLayout::addInstancedBinding(const UnsignedInt binding, const UnsignedInt stride, const UnsignedInt divisor) { if(!_state) _state.emplace(); + /* Ensure order for efficient comparisons */ + CORRADE_ASSERT(_state->bindings.empty() || _state->bindings.back().binding < binding, + "Vk::MeshLayout::addInstancedBinding(): binding" << binding << "can't be ordered after" << _state->bindings.back().binding, *this); + VkVertexInputBindingDescription description{}; description.binding = binding; description.stride = stride; @@ -166,6 +174,10 @@ MeshLayout& MeshLayout::addInstancedBinding(const UnsignedInt binding, const Uns MeshLayout& MeshLayout::addAttribute(const UnsignedInt location, const UnsignedInt binding, const VertexFormat format, const UnsignedInt offset) { if(!_state) _state.emplace(); + /* Ensure order for efficient comparisons */ + CORRADE_ASSERT(_state->attributes.empty() || _state->attributes.back().location < location, + "Vk::MeshLayout::addAttribute(): location" << location << "can't be ordered after" << _state->attributes.back().location, *this); + VkVertexInputAttributeDescription description{}; description.location = location; description.binding = binding; diff --git a/src/Magnum/Vk/MeshLayout.h b/src/Magnum/Vk/MeshLayout.h index c69fc4f17..566c956ff 100644 --- a/src/Magnum/Vk/MeshLayout.h +++ b/src/Magnum/Vk/MeshLayout.h @@ -253,7 +253,9 @@ class MAGNUM_VK_EXPORT MeshLayout { * @brief Add a buffer binding * @param binding Binding index, to which a buffer subrange will be * bound when drawing the mesh. Has to be unique among all - * @ref addBinding() and @ref addInstancedBinding() calls. + * @ref addBinding() and @ref addInstancedBinding() calls, and + * monotonically increasing in order to make the layouts + * efficiently comparable. * @param stride Binding stride, in bytes * @return Reference to self (for method chaining) * @@ -273,7 +275,9 @@ class MAGNUM_VK_EXPORT MeshLayout { * @brief Add an instanced buffer binding * @param binding Binding index, to which a buffer subrange will be * bound when drawing the mesh. Has to be unique among all - * @ref addBinding() and @ref addInstancedBinding() calls. + * @ref addBinding() and @ref addInstancedBinding() calls, and + * monotonically increasing in order to make the layouts + * efficiently comparable. * @param stride Binding stride, in bytes * @param divisor Attribute divisor. @cpp 1 @ce means the attribute * will be advanced for each instance, larger values will mean @@ -306,7 +310,9 @@ class MAGNUM_VK_EXPORT MeshLayout { /** * @brief Add an attribute - * @param location Attribute location, matching a shader input + * @param location Attribute location, matching a shader input. + * Has to be monotonically increasing in order to make the layouts + * efficiently comparable. * @param binding Binding index, corresponding to the @p binding * parameter of one of the @ref addBinding() / * @ref addInstancedBinding() calls. diff --git a/src/Magnum/Vk/Test/MeshLayoutTest.cpp b/src/Magnum/Vk/Test/MeshLayoutTest.cpp index 3a3cc9acc..aba5f5328 100644 --- a/src/Magnum/Vk/Test/MeshLayoutTest.cpp +++ b/src/Magnum/Vk/Test/MeshLayoutTest.cpp @@ -53,7 +53,9 @@ struct MeshLayoutTest: TestSuite::Tester { void addBinding(); void addInstancedBinding(); void addInstancedBindingDivisor(); + void addBindingWrongOrder(); template void addAttribute(); + void addAttributeWrongOrder(); void debugMeshPrimitive(); }; @@ -74,8 +76,10 @@ MeshLayoutTest::MeshLayoutTest() { &MeshLayoutTest::addBinding, &MeshLayoutTest::addInstancedBinding, &MeshLayoutTest::addInstancedBindingDivisor, + &MeshLayoutTest::addBindingWrongOrder, &MeshLayoutTest::addAttribute, &MeshLayoutTest::addAttribute, + &MeshLayoutTest::addAttributeWrongOrder, &MeshLayoutTest::debugMeshPrimitive}); } @@ -322,6 +326,23 @@ void MeshLayoutTest::addInstancedBindingDivisor() { CORRADE_COMPARE(vertexDivisorInfo.pVertexBindingDivisors[1].divisor, 0); } +void MeshLayoutTest::addBindingWrongOrder() { + #ifdef CORRADE_NO_ASSERT + CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions"); + #endif + + MeshLayout layout{MeshPrimitive::Triangles}; + layout.addBinding(15, 23); + + std::ostringstream out; + Error redirectError{&out}; + layout.addBinding(15, 27) + .addInstancedBinding(15, 27); + CORRADE_COMPARE(out.str(), + "Vk::MeshLayout::addBinding(): binding 15 can't be ordered after 15\n" + "Vk::MeshLayout::addInstancedBinding(): binding 15 can't be ordered after 15\n"); +} + template void MeshLayoutTest::addAttribute() { setTestCaseTemplateName(VertexFormatTraits::name()); @@ -341,6 +362,21 @@ template void MeshLayoutTest::addAttribute() { CORRADE_COMPARE(layout.vkPipelineVertexInputStateCreateInfo().pVertexAttributeDescriptions[1].offset, 22); } +void MeshLayoutTest::addAttributeWrongOrder() { + #ifdef CORRADE_NO_ASSERT + CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions"); + #endif + + MeshLayout layout{MeshPrimitive::Triangles}; + layout.addAttribute(5, 17, VertexFormat{}, 0); + + std::ostringstream out; + Error redirectError{&out}; + layout.addAttribute(5, 25, VertexFormat{}, 1); + CORRADE_COMPARE(out.str(), + "Vk::MeshLayout::addAttribute(): location 5 can't be ordered after 5\n"); +} + void MeshLayoutTest::debugMeshPrimitive() { std::ostringstream out; Debug{&out} << MeshPrimitive::TriangleFan << MeshPrimitive(-10007655);