From c9634508e3cd2a7aca427e9a81d7b1b674c54c13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Wed, 26 Feb 2020 18:25:55 +0100 Subject: [PATCH] Enlarge MeshPrimitive to four bytes, allow wrapping impl-specific values. And also handle them specially in GL::meshPrimitive() and Vk::vkPrimitiveTopology(). --- doc/changelog.dox | 7 ++++ src/Magnum/GL/Mesh.cpp | 17 +++++++++- src/Magnum/GL/Mesh.h | 21 ++++++++++++ src/Magnum/GL/Test/MeshTest.cpp | 19 ++++++++++- src/Magnum/Magnum.h | 2 +- src/Magnum/Mesh.cpp | 4 +++ src/Magnum/Mesh.h | 54 ++++++++++++++++++++++++++++-- src/Magnum/Test/CMakeLists.txt | 1 + src/Magnum/Test/MeshTest.cpp | 56 ++++++++++++++++++++++++++++++++ src/Magnum/Vk/Enums.cpp | 6 ++++ src/Magnum/Vk/Enums.h | 10 +++++- src/Magnum/Vk/Test/EnumsTest.cpp | 8 +++++ 12 files changed, 199 insertions(+), 6 deletions(-) diff --git a/doc/changelog.dox b/doc/changelog.dox index 12fd83728..2c7aa5a5a 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -196,6 +196,10 @@ See also: @subsection changelog-latest-changes Changes and improvements +- The @ref MeshPrimitive type can now store implementation-specific primitive + types similarly to @ref PixelFormat and the new @ref VertexFormat. + Implementation-specific types are then simply passed through in + @ref GL::meshPrimitive() and @ref Vk::vkPrimitiveTopology(). - The @ref PixelFormat and @ref CompressedPixelFormat enums can now be saved and retrieved from @ref Corrade::Utility::Configuration / @ref Corrade::Utility::Arguments @@ -468,6 +472,9 @@ See also: @ref Shaders::Phong::bindDiffuseTexture(), @ref Shaders::Phong::bindSpecularTexture() and @ref Shaders::Phong::bindTextures() instead +- @ref MeshPrimitive is now four bytes instead of one, to allow wrapping + implementation-specific values using @ref meshPrimitiveWrap() and + @ref meshPrimitiveUnwrap() - @ref MeshPrimitive and @ref MeshIndexType now reserve the zero value to indicate an invalid primitive / type, better catching accidentally forgotten initialization. Valid code shouldn't be affected by this change, diff --git a/src/Magnum/GL/Mesh.cpp b/src/Magnum/GL/Mesh.cpp index 729d407d7..80980bd5e 100644 --- a/src/Magnum/GL/Mesh.cpp +++ b/src/Magnum/GL/Mesh.cpp @@ -65,10 +65,25 @@ constexpr MeshIndexType IndexTypeMapping[]{ } +bool hasMeshPrimitive(const Magnum::MeshPrimitive primitive) { + if(isMeshPrimitiveImplementationSpecific(primitive)) + return true; + + CORRADE_ASSERT(UnsignedInt(primitive) - 1 < Containers::arraySize(PrimitiveMapping), + "GL::hasPrimitive(): invalid primitive" << primitive, {}); + return UnsignedInt(PrimitiveMapping[UnsignedInt(primitive) - 1]) != ~UnsignedInt{}; +} + MeshPrimitive meshPrimitive(const Magnum::MeshPrimitive primitive) { + if(isMeshPrimitiveImplementationSpecific(primitive)) + return meshPrimitiveUnwrap(primitive); + CORRADE_ASSERT(UnsignedInt(primitive) - 1 < Containers::arraySize(PrimitiveMapping), "GL::meshPrimitive(): invalid primitive" << primitive, {}); - return PrimitiveMapping[UnsignedInt(primitive) - 1]; + const MeshPrimitive out = PrimitiveMapping[UnsignedInt(primitive) - 1]; + CORRADE_ASSERT(out != MeshPrimitive(~UnsignedInt{}), + "GL::meshPrimitive(): unsupported primitive" << primitive, {}); + return out; } MeshIndexType meshIndexType(const Magnum::MeshIndexType type) { diff --git a/src/Magnum/GL/Mesh.h b/src/Magnum/GL/Mesh.h index f3d230f99..9b953f094 100644 --- a/src/Magnum/GL/Mesh.h +++ b/src/Magnum/GL/Mesh.h @@ -136,9 +136,30 @@ enum class MeshPrimitive: GLenum { #endif }; +/** +@brief Check availability of a generic mesh primitive +@m_since_latest + +Returns @cpp false @ce if OpenGL doesn't support such primitive, @cpp true @ce +otherwise. Moreover, returns @cpp true @ce also for all formats that are +@ref isMeshPrimitiveImplementationSpecific(). The @p primitive value is +expected to be valid. +@see @ref meshPrimitive() +*/ +MAGNUM_GL_EXPORT bool hasMeshPrimitive(Magnum::MeshPrimitive primitive); + /** @brief Convert generic mesh primitive to OpenGL mesh primitive +In case @ref isMeshPrimitiveImplementationSpecific() returns @cpp false @ce for +@p primitive, maps it to a corresponding OpenGL mesh primitive. In case +@ref isMeshPrimitiveImplementationSpecific() returns @cpp true @ce, assumes +@p primitive stores OpenGL-specific mesh primitive and returns +@ref meshPrimitiveUnwrap() cast to @ref GL::MeshPrimitive. + +Not all generic mesh primitives are available in OpenGL and this function +expects that given primitive is available. Use @ref hasMeshPrimitive() to +query availability of given primitive. @see @ref meshIndexType() */ MAGNUM_GL_EXPORT MeshPrimitive meshPrimitive(Magnum::MeshPrimitive primitive); diff --git a/src/Magnum/GL/Test/MeshTest.cpp b/src/Magnum/GL/Test/MeshTest.cpp index ec199c527..2cbe8422d 100644 --- a/src/Magnum/GL/Test/MeshTest.cpp +++ b/src/Magnum/GL/Test/MeshTest.cpp @@ -50,7 +50,10 @@ struct MeshTest: TestSuite::Tester { void drawViewCountNotSet(); void mapPrimitive(); + void mapPrimitiveImplementationSpecific(); + void mapPrimitiveUnsupported(); void mapPrimitiveInvalid(); + void mapIndexType(); void mapIndexTypeInvalid(); @@ -69,7 +72,10 @@ MeshTest::MeshTest() { &MeshTest::drawViewCountNotSet, &MeshTest::mapPrimitive, + &MeshTest::mapPrimitiveImplementationSpecific, + &MeshTest::mapPrimitiveUnsupported, &MeshTest::mapPrimitiveInvalid, + &MeshTest::mapIndexType, &MeshTest::mapIndexTypeInvalid, @@ -166,7 +172,8 @@ void MeshTest::mapPrimitive() { switch(primitive) { #define _c(primitive) \ case Magnum::MeshPrimitive::primitive: \ - CORRADE_VERIFY(UnsignedInt(meshPrimitive(Magnum::MeshPrimitive::primitive)) >= 0); \ + if(hasMeshPrimitive(Magnum::MeshPrimitive::primitive)) \ + CORRADE_VERIFY(UnsignedInt(meshPrimitive(Magnum::MeshPrimitive::primitive)) >= 0); \ break; #include "Magnum/Implementation/meshPrimitiveMapping.hpp" #undef _c @@ -177,6 +184,16 @@ void MeshTest::mapPrimitive() { } } +void MeshTest::mapPrimitiveImplementationSpecific() { + CORRADE_VERIFY(hasMeshPrimitive(meshPrimitiveWrap(GL_LINES))); + CORRADE_COMPARE(meshPrimitive(meshPrimitiveWrap(GL_LINES)), + MeshPrimitive::Lines); +} + +void MeshTest::mapPrimitiveUnsupported() { + CORRADE_SKIP("All primitive types are supported."); +} + void MeshTest::mapPrimitiveInvalid() { std::ostringstream out; Error redirectError{&out}; diff --git a/src/Magnum/Magnum.h b/src/Magnum/Magnum.h index 793e00920..8cfd87da3 100644 --- a/src/Magnum/Magnum.h +++ b/src/Magnum/Magnum.h @@ -860,7 +860,7 @@ typedef BasicMutableCompressedImageView<1> MutableCompressedImageView1D; typedef BasicMutableCompressedImageView<2> MutableCompressedImageView2D; typedef BasicMutableCompressedImageView<3> MutableCompressedImageView3D; -enum class MeshPrimitive: UnsignedByte; +enum class MeshPrimitive: UnsignedInt; enum class MeshIndexType: UnsignedByte; enum class VertexFormat: UnsignedInt; diff --git a/src/Magnum/Mesh.cpp b/src/Magnum/Mesh.cpp index 65fa64b11..6ba49fed2 100644 --- a/src/Magnum/Mesh.cpp +++ b/src/Magnum/Mesh.cpp @@ -56,6 +56,10 @@ constexpr const char* MeshPrimitiveNames[] { Debug& operator<<(Debug& debug, const MeshPrimitive value) { debug << "MeshPrimitive" << Debug::nospace; + if(isMeshPrimitiveImplementationSpecific(value)) { + return debug << "::ImplementationSpecific(" << Debug::nospace << reinterpret_cast(meshPrimitiveUnwrap(value)) << Debug::nospace << ")"; + } + if(UnsignedInt(value) - 1 < Containers::arraySize(MeshPrimitiveNames)) { return debug << "::" << Debug::nospace << MeshPrimitiveNames[UnsignedInt(value) - 1]; } diff --git a/src/Magnum/Mesh.h b/src/Magnum/Mesh.h index 642711792..791ba69d6 100644 --- a/src/Magnum/Mesh.h +++ b/src/Magnum/Mesh.h @@ -26,9 +26,10 @@ */ /** @file - * @brief Enum @ref Magnum::MeshPrimitive, @ref Magnum::MeshIndexType, function @ref Magnum::meshIndexTypeSize() + * @brief Enum @ref Magnum::MeshPrimitive, @ref Magnum::MeshIndexType, function @ref Magnum::isMeshPrimitiveImplementationSpecific(), @ref Magnum::meshPrimitiveWrap(), @ref Magnum::meshPrimitiveUnwrap(), @ref Magnum::meshIndexTypeSize() */ +#include #include #include "Magnum/Magnum.h" @@ -39,6 +40,11 @@ namespace Magnum { /** @brief Mesh primitive type +Can act also as a wrapper for implementation-specific mesh primitive types +using @ref meshPrimitiveWrap() and @ref meshPrimitiveUnwrap(). Distinction +between generic and implementation-specific primitive types can be done using +@ref isMeshPrimitiveImplementationSpecific(). + In case of OpenGL, corresponds to @ref GL::MeshPrimitive and is convertible to it using @ref GL::meshPrimitive(). See documentation of each value for more information about the mapping. @@ -52,7 +58,7 @@ For D3D, corresponds to @m_class{m-doc-external} [D3D_PRIMITIVE_TOPOLOGY](https: for Metal, corresponds to @m_class{m-doc-external} [MTLPrimitiveType](https://developer.apple.com/documentation/metal/mtlprimitivetype?language=objc). See documentation of each value for more information about the mapping. */ -enum class MeshPrimitive: UnsignedByte { +enum class MeshPrimitive: UnsignedInt { /* Zero reserved for an invalid type (but not being a named value) */ /** @@ -135,6 +141,50 @@ enum class MeshPrimitive: UnsignedByte { /** @debugoperatorenum{MeshPrimitive} */ MAGNUM_EXPORT Debug& operator<<(Debug& debug, MeshPrimitive value); +/** +@brief Whether a @ref MeshPrimitive value wraps an implementation-specific identifier +@m_since_latest + +Returns @cpp true @ce if value of @p primitive has its highest bit set, +@cpp false @ce otherwise. Use @ref meshPrimitiveWrap() and @ref meshPrimitiveUnwrap() +to wrap/unwrap an implementation-specific indentifier to/from +@ref MeshPrimitive. +*/ +constexpr bool isMeshPrimitiveImplementationSpecific(MeshPrimitive primitive) { + return UnsignedInt(primitive) & (1u << 31); +} + +/** +@brief Wrap an implementation-specific mesh primitive identifier in @ref MeshPrimitive +@m_since_latest + +Sets the highest bit on @p primitive to mark it as implementation-specific. +Expects that @p primitive fits into the remaining bits. Use +@ref meshPrimitiveUnwrap() for the inverse operation. +@see @ref isMeshPrimitiveImplementationSpecific() +*/ +template constexpr MeshPrimitive meshPrimitiveWrap(T implementationSpecific) { + static_assert(sizeof(T) <= 4, "types larger than 32bits are not supported"); + return CORRADE_CONSTEXPR_ASSERT(!(UnsignedInt(implementationSpecific) & (1u << 31)), + "meshPrimitiveWrap(): implementation-specific value" << reinterpret_cast(implementationSpecific) << "already wrapped or too large"), + MeshPrimitive((1u << 31)|UnsignedInt(implementationSpecific)); +} + +/** +@brief Unwrap an implementation-specific mesh primitive identifier from @ref MeshPrimitive +@m_since_latest + +Unsets the highest bit from @p primitive to extract the implementation-specific +value. Expects that @p primitive has it set. Use @ref meshPrimitiveWrap() for +the inverse operation. +@see @ref isMeshPrimitiveImplementationSpecific() +*/ +template constexpr T meshPrimitiveUnwrap(MeshPrimitive primitive) { + return CORRADE_CONSTEXPR_ASSERT(UnsignedInt(primitive) & (1u << 31), + "meshPrimitiveUnwrap():" << primitive << "isn't a wrapped implementation-specific value"), + T(UnsignedInt(primitive) & ~(1u << 31)); +} + /** @brief Mesh index type diff --git a/src/Magnum/Test/CMakeLists.txt b/src/Magnum/Test/CMakeLists.txt index e7cd1e216..53d29e0b4 100644 --- a/src/Magnum/Test/CMakeLists.txt +++ b/src/Magnum/Test/CMakeLists.txt @@ -49,6 +49,7 @@ set_target_properties( PROPERTIES FOLDER "Magnum/Test") set_property(TARGET + MeshTest PixelFormatTest ResourceManagerTest VertexFormatTest diff --git a/src/Magnum/Test/MeshTest.cpp b/src/Magnum/Test/MeshTest.cpp index 5a492aef8..10e2d9bf4 100644 --- a/src/Magnum/Test/MeshTest.cpp +++ b/src/Magnum/Test/MeshTest.cpp @@ -39,10 +39,17 @@ struct MeshTest: TestSuite::Tester { void primitiveMapping(); void indexTypeMapping(); + void primitiveIsImplementationSpecific(); + void primitiveWrap(); + void primitiveWrapInvalid(); + void primitiveUnwrap(); + void primitiveUnwrapInvalid(); + void indexTypeSize(); void indexTypeSizeInvalid(); void debugPrimitive(); + void debugPrimitiveImplementationSpecific(); void debugIndexType(); void configurationPrimitive(); @@ -53,10 +60,17 @@ MeshTest::MeshTest() { addTests({&MeshTest::primitiveMapping, &MeshTest::indexTypeMapping, + &MeshTest::primitiveIsImplementationSpecific, + &MeshTest::primitiveWrap, + &MeshTest::primitiveWrapInvalid, + &MeshTest::primitiveUnwrap, + &MeshTest::primitiveUnwrapInvalid, + &MeshTest::indexTypeSize, &MeshTest::indexTypeSizeInvalid, &MeshTest::debugPrimitive, + &MeshTest::debugPrimitiveImplementationSpecific, &MeshTest::debugIndexType, &MeshTest::configurationPrimitive, @@ -139,6 +153,41 @@ void MeshTest::indexTypeMapping() { CORRADE_COMPARE(firstUnhandled, 0xff); } +void MeshTest::primitiveIsImplementationSpecific() { + constexpr bool a = isMeshPrimitiveImplementationSpecific(MeshPrimitive::Lines); + constexpr bool b = isMeshPrimitiveImplementationSpecific(MeshPrimitive(0x8000dead)); + CORRADE_VERIFY(!a); + CORRADE_VERIFY(b); +} + +void MeshTest::primitiveWrap() { + constexpr MeshPrimitive a = meshPrimitiveWrap(0xdead); + CORRADE_COMPARE(UnsignedInt(a), 0x8000dead); +} + +void MeshTest::primitiveWrapInvalid() { + std::ostringstream out; + Error redirectError{&out}; + + meshPrimitiveWrap(0xdeadbeef); + + CORRADE_COMPARE(out.str(), "meshPrimitiveWrap(): implementation-specific value 0xdeadbeef already wrapped or too large\n"); +} + +void MeshTest::primitiveUnwrap() { + constexpr UnsignedInt a = meshPrimitiveUnwrap(MeshPrimitive(0x8000dead)); + CORRADE_COMPARE(a, 0xdead); +} + +void MeshTest::primitiveUnwrapInvalid() { + std::ostringstream out; + Error redirectError{&out}; + + meshPrimitiveUnwrap(MeshPrimitive::Triangles); + + CORRADE_COMPARE(out.str(), "meshPrimitiveUnwrap(): MeshPrimitive::Triangles isn't a wrapped implementation-specific value\n"); +} + void MeshTest::indexTypeSize() { CORRADE_COMPARE(meshIndexTypeSize(MeshIndexType::UnsignedByte), 1); CORRADE_COMPARE(meshIndexTypeSize(MeshIndexType::UnsignedShort), 2); @@ -163,6 +212,13 @@ void MeshTest::debugPrimitive() { CORRADE_COMPARE(o.str(), "MeshPrimitive::TriangleFan MeshPrimitive(0xfe)\n"); } +void MeshTest::debugPrimitiveImplementationSpecific() { + std::ostringstream out; + Debug{&out} << meshPrimitiveWrap(0xdead); + + CORRADE_COMPARE(out.str(), "MeshPrimitive::ImplementationSpecific(0xdead)\n"); +} + void MeshTest::debugIndexType() { std::ostringstream o; Debug(&o) << MeshIndexType::UnsignedShort << MeshIndexType(0xfe); diff --git a/src/Magnum/Vk/Enums.cpp b/src/Magnum/Vk/Enums.cpp index 47e236dd3..cacdb8953 100644 --- a/src/Magnum/Vk/Enums.cpp +++ b/src/Magnum/Vk/Enums.cpp @@ -102,12 +102,18 @@ constexpr VkSamplerAddressMode SamplerAddressModeMapping[]{ } bool hasVkPrimitiveTopology(const Magnum::MeshPrimitive primitive) { + if(isMeshPrimitiveImplementationSpecific(primitive)) + return true; + CORRADE_ASSERT(UnsignedInt(primitive) - 1 < Containers::arraySize(PrimitiveTopologyMapping), "Vk::hasVkPrimitiveTopology(): invalid primitive" << primitive, {}); return UnsignedInt(PrimitiveTopologyMapping[UnsignedInt(primitive) - 1]) != ~UnsignedInt{}; } VkPrimitiveTopology vkPrimitiveTopology(const Magnum::MeshPrimitive primitive) { + if(isMeshPrimitiveImplementationSpecific(primitive)) + return meshPrimitiveUnwrap(primitive); + CORRADE_ASSERT(UnsignedInt(primitive) - 1 < Containers::arraySize(PrimitiveTopologyMapping), "Vk::vkPrimitiveTopology(): invalid primitive" << primitive, {}); const VkPrimitiveTopology out = PrimitiveTopologyMapping[UnsignedInt(primitive) - 1]; diff --git a/src/Magnum/Vk/Enums.h b/src/Magnum/Vk/Enums.h index 39f4ed7f0..230fd0aae 100644 --- a/src/Magnum/Vk/Enums.h +++ b/src/Magnum/Vk/Enums.h @@ -40,7 +40,9 @@ namespace Magnum { namespace Vk { In particular, Vulkan doesn't support the @ref MeshPrimitive::LineLoop primitive. Returns @cpp false @ce if Vulkan doesn't support such primitive, -@cpp true @ce otherwise. The @p primitive value is expected to be valid. +@cpp true @ce otherwise. Moreover, returns @cpp true @ce also for all types +that are @ref isMeshPrimitiveImplementationSpecific(). The @p primitive value +is expected to be valid. @see @ref vkPrimitiveTopology() */ MAGNUM_VK_EXPORT bool hasVkPrimitiveTopology(Magnum::MeshPrimitive primitive); @@ -48,6 +50,12 @@ MAGNUM_VK_EXPORT bool hasVkPrimitiveTopology(Magnum::MeshPrimitive primitive); /** @brief Convert generic mesh primitive to Vulkan primitive topology +In case @ref isMeshPrimitiveImplementationSpecific() returns @cpp false @ce for +@p primitive, maps it to a corresponding Vulkan primitive topology. In case +@ref isMeshPrimitiveImplementationSpecific() returns @cpp true @ce, assumes +@p primitive stores a Vulkan-specific primitive topology and returns +@ref meshPrimitiveUnwrap() cast to @type_vk{VkPrimitiveTopology}. + Not all generic mesh primitives are available in Vulkan and this function expects that given primitive is available. Use @ref hasVkPrimitiveTopology() to query availability of given primitive. diff --git a/src/Magnum/Vk/Test/EnumsTest.cpp b/src/Magnum/Vk/Test/EnumsTest.cpp index baea77c7d..ee33252f9 100644 --- a/src/Magnum/Vk/Test/EnumsTest.cpp +++ b/src/Magnum/Vk/Test/EnumsTest.cpp @@ -39,6 +39,7 @@ struct EnumsTest: TestSuite::Tester { explicit EnumsTest(); void mapVkPrimitiveTopology(); + void mapVkPrimitiveTopologyImplementationSpecific(); void mapVkPrimitiveTopologyUnsupported(); void mapVkPrimitiveTopologyInvalid(); @@ -75,6 +76,7 @@ struct EnumsTest: TestSuite::Tester { EnumsTest::EnumsTest() { addTests({&EnumsTest::mapVkPrimitiveTopology, + &EnumsTest::mapVkPrimitiveTopologyImplementationSpecific, &EnumsTest::mapVkPrimitiveTopologyUnsupported, &EnumsTest::mapVkPrimitiveTopologyInvalid, @@ -152,6 +154,12 @@ void EnumsTest::mapVkPrimitiveTopology() { } } +void EnumsTest::mapVkPrimitiveTopologyImplementationSpecific() { + CORRADE_VERIFY(hasVkPrimitiveTopology(meshPrimitiveWrap(VK_PRIMITIVE_TOPOLOGY_LINE_LIST_WITH_ADJACENCY))); + CORRADE_COMPARE(vkPrimitiveTopology(meshPrimitiveWrap(VK_PRIMITIVE_TOPOLOGY_LINE_LIST_WITH_ADJACENCY)), + VK_PRIMITIVE_TOPOLOGY_LINE_LIST_WITH_ADJACENCY); +} + void EnumsTest::mapVkPrimitiveTopologyUnsupported() { CORRADE_VERIFY(!hasVkPrimitiveTopology(Magnum::MeshPrimitive::LineLoop));