ic
/
magnum
mirror of https://github.com/mosra/magnum.git
6
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

Assert non-implementation-specific PixelFormat in Image* constructors. 

And document that. Because the pixel size cannot be determined for it,
and one has to either pass it explicitly or use the templated overload
that figures it out implicitly via ADL. This asserted before, but only
deep inside in pixelFormatSize(), which may be confusing.

I need to do a similar treatment for compressed images with block size
properties so let's first make it behave properly for uncompressed.
pull/651/merge
Vladimír Vondruš 1 year ago
parent
ad309d3d0f
commit
d51921ff00
9 changed files with 117 additions and 5 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 4
      src/Magnum/Image.cpp
  2. 16
      src/Magnum/Image.h
  3. 4
      src/Magnum/ImageView.cpp
  4. 16
      src/Magnum/ImageView.h
  5. 22
      src/Magnum/Test/ImageTest.cpp
  6. 24
      src/Magnum/Test/ImageViewTest.cpp
  7. 10
      src/Magnum/Trade/ImageData.cpp
  8. 8
      src/Magnum/Trade/ImageData.h
  9. 18
      src/Magnum/Trade/Test/ImageDataTest.cpp

4
src/Magnum/Image.cpp
Unescape View File

@ -32,7 +32,7 @@
namespace Magnum {
template<UnsignedInt dimensions> Image<dimensions>::Image(const PixelStorage storage, const PixelFormat format, const VectorTypeFor<dimensions, Int>& size, Containers::Array<char>&& data, const ImageFlags<dimensions> flags) noexcept: Image{storage, format, {}, pixelFormatSize(format), size, Utility::move(data), flags} {}
template<UnsignedInt dimensions> Image<dimensions>::Image(const PixelStorage storage, const PixelFormat format, const VectorTypeFor<dimensions, Int>& size, Containers::Array<char>&& data, const ImageFlags<dimensions> flags) noexcept: Image{storage, format, {}, (CORRADE_CONSTEXPR_ASSERT(!isPixelFormatImplementationSpecific(format), "Image: can't determine size of an implementation-specific pixel format" << Debug::hex << pixelFormatUnwrap(format) << Debug::nospace << ", pass it explicitly"), pixelFormatSize(format)), size, Utility::move(data), flags} {}
template<UnsignedInt dimensions> Image<dimensions>::Image(const PixelStorage storage, const UnsignedInt format, const UnsignedInt formatExtra, const UnsignedInt pixelSize, const VectorTypeFor<dimensions, Int>& size, Containers::Array<char>&& data, const ImageFlags<dimensions> flags) noexcept: Image{storage, pixelFormatWrap(format), formatExtra, pixelSize, size, Utility::move(data), flags} {}
@ -43,7 +43,7 @@ template<UnsignedInt dimensions> Image<dimensions>::Image(const PixelStorage sto
#endif
}
template<UnsignedInt dimensions> Image<dimensions>::Image(const PixelStorage storage, const PixelFormat format) noexcept: Image{storage, format, {}, pixelFormatSize(format)} {}
template<UnsignedInt dimensions> Image<dimensions>::Image(const PixelStorage storage, const PixelFormat format) noexcept: Image{storage, format, {}, (CORRADE_CONSTEXPR_ASSERT(!isPixelFormatImplementationSpecific(format), "Image: can't determine size of an implementation-specific pixel format" << Debug::hex << pixelFormatUnwrap(format) << Debug::nospace << ", pass it explicitly"), pixelFormatSize(format))} {}
template<UnsignedInt dimensions> Image<dimensions>::Image(const PixelStorage storage, const UnsignedInt format, const UnsignedInt formatExtra, const UnsignedInt pixelSize) noexcept: Image{storage, pixelFormatWrap(format), formatExtra, pixelSize} {}

16
src/Magnum/Image.h
Unescape View File

@ -141,6 +141,14 @@ template<UnsignedInt dimensions> class Image {
* parameters. For a 3D image, if @p flags contain
* @ref ImageFlag3D::CubeMap, the @p size is expected to match its
* restrictions.
*
* The @p format is expected to not be implementation-specific, use the
* @ref Image(PixelStorage, PixelFormat, UnsignedInt, UnsignedInt, const VectorTypeFor<dimensions, Int>&, Containers::Array<char>&&, ImageFlags<dimensions>)
* overload to explicitly pass an implementation-specific
* @ref PixelFormat along with a pixel size, or the
* @ref Image(PixelStorage, T, const VectorTypeFor<dimensions, Int>&, Containers::Array<char>&&, ImageFlags<dimensions>)
* overload with the original implementation-specific enum type to have
* the pixel size determined implicitly.
*/
explicit Image(PixelStorage storage, PixelFormat format, const VectorTypeFor<dimensions, Int>& size, Containers::Array<char>&& data, ImageFlags<dimensions> flags = {}) noexcept;
@ -164,6 +172,14 @@ template<UnsignedInt dimensions> class Image {
* Size is set to zero, data pointer to @cpp nullptr @ce and data
* layout flags are empty. Move over a non-empty instance to make it
* useful.
*
* The @p format is expected to not be implementation-specific, use the
* @ref Image(PixelStorage, PixelFormat, UnsignedInt, UnsignedInt)
* overload to explicitly pass an implementation-specific
* @ref PixelFormat along with a pixel size, or the
* @ref Image(PixelStorage, T) overload with the original
* implementation-specific enum type to have the pixel size determined
* implicitly.
*/
/* No ImageFlags parameter here as this constructor is mainly used to
query GL textures, and there the flags are forcibly reset */

4
src/Magnum/ImageView.cpp
Unescape View File

@ -31,7 +31,7 @@
namespace Magnum {
template<UnsignedInt dimensions, class T> ImageView<dimensions, T>::ImageView(const PixelStorage storage, const PixelFormat format, const VectorTypeFor<dimensions, Int>& size, const Containers::ArrayView<ErasedType> data, const ImageFlags<dimensions> flags) noexcept: ImageView{storage, format, {}, pixelFormatSize(format), size, data, flags} {}
template<UnsignedInt dimensions, class T> ImageView<dimensions, T>::ImageView(const PixelStorage storage, const PixelFormat format, const VectorTypeFor<dimensions, Int>& size, const Containers::ArrayView<ErasedType> data, const ImageFlags<dimensions> flags) noexcept: ImageView{storage, format, {}, (CORRADE_CONSTEXPR_ASSERT(!isPixelFormatImplementationSpecific(format), "ImageView: can't determine size of an implementation-specific pixel format" << Debug::hex << pixelFormatUnwrap(format) << Debug::nospace << ", pass it explicitly"), pixelFormatSize(format)), size, data, flags} {}
template<UnsignedInt dimensions, class T> ImageView<dimensions, T>::ImageView(const PixelStorage storage, const UnsignedInt format, const UnsignedInt formatExtra, const UnsignedInt pixelSize, const VectorTypeFor<dimensions, Int>& size, const Containers::ArrayView<ErasedType> data, const ImageFlags<dimensions> flags) noexcept: ImageView{storage, pixelFormatWrap(format), formatExtra, pixelSize, size, data, flags} {}
@ -42,7 +42,7 @@ template<UnsignedInt dimensions, class T> ImageView<dimensions, T>::ImageView(co
#endif
}
template<UnsignedInt dimensions, class T> ImageView<dimensions, T>::ImageView(const PixelStorage storage, const PixelFormat format, const VectorTypeFor<dimensions, Int>& size, const ImageFlags<dimensions> flags) noexcept: ImageView{storage, format, {}, pixelFormatSize(format), size, flags} {}
template<UnsignedInt dimensions, class T> ImageView<dimensions, T>::ImageView(const PixelStorage storage, const PixelFormat format, const VectorTypeFor<dimensions, Int>& size, const ImageFlags<dimensions> flags) noexcept: ImageView{storage, format, {}, (CORRADE_CONSTEXPR_ASSERT(!isPixelFormatImplementationSpecific(format), "ImageView: can't determine size of an implementation-specific pixel format" << Debug::hex << pixelFormatUnwrap(format) << Debug::nospace << ", pass it explicitly"), pixelFormatSize(format)), size, flags} {}
template<UnsignedInt dimensions, class T> ImageView<dimensions, T>::ImageView(const PixelStorage storage, const UnsignedInt format, const UnsignedInt formatExtra, const UnsignedInt pixelSize, const VectorTypeFor<dimensions, Int>& size, const ImageFlags<dimensions> flags) noexcept: ImageView{storage, pixelFormatWrap(format), formatExtra, pixelSize, size, flags} {}

16
src/Magnum/ImageView.h
Unescape View File

@ -185,6 +185,14 @@ template<UnsignedInt dimensions, class T> class ImageView {
* parameters. For a 3D image, if @p flags contain
* @ref ImageFlag3D::CubeMap, the @p size is expected to match its
* restrictions.
*
* The @p format is expected to not be implementation-specific, use the
* @ref ImageView(PixelStorage, PixelFormat, UnsignedInt, UnsignedInt, const VectorTypeFor<dimensions, Int>&, Containers::ArrayView<ErasedType>, ImageFlags<dimensions>)
* overload to explicitly pass an implementation-specific
* @ref PixelFormat along with a pixel size, or the
* @ref ImageView(PixelStorage, U, const VectorTypeFor<dimensions, Int>&, Containers::ArrayView<ErasedType>, ImageFlags<dimensions>)
* overload with the original implementation-specific enum type to have
* the pixel size determined implicitly.
*/
explicit ImageView(PixelStorage storage, PixelFormat format, const VectorTypeFor<dimensions, Int>& size, Containers::ArrayView<ErasedType> data, ImageFlags<dimensions> flags = {}) noexcept;
@ -211,6 +219,14 @@ template<UnsignedInt dimensions, class T> class ImageView {
* assign a memory view to the image. For a 3D image, if @p flags
* contain @ref ImageFlag3D::CubeMap, the @p size is expected to match
* its restrictions.
*
* The @p format is expected to not be implementation-specific, use the
* @ref ImageView(PixelStorage, PixelFormat, UnsignedInt, UnsignedInt, const VectorTypeFor<dimensions, Int>&, ImageFlags<dimensions>)
* overload to explicitly pass an implementation-specific
* @ref PixelFormat along with a pixel size, or the
* @ref ImageView(PixelStorage, U, const VectorTypeFor<dimensions, Int>&, ImageFlags<dimensions>)
* overload with the original implementation-specific enum type to have
* the pixel size determined implicitly.
*/
explicit ImageView(PixelStorage storage, PixelFormat format, const VectorTypeFor<dimensions, Int>& size, ImageFlags<dimensions> flags = {}) noexcept;

22
src/Magnum/Test/ImageTest.cpp
Unescape View File

@ -32,6 +32,7 @@
#include <Corrade/Containers/StridedArrayView.h>
#include <Corrade/Containers/String.h>
#include <Corrade/TestSuite/Tester.h>
#include <Corrade/TestSuite/Compare/String.h>
#include <Corrade/Utility/DebugStl.h> /** @todo remove once dataProperties() std::pair is gone */
#include "Magnum/ImageView.h"
@ -51,6 +52,7 @@ struct ImageTest: TestSuite::Tester {
void constructCompressedGenericPlaceholder();
void constructCompressedImplementationSpecific();
void constructUnknownImplementationSpecificPixelSize();
void constructInvalidSize();
void constructInvalidCubeMap();
void constructCompressedInvalidSize();
@ -108,6 +110,7 @@ ImageTest::ImageTest() {
&ImageTest::constructCompressedGenericPlaceholder,
&ImageTest::constructCompressedImplementationSpecific,
&ImageTest::constructUnknownImplementationSpecificPixelSize,
&ImageTest::constructInvalidSize,
&ImageTest::constructInvalidCubeMap,
&ImageTest::constructCompressedInvalidSize,
@ -471,6 +474,25 @@ void ImageTest::constructCompressedImplementationSpecific() {
/* Manual properties not implemented yet */
}
void ImageTest::constructUnknownImplementationSpecificPixelSize() {
CORRADE_SKIP_IF_NO_ASSERT();
Containers::String out;
Error redirectError{&out};
Image2D{pixelFormatWrap(0x666), {1, 1}, Containers::Array<char>{NoInit, 1}};
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
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: 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",
TestSuite::Compare::String);
}
void ImageTest::constructInvalidSize() {
CORRADE_SKIP_IF_NO_ASSERT();

24
src/Magnum/Test/ImageViewTest.cpp
Unescape View File

@ -32,6 +32,7 @@
#include <Corrade/Containers/StridedArrayView.h>
#include <Corrade/Containers/String.h>
#include <Corrade/TestSuite/Tester.h>
#include <Corrade/TestSuite/Compare/String.h>
#include <Corrade/Utility/DebugStl.h> /** @todo remove once dataProperties() std::pair is gone */
#include "Magnum/PixelFormat.h"
@ -60,6 +61,7 @@ struct ImageViewTest: TestSuite::Tester {
void constructCompressedFromMutable();
void constructNullptr();
void constructUnknownImplementationSpecificPixelSize();
void constructInvalidSize();
void constructInvalidCubeMap();
void constructCompressedInvalidSize();
@ -115,6 +117,7 @@ ImageViewTest::ImageViewTest() {
&ImageViewTest::constructCompressedFromMutable,
&ImageViewTest::constructNullptr,
&ImageViewTest::constructUnknownImplementationSpecificPixelSize,
&ImageViewTest::constructInvalidSize,
&ImageViewTest::constructInvalidCubeMap,
&ImageViewTest::constructCompressedInvalidSize,
@ -665,6 +668,27 @@ void ImageViewTest::constructNullptr() {
CORRADE_COMPARE(out, "ImageView: data too small, got 0 but expected at least 12 bytes\n");
}
void ImageViewTest::constructUnknownImplementationSpecificPixelSize() {
CORRADE_SKIP_IF_NO_ASSERT();
char data[1];
Containers::String out;
Error redirectError{&out};
ImageView2D{pixelFormatWrap(0x666), {1, 1}, data};
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
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: 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",
TestSuite::Compare::String);
}
void ImageViewTest::constructInvalidSize() {
CORRADE_SKIP_IF_NO_ASSERT();

10
src/Magnum/Trade/ImageData.cpp
Unescape View File

@ -34,7 +34,15 @@
namespace Magnum { namespace Trade {
template<UnsignedInt dimensions> ImageData<dimensions>::ImageData(const PixelStorage storage, const PixelFormat format, const VectorTypeFor<dimensions, Int>& size, Containers::Array<char>&& data, const ImageFlags<dimensions> flags, const void* const importerState) noexcept: ImageData{storage, format, {}, pixelFormatSize(format), size, Utility::move(data), flags, importerState} {}
template<UnsignedInt dimensions> ImageData<dimensions>::ImageData(const PixelStorage storage, const PixelFormat format, const VectorTypeFor<dimensions, Int>& size, Containers::Array<char>&& data, const ImageFlags<dimensions> flags, const void* const importerState) noexcept: ImageData{storage, format, {}, (
/* Unlike with Image and ImageView have to do it like this to avoid an
ungraceful assertion from pixelFormatSize() afterwards */
#ifndef CORRADE_NO_ASSERT
isPixelFormatImplementationSpecific(format) ?
(CORRADE_CONSTEXPR_ASSERT(false, "Trade::ImageData: can't determine size of an implementation-specific pixel format" << Debug::hex << pixelFormatUnwrap(format) << Debug::nospace << ", pass it explicitly"), 0) :
#endif
pixelFormatSize(format)
), size, Utility::move(data), flags, importerState} {}
template<UnsignedInt dimensions> ImageData<dimensions>::ImageData(const PixelStorage storage, const PixelFormat format, const VectorTypeFor<dimensions, Int>& size, const DataFlags dataFlags, const Containers::ArrayView<const void> data, const ImageFlags<dimensions> flags, const void* const importerState) noexcept: ImageData{storage, format, size, Containers::Array<char>{const_cast<char*>(static_cast<const char*>(data.data())), data.size(), Implementation::nonOwnedArrayDeleter}, flags, importerState} {
CORRADE_ASSERT(!(dataFlags & DataFlag::Owned),

8
src/Magnum/Trade/ImageData.h
Unescape View File

@ -165,6 +165,14 @@ template<UnsignedInt dimensions> class ImageData {
* @ref DataFlag::Owned and @ref DataFlag::Mutable. For non-owned data
* use the @ref ImageData(PixelStorage, PixelFormat, const VectorTypeFor<dimensions, Int>&, DataFlags, Containers::ArrayView<const void>, ImageFlags<dimensions>, const void*)
* constructor instead.
*
* The @p format is expected to not be implementation-specific, use the
* @ref ImageData(PixelStorage, PixelFormat, UnsignedInt, UnsignedInt, const VectorTypeFor<dimensions, Int>&, Containers::Array<char>&&, ImageFlags<dimensions>, const void*)
* overload to explicitly pass an implementation-specific
* @ref PixelFormat along with a pixel size, or the
* @ref ImageData(PixelStorage, T, const VectorTypeFor<dimensions, Int>&, Containers::Array<char>&&, ImageFlags<dimensions>, const void*)
* overload with the original implementation-specific enum type to have
* the pixel size determined implicitly.
*/
explicit ImageData(PixelStorage storage, PixelFormat format, const VectorTypeFor<dimensions, Int>& size, Containers::Array<char>&& data, ImageFlags<dimensions> flags = {}, const void* importerState = nullptr) noexcept;

18
src/Magnum/Trade/Test/ImageDataTest.cpp
Unescape View File

@ -32,6 +32,7 @@
#include <Corrade/Containers/StridedArrayView.h>
#include <Corrade/Containers/String.h>
#include <Corrade/TestSuite/Tester.h>
#include <Corrade/TestSuite/Compare/String.h>
#include <Corrade/Utility/DebugStl.h> /** @todo remove once dataProperties() std::pair is gone */
#include "Magnum/ImageView.h"
@ -57,6 +58,7 @@ struct ImageDataTest: TestSuite::Tester {
void constructCompressedGenericNotOwnedFlagOwned();
void constructCompressedImplementationSpecificNotOwnedFlagOwned();
void constructUnknownImplementationSpecificPixelSize();
void constructInvalidSize();
void constructInvalidCubeMap();
void constructCompressedInvalidSize();
@ -132,6 +134,7 @@ ImageDataTest::ImageDataTest() {
&ImageDataTest::constructCompressedGenericNotOwnedFlagOwned,
&ImageDataTest::constructCompressedImplementationSpecificNotOwnedFlagOwned,
&ImageDataTest::constructUnknownImplementationSpecificPixelSize,
&ImageDataTest::constructInvalidSize,
&ImageDataTest::constructInvalidCubeMap,
&ImageDataTest::constructCompressedInvalidSize,
@ -655,6 +658,21 @@ void ImageDataTest::constructCompressedImplementationSpecificNotOwnedFlagOwned()
"Trade::ImageData: can't construct a non-owned instance with Trade::DataFlag::Owned\n");
}
void ImageDataTest::constructUnknownImplementationSpecificPixelSize() {
CORRADE_SKIP_IF_NO_ASSERT();
char data[1];
Containers::String out;
Error redirectError{&out};
ImageData2D{pixelFormatWrap(0x666), {1, 1}, Containers::Array<char>{NoInit, 1}};
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",
TestSuite::Compare::String);
}
void ImageDataTest::constructInvalidSize() {
CORRADE_SKIP_IF_NO_ASSERT();

Write Preview
Loading…
Cancel
Save