diff --git a/src/Magnum/DebugTools/Test/CompareImageTest.cpp b/src/Magnum/DebugTools/Test/CompareImageTest.cpp index 0d9dd9673..c54c46ee0 100644 --- a/src/Magnum/DebugTools/Test/CompareImageTest.cpp +++ b/src/Magnum/DebugTools/Test/CompareImageTest.cpp @@ -294,7 +294,7 @@ void CompareImageTest::formatUnknown() { Containers::String out; Error redirectError{&out}; - ImageView2D image{PixelStorage{}, PixelFormat(0xdead), 0, 0, {}}; + ImageView2D image{PixelStorage{}, PixelFormat(0xdead), 0, 1, {}}; Implementation::calculateImageDelta(image.format(), image.pixels(), image); CORRADE_COMPARE(out, "DebugTools::CompareImage: unknown format PixelFormat(0xdead)\n"); @@ -318,7 +318,7 @@ void CompareImageTest::formatImplementationSpecific() { Containers::String out; Error redirectError{&out}; - ImageView2D image{PixelStorage{}, pixelFormatWrap(0xdead), 0, 0, {}}; + ImageView2D image{PixelStorage{}, pixelFormatWrap(0xdead), 0, 1, {}}; Implementation::calculateImageDelta(image.format(), image.pixels(), image); CORRADE_COMPARE(out, "DebugTools::CompareImage: can't compare implementation-specific pixel formats\n"); diff --git a/src/Magnum/Image.cpp b/src/Magnum/Image.cpp index 16a7c9f68..99b0e477b 100644 --- a/src/Magnum/Image.cpp +++ b/src/Magnum/Image.cpp @@ -36,9 +36,10 @@ template Image::Image(const PixelStorage sto template Image::Image(const PixelStorage storage, const UnsignedInt format, const UnsignedInt formatExtra, const UnsignedInt pixelSize, const VectorTypeFor& size, Containers::Array&& data, const ImageFlags flags) noexcept: Image{storage, pixelFormatWrap(format), formatExtra, pixelSize, size, Utility::move(data), flags} {} -template Image::Image(const PixelStorage storage, const PixelFormat format, const UnsignedInt formatExtra, const UnsignedInt pixelSize, const VectorTypeFor& size, Containers::Array&& data, const ImageFlags flags) noexcept: _storage{storage}, _format{format}, _formatExtra{formatExtra}, _pixelSize{pixelSize}, _flags{flags}, _size{size}, _data{Utility::move(data)} { - CORRADE_ASSERT(Implementation::imageDataSize(*this) <= _data.size(), "Image: data too small, got" << _data.size() << "but expected at least" << Implementation::imageDataSize(*this) << "bytes", ); +template Image::Image(const PixelStorage storage, const PixelFormat format, const UnsignedInt formatExtra, const UnsignedInt pixelSize, const VectorTypeFor& size, Containers::Array&& data, const ImageFlags flags) noexcept: _storage{storage}, _format{format}, _formatExtra{formatExtra}, _pixelSize{UnsignedByte(pixelSize)}, _flags{flags}, _size{size}, _data{Utility::move(data)} { #ifndef CORRADE_NO_ASSERT + Implementation::checkPixelSize("Image:", pixelSize); + CORRADE_ASSERT(Implementation::imageDataSize(*this) <= _data.size(), "Image: data too small, got" << _data.size() << "but expected at least" << Implementation::imageDataSize(*this) << "bytes", ); Implementation::checkImageFlagsForSize("Image:", flags, size); #endif } @@ -47,7 +48,11 @@ template Image::Image(const PixelStorage sto template Image::Image(const PixelStorage storage, const UnsignedInt format, const UnsignedInt formatExtra, const UnsignedInt pixelSize) noexcept: Image{storage, pixelFormatWrap(format), formatExtra, pixelSize} {} -template Image::Image(const PixelStorage storage, const PixelFormat format, const UnsignedInt formatExtra, const UnsignedInt pixelSize) noexcept: _storage{storage}, _format{format}, _formatExtra{formatExtra}, _pixelSize{pixelSize}, _data{} {} +template Image::Image(const PixelStorage storage, const PixelFormat format, const UnsignedInt formatExtra, const UnsignedInt pixelSize) noexcept: _storage{storage}, _format{format}, _formatExtra{formatExtra}, _pixelSize{UnsignedByte(pixelSize)}, _data{} { + #ifndef CORRADE_NO_ASSERT + Implementation::checkPixelSize("Image:", pixelSize); + #endif +} template Image::Image(Image&& other) noexcept: _storage{Utility::move(other._storage)}, _format{Utility::move(other._format)}, _formatExtra{Utility::move(other._formatExtra)}, _pixelSize{Utility::move(other._pixelSize)}, _flags{Utility::move(other._flags)}, _size{Utility::move(other._size)}, _data{Utility::move(other._data)} { other._size = {}; diff --git a/src/Magnum/Image.h b/src/Magnum/Image.h index d315362ed..9da6f5a74 100644 --- a/src/Magnum/Image.h +++ b/src/Magnum/Image.h @@ -211,7 +211,8 @@ template class Image { * @ref pixelFormatSize(), this allows you to specify an * implementation-specific pixel format and pixel size directly. Uses * @ref pixelFormatWrap() internally to wrap @p format in - * @ref PixelFormat. + * @ref PixelFormat. The @p pixelSize is expected to be non-zero and + * less than @cpp 256 @ce. * * The @p data array is expected to be of proper size for given * parameters. For a 3D image, if @p flags contain @@ -238,7 +239,8 @@ template class Image { * is determined automatically using @ref pixelFormatSize(), this * allows you to specify an implementation-specific pixel format and * pixel size directly. Uses @ref pixelFormatWrap() internally to wrap - * @p format in @ref PixelFormat. + * @p format in @ref PixelFormat. The @p pixelSize is expected to be + * non-zero and less than @cpp 256 @ce. */ /* No ImageFlags parameter here as this constructor is mainly used to query GL textures, and there the flags are forcibly reset */ @@ -510,8 +512,8 @@ template class Image { UnsignedInt _formatExtra; /** @todo this could be a short, saving 8 bytes for 1D and 3D images on 64bit and 4 bytes for all dimensions on 32bit. Worth the pain? */ - UnsignedInt _pixelSize; - /* 2-byte gap */ + UnsignedByte _pixelSize; + /* 1 byte free */ ImageFlags _flags; VectorTypeFor _size; Containers::Array _data; diff --git a/src/Magnum/ImageView.cpp b/src/Magnum/ImageView.cpp index 0735d80cd..dd2b861d0 100644 --- a/src/Magnum/ImageView.cpp +++ b/src/Magnum/ImageView.cpp @@ -35,9 +35,10 @@ template ImageView::ImageView(co template ImageView::ImageView(const PixelStorage storage, const UnsignedInt format, const UnsignedInt formatExtra, const UnsignedInt pixelSize, const VectorTypeFor& size, const Containers::ArrayView data, const ImageFlags flags) noexcept: ImageView{storage, pixelFormatWrap(format), formatExtra, pixelSize, size, data, flags} {} -template ImageView::ImageView(const PixelStorage storage, const PixelFormat format, const UnsignedInt formatExtra, const UnsignedInt pixelSize, const VectorTypeFor& size, const Containers::ArrayView data, const ImageFlags flags) noexcept: _storage{storage}, _format{format}, _formatExtra{formatExtra}, _pixelSize{pixelSize}, _flags{flags}, _size{size}, _data{reinterpret_cast(data.data()), data.size()} { - CORRADE_ASSERT(Implementation::imageDataSize(*this) <= _data.size(), "ImageView: data too small, got" << _data.size() << "but expected at least" << Implementation::imageDataSize(*this) << "bytes", ); +template ImageView::ImageView(const PixelStorage storage, const PixelFormat format, const UnsignedInt formatExtra, const UnsignedInt pixelSize, const VectorTypeFor& size, const Containers::ArrayView data, const ImageFlags flags) noexcept: _storage{storage}, _format{format}, _formatExtra{formatExtra}, _pixelSize{UnsignedByte(pixelSize)}, _flags{flags}, _size{size}, _data{reinterpret_cast(data.data()), data.size()} { #ifndef CORRADE_NO_ASSERT + Implementation::checkPixelSize("ImageView:", pixelSize); + CORRADE_ASSERT(Implementation::imageDataSize(*this) <= _data.size(), "ImageView: data too small, got" << _data.size() << "but expected at least" << Implementation::imageDataSize(*this) << "bytes", ); Implementation::checkImageFlagsForSize("ImageView:", flags, size); #endif } @@ -46,8 +47,9 @@ template ImageView::ImageView(co template ImageView::ImageView(const PixelStorage storage, const UnsignedInt format, const UnsignedInt formatExtra, const UnsignedInt pixelSize, const VectorTypeFor& size, const ImageFlags flags) noexcept: ImageView{storage, pixelFormatWrap(format), formatExtra, pixelSize, size, flags} {} -template ImageView::ImageView(const PixelStorage storage, const PixelFormat format, const UnsignedInt formatExtra, const UnsignedInt pixelSize, const VectorTypeFor& size, const ImageFlags flags) noexcept: _storage{storage}, _format{format}, _formatExtra{formatExtra}, _pixelSize{pixelSize}, _flags{flags}, _size{size}, _data{nullptr} { +template ImageView::ImageView(const PixelStorage storage, const PixelFormat format, const UnsignedInt formatExtra, const UnsignedInt pixelSize, const VectorTypeFor& size, const ImageFlags flags) noexcept: _storage{storage}, _format{format}, _formatExtra{formatExtra}, _pixelSize{UnsignedByte(pixelSize)}, _flags{flags}, _size{size}, _data{nullptr} { #ifndef CORRADE_NO_ASSERT + Implementation::checkPixelSize("ImageView:", pixelSize); Implementation::checkImageFlagsForSize("ImageView:", flags, size); #endif } diff --git a/src/Magnum/ImageView.h b/src/Magnum/ImageView.h index 4373bf9d2..f2a744b98 100644 --- a/src/Magnum/ImageView.h +++ b/src/Magnum/ImageView.h @@ -256,7 +256,8 @@ template class ImageView { * @ref pixelFormatSize(), this allows you to specify an * implementation-specific pixel format and pixel size directly. Uses * @ref pixelFormatWrap() internally to wrap @p format in - * @ref PixelFormat. + * @ref PixelFormat. The @p pixelSize is expected to be non-zero and + * less than @cpp 256 @ce. * * The @p data array is expected to be of proper size for given * parameters. For a 3D image, if @p flags contain @@ -286,7 +287,8 @@ template class ImageView { * @ref pixelFormatSize(), this allows you to specify an * implementation-specific pixel format and pixel size directly. Uses * @ref pixelFormatWrap() internally to wrap @p format in - * @ref PixelFormat. + * @ref PixelFormat. The @p pixelSize is expected to be non-zero and + * less than @cpp 256 @ce. * * Data pointer is set to @cpp nullptr @ce, call @ref setData() to * assign a memory view to the image. For a 3D image, if @p flags @@ -532,7 +534,8 @@ template class ImageView { PixelStorage _storage; PixelFormat _format; UnsignedInt _formatExtra; - UnsignedInt _pixelSize; + UnsignedByte _pixelSize; + /* 1 byte free */ ImageFlags _flags; VectorTypeFor _size; Containers::ArrayView _data; diff --git a/src/Magnum/Implementation/ImageProperties.h b/src/Magnum/Implementation/ImageProperties.h index 05d621edb..888cd7850 100644 --- a/src/Magnum/Implementation/ImageProperties.h +++ b/src/Magnum/Implementation/ImageProperties.h @@ -39,6 +39,16 @@ namespace Magnum { namespace Implementation { /* Used in *Image and Compressed*Image constructors */ #ifndef CORRADE_NO_ASSERT +inline void checkPixelSize(const char* + #ifndef CORRADE_STANDARD_ASSERT + const prefix + #endif + , const UnsignedInt pixelSize) +{ + CORRADE_ASSERT(pixelSize && pixelSize < 256, + prefix << "expected pixel size to be non-zero and less than 256 but got" << pixelSize, ); +} + inline void checkImageFlagsForSize(const char*, const ImageFlags1D, const Math::Vector<1, Int>&) {} inline void checkImageFlagsForSize(const char*, const ImageFlags2D, const Vector2i&) {} inline void checkImageFlagsForSize(const char* diff --git a/src/Magnum/Test/ImageTest.cpp b/src/Magnum/Test/ImageTest.cpp index 49523c8a2..25087052d 100644 --- a/src/Magnum/Test/ImageTest.cpp +++ b/src/Magnum/Test/ImageTest.cpp @@ -53,6 +53,7 @@ struct ImageTest: TestSuite::Tester { void constructCompressedImplementationSpecific(); void constructUnknownImplementationSpecificPixelSize(); + void constructInvalidPixelSize(); void constructInvalidSize(); void constructInvalidCubeMap(); void constructCompressedInvalidSize(); @@ -111,6 +112,7 @@ ImageTest::ImageTest() { &ImageTest::constructCompressedImplementationSpecific, &ImageTest::constructUnknownImplementationSpecificPixelSize, + &ImageTest::constructInvalidPixelSize, &ImageTest::constructInvalidSize, &ImageTest::constructInvalidCubeMap, &ImageTest::constructCompressedInvalidSize, @@ -481,13 +483,32 @@ void ImageTest::constructUnknownImplementationSpecificPixelSize() { Image2D{pixelFormatWrap(0x777)}; CORRADE_COMPARE_AS(out, "Image: can't determine size of an implementation-specific pixel format 0x666, pass it explicitly\n" - /* The second message is printed because it cannot exit the + /* The next messages are printed because it cannot exit the construction from the middle of the member initializer list. It does so with non-graceful asserts tho and just one message is printed. */ "pixelFormatSize(): can't determine size of an implementation-specific format 0x666\n" + "Image: expected pixel size to be non-zero and less than 256 but got 0\n" "Image: can't determine size of an implementation-specific pixel format 0x777, pass it explicitly\n" - "pixelFormatSize(): can't determine size of an implementation-specific format 0x777\n", + "pixelFormatSize(): can't determine size of an implementation-specific format 0x777\n" + "Image: expected pixel size to be non-zero and less than 256 but got 0\n", + TestSuite::Compare::String); +} + +void ImageTest::constructInvalidPixelSize() { + CORRADE_SKIP_IF_NO_ASSERT(); + + Containers::String out; + Error redirectError{&out}; + Image2D{PixelStorage{}, 666, 0, 0, {}, nullptr}; + Image2D{PixelStorage{}, 666, 0, 256, {}, nullptr}; + Image2D{PixelStorage{}, 666, 0, 0}; + Image2D{PixelStorage{}, 666, 0, 256}; + CORRADE_COMPARE_AS(out, + "Image: expected pixel size to be non-zero and less than 256 but got 0\n" + "Image: expected pixel size to be non-zero and less than 256 but got 256\n" + "Image: expected pixel size to be non-zero and less than 256 but got 0\n" + "Image: expected pixel size to be non-zero and less than 256 but got 256\n", TestSuite::Compare::String); } diff --git a/src/Magnum/Test/ImageViewTest.cpp b/src/Magnum/Test/ImageViewTest.cpp index 91aed819d..0499829d4 100644 --- a/src/Magnum/Test/ImageViewTest.cpp +++ b/src/Magnum/Test/ImageViewTest.cpp @@ -61,6 +61,7 @@ struct ImageViewTest: TestSuite::Tester { void constructCompressedFromMutable(); void constructUnknownImplementationSpecificPixelSize(); + void constructInvalidPixelSize(); void constructInvalidSize(); void constructInvalidCubeMap(); void constructCompressedInvalidSize(); @@ -116,6 +117,7 @@ ImageViewTest::ImageViewTest() { &ImageViewTest::constructCompressedFromMutable, &ImageViewTest::constructUnknownImplementationSpecificPixelSize, + &ImageViewTest::constructInvalidPixelSize, &ImageViewTest::constructInvalidSize, &ImageViewTest::constructInvalidCubeMap, &ImageViewTest::constructCompressedInvalidSize, @@ -666,13 +668,32 @@ void ImageViewTest::constructUnknownImplementationSpecificPixelSize() { ImageView2D{pixelFormatWrap(0x777), {1, 1}}; CORRADE_COMPARE_AS(out, "ImageView: can't determine size of an implementation-specific pixel format 0x666, pass it explicitly\n" - /* The second message is printed because it cannot exit the + /* The next messages are printed because it cannot exit the construction from the middle of the member initializer list. It does so with non-graceful asserts tho and just one message is printed. */ "pixelFormatSize(): can't determine size of an implementation-specific format 0x666\n" + "ImageView: expected pixel size to be non-zero and less than 256 but got 0\n" "ImageView: can't determine size of an implementation-specific pixel format 0x777, pass it explicitly\n" - "pixelFormatSize(): can't determine size of an implementation-specific format 0x777\n", + "pixelFormatSize(): can't determine size of an implementation-specific format 0x777\n" + "ImageView: expected pixel size to be non-zero and less than 256 but got 0\n", + TestSuite::Compare::String); +} + +void ImageViewTest::constructInvalidPixelSize() { + CORRADE_SKIP_IF_NO_ASSERT(); + + Containers::String out; + Error redirectError{&out}; + ImageView2D{PixelStorage{}, 666, 0, 0, {}, nullptr}; + ImageView2D{PixelStorage{}, 666, 0, 256, {}, nullptr}; + ImageView2D{PixelStorage{}, 666, 0, 0, {}}; + ImageView2D{PixelStorage{}, 666, 0, 256, {}}; + CORRADE_COMPARE_AS(out, + "ImageView: expected pixel size to be non-zero and less than 256 but got 0\n" + "ImageView: expected pixel size to be non-zero and less than 256 but got 256\n" + "ImageView: expected pixel size to be non-zero and less than 256 but got 0\n" + "ImageView: expected pixel size to be non-zero and less than 256 but got 256\n", TestSuite::Compare::String); } diff --git a/src/Magnum/Trade/ImageData.cpp b/src/Magnum/Trade/ImageData.cpp index 77c80fdbf..02dc406fc 100644 --- a/src/Magnum/Trade/ImageData.cpp +++ b/src/Magnum/Trade/ImageData.cpp @@ -60,9 +60,10 @@ template ImageData::ImageData(const PixelFor template ImageData::ImageData(const PixelStorage storage, const UnsignedInt format, const UnsignedInt formatExtra, const UnsignedInt pixelSize, const VectorTypeFor& size, Containers::Array&& data, const ImageFlags flags, const void* const importerState) noexcept: ImageData{storage, pixelFormatWrap(format), formatExtra, pixelSize, size, Utility::move(data), flags, importerState} {} -template ImageData::ImageData(const PixelStorage storage, const PixelFormat format, const UnsignedInt formatExtra, const UnsignedInt pixelSize, const VectorTypeFor& size, Containers::Array&& data, const ImageFlags flags, const void* const importerState) noexcept: _dataFlags{DataFlag::Owned|DataFlag::Mutable}, _compressed{false}, _flags{flags}, _storage{storage}, _format{format}, _formatExtra{formatExtra}, _pixelSize{pixelSize}, _size{size}, _data{Utility::move(data)}, _importerState{importerState} { - CORRADE_ASSERT(Magnum::Implementation::imageDataSize(*this) <= _data.size(), "Trade::ImageData: data too small, got" << _data.size() << "but expected at least" << Magnum::Implementation::imageDataSize(*this) << "bytes", ); +template ImageData::ImageData(const PixelStorage storage, const PixelFormat format, const UnsignedInt formatExtra, const UnsignedInt pixelSize, const VectorTypeFor& size, Containers::Array&& data, const ImageFlags flags, const void* const importerState) noexcept: _dataFlags{DataFlag::Owned|DataFlag::Mutable}, _compressed{false}, _flags{flags}, _storage{storage}, _format{format}, _formatExtra{formatExtra}, _pixelSize{UnsignedByte(pixelSize)}, _size{size}, _data{Utility::move(data)}, _importerState{importerState} { #ifndef CORRADE_NO_ASSERT + Magnum::Implementation::checkPixelSize("Trade::ImageData:", pixelSize); + CORRADE_ASSERT(Magnum::Implementation::imageDataSize(*this) <= _data.size(), "Trade::ImageData: data too small, got" << _data.size() << "but expected at least" << Magnum::Implementation::imageDataSize(*this) << "bytes", ); Magnum::Implementation::checkImageFlagsForSize("Trade::ImageData:", flags, size); #endif } diff --git a/src/Magnum/Trade/ImageData.h b/src/Magnum/Trade/ImageData.h index c236e807f..4bf4c3acf 100644 --- a/src/Magnum/Trade/ImageData.h +++ b/src/Magnum/Trade/ImageData.h @@ -280,7 +280,8 @@ template class ImageData { * @ref pixelFormatSize(), this allows you to specify an * implementation-specific pixel format and pixel size directly. Uses * @ref pixelFormatWrap() internally to wrap @p format in - * @ref Magnum::PixelFormat "PixelFormat". + * @ref Magnum::PixelFormat "PixelFormat". The @p pixelSize is expected + * to be non-zero and less than @cpp 256 @ce. * * The @p data array is expected to be of proper size for given * parameters. For a 3D image, if @p flags contain @@ -917,7 +918,8 @@ template class ImageData { CompressedPixelFormat _compressedFormat; }; UnsignedInt _formatExtra; - UnsignedInt _pixelSize; + UnsignedByte _pixelSize; + /* 3 bytes free */ VectorTypeFor _size; Containers::Array _data; const void* _importerState; diff --git a/src/Magnum/Trade/Test/ImageDataTest.cpp b/src/Magnum/Trade/Test/ImageDataTest.cpp index eb29c9f8e..ddd94a19d 100644 --- a/src/Magnum/Trade/Test/ImageDataTest.cpp +++ b/src/Magnum/Trade/Test/ImageDataTest.cpp @@ -59,6 +59,7 @@ struct ImageDataTest: TestSuite::Tester { void constructCompressedImplementationSpecificNotOwnedFlagOwned(); void constructUnknownImplementationSpecificPixelSize(); + void constructInvalidPixelSize(); void constructInvalidSize(); void constructInvalidCubeMap(); void constructCompressedInvalidSize(); @@ -135,6 +136,7 @@ ImageDataTest::ImageDataTest() { &ImageDataTest::constructCompressedImplementationSpecificNotOwnedFlagOwned, &ImageDataTest::constructUnknownImplementationSpecificPixelSize, + &ImageDataTest::constructInvalidPixelSize, &ImageDataTest::constructInvalidSize, &ImageDataTest::constructInvalidCubeMap, &ImageDataTest::constructCompressedInvalidSize, @@ -667,7 +669,30 @@ void ImageDataTest::constructUnknownImplementationSpecificPixelSize() { ImageData2D{pixelFormatWrap(0x777), {1, 1}, DataFlags{}, data}; CORRADE_COMPARE_AS(out, "Trade::ImageData: can't determine size of an implementation-specific pixel format 0x666, pass it explicitly\n" - "Trade::ImageData: can't determine size of an implementation-specific pixel format 0x777, pass it explicitly\n", + /* The next messages are printed because it cannot exit the + construction from the middle of the member initializer list. It does + so with non-graceful asserts tho and just one message is printed. */ + "Trade::ImageData: expected pixel size to be non-zero and less than 256 but got 0\n" + + "Trade::ImageData: can't determine size of an implementation-specific pixel format 0x777, pass it explicitly\n" + "Trade::ImageData: expected pixel size to be non-zero and less than 256 but got 0\n", + TestSuite::Compare::String); +} + +void ImageDataTest::constructInvalidPixelSize() { + CORRADE_SKIP_IF_NO_ASSERT(); + + Containers::String out; + Error redirectError{&out}; + ImageData2D{PixelStorage{}, 666, 0, 0, {}, nullptr}; + ImageData2D{PixelStorage{}, 666, 0, 256, {}, nullptr}; + ImageData2D{PixelStorage{}, 666, 0, 0, {}, DataFlags{}, nullptr}; + ImageData2D{PixelStorage{}, 666, 0, 256, {}, DataFlags{}, nullptr}; + CORRADE_COMPARE_AS(out, + "Trade::ImageData: expected pixel size to be non-zero and less than 256 but got 0\n" + "Trade::ImageData: expected pixel size to be non-zero and less than 256 but got 256\n" + "Trade::ImageData: expected pixel size to be non-zero and less than 256 but got 0\n" + "Trade::ImageData: expected pixel size to be non-zero and less than 256 but got 256\n", TestSuite::Compare::String); }