Reserve zero MeshPrimitive and MeshIndexType for invalid values.

Better for checking accidents, as picking a wrong primitive / index type
can lead to *serious* rendering issues. Similarly to a change done to
(Compressed)PixelFormat in 2019.10.

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
 

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

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");
 }
 

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<void*>(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<void*>(UnsignedInt(value)) << Debug::nospace << ")";
@@ -89,31 +89,31 @@ Debug& operator<<(Debug& debug, const MeshIndexType value) {
 namespace Corrade { namespace Utility {
 
 std::string ConfigurationValue<Magnum::MeshPrimitive>::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<Magnum::MeshPrimitive>::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<Magnum::MeshIndexType>::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<Magnum::MeshIndexType>::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 {};
 }
 
 }}

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<Magnum::MeshPrimitive> {
     /**
      * @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<Magnum::MeshIndexType> {
     /**
      * @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);
 };

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.

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<MeshPrimitive>("primitive"), MeshPrimitive::LineStrip);
 
+    c.setValue("zero", MeshPrimitive{});
+    CORRADE_COMPARE(c.value("zero"), "");
+    CORRADE_COMPARE(c.value<MeshPrimitive>("zero"), MeshPrimitive{});
+
     c.setValue("invalid", MeshPrimitive(0xdead));
     CORRADE_COMPARE(c.value("invalid"), "");
-    CORRADE_COMPARE(c.value<MeshPrimitive>("invalid"), MeshPrimitive::Points);
+    CORRADE_COMPARE(c.value<MeshPrimitive>("invalid"), MeshPrimitive{});
 }
 
 void MeshTest::configurationIndexType() {
@@ -182,9 +189,13 @@ void MeshTest::configurationIndexType() {
     CORRADE_COMPARE(c.value("type"), "UnsignedShort");
     CORRADE_COMPARE(c.value<MeshIndexType>("type"), MeshIndexType::UnsignedShort);
 
+    c.setValue("zero", MeshIndexType{});
+    CORRADE_COMPARE(c.value("zero"), "");
+    CORRADE_COMPARE(c.value<MeshIndexType>("zero"), MeshIndexType{});
+
     c.setValue("invalid", MeshIndexType(0xdead));
     CORRADE_COMPARE(c.value("invalid"), "");
-    CORRADE_COMPARE(c.value<MeshIndexType>("invalid"), MeshIndexType::UnsignedInt);
+    CORRADE_COMPARE(c.value<MeshIndexType>("invalid"), MeshIndexType{});
 }
 
 }}}

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;

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");
 }