diff --git a/doc/changelog.dox b/doc/changelog.dox index 006d5dc65..640f08025 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -740,6 +740,10 @@ See also: CMake / preprocessor variable has been moved to Corrade --- you need to toggle it when building Corrade and use @ref CORRADE_BUILD_MULTITHREADED instead +- @ref PixelFormat and @ref CompressedPixelFormat now reserve the zero value + to indicate an invalid format, 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. @section changelog-2019-01 2019.01 diff --git a/src/Magnum/GL/PixelFormat.cpp b/src/Magnum/GL/PixelFormat.cpp index 74b145cf8..1f80b2faa 100644 --- a/src/Magnum/GL/PixelFormat.cpp +++ b/src/Magnum/GL/PixelFormat.cpp @@ -54,18 +54,18 @@ bool hasPixelFormat(const Magnum::PixelFormat format) { if(isPixelFormatImplementationSpecific(format)) return true; - CORRADE_ASSERT(UnsignedInt(format) < Containers::arraySize(FormatMapping), + CORRADE_ASSERT(UnsignedInt(format) - 1 < Containers::arraySize(FormatMapping), "GL::hasPixelFormat(): invalid format" << format, {}); - return UnsignedInt(FormatMapping[UnsignedInt(format)].format); + return UnsignedInt(FormatMapping[UnsignedInt(format) - 1].format); } PixelFormat pixelFormat(const Magnum::PixelFormat format) { if(isPixelFormatImplementationSpecific(format)) return pixelFormatUnwrap(format); - CORRADE_ASSERT(UnsignedInt(format) < Containers::arraySize(FormatMapping), + CORRADE_ASSERT(UnsignedInt(format) - 1 < Containers::arraySize(FormatMapping), "GL::pixelFormat(): invalid format" << format, {}); - const PixelFormat out = FormatMapping[UnsignedInt(format)].format; + const PixelFormat out = FormatMapping[UnsignedInt(format) - 1].format; CORRADE_ASSERT(UnsignedInt(out), "GL::pixelFormat(): format" << format << "is not supported on this target", {}); return out; @@ -78,9 +78,9 @@ PixelType pixelType(const Magnum::PixelFormat format, const UnsignedInt extra) { return PixelType(extra); } - CORRADE_ASSERT(UnsignedInt(format) < Containers::arraySize(FormatMapping), + CORRADE_ASSERT(UnsignedInt(format) - 1 < Containers::arraySize(FormatMapping), "GL::pixelType(): invalid format" << format, {}); - const PixelType out = FormatMapping[UnsignedInt(format)].type; + const PixelType out = FormatMapping[UnsignedInt(format) - 1].type; CORRADE_ASSERT(UnsignedInt(out), "GL::pixelType(): format" << format << "is not supported on this target", {}); return out; @@ -373,18 +373,18 @@ bool hasCompressedPixelFormat(const Magnum::CompressedPixelFormat format) { if(isCompressedPixelFormatImplementationSpecific(format)) return true; - CORRADE_ASSERT(UnsignedInt(format) < Containers::arraySize(CompressedFormatMapping), + CORRADE_ASSERT(UnsignedInt(format) - 1 < Containers::arraySize(CompressedFormatMapping), "GL::hasCompressedPixelFormat(): invalid format" << format, {}); - return UnsignedInt(CompressedFormatMapping[UnsignedInt(format)]); + return UnsignedInt(CompressedFormatMapping[UnsignedInt(format) - 1]); } CompressedPixelFormat compressedPixelFormat(const Magnum::CompressedPixelFormat format) { if(isCompressedPixelFormatImplementationSpecific(format)) return compressedPixelFormatUnwrap(format); - CORRADE_ASSERT(UnsignedInt(format) < Containers::arraySize(CompressedFormatMapping), + CORRADE_ASSERT(UnsignedInt(format) - 1 < Containers::arraySize(CompressedFormatMapping), "GL::compressedPixelFormat(): invalid format" << format, {}); - const CompressedPixelFormat out = CompressedFormatMapping[UnsignedInt(format)]; + const CompressedPixelFormat out = CompressedFormatMapping[UnsignedInt(format) - 1]; CORRADE_ASSERT(UnsignedInt(out), "GL::compressedPixelFormat(): format" << format << "is not supported on this target", {}); return out; diff --git a/src/Magnum/GL/Test/PixelFormatTest.cpp b/src/Magnum/GL/Test/PixelFormatTest.cpp index 5a90b6b92..199a22044 100644 --- a/src/Magnum/GL/Test/PixelFormatTest.cpp +++ b/src/Magnum/GL/Test/PixelFormatTest.cpp @@ -90,8 +90,8 @@ void PixelFormatTest::mapFormatType() { /* This goes through the first 16 bits, which should be enough. Going through 32 bits takes 8 seconds, too much. */ UnsignedInt firstUnhandled = 0xffff; - UnsignedInt nextHandled = 0; - for(UnsignedInt i = 0; i <= 0xffff; ++i) { + UnsignedInt nextHandled = 1; /* 0 is an invalid format */ + for(UnsignedInt i = 1; i <= 0xffff; ++i) { const auto format = Magnum::PixelFormat(i); /* Each case verifies: - that the cases are ordered by number (so insertion here is done in @@ -161,10 +161,14 @@ void PixelFormatTest::mapFormatInvalid() { std::ostringstream out; Error redirectError{&out}; + hasPixelFormat(Magnum::PixelFormat{}); hasPixelFormat(Magnum::PixelFormat(0x123)); + pixelFormat(Magnum::PixelFormat{}); pixelFormat(Magnum::PixelFormat(0x123)); CORRADE_COMPARE(out.str(), + "GL::hasPixelFormat(): invalid format PixelFormat(0x0)\n" "GL::hasPixelFormat(): invalid format PixelFormat(0x123)\n" + "GL::pixelFormat(): invalid format PixelFormat(0x0)\n" "GL::pixelFormat(): invalid format PixelFormat(0x123)\n"); } @@ -197,8 +201,11 @@ void PixelFormatTest::mapTypeUnsupported() { void PixelFormatTest::mapTypeInvalid() { std::ostringstream out; Error redirectError{&out}; + pixelType(Magnum::PixelFormat{}); pixelType(Magnum::PixelFormat(0x123)); - CORRADE_COMPARE(out.str(), "GL::pixelType(): invalid format PixelFormat(0x123)\n"); + CORRADE_COMPARE(out.str(), + "GL::pixelType(): invalid format PixelFormat(0x0)\n" + "GL::pixelType(): invalid format PixelFormat(0x123)\n"); } void PixelFormatTest::size() { @@ -232,8 +239,8 @@ void PixelFormatTest::mapCompressedFormat() { /* This goes through the first 16 bits, which should be enough. Going through 32 bits takes 8 seconds, too much. */ UnsignedInt firstUnhandled = 0xffff; - UnsignedInt nextHandled = 0; - for(UnsignedInt i = 0; i <= 0xffff; ++i) { + UnsignedInt nextHandled = 1; /* 0 is an invalid format */ + for(UnsignedInt i = 1; i <= 0xffff; ++i) { const auto format = Magnum::CompressedPixelFormat(i); /* Each case verifies: - that the cases are ordered by number (so insertion here is done in @@ -302,10 +309,14 @@ void PixelFormatTest::mapCompressedFormatInvalid() { std::ostringstream out; Error redirectError{&out}; + hasCompressedPixelFormat(Magnum::CompressedPixelFormat{}); hasCompressedPixelFormat(Magnum::CompressedPixelFormat(0x123)); + compressedPixelFormat(Magnum::CompressedPixelFormat{}); compressedPixelFormat(Magnum::CompressedPixelFormat(0x123)); CORRADE_COMPARE(out.str(), + "GL::hasCompressedPixelFormat(): invalid format CompressedPixelFormat(0x0)\n" "GL::hasCompressedPixelFormat(): invalid format CompressedPixelFormat(0x123)\n" + "GL::compressedPixelFormat(): invalid format CompressedPixelFormat(0x0)\n" "GL::compressedPixelFormat(): invalid format CompressedPixelFormat(0x123)\n"); } diff --git a/src/Magnum/PixelFormat.cpp b/src/Magnum/PixelFormat.cpp index d5234385a..4bea9aec3 100644 --- a/src/Magnum/PixelFormat.cpp +++ b/src/Magnum/PixelFormat.cpp @@ -104,7 +104,7 @@ UnsignedInt pixelSize(const PixelFormat format) { #pragma GCC diagnostic pop #endif - CORRADE_ASSERT_UNREACHABLE(); /* LCOV_EXCL_LINE */ + CORRADE_ASSERT(false, "pixelSize(): invalid pixel format" << format, {}); } #ifndef DOXYGEN_GENERATING_OUTPUT diff --git a/src/Magnum/PixelFormat.h b/src/Magnum/PixelFormat.h index 672ef7843..591000a57 100644 --- a/src/Magnum/PixelFormat.h +++ b/src/Magnum/PixelFormat.h @@ -58,6 +58,8 @@ presence. @see @ref pixelSize(), @ref CompressedPixelFormat, @ref Image, @ref ImageView */ enum class PixelFormat: UnsignedInt { + /* Zero reserved for an invalid format (but not being a named value) */ + /** * Red component, normalized unsigned byte. * @@ -65,7 +67,7 @@ enum class PixelFormat: UnsignedInt { * @ref GL::PixelType::UnsignedByte, @ref GL::TextureFormat::R8 / * @def_vk_keyword{FORMAT_R8_UNORM,Format}. */ - R8Unorm, + R8Unorm = 1, /** * Red and green component, normalized unsigned byte. @@ -606,6 +608,8 @@ to check for its presence. @see @ref PixelFormat, @ref CompressedImage, @ref CompressedImageView */ enum class CompressedPixelFormat: UnsignedInt { + /* Zero reserved for an invalid format (but not being a named value) */ + /** * [S3TC](https://en.wikipedia.org/wiki/S3_Texture_Compression) BC1 * compressed RGB, normalized unsigned byte (DXT1). @@ -614,7 +618,7 @@ enum class CompressedPixelFormat: UnsignedInt { * @ref GL::TextureFormat::CompressedRGBS3tcDxt1 / * @def_vk_keyword{FORMAT_BC1_RGB_UNORM_BLOCK,Format}. */ - Bc1RGBUnorm, + Bc1RGBUnorm = 1, /** * [S3TC](https://en.wikipedia.org/wiki/S3_Texture_Compression) BC1 diff --git a/src/Magnum/Test/PixelFormatTest.cpp b/src/Magnum/Test/PixelFormatTest.cpp index 12b6bc834..eeab64374 100644 --- a/src/Magnum/Test/PixelFormatTest.cpp +++ b/src/Magnum/Test/PixelFormatTest.cpp @@ -35,6 +35,7 @@ struct PixelFormatTest: TestSuite::Tester { explicit PixelFormatTest(); void size(); + void sizeInvalid(); void sizeImplementationSpecific(); void isImplementationSpecific(); @@ -58,6 +59,7 @@ struct PixelFormatTest: TestSuite::Tester { PixelFormatTest::PixelFormatTest() { addTests({&PixelFormatTest::size, + &PixelFormatTest::sizeInvalid, &PixelFormatTest::sizeImplementationSpecific, &PixelFormatTest::isImplementationSpecific, @@ -90,6 +92,18 @@ void PixelFormatTest::size() { CORRADE_COMPARE(pixelSize(PixelFormat::RGBA32F), 16); } +void PixelFormatTest::sizeInvalid() { + std::ostringstream out; + Error redirectError{&out}; + + pixelSize(PixelFormat{}); + pixelSize(PixelFormat(0xdead)); + + CORRADE_COMPARE(out.str(), + "pixelSize(): invalid pixel format PixelFormat(0x0)\n" + "pixelSize(): invalid pixel format PixelFormat(0xdead)\n"); +} + void PixelFormatTest::sizeImplementationSpecific() { std::ostringstream out; Error redirectError{&out}; diff --git a/src/Magnum/Text/AbstractGlyphCache.cpp b/src/Magnum/Text/AbstractGlyphCache.cpp index d05eea40e..4cac1e850 100644 --- a/src/Magnum/Text/AbstractGlyphCache.cpp +++ b/src/Magnum/Text/AbstractGlyphCache.cpp @@ -27,6 +27,7 @@ #include "Magnum/Image.h" #include "Magnum/ImageView.h" +#include "Magnum/PixelFormat.h" #include "Magnum/TextureTools/Atlas.h" namespace Magnum { namespace Text { @@ -65,13 +66,13 @@ void AbstractGlyphCache::setImage(const Vector2i& offset, const ImageView2D& ima Image2D AbstractGlyphCache::image() { CORRADE_ASSERT(features() & GlyphCacheFeature::ImageDownload, - "Text::AbstractGlyphCache::image(): feature not supported", Image2D{{}}); + "Text::AbstractGlyphCache::image(): feature not supported", Image2D{PixelFormat::R8Unorm}); return doImage(); } Image2D AbstractGlyphCache::doImage() { - CORRADE_ASSERT(false, "Text::AbstractGlyphCache::image(): feature advertised but not implemented", Image2D{{}}); + CORRADE_ASSERT(false, "Text::AbstractGlyphCache::image(): feature advertised but not implemented", Image2D{PixelFormat::R8Unorm}); } }} diff --git a/src/Magnum/Text/Test/AbstractGlyphCacheTest.cpp b/src/Magnum/Text/Test/AbstractGlyphCacheTest.cpp index 95f1fa272..c22459c42 100644 --- a/src/Magnum/Text/Test/AbstractGlyphCacheTest.cpp +++ b/src/Magnum/Text/Test/AbstractGlyphCacheTest.cpp @@ -127,7 +127,7 @@ void AbstractGlyphCacheTest::setImage() { Vector2i imageOffset, imageSize; } cache{{100, 200}}; - cache.setImage({80, 175}, ImageView2D{{}, {20, 25}}); + cache.setImage({80, 175}, ImageView2D{PixelFormat::R8Unorm, {20, 25}}); CORRADE_COMPARE(cache.imageOffset, (Vector2i{80, 175})); CORRADE_COMPARE(cache.imageSize, (Vector2i{20, 25})); @@ -138,9 +138,9 @@ void AbstractGlyphCacheTest::setImageOutOfBounds() { std::ostringstream out; Error redirectError{&out}; - cache.setImage({80, 175}, ImageView2D{{}, {20, 25}}); - cache.setImage({81, 175}, ImageView2D{{}, {20, 25}}); - cache.setImage({80, -1}, ImageView2D{{}, {20, 25}}); + cache.setImage({80, 175}, ImageView2D{PixelFormat::R8Unorm, {20, 25}}); + cache.setImage({81, 175}, ImageView2D{PixelFormat::R8Unorm, {20, 25}}); + cache.setImage({80, -1}, ImageView2D{PixelFormat::R8Unorm, {20, 25}}); CORRADE_COMPARE(out.str(), "Text::AbstractGlyphCache::setImage(): Range({81, 175}, {101, 200}) out of bounds for texture size Vector(100, 200)\n" diff --git a/src/Magnum/Trade/Test/AbstractImporterTest.cpp b/src/Magnum/Trade/Test/AbstractImporterTest.cpp index 294767898..b98a626e8 100644 --- a/src/Magnum/Trade/Test/AbstractImporterTest.cpp +++ b/src/Magnum/Trade/Test/AbstractImporterTest.cpp @@ -30,6 +30,7 @@ #include #include +#include "Magnum/PixelFormat.h" #include "Magnum/FileCallback.h" #include "Magnum/Trade/AbstractImporter.h" #include "Magnum/Trade/AnimationData.h" @@ -2622,7 +2623,7 @@ void AbstractImporterTest::image1D() { else return {}; } Containers::Optional doImage1D(UnsignedInt id) override { - if(id == 7) return ImageData1D{PixelStorage{}, {}, {}, {}, &state}; + if(id == 7) return ImageData1D{PixelFormat::RGBA8Unorm, {}, {}, &state}; else return {}; } } importer; @@ -2787,7 +2788,7 @@ void AbstractImporterTest::image2D() { else return {}; } Containers::Optional doImage2D(UnsignedInt id) override { - if(id == 7) return ImageData2D{PixelStorage{}, {}, {}, {}, &state}; + if(id == 7) return ImageData2D{PixelFormat::RGBA8Unorm, {}, {}, &state}; else return {}; } } importer; @@ -2952,7 +2953,7 @@ void AbstractImporterTest::image3D() { else return {}; } Containers::Optional doImage3D(UnsignedInt id) override { - if(id == 7) return ImageData3D{PixelStorage{}, {}, {}, {}, &state}; + if(id == 7) return ImageData3D{PixelFormat::RGBA8Unorm, {}, {}, &state}; else return {}; } } importer; diff --git a/src/Magnum/Vk/Enums.cpp b/src/Magnum/Vk/Enums.cpp index 8298f4e26..ab182b63f 100644 --- a/src/Magnum/Vk/Enums.cpp +++ b/src/Magnum/Vk/Enums.cpp @@ -124,27 +124,27 @@ bool hasVkFormat(const Magnum::PixelFormat format) { if(isPixelFormatImplementationSpecific(format)) return true; - CORRADE_ASSERT(UnsignedInt(format) < Containers::arraySize(FormatMapping), + CORRADE_ASSERT(UnsignedInt(format) - 1 < Containers::arraySize(FormatMapping), "Vk::hasVkFormat(): invalid format" << format, {}); - return UnsignedInt(FormatMapping[UnsignedInt(format)]); + return UnsignedInt(FormatMapping[UnsignedInt(format) - 1]); } bool hasVkFormat(const Magnum::CompressedPixelFormat format) { if(isCompressedPixelFormatImplementationSpecific(format)) return true; - CORRADE_ASSERT(UnsignedInt(format) < Containers::arraySize(CompressedFormatMapping), + CORRADE_ASSERT(UnsignedInt(format) - 1 < Containers::arraySize(CompressedFormatMapping), "Vk::hasVkFormat(): invalid format" << format, {}); - return UnsignedInt(CompressedFormatMapping[UnsignedInt(format)]); + return UnsignedInt(CompressedFormatMapping[UnsignedInt(format) - 1]); } VkFormat vkFormat(const Magnum::PixelFormat format) { if(isPixelFormatImplementationSpecific(format)) return pixelFormatUnwrap(format); - CORRADE_ASSERT(UnsignedInt(format) < Containers::arraySize(FormatMapping), + CORRADE_ASSERT(UnsignedInt(format) - 1 < Containers::arraySize(FormatMapping), "Vk::vkFormat(): invalid format" << format, {}); - const VkFormat out = FormatMapping[UnsignedInt(format)]; + const VkFormat out = FormatMapping[UnsignedInt(format) - 1]; CORRADE_ASSERT(UnsignedInt(out), "Vk::vkFormat(): unsupported format" << format, {}); return out; @@ -154,9 +154,9 @@ VkFormat vkFormat(const Magnum::CompressedPixelFormat format) { if(isCompressedPixelFormatImplementationSpecific(format)) return compressedPixelFormatUnwrap(format); - CORRADE_ASSERT(UnsignedInt(format) < Containers::arraySize(CompressedFormatMapping), + CORRADE_ASSERT(UnsignedInt(format) - 1 < Containers::arraySize(CompressedFormatMapping), "Vk::vkFormat(): invalid format" << format, {}); - const VkFormat out = CompressedFormatMapping[UnsignedInt(format)]; + const VkFormat out = CompressedFormatMapping[UnsignedInt(format) - 1]; CORRADE_ASSERT(UnsignedInt(out), "Vk::vkFormat(): unsupported format" << format, {}); return out; diff --git a/src/Magnum/Vk/Test/EnumsTest.cpp b/src/Magnum/Vk/Test/EnumsTest.cpp index 157059bc0..73dabf523 100644 --- a/src/Magnum/Vk/Test/EnumsTest.cpp +++ b/src/Magnum/Vk/Test/EnumsTest.cpp @@ -181,8 +181,8 @@ void EnumsTest::mapVkFormat() { /* This goes through the first 16 bits, which should be enough. Going through 32 bits takes 8 seconds, too much. */ UnsignedInt firstUnhandled = 0xffff; - UnsignedInt nextHandled = 0; - for(UnsignedInt i = 0; i <= 0xffff; ++i) { + UnsignedInt nextHandled = 1; /* 0 is an invalid format */ + for(UnsignedInt i = 1; i <= 0xffff; ++i) { const auto format = Magnum::PixelFormat(i); /* Each case verifies: - that the cases are ordered by number (so insertion here is done in @@ -250,10 +250,14 @@ void EnumsTest::mapVkFormatInvalid() { std::ostringstream out; Error redirectError{&out}; + hasVkFormat(Magnum::PixelFormat{}); hasVkFormat(Magnum::PixelFormat(0x123)); + vkFormat(Magnum::PixelFormat{}); vkFormat(Magnum::PixelFormat(0x123)); CORRADE_COMPARE(out.str(), + "Vk::hasVkFormat(): invalid format PixelFormat(0x0)\n" "Vk::hasVkFormat(): invalid format PixelFormat(0x123)\n" + "Vk::vkFormat(): invalid format PixelFormat(0x0)\n" "Vk::vkFormat(): invalid format PixelFormat(0x123)\n"); } @@ -265,8 +269,8 @@ void EnumsTest::mapVkFormatCompressed() { /* This goes through the first 16 bits, which should be enough. Going through 32 bits takes 8 seconds, too much. */ UnsignedInt firstUnhandled = 0xffff; - UnsignedInt nextHandled = 0; - for(UnsignedInt i = 0; i <= 0xffff; ++i) { + UnsignedInt nextHandled = 1; /* 0 is an invalid format */ + for(UnsignedInt i = 1; i <= 0xffff; ++i) { const auto format = Magnum::CompressedPixelFormat(i); /* Each case verifies: - that the cases are ordered by number (so insertion here is done in @@ -335,10 +339,14 @@ void EnumsTest::mapVkFormatCompressedInvalid() { std::ostringstream out; Error redirectError{&out}; + hasVkFormat(Magnum::CompressedPixelFormat{}); hasVkFormat(Magnum::CompressedPixelFormat(0x123)); + vkFormat(Magnum::CompressedPixelFormat{}); vkFormat(Magnum::CompressedPixelFormat(0x123)); CORRADE_COMPARE(out.str(), + "Vk::hasVkFormat(): invalid format CompressedPixelFormat(0x0)\n" "Vk::hasVkFormat(): invalid format CompressedPixelFormat(0x123)\n" + "Vk::vkFormat(): invalid format CompressedPixelFormat(0x0)\n" "Vk::vkFormat(): invalid format CompressedPixelFormat(0x123)\n"); }