diff --git a/doc/changelog.dox b/doc/changelog.dox index 6f6fc89ad..137ca7736 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -418,6 +418,14 @@ See also: @ref Shaders::Phong::bindDiffuseTexture(), @ref Shaders::Phong::bindSpecularTexture() and @ref Shaders::Phong::bindTextures() instead +- @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, + but broken code that seemingly worked before might start throwing + assertions now. In contrast, @ref SamplerFilter, @ref SamplerMipmap and + @ref SamplerWrapping keep the zero value as a reasonable default. This + follows a similar change done for @ref PixelFormat and + @ref CompressedPixelFormat in 2019.10. @subsection changelog-latest-documentation Documentation diff --git a/src/Magnum/GL/Mesh.cpp b/src/Magnum/GL/Mesh.cpp index 438a21820..729d407d7 100644 --- a/src/Magnum/GL/Mesh.cpp +++ b/src/Magnum/GL/Mesh.cpp @@ -66,15 +66,15 @@ constexpr MeshIndexType IndexTypeMapping[]{ } MeshPrimitive meshPrimitive(const Magnum::MeshPrimitive primitive) { - CORRADE_ASSERT(UnsignedInt(primitive) < Containers::arraySize(PrimitiveMapping), + CORRADE_ASSERT(UnsignedInt(primitive) - 1 < Containers::arraySize(PrimitiveMapping), "GL::meshPrimitive(): invalid primitive" << primitive, {}); - return PrimitiveMapping[UnsignedInt(primitive)]; + return PrimitiveMapping[UnsignedInt(primitive) - 1]; } MeshIndexType meshIndexType(const Magnum::MeshIndexType type) { - CORRADE_ASSERT(UnsignedInt(type) < Containers::arraySize(IndexTypeMapping), + CORRADE_ASSERT(UnsignedInt(type) - 1 < Containers::arraySize(IndexTypeMapping), "GL::meshIndexType(): invalid type" << type, {}); - return IndexTypeMapping[UnsignedInt(type)]; + return IndexTypeMapping[UnsignedInt(type) - 1]; } #ifndef DOXYGEN_GENERATING_OUTPUT diff --git a/src/Magnum/GL/Test/MeshTest.cpp b/src/Magnum/GL/Test/MeshTest.cpp index 880302abe..e91279b58 100644 --- a/src/Magnum/GL/Test/MeshTest.cpp +++ b/src/Magnum/GL/Test/MeshTest.cpp @@ -181,8 +181,10 @@ void MeshTest::mapPrimitiveInvalid() { std::ostringstream out; Error redirectError{&out}; + meshPrimitive(Magnum::MeshPrimitive{}); meshPrimitive(Magnum::MeshPrimitive(0x123)); CORRADE_COMPARE(out.str(), + "GL::meshPrimitive(): invalid primitive MeshPrimitive(0x0)\n" "GL::meshPrimitive(): invalid primitive MeshPrimitive(0x123)\n"); } @@ -218,8 +220,10 @@ void MeshTest::mapIndexTypeInvalid() { std::ostringstream out; Error redirectError{&out}; + meshIndexType(Magnum::MeshIndexType(0x0)); meshIndexType(Magnum::MeshIndexType(0x123)); CORRADE_COMPARE(out.str(), + "GL::meshIndexType(): invalid type MeshIndexType(0x0)\n" "GL::meshIndexType(): invalid type MeshIndexType(0x123)\n"); } diff --git a/src/Magnum/Mesh.cpp b/src/Magnum/Mesh.cpp index 97e8ad83d..65fa64b11 100644 --- a/src/Magnum/Mesh.cpp +++ b/src/Magnum/Mesh.cpp @@ -56,8 +56,8 @@ constexpr const char* MeshPrimitiveNames[] { Debug& operator<<(Debug& debug, const MeshPrimitive value) { debug << "MeshPrimitive" << Debug::nospace; - if(UnsignedInt(value) < Containers::arraySize(MeshPrimitiveNames)) { - return debug << "::" << Debug::nospace << MeshPrimitiveNames[UnsignedInt(value)]; + if(UnsignedInt(value) - 1 < Containers::arraySize(MeshPrimitiveNames)) { + return debug << "::" << Debug::nospace << MeshPrimitiveNames[UnsignedInt(value) - 1]; } return debug << "(" << Debug::nospace << reinterpret_cast(UnsignedInt(value)) << Debug::nospace << ")"; @@ -76,8 +76,8 @@ constexpr const char* MeshIndexTypeNames[] { Debug& operator<<(Debug& debug, const MeshIndexType value) { debug << "MeshIndexType" << Debug::nospace; - if(UnsignedInt(value) < Containers::arraySize(MeshIndexTypeNames)) { - return debug << "::" << Debug::nospace << MeshIndexTypeNames[UnsignedInt(value)]; + if(UnsignedInt(value) - 1 < Containers::arraySize(MeshIndexTypeNames)) { + return debug << "::" << Debug::nospace << MeshIndexTypeNames[UnsignedInt(value) - 1]; } return debug << "(" << Debug::nospace << reinterpret_cast(UnsignedInt(value)) << Debug::nospace << ")"; @@ -89,31 +89,31 @@ Debug& operator<<(Debug& debug, const MeshIndexType value) { namespace Corrade { namespace Utility { std::string ConfigurationValue::toString(Magnum::MeshPrimitive value, ConfigurationValueFlags) { - if(Magnum::UnsignedInt(value) < Containers::arraySize(Magnum::MeshPrimitiveNames)) - return Magnum::MeshPrimitiveNames[Magnum::UnsignedInt(value)]; + if(Magnum::UnsignedInt(value) - 1 < Containers::arraySize(Magnum::MeshPrimitiveNames)) + return Magnum::MeshPrimitiveNames[Magnum::UnsignedInt(value) - 1]; return {}; } Magnum::MeshPrimitive ConfigurationValue::fromString(const std::string& stringValue, ConfigurationValueFlags) { for(std::size_t i = 0; i != Containers::arraySize(Magnum::MeshPrimitiveNames); ++i) - if(stringValue == Magnum::MeshPrimitiveNames[i]) return Magnum::MeshPrimitive(i); + if(stringValue == Magnum::MeshPrimitiveNames[i]) return Magnum::MeshPrimitive(i + 1); - return Magnum::MeshPrimitive::Points; + return {}; } std::string ConfigurationValue::toString(Magnum::MeshIndexType value, ConfigurationValueFlags) { - if(Magnum::UnsignedInt(value) < Containers::arraySize(Magnum::MeshIndexTypeNames)) - return Magnum::MeshIndexTypeNames[Magnum::UnsignedInt(value)]; + if(Magnum::UnsignedInt(value) - 1 < Containers::arraySize(Magnum::MeshIndexTypeNames)) + return Magnum::MeshIndexTypeNames[Magnum::UnsignedInt(value) - 1]; return {}; } Magnum::MeshIndexType ConfigurationValue::fromString(const std::string& stringValue, ConfigurationValueFlags) { for(std::size_t i = 0; i != Containers::arraySize(Magnum::MeshIndexTypeNames); ++i) - if(stringValue == Magnum::MeshIndexTypeNames[i]) return Magnum::MeshIndexType(i); + if(stringValue == Magnum::MeshIndexTypeNames[i]) return Magnum::MeshIndexType(i + 1); - return Magnum::MeshIndexType::UnsignedInt; + return {}; } }} diff --git a/src/Magnum/Mesh.h b/src/Magnum/Mesh.h index 4816b9728..23bf1762e 100644 --- a/src/Magnum/Mesh.h +++ b/src/Magnum/Mesh.h @@ -53,6 +53,8 @@ for Metal, corresponds to @m_class{m-doc-external} [MTLPrimitiveType](https://de See documentation of each value for more information about the mapping. */ enum class MeshPrimitive: UnsignedInt { + /* Zero reserved for an invalid type (but not being a named value) */ + /** * Single points. * @@ -62,7 +64,7 @@ enum class MeshPrimitive: UnsignedInt { * or @m_class{m-doc-external} [MTLPrimitiveTypePoint](https://developer.apple.com/documentation/metal/mtlprimitivetype/mtlprimitivetypepoint?language=objc). * @m_keywords{D3D_PRIMITIVE_TOPOLOGY_POINTLIST MTLPrimitiveTypePoint} */ - Points, + Points = 1, /** * Each pair of vertices defines a single line, lines aren't @@ -147,6 +149,8 @@ there, use @ref Vk::hasVkIndexType() to check for its presence. @see @ref meshIndexTypeSize() */ enum class MeshIndexType: UnsignedInt { + /* Zero reserved for an invalid type (but not being a named value) */ + /** * Unsigned byte * @@ -155,7 +159,7 @@ enum class MeshIndexType: UnsignedInt { * suggest (via debug output) using 16-byte types instead for better * efficiency. */ - UnsignedByte, + UnsignedByte = 1, /** * Unsigned short @@ -198,7 +202,7 @@ template<> struct MAGNUM_EXPORT ConfigurationValue { /** * @brief Reads enum value as string * - * If the value is invalid, returns @ref Magnum::MeshPrimitive::Points "MeshPrimitive::Points". + * If the value is invalid, returns a zero (invalid) primitive. */ static Magnum::MeshPrimitive fromString(const std::string& stringValue, ConfigurationValueFlags); }; @@ -217,7 +221,7 @@ template<> struct MAGNUM_EXPORT ConfigurationValue { /** * @brief Read enum value as string * - * If the value is invalid, returns @ref Magnum::MeshIndexType::UnsignedInt "MeshIndexType::UnsignedInt". + * If the value is invalid, returns a zero (invalid) type. */ static Magnum::MeshIndexType fromString(const std::string& stringValue, ConfigurationValueFlags); }; diff --git a/src/Magnum/Sampler.h b/src/Magnum/Sampler.h index 369ecc639..188c3a196 100644 --- a/src/Magnum/Sampler.h +++ b/src/Magnum/Sampler.h @@ -47,13 +47,17 @@ information about the mapping. @see @ref SamplerMipmap, @ref SamplerWrapping */ enum class SamplerFilter: UnsignedInt { + /* Unlike with MeshIndexType, MeshPrimitive, VertexFormat, PixelFormat + etc., this enum doesn't have zero as an invalid value -- Nearest is a + good default */ + /** * Nearest neighbor filtering. * * Corresponds to @ref GL::SamplerFilter::Nearest / * @def_vk_keyword{FILTER_NEAREST,Filter}. */ - Nearest, + Nearest = 0, /** * Linear interpolation filtering. @@ -77,6 +81,10 @@ each value for more information about the mapping. @see @ref SamplerFilter, @ref SamplerWrapping */ enum class SamplerMipmap: UnsignedInt { + /* Unlike with MeshIndexType, MeshPrimitive, VertexFormat, PixelFormat + etc., this enum doesn't have zero as an invalid value -- Base is a + good default */ + /** * Select base mip level * @@ -85,7 +93,7 @@ enum class SamplerMipmap: UnsignedInt { * @def_vk_keyword{SAMPLER_MIPMAP_MODE_NEAREST,SamplerMipmapMode} and you * have to configure the sampler to use just a single mipmap level. */ - Base, + Base = 0, /** * Select nearest mip level. @@ -119,13 +127,17 @@ presence. @see @ref SamplerFilter, @ref SamplerMipmap */ enum class SamplerWrapping: UnsignedInt { + /* Unlike with MeshIndexType, MeshPrimitive, VertexFormat, PixelFormat + etc., this enum doesn't have zero as an invalid value -- Repeat is a + good default */ + /** * Repeat texture. * * Corresponds to @ref GL::SamplerWrapping::Repeat / * @def_vk_keyword{SAMPLER_ADDRESS_MODE_REPEAT,SamplerAddressMode}. */ - Repeat, + Repeat = 0, /** * Repeat mirrored texture. diff --git a/src/Magnum/Test/MeshTest.cpp b/src/Magnum/Test/MeshTest.cpp index d212d4bf5..c121a9c72 100644 --- a/src/Magnum/Test/MeshTest.cpp +++ b/src/Magnum/Test/MeshTest.cpp @@ -63,8 +63,8 @@ MeshTest::MeshTest() { void MeshTest::primitiveMapping() { /* This goes through the first 8 bits, which should be enough. */ UnsignedInt firstUnhandled = 0xff; - UnsignedInt nextHandled = 0; - for(UnsignedInt i = 0; i <= 0xff; ++i) { + UnsignedInt nextHandled = 1; /* 0 is an invalid primitive */ + for(UnsignedInt i = 1; i <= 0xff; ++i) { const auto primitive = MeshPrimitive(i); /* Each case verifies: - that the entries are ordered by number by comparing a function to @@ -101,8 +101,8 @@ void MeshTest::primitiveMapping() { void MeshTest::indexTypeMapping() { /* This goes through the first 8 bits, which should be enough. */ UnsignedInt firstUnhandled = 0xff; - UnsignedInt nextHandled = 0; - for(UnsignedInt i = 0; i <= 0xff; ++i) { + UnsignedInt nextHandled = 1; /* 0 is an invalid type */ + for(UnsignedInt i = 1; i <= 0xff; ++i) { const auto type = MeshIndexType(i); /* Each case verifies: - that the entries are ordered by number by comparing a function to @@ -146,9 +146,12 @@ void MeshTest::indexTypeSizeInvalid() { std::ostringstream out; Error redirectError{&out}; + meshIndexTypeSize(MeshIndexType{}); meshIndexTypeSize(MeshIndexType(0xdead)); - CORRADE_COMPARE(out.str(), "meshIndexTypeSize(): invalid type MeshIndexType(0xdead)\n"); + CORRADE_COMPARE(out.str(), + "meshIndexTypeSize(): invalid type MeshIndexType(0x0)\n" + "meshIndexTypeSize(): invalid type MeshIndexType(0xdead)\n"); } void MeshTest::debugPrimitive() { @@ -170,9 +173,13 @@ void MeshTest::configurationPrimitive() { CORRADE_COMPARE(c.value("primitive"), "LineStrip"); CORRADE_COMPARE(c.value("primitive"), MeshPrimitive::LineStrip); + c.setValue("zero", MeshPrimitive{}); + CORRADE_COMPARE(c.value("zero"), ""); + CORRADE_COMPARE(c.value("zero"), MeshPrimitive{}); + c.setValue("invalid", MeshPrimitive(0xdead)); CORRADE_COMPARE(c.value("invalid"), ""); - CORRADE_COMPARE(c.value("invalid"), MeshPrimitive::Points); + CORRADE_COMPARE(c.value("invalid"), MeshPrimitive{}); } void MeshTest::configurationIndexType() { @@ -182,9 +189,13 @@ void MeshTest::configurationIndexType() { CORRADE_COMPARE(c.value("type"), "UnsignedShort"); CORRADE_COMPARE(c.value("type"), MeshIndexType::UnsignedShort); + c.setValue("zero", MeshIndexType{}); + CORRADE_COMPARE(c.value("zero"), ""); + CORRADE_COMPARE(c.value("zero"), MeshIndexType{}); + c.setValue("invalid", MeshIndexType(0xdead)); CORRADE_COMPARE(c.value("invalid"), ""); - CORRADE_COMPARE(c.value("invalid"), MeshIndexType::UnsignedInt); + CORRADE_COMPARE(c.value("invalid"), MeshIndexType{}); } }}} diff --git a/src/Magnum/Vk/Enums.cpp b/src/Magnum/Vk/Enums.cpp index 75844d238..6e8a69c47 100644 --- a/src/Magnum/Vk/Enums.cpp +++ b/src/Magnum/Vk/Enums.cpp @@ -93,30 +93,30 @@ constexpr VkSamplerAddressMode SamplerAddressModeMapping[]{ } bool hasVkPrimitiveTopology(const Magnum::MeshPrimitive primitive) { - CORRADE_ASSERT(UnsignedInt(primitive) < Containers::arraySize(PrimitiveTopologyMapping), + CORRADE_ASSERT(UnsignedInt(primitive) - 1 < Containers::arraySize(PrimitiveTopologyMapping), "Vk::hasVkPrimitiveTopology(): invalid primitive" << primitive, {}); - return UnsignedInt(PrimitiveTopologyMapping[UnsignedInt(primitive)]) != ~UnsignedInt{}; + return UnsignedInt(PrimitiveTopologyMapping[UnsignedInt(primitive) - 1]) != ~UnsignedInt{}; } VkPrimitiveTopology vkPrimitiveTopology(const Magnum::MeshPrimitive primitive) { - CORRADE_ASSERT(UnsignedInt(primitive) < Containers::arraySize(PrimitiveTopologyMapping), + CORRADE_ASSERT(UnsignedInt(primitive) - 1 < Containers::arraySize(PrimitiveTopologyMapping), "Vk::vkPrimitiveTopology(): invalid primitive" << primitive, {}); - const VkPrimitiveTopology out = PrimitiveTopologyMapping[UnsignedInt(primitive)]; + const VkPrimitiveTopology out = PrimitiveTopologyMapping[UnsignedInt(primitive) - 1]; CORRADE_ASSERT(out != VkPrimitiveTopology(~UnsignedInt{}), "Vk::vkPrimitiveTopology(): unsupported primitive" << primitive, {}); return out; } bool hasVkIndexType(const Magnum::MeshIndexType type) { - CORRADE_ASSERT(UnsignedInt(type) < Containers::arraySize(IndexTypeMapping), + CORRADE_ASSERT(UnsignedInt(type) - 1 < Containers::arraySize(IndexTypeMapping), "Vk::hasVkIndexType(): invalid type" << type, {}); - return UnsignedInt(IndexTypeMapping[UnsignedInt(type)]) != ~UnsignedInt{}; + return UnsignedInt(IndexTypeMapping[UnsignedInt(type) - 1]) != ~UnsignedInt{}; } VkIndexType vkIndexType(const Magnum::MeshIndexType type) { - CORRADE_ASSERT(UnsignedInt(type) < Containers::arraySize(IndexTypeMapping), + CORRADE_ASSERT(UnsignedInt(type) - 1 < Containers::arraySize(IndexTypeMapping), "Vk::vkIndexType(): invalid type" << type, {}); - const VkIndexType out = IndexTypeMapping[UnsignedInt(type)]; + const VkIndexType out = IndexTypeMapping[UnsignedInt(type) - 1]; CORRADE_ASSERT(out != VkIndexType(~UnsignedInt{}), "Vk::vkIndexType(): unsupported type" << type, {}); return out; diff --git a/src/Magnum/Vk/Test/EnumsTest.cpp b/src/Magnum/Vk/Test/EnumsTest.cpp index 3671f6638..3896e1a5e 100644 --- a/src/Magnum/Vk/Test/EnumsTest.cpp +++ b/src/Magnum/Vk/Test/EnumsTest.cpp @@ -157,10 +157,14 @@ void EnumsTest::mapVkPrimitiveTopologyInvalid() { std::ostringstream out; Error redirectError{&out}; + hasVkPrimitiveTopology(Magnum::MeshPrimitive{}); hasVkPrimitiveTopology(Magnum::MeshPrimitive(0x123)); + vkPrimitiveTopology(Magnum::MeshPrimitive{}); vkPrimitiveTopology(Magnum::MeshPrimitive(0x123)); CORRADE_COMPARE(out.str(), + "Vk::hasVkPrimitiveTopology(): invalid primitive MeshPrimitive(0x0)\n" "Vk::hasVkPrimitiveTopology(): invalid primitive MeshPrimitive(0x123)\n" + "Vk::vkPrimitiveTopology(): invalid primitive MeshPrimitive(0x0)\n" "Vk::vkPrimitiveTopology(): invalid primitive MeshPrimitive(0x123)\n"); } @@ -213,10 +217,14 @@ void EnumsTest::mapVkIndexTypeInvalid() { std::ostringstream out; Error redirectError{&out}; + hasVkIndexType(Magnum::MeshIndexType(0x0)); hasVkIndexType(Magnum::MeshIndexType(0x123)); + vkIndexType(Magnum::MeshIndexType(0x0)); vkIndexType(Magnum::MeshIndexType(0x123)); CORRADE_COMPARE(out.str(), + "Vk::hasVkIndexType(): invalid type MeshIndexType(0x0)\n" "Vk::hasVkIndexType(): invalid type MeshIndexType(0x123)\n" + "Vk::vkIndexType(): invalid type MeshIndexType(0x0)\n" "Vk::vkIndexType(): invalid type MeshIndexType(0x123)\n"); }