From a35a3ec271512e1d8bf3335890a25cd5becdd9e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sun, 16 Jan 2022 18:57:20 +0100 Subject: [PATCH] Add an ability to store implementation-specific values in MeshIndexType. The type is now extended to 32 bits. In the GL and Vk libraries it means one can now do things like MeshIndexType type = meshIndexTypeWrap(GL_UNSIGNED_BYTE); and passing that to GL::Mesh or Vk::Mesh will cause it to use the value directly, instead of doing a mapping from a generic type. The *real* use case for this is however to allow custom index buffer representations in Trade::MeshData. Support for that will be hooked up in the following commit. --- doc/changelog.dox | 3 ++ src/Magnum/GL/Mesh.cpp | 3 ++ src/Magnum/GL/Mesh.h | 34 +++++++++++++-- src/Magnum/GL/Test/MeshTest.cpp | 7 +++ src/Magnum/Magnum.h | 2 +- src/Magnum/Mesh.cpp | 9 +++- src/Magnum/Mesh.h | 52 +++++++++++++++++++++- src/Magnum/Test/MeshTest.cpp | 77 ++++++++++++++++++++++++++++++++- src/Magnum/Trade/MeshData.h | 1 - src/Magnum/Vk/Mesh.cpp | 3 ++ src/Magnum/Vk/Mesh.h | 26 ++++++++++- src/Magnum/Vk/Test/MeshTest.cpp | 7 +++ 12 files changed, 213 insertions(+), 11 deletions(-) diff --git a/doc/changelog.dox b/doc/changelog.dox index 3d19325dc..cd06a0a0e 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -285,6 +285,9 @@ See also: - Added @ref MeshPrimitive::Meshlets as a placeholder for future meshlet support in @ref Trade::MeshData +- @ref MeshIndexType was enlarged to 32 bits and can now wrap + implementation-specific values similar to @ref PixelFormat, + @ref CompressedPixelFormat, @ref VertexFormat and @ref MeshPrimitive @subsubsection changelog-latest-changes-debugtools DebugTools library diff --git a/src/Magnum/GL/Mesh.cpp b/src/Magnum/GL/Mesh.cpp index ba20441c3..dab4e7629 100644 --- a/src/Magnum/GL/Mesh.cpp +++ b/src/Magnum/GL/Mesh.cpp @@ -96,6 +96,9 @@ MeshPrimitive meshPrimitive(const Magnum::MeshPrimitive primitive) { } MeshIndexType meshIndexType(const Magnum::MeshIndexType type) { + if(isMeshIndexTypeImplementationSpecific(type)) + return meshIndexTypeUnwrap(type); + CORRADE_ASSERT(UnsignedInt(type) - 1 < Containers::arraySize(IndexTypeMapping), "GL::meshIndexType(): invalid type" << type, {}); return IndexTypeMapping[UnsignedInt(type) - 1]; diff --git a/src/Magnum/GL/Mesh.h b/src/Magnum/GL/Mesh.h index a867600c1..bf9180e0a 100644 --- a/src/Magnum/GL/Mesh.h +++ b/src/Magnum/GL/Mesh.h @@ -197,6 +197,11 @@ enum class MeshIndexType: GLenum { /** @brief Convert generic mesh index type to OpenGL mesh index type +In case @ref isMeshIndexTypeImplementationSpecific() returns @cpp false @ce for +@p type, maps it to a corresponding OpenGL type. In case +@ref isMeshIndexTypeImplementationSpecific() returns @cpp true @ce, assumes +@p type stores a Vulkan-specific format and returns @ref meshIndexTypeUnwrap() +cast to @ref MeshIndexType. @see @ref meshPrimitive(), @ref meshIndexTypeSize() */ MAGNUM_GL_EXPORT MeshIndexType meshIndexType(Magnum::MeshIndexType type); @@ -1029,7 +1034,14 @@ class MAGNUM_GL_EXPORT Mesh: public AbstractObject { return setIndexBuffer(buffer, offset, type, 0, 0); } - /** @overload */ + /** @overload + * @brief Set index buffer with a generic index type + * + * Note that implementation-specific values are passed as-is with + * @ref meshIndexTypeUnwrap(). It's the user responsibility to ensure + * an implementation-specific value actually represents a valid OpenGL + * index type. + */ Mesh& setIndexBuffer(Buffer& buffer, GLintptr offset, Magnum::MeshIndexType type) { return setIndexBuffer(buffer, offset, meshIndexType(type), 0, 0); } @@ -1043,7 +1055,14 @@ class MAGNUM_GL_EXPORT Mesh: public AbstractObject { */ Mesh& setIndexBuffer(Buffer&& buffer, GLintptr offset, MeshIndexType type, UnsignedInt start, UnsignedInt end); - /** @overload */ + /** @overload + * @brief Set index buffer with a generic index type and ownership transfer + * + * Note that implementation-specific values are passed as-is with + * @ref meshIndexTypeUnwrap(). It's the user responsibility to ensure + * an implementation-specific value actually represents a valid OpenGL + * index type. + */ Mesh& setIndexBuffer(Buffer&& buffer, GLintptr offset, Magnum::MeshIndexType type, UnsignedInt start, UnsignedInt end) { return setIndexBuffer(std::move(buffer), offset, meshIndexType(type), start, end); } @@ -1058,9 +1077,18 @@ class MAGNUM_GL_EXPORT Mesh: public AbstractObject { Mesh& setIndexBuffer(Buffer&& buffer, GLintptr offset, MeshIndexType type) { return setIndexBuffer(std::move(buffer), offset, type, 0, 0); } + + /** @overload + * @brief Set index buffer with a generic index type and ownership transfer + * + * Note that implementation-specific values are passed as-is with + * @ref meshIndexTypeUnwrap(). It's the user responsibility to ensure + * an implementation-specific value actually represents a valid OpenGL + * index type. + */ Mesh& setIndexBuffer(Buffer&& buffer, GLintptr offset, Magnum::MeshIndexType type) { return setIndexBuffer(std::move(buffer), offset, meshIndexType(type), 0, 0); - } /**< @overload */ + } #ifdef MAGNUM_BUILD_DEPRECATED /** diff --git a/src/Magnum/GL/Test/MeshTest.cpp b/src/Magnum/GL/Test/MeshTest.cpp index b2697483d..0b7103d75 100644 --- a/src/Magnum/GL/Test/MeshTest.cpp +++ b/src/Magnum/GL/Test/MeshTest.cpp @@ -55,6 +55,7 @@ struct MeshTest: TestSuite::Tester { void mapPrimitiveInvalid(); void mapIndexType(); + void mapIndexTypeImplementationSpecific(); void mapIndexTypeInvalid(); void debugPrimitive(); @@ -77,6 +78,7 @@ MeshTest::MeshTest() { &MeshTest::mapPrimitiveInvalid, &MeshTest::mapIndexType, + &MeshTest::mapIndexTypeImplementationSpecific, &MeshTest::mapIndexTypeInvalid, &MeshTest::debugPrimitive, @@ -252,6 +254,11 @@ void MeshTest::mapIndexType() { } } +void MeshTest::mapIndexTypeImplementationSpecific() { + CORRADE_COMPARE(meshIndexType(meshIndexTypeWrap(GL_UNSIGNED_BYTE)), + MeshIndexType::UnsignedByte); +} + void MeshTest::mapIndexTypeInvalid() { #ifdef CORRADE_NO_ASSERT CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions"); diff --git a/src/Magnum/Magnum.h b/src/Magnum/Magnum.h index 8717216a8..ff17000b9 100644 --- a/src/Magnum/Magnum.h +++ b/src/Magnum/Magnum.h @@ -1257,7 +1257,7 @@ typedef BasicMutableCompressedImageView<2> MutableCompressedImageView2D; typedef BasicMutableCompressedImageView<3> MutableCompressedImageView3D; enum class MeshPrimitive: UnsignedInt; -enum class MeshIndexType: UnsignedByte; +enum class MeshIndexType: UnsignedInt; enum class VertexFormat: UnsignedInt; enum class PixelFormat: UnsignedInt; diff --git a/src/Magnum/Mesh.cpp b/src/Magnum/Mesh.cpp index f2614dbbf..301f31452 100644 --- a/src/Magnum/Mesh.cpp +++ b/src/Magnum/Mesh.cpp @@ -70,6 +70,10 @@ constexpr const char* MeshIndexTypeNames[] { Debug& operator<<(Debug& debug, const MeshIndexType value) { debug << "MeshIndexType" << Debug::nospace; + if(isMeshIndexTypeImplementationSpecific(value)) { + return debug << "::ImplementationSpecific(" << Debug::nospace << reinterpret_cast(meshIndexTypeUnwrap(value)) << Debug::nospace << ")"; + } + if(UnsignedInt(value) - 1 < Containers::arraySize(MeshIndexTypeNames)) { return debug << "::" << Debug::nospace << MeshIndexTypeNames[UnsignedInt(value) - 1]; } @@ -78,7 +82,10 @@ Debug& operator<<(Debug& debug, const MeshIndexType value) { } #endif -UnsignedInt meshIndexTypeSize(MeshIndexType type) { +UnsignedInt meshIndexTypeSize(const MeshIndexType type) { + CORRADE_ASSERT(!isMeshIndexTypeImplementationSpecific(type), + "meshIndexTypeSize(): can't determine size of an implementation-specific type" << reinterpret_cast(meshIndexTypeUnwrap(type)), {}); + switch(type) { case MeshIndexType::UnsignedByte: return 1; case MeshIndexType::UnsignedShort: return 2; diff --git a/src/Magnum/Mesh.h b/src/Magnum/Mesh.h index e25c400f3..92a8ebca4 100644 --- a/src/Magnum/Mesh.h +++ b/src/Magnum/Mesh.h @@ -246,6 +246,12 @@ template constexpr T meshPrimitiveUnwrap(MeshPrimitive pr /** @brief Mesh index type +A counterpart to @ref VertexFormat describing a mesh index type. Can act also +as a wrapper for implementation-specific mesh index type valueus uing +@ref meshIndexTypeWrap() and @ref meshIndexTypeUnwrap(). Distinction between +generic and implementation-specific types can be done using +@ref isMeshIndexTypeImplementationSpecific(). + In case of OpenGL, corresponds to @ref GL::MeshIndexType and is convertible to it using @ref GL::meshIndexType(). See documentation of each value for more information about the mapping. @@ -259,7 +265,7 @@ for Metal, corresponds to @m_class{m-doc-external} [MTLIndexType](https://develo See documentation of each value for more information about the mapping. @see @ref meshIndexTypeSize() */ -enum class MeshIndexType: UnsignedByte { +enum class MeshIndexType: UnsignedInt { /* Zero reserved for an invalid type (but not being a named value) */ /** @@ -299,6 +305,50 @@ enum class MeshIndexType: UnsignedByte { /** @debugoperatorenum{MeshIndexType} */ MAGNUM_EXPORT Debug& operator<<(Debug& debug, MeshIndexType value); +/** +@brief Whether a @ref MeshIndexType value wraps an implementation-specific identifier +@m_since_latest + +Returns @cpp true @ce if value of @p type has its highest bit set, +@cpp false @ce otherwise. Use @ref meshIndexTypeWrap() and +@ref meshIndexTypeUnwrap() to wrap/unwrap an implementation-specific +indentifier to/from @ref MeshIndexType. +*/ +constexpr bool isMeshIndexTypeImplementationSpecific(MeshIndexType type) { + return UnsignedInt(type) & (1u << 31); +} + +/** +@brief Wrap an implementation-specific mesh index type identifier in @ref MeshIndexType +@m_since_latest + +Sets the highest bit on @p implementationSpecific to mark it as +implementation-specific. Expects that @p implementationSpecific fits into the +remaining 31 bits. Use @ref meshIndexTypeUnwrap() for the inverse operation. +@see @ref isMeshIndexTypeImplementationSpecific() +*/ +template constexpr MeshIndexType meshIndexTypeWrap(T implementationSpecific) { + static_assert(sizeof(T) <= 4, "types larger than 32bits are not supported"); + return CORRADE_CONSTEXPR_ASSERT(!(UnsignedInt(implementationSpecific) & (1u << 31)), + "meshIndexTypeWrap(): implementation-specific value" << reinterpret_cast(implementationSpecific) << "already wrapped or too large"), + MeshIndexType((1u << 31)|UnsignedInt(implementationSpecific)); +} + +/** +@brief Unwrap an implementation-specific mesh index type identifier from @ref MeshIndexType +@m_since_latest + +Unsets the highest bit from @p type to extract the implementation-specific +value. Expects that @p type has it set. Use @ref meshIndexTypeWrap() for +the inverse operation. +@see @ref isMeshIndexTypeImplementationSpecific() +*/ +template constexpr T meshIndexTypeUnwrap(MeshIndexType type) { + return CORRADE_CONSTEXPR_ASSERT(UnsignedInt(type) & (1u << 31), + "meshIndexTypeUnwrap():" << type << "isn't a wrapped implementation-specific value"), + T(UnsignedInt(type) & ~(1u << 31)); +} + /** @brief Size of given mesh index type */ MAGNUM_EXPORT UnsignedInt meshIndexTypeSize(MeshIndexType type); diff --git a/src/Magnum/Test/MeshTest.cpp b/src/Magnum/Test/MeshTest.cpp index 72e492f2c..0c0b084de 100644 --- a/src/Magnum/Test/MeshTest.cpp +++ b/src/Magnum/Test/MeshTest.cpp @@ -44,12 +44,19 @@ struct MeshTest: TestSuite::Tester { void primitiveUnwrapInvalid(); void indexTypeMapping(); + void indexTypeIsImplementationSpecific(); + void indexTypeWrap(); + void indexTypeWrapInvalid(); + void indexTypeUnwrap(); + void indexTypeUnwrapInvalid(); void indexTypeSize(); void indexTypeSizeInvalid(); + void indexTypeSizeImplementationSpecific(); void debugPrimitive(); void debugPrimitiveImplementationSpecific(); void debugIndexType(); + void debugIndexTypeImplementationSpecific(); void configurationPrimitive(); void configurationIndexType(); @@ -64,12 +71,19 @@ MeshTest::MeshTest() { &MeshTest::primitiveUnwrapInvalid, &MeshTest::indexTypeMapping, + &MeshTest::indexTypeIsImplementationSpecific, + &MeshTest::indexTypeWrap, + &MeshTest::indexTypeWrapInvalid, + &MeshTest::indexTypeUnwrap, + &MeshTest::indexTypeUnwrapInvalid, &MeshTest::indexTypeSize, &MeshTest::indexTypeSizeInvalid, + &MeshTest::indexTypeSizeImplementationSpecific, &MeshTest::debugPrimitive, &MeshTest::debugPrimitiveImplementationSpecific, &MeshTest::debugIndexType, + &MeshTest::debugIndexTypeImplementationSpecific, &MeshTest::configurationPrimitive, &MeshTest::configurationIndexType}); @@ -194,6 +208,47 @@ void MeshTest::indexTypeMapping() { CORRADE_COMPARE(firstUnhandled, 0xff); } +void MeshTest::indexTypeIsImplementationSpecific() { + constexpr bool a = isMeshIndexTypeImplementationSpecific(MeshIndexType::UnsignedShort); + constexpr bool b = isMeshIndexTypeImplementationSpecific(MeshIndexType(0x8000dead)); + CORRADE_VERIFY(!a); + CORRADE_VERIFY(b); +} + +void MeshTest::indexTypeWrap() { + constexpr MeshIndexType a = meshIndexTypeWrap(0xdead); + CORRADE_COMPARE(UnsignedInt(a), 0x8000dead); +} + +void MeshTest::indexTypeWrapInvalid() { + #ifdef CORRADE_NO_ASSERT + CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions"); + #endif + + std::ostringstream out; + Error redirectError{&out}; + meshIndexTypeWrap(0xdeadbeef); + CORRADE_COMPARE(out.str(), "meshIndexTypeWrap(): implementation-specific value 0xdeadbeef already wrapped or too large\n"); +} + +void MeshTest::indexTypeUnwrap() { + constexpr UnsignedInt a = meshIndexTypeUnwrap(MeshIndexType(0x8000dead)); + CORRADE_COMPARE(a, 0xdead); +} + +void MeshTest::indexTypeUnwrapInvalid() { + #ifdef CORRADE_NO_ASSERT + CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions"); + #endif + + std::ostringstream out; + Error redirectError{&out}; + + meshIndexTypeUnwrap(MeshIndexType::UnsignedInt); + + CORRADE_COMPARE(out.str(), "meshIndexTypeUnwrap(): MeshIndexType::UnsignedInt isn't a wrapped implementation-specific value\n"); +} + void MeshTest::indexTypeSize() { CORRADE_COMPARE(meshIndexTypeSize(MeshIndexType::UnsignedByte), 1); CORRADE_COMPARE(meshIndexTypeSize(MeshIndexType::UnsignedShort), 2); @@ -209,11 +264,22 @@ void MeshTest::indexTypeSizeInvalid() { Error redirectError{&out}; meshIndexTypeSize(MeshIndexType{}); - meshIndexTypeSize(MeshIndexType(0xfe)); + meshIndexTypeSize(MeshIndexType(0xbadcafe)); CORRADE_COMPARE(out.str(), "meshIndexTypeSize(): invalid type MeshIndexType(0x0)\n" - "meshIndexTypeSize(): invalid type MeshIndexType(0xfe)\n"); + "meshIndexTypeSize(): invalid type MeshIndexType(0xbadcafe)\n"); +} + +void MeshTest::indexTypeSizeImplementationSpecific() { + #ifdef CORRADE_NO_ASSERT + CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions"); + #endif + + std::ostringstream out; + Error redirectError{&out}; + meshIndexTypeSize(meshIndexTypeWrap(0xdead)); + CORRADE_COMPARE(out.str(), "meshIndexTypeSize(): can't determine size of an implementation-specific type 0xdead\n"); } void MeshTest::debugPrimitive() { @@ -235,6 +301,13 @@ void MeshTest::debugIndexType() { CORRADE_COMPARE(o.str(), "MeshIndexType::UnsignedShort MeshIndexType(0xfe)\n"); } +void MeshTest::debugIndexTypeImplementationSpecific() { + std::ostringstream out; + Debug{&out} << meshIndexTypeWrap(0xdead); + + CORRADE_COMPARE(out.str(), "MeshIndexType::ImplementationSpecific(0xdead)\n"); +} + void MeshTest::configurationPrimitive() { Utility::Configuration c; diff --git a/src/Magnum/Trade/MeshData.h b/src/Magnum/Trade/MeshData.h index a6b2ac489..cc72deaf2 100644 --- a/src/Magnum/Trade/MeshData.h +++ b/src/Magnum/Trade/MeshData.h @@ -1932,7 +1932,6 @@ class MAGNUM_TRADE_EXPORT MeshData { UnsignedInt _indexCount, _vertexCount; MeshPrimitive _primitive; MeshIndexType _indexType; - /* 3 byte padding, reserved for 4-byte MeshIndexType */ Short _indexStride; DataFlags _indexDataFlags, _vertexDataFlags; /* 4 byte padding on 64bit */ diff --git a/src/Magnum/Vk/Mesh.cpp b/src/Magnum/Vk/Mesh.cpp index f304ed758..086d15fb3 100644 --- a/src/Magnum/Vk/Mesh.cpp +++ b/src/Magnum/Vk/Mesh.cpp @@ -48,6 +48,9 @@ constexpr MeshIndexType IndexTypeMapping[]{ } MeshIndexType meshIndexType(const Magnum::MeshIndexType type) { + if(isMeshIndexTypeImplementationSpecific(type)) + return meshIndexTypeUnwrap(type); + CORRADE_ASSERT(UnsignedInt(type) - 1 < Containers::arraySize(IndexTypeMapping), "Vk::meshIndexType(): invalid type" << type, {}); return IndexTypeMapping[UnsignedInt(type) - 1]; diff --git a/src/Magnum/Vk/Mesh.h b/src/Magnum/Vk/Mesh.h index fea6da5e0..26b070fce 100644 --- a/src/Magnum/Vk/Mesh.h +++ b/src/Magnum/Vk/Mesh.h @@ -81,6 +81,12 @@ MAGNUM_VK_EXPORT Debug& operator<<(Debug& debug, MeshIndexType value); @brief Convert a generic index type to Vulkan index type @m_since_latest +In case @ref isMeshIndexTypeImplementationSpecific() returns @cpp false @ce for +@p type, maps it to a corresponding Vulkan type. In case +@ref isMeshIndexTypeImplementationSpecific() returns @cpp true @ce, assumes +@p type stores a Vulkan-specific format and returns @ref meshIndexTypeUnwrap() +cast to @ref MeshIndexType. + @see @ref meshPrimitive(), @ref vertexFormat() */ MAGNUM_VK_EXPORT MeshIndexType meshIndexType(Magnum::MeshIndexType type); @@ -310,7 +316,15 @@ class MAGNUM_VK_EXPORT Mesh { * @see @ref setCount(), @ref setIndexOffset() */ Mesh& setIndexBuffer(VkBuffer buffer, UnsignedLong offset, MeshIndexType indexType); - /** @overload */ + + /** @overload + * @brief Set an index buffer with a generic index type + * + * Note that implementation-specific values are passed as-is with + * @ref meshIndexTypeUnwrap(). It's the user responsibility to ensure + * an implementation-specific actually represents a valid Vulkan index + * type. + */ Mesh& setIndexBuffer(VkBuffer buffer, UnsignedLong offset, Magnum::MeshIndexType indexType); /** @@ -322,7 +336,15 @@ class MAGNUM_VK_EXPORT Mesh { * thus doesn't have to be managed separately. */ Mesh& setIndexBuffer(Buffer&& buffer, UnsignedLong offset, MeshIndexType indexType); - /** @overload */ + + /** @overload + * @brief Set an index buffer with a generic index type and take over its ownership + * + * Note that implementation-specific values are passed as-is with + * @ref meshIndexTypeUnwrap(). It's the user responsibility to ensure + * an implementation-specific actually represents a valid Vulkan index + * type. + */ Mesh& setIndexBuffer(Buffer&& buffer, UnsignedLong offset, Magnum::MeshIndexType indexType); /** @brief Layout of this mesh */ diff --git a/src/Magnum/Vk/Test/MeshTest.cpp b/src/Magnum/Vk/Test/MeshTest.cpp index 25c429a89..0734c0995 100644 --- a/src/Magnum/Vk/Test/MeshTest.cpp +++ b/src/Magnum/Vk/Test/MeshTest.cpp @@ -39,6 +39,7 @@ struct MeshTest: TestSuite::Tester { explicit MeshTest(); void mapIndexType(); + void mapIndexTypeImplementationSpecific(); void mapIndexTypeInvalid(); void construct(); @@ -60,6 +61,7 @@ struct MeshTest: TestSuite::Tester { MeshTest::MeshTest() { addTests({&MeshTest::mapIndexType, + &MeshTest::mapIndexTypeImplementationSpecific, &MeshTest::mapIndexTypeInvalid, &MeshTest::construct, @@ -95,6 +97,11 @@ void MeshTest::mapIndexType() { CORRADE_COMPARE(meshIndexType(Magnum::MeshIndexType::UnsignedInt), MeshIndexType::UnsignedInt); } +void MeshTest::mapIndexTypeImplementationSpecific() { + CORRADE_COMPARE(meshIndexType(meshIndexTypeWrap(VK_INDEX_TYPE_UINT32)), + MeshIndexType::UnsignedInt); +} + void MeshTest::mapIndexTypeInvalid() { #ifdef CORRADE_NO_ASSERT CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");