Browse Source

Deprecate passing nullptr to images.

Causes too much pain. Ugh.
pull/364/head
Vladimír Vondruš 7 years ago
parent
commit
069c81b9cb
  1. 3
      doc/changelog.dox
  2. 10
      src/Magnum/DebugTools/Test/CompareImageTest.cpp
  3. 8
      src/Magnum/ImageView.cpp
  4. 109
      src/Magnum/Test/ImageViewTest.cpp
  5. 8
      src/Magnum/Text/Test/AbstractGlyphCacheTest.cpp
  6. 8
      src/Magnum/Trade/Test/AbstractImageConverterTest.cpp

3
doc/changelog.dox

@ -550,6 +550,9 @@ See also:
use the non-templated version together with use the non-templated version together with
@ref Corrade::Containers::arrayCast() instead for properly bounds-checked @ref Corrade::Containers::arrayCast() instead for properly bounds-checked
type conversion type conversion
- Passing @cpp nullptr @ce to @ref ImageView constructors is deprecated and
will print a warning at runtime. Use a constructor without the @p data
parameter instead.
@subsection changelog-latest-compatibility Potential compatibility breakages, removed APIs @subsection changelog-latest-compatibility Potential compatibility breakages, removed APIs

10
src/Magnum/DebugTools/Test/CompareImageTest.cpp

@ -493,8 +493,9 @@ void CompareImageTest::pixelDeltaSpecials() {
void CompareImageTest::compareDifferentSize() { void CompareImageTest::compareDifferentSize() {
std::stringstream out; std::stringstream out;
ImageView2D a{PixelFormat::RG8UI, {3, 4}, nullptr}; char data[8*5];
ImageView2D b{PixelFormat::RG8UI, {3, 5}, nullptr}; ImageView2D a{PixelFormat::RG8UI, {3, 4}, data};
ImageView2D b{PixelFormat::RG8UI, {3, 5}, data};
{ {
TestSuite::Comparator<CompareImage> compare{{}, {}}; TestSuite::Comparator<CompareImage> compare{{}, {}};
@ -511,8 +512,9 @@ void CompareImageTest::compareDifferentSize() {
void CompareImageTest::compareDifferentFormat() { void CompareImageTest::compareDifferentFormat() {
std::stringstream out; std::stringstream out;
ImageView2D a{PixelFormat::RGBA32F, {3, 4}, nullptr}; char data[16*12];
ImageView2D b{PixelFormat::RGB32F, {3, 4}, nullptr}; ImageView2D a{PixelFormat::RGBA32F, {3, 4}, data};
ImageView2D b{PixelFormat::RGB32F, {3, 4}, data};
{ {
TestSuite::Comparator<CompareImage> compare{{}, {}}; TestSuite::Comparator<CompareImage> compare{{}, {}};

8
src/Magnum/ImageView.cpp

@ -35,7 +35,15 @@ template<UnsignedInt dimensions, class T> ImageView<dimensions, T>::ImageView(co
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) noexcept: ImageView{storage, pixelFormatWrap(format), formatExtra, pixelSize, size, data} {} 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) noexcept: ImageView{storage, pixelFormatWrap(format), formatExtra, pixelSize, size, data} {}
template<UnsignedInt dimensions, class T> ImageView<dimensions, T>::ImageView(const PixelStorage storage, const PixelFormat format, const UnsignedInt formatExtra, const UnsignedInt pixelSize, const VectorTypeFor<dimensions, Int>& size, const Containers::ArrayView<ErasedType> data) noexcept: _storage{storage}, _format{format}, _formatExtra{formatExtra}, _pixelSize{pixelSize}, _size{size}, _data{reinterpret_cast<Type*>(data.data()), data.size()} { template<UnsignedInt dimensions, class T> ImageView<dimensions, T>::ImageView(const PixelStorage storage, const PixelFormat format, const UnsignedInt formatExtra, const UnsignedInt pixelSize, const VectorTypeFor<dimensions, Int>& size, const Containers::ArrayView<ErasedType> data) noexcept: _storage{storage}, _format{format}, _formatExtra{formatExtra}, _pixelSize{pixelSize}, _size{size}, _data{reinterpret_cast<Type*>(data.data()), data.size()} {
#ifdef MAGNUM_BUILD_DEPRECATED
#ifndef CORRADE_NO_ASSERT
if(size.product() && !_data && !_data.size())
Warning{} << "ImageView::ImageView(): passing empty data to a non-empty view is deprecated, use a constructor without the data parameter instead";
#endif
CORRADE_ASSERT(!_data || Implementation::imageDataSize(*this) <= _data.size(), "ImageView::ImageView(): data too small, got" << _data.size() << "but expected at least" << Implementation::imageDataSize(*this) << "bytes", ); CORRADE_ASSERT(!_data || Implementation::imageDataSize(*this) <= _data.size(), "ImageView::ImageView(): data too small, got" << _data.size() << "but expected at least" << Implementation::imageDataSize(*this) << "bytes", );
#else
CORRADE_ASSERT(Implementation::imageDataSize(*this) <= _data.size(), "ImageView::ImageView(): data too small, got" << _data.size() << "but expected at least" << Implementation::imageDataSize(*this) << "bytes", );
#endif
} }
template<UnsignedInt dimensions, class T> ImageView<dimensions, T>::ImageView(const PixelStorage storage, const PixelFormat format, const VectorTypeFor<dimensions, Int>& size) noexcept: ImageView{storage, format, {}, Magnum::pixelSize(format), size} {} template<UnsignedInt dimensions, class T> ImageView<dimensions, T>::ImageView(const PixelStorage storage, const PixelFormat format, const VectorTypeFor<dimensions, Int>& size) noexcept: ImageView{storage, format, {}, Magnum::pixelSize(format), size} {}

109
src/Magnum/Test/ImageViewTest.cpp

@ -39,10 +39,8 @@ struct ImageViewTest: TestSuite::Tester {
template<class T> void constructGeneric(); template<class T> void constructGeneric();
template<class T> void constructGenericEmpty(); template<class T> void constructGenericEmpty();
template<class T> void constructGenericEmptyNullptr();
template<class T> void constructImplementationSpecific(); template<class T> void constructImplementationSpecific();
template<class T> void constructImplementationSpecificEmpty(); template<class T> void constructImplementationSpecificEmpty();
template<class T> void constructImplementationSpecificEmptyNullptr();
template<class T> void constructCompressedGeneric(); template<class T> void constructCompressedGeneric();
template<class T> void constructCompressedGenericEmpty(); template<class T> void constructCompressedGenericEmpty();
template<class T> void constructCompressedImplementationSpecific(); template<class T> void constructCompressedImplementationSpecific();
@ -51,6 +49,7 @@ struct ImageViewTest: TestSuite::Tester {
void constructFromMutable(); void constructFromMutable();
void constructCompressedFromMutable(); void constructCompressedFromMutable();
void constructNullptr();
void constructInvalidSize(); void constructInvalidSize();
void constructCompressedInvalidSize(); void constructCompressedInvalidSize();
@ -81,14 +80,10 @@ ImageViewTest::ImageViewTest() {
&ImageViewTest::constructGeneric<char>, &ImageViewTest::constructGeneric<char>,
&ImageViewTest::constructGenericEmpty<const char>, &ImageViewTest::constructGenericEmpty<const char>,
&ImageViewTest::constructGenericEmpty<char>, &ImageViewTest::constructGenericEmpty<char>,
&ImageViewTest::constructGenericEmptyNullptr<const char>,
&ImageViewTest::constructGenericEmptyNullptr<char>,
&ImageViewTest::constructImplementationSpecific<const char>, &ImageViewTest::constructImplementationSpecific<const char>,
&ImageViewTest::constructImplementationSpecific<char>, &ImageViewTest::constructImplementationSpecific<char>,
&ImageViewTest::constructImplementationSpecificEmpty<const char>, &ImageViewTest::constructImplementationSpecificEmpty<const char>,
&ImageViewTest::constructImplementationSpecificEmpty<char>, &ImageViewTest::constructImplementationSpecificEmpty<char>,
&ImageViewTest::constructImplementationSpecificEmptyNullptr<const char>,
&ImageViewTest::constructImplementationSpecificEmptyNullptr<char>,
&ImageViewTest::constructCompressedGeneric<const char>, &ImageViewTest::constructCompressedGeneric<const char>,
&ImageViewTest::constructCompressedGeneric<char>, &ImageViewTest::constructCompressedGeneric<char>,
&ImageViewTest::constructCompressedGenericEmpty<const char>, &ImageViewTest::constructCompressedGenericEmpty<const char>,
@ -101,6 +96,7 @@ ImageViewTest::ImageViewTest() {
&ImageViewTest::constructFromMutable, &ImageViewTest::constructFromMutable,
&ImageViewTest::constructCompressedFromMutable, &ImageViewTest::constructCompressedFromMutable,
&ImageViewTest::constructNullptr,
&ImageViewTest::constructInvalidSize, &ImageViewTest::constructInvalidSize,
&ImageViewTest::constructCompressedInvalidSize, &ImageViewTest::constructCompressedInvalidSize,
@ -205,34 +201,6 @@ template<class T> void ImageViewTest::constructGenericEmpty() {
} }
} }
template<class T> void ImageViewTest::constructGenericEmptyNullptr() {
setTestCaseTemplateName(MutabilityTraits<T>::name());
/* This should be deprecated/removed, as it doesn't provide anything over
the above and can lead to silent errors */
{
ImageView<2, T> a{PixelFormat::RG32F, {2, 6}, nullptr};
CORRADE_COMPARE(a.storage().alignment(), 4);
CORRADE_COMPARE(a.format(), PixelFormat::RG32F);
CORRADE_COMPARE(a.formatExtra(), 0);
CORRADE_COMPARE(a.pixelSize(), 8);
CORRADE_COMPARE(a.size(), (Vector2i{2, 6}));
CORRADE_COMPARE(a.data(), nullptr);
} {
ImageView<2, T> a{PixelStorage{}.setAlignment(1),
PixelFormat::RGB16F, {8, 3}, nullptr};
CORRADE_COMPARE(a.storage().alignment(), 1);
CORRADE_COMPARE(a.format(), PixelFormat::RGB16F);
CORRADE_COMPARE(a.formatExtra(), 0);
CORRADE_COMPARE(a.pixelSize(), 6);
CORRADE_COMPARE(a.size(), (Vector2i{8, 3}));
CORRADE_COMPARE(a.data(), nullptr);
}
}
template<class T> void ImageViewTest::constructImplementationSpecific() { template<class T> void ImageViewTest::constructImplementationSpecific() {
setTestCaseTemplateName(MutabilityTraits<T>::name()); setTestCaseTemplateName(MutabilityTraits<T>::name());
@ -361,68 +329,6 @@ template<class T> void ImageViewTest::constructImplementationSpecificEmpty() {
} }
} }
template<class T> void ImageViewTest::constructImplementationSpecificEmptyNullptr() {
setTestCaseTemplateName(MutabilityTraits<T>::name());
/* This should be deprecated/removed, as it doesn't provide anything over
the above and can lead to silent errors */
/* Single format */
{
ImageView<2, T> a{Vk::PixelFormat::R32G32B32F, {2, 16}, nullptr};
CORRADE_COMPARE(a.storage().alignment(), 4);
CORRADE_COMPARE(a.format(), pixelFormatWrap(Vk::PixelFormat::R32G32B32F));
CORRADE_COMPARE(a.formatExtra(), 0);
CORRADE_COMPARE(a.pixelSize(), 12);
CORRADE_COMPARE(a.size(), (Vector2i{2, 16}));
CORRADE_COMPARE(a.data(), nullptr);
} {
ImageView<2, T> a{PixelStorage{}.setAlignment(1),
Vk::PixelFormat::R32G32B32F, {1, 2}, nullptr};
CORRADE_COMPARE(a.storage().alignment(), 1);
CORRADE_COMPARE(a.format(), pixelFormatWrap(Vk::PixelFormat::R32G32B32F));
CORRADE_COMPARE(a.formatExtra(), 0);
CORRADE_COMPARE(a.pixelSize(), 12);
CORRADE_COMPARE(a.size(), (Vector2i{1, 2}));
CORRADE_COMPARE(a.data(), nullptr);
}
/* Format + extra */
{
ImageView<2, T> a{GL::PixelFormat::RGB, GL::PixelType::UnsignedShort, {1, 3}, nullptr};
CORRADE_COMPARE(a.storage().alignment(), 4);
CORRADE_COMPARE(a.format(), pixelFormatWrap(GL::PixelFormat::RGB));
CORRADE_COMPARE(a.formatExtra(), UnsignedInt(GL::PixelType::UnsignedShort));
CORRADE_COMPARE(a.pixelSize(), 6);
CORRADE_COMPARE(a.size(), (Vector2i{1, 3}));
CORRADE_COMPARE(a.data(), nullptr);
} {
ImageView<2, T> a{PixelStorage{}.setAlignment(1),
GL::PixelFormat::RGB, GL::PixelType::UnsignedShort, {8, 2}, nullptr};
CORRADE_COMPARE(a.format(), pixelFormatWrap(GL::PixelFormat::RGB));
CORRADE_COMPARE(a.formatExtra(), UnsignedInt(GL::PixelType::UnsignedShort));
CORRADE_COMPARE(a.pixelSize(), 6);
CORRADE_COMPARE(a.size(), (Vector2i{8, 2}));
CORRADE_COMPARE(a.data(), nullptr);
}
/* Manual pixel size */
{
ImageView<2, T> a{PixelStorage{}.setAlignment(1), 666, 1337, 6, {3, 3}, nullptr};
CORRADE_COMPARE(a.storage().alignment(), 1);
CORRADE_COMPARE(a.format(), pixelFormatWrap(GL::PixelFormat::RGB));
CORRADE_COMPARE(a.formatExtra(), UnsignedInt(GL::PixelType::UnsignedShort));
CORRADE_COMPARE(a.pixelSize(), 6);
CORRADE_COMPARE(a.size(), (Vector2i{3, 3}));
CORRADE_COMPARE(a.data(), nullptr);
}
}
template<class T> void ImageViewTest::constructCompressedGeneric() { template<class T> void ImageViewTest::constructCompressedGeneric() {
setTestCaseTemplateName(MutabilityTraits<T>::name()); setTestCaseTemplateName(MutabilityTraits<T>::name());
@ -566,6 +472,17 @@ void ImageViewTest::constructCompressedFromMutable() {
CORRADE_COMPARE(b.data().size(), 8); CORRADE_COMPARE(b.data().size(), 8);
} }
void ImageViewTest::constructNullptr() {
#ifdef CORRADE_BUILD_DEPRECATED
CORRADE_SKIP("This is still allowed on a deprecated build, can't test.");
#endif
std::ostringstream out;
Error redirectError{&out};
ImageView2D{PixelFormat::RGB8Unorm, {1, 3}, nullptr};
CORRADE_COMPARE(out.str(), "ImageView::ImageView(): data too small, got 0 but expected at least 12 bytes\n");
}
void ImageViewTest::constructInvalidSize() { void ImageViewTest::constructInvalidSize() {
std::ostringstream out; std::ostringstream out;
Error redirectError{&out}; Error redirectError{&out};

8
src/Magnum/Text/Test/AbstractGlyphCacheTest.cpp

@ -127,7 +127,7 @@ void AbstractGlyphCacheTest::setImage() {
Vector2i imageOffset, imageSize; Vector2i imageOffset, imageSize;
} cache{{100, 200}}; } cache{{100, 200}};
cache.setImage({80, 175}, ImageView2D{{}, {20, 25}, nullptr}); cache.setImage({80, 175}, ImageView2D{{}, {20, 25}});
CORRADE_COMPARE(cache.imageOffset, (Vector2i{80, 175})); CORRADE_COMPARE(cache.imageOffset, (Vector2i{80, 175}));
CORRADE_COMPARE(cache.imageSize, (Vector2i{20, 25})); CORRADE_COMPARE(cache.imageSize, (Vector2i{20, 25}));
@ -138,9 +138,9 @@ void AbstractGlyphCacheTest::setImageOutOfBounds() {
std::ostringstream out; std::ostringstream out;
Error redirectError{&out}; Error redirectError{&out};
cache.setImage({80, 175}, ImageView2D{{}, {20, 25}, nullptr}); cache.setImage({80, 175}, ImageView2D{{}, {20, 25}});
cache.setImage({81, 175}, ImageView2D{{}, {20, 25}, nullptr}); cache.setImage({81, 175}, ImageView2D{{}, {20, 25}});
cache.setImage({80, -1}, ImageView2D{{}, {20, 25}, nullptr}); cache.setImage({80, -1}, ImageView2D{{}, {20, 25}});
CORRADE_COMPARE(out.str(), CORRADE_COMPARE(out.str(),
"Text::AbstractGlyphCache::setImage(): Range({81, 175}, {101, 200}) out of bounds for texture size Vector(100, 200)\n" "Text::AbstractGlyphCache::setImage(): Range({81, 175}, {101, 200}) out of bounds for texture size Vector(100, 200)\n"

8
src/Magnum/Trade/Test/AbstractImageConverterTest.cpp

@ -188,7 +188,7 @@ void AbstractImageConverterTest::exportToImageNotImplemented() {
Error redirectError{&out}; Error redirectError{&out};
Converter converter; Converter converter;
converter.exportToImage(ImageView2D{PixelFormat::R8Unorm, {4, 6}, Containers::ArrayView<char>{nullptr, 24}}); converter.exportToImage(ImageView2D{PixelFormat::R8Unorm, {4, 6}, Containers::ArrayView<char>{nullptr, 128}});
CORRADE_COMPARE(out.str(), "Trade::AbstractImageConverter::exportToImage(): feature advertised but not implemented\n"); CORRADE_COMPARE(out.str(), "Trade::AbstractImageConverter::exportToImage(): feature advertised but not implemented\n");
} }
@ -201,7 +201,7 @@ void AbstractImageConverterTest::exportToCompressedImage() {
}; };
Converter converter; Converter converter;
Containers::Optional<CompressedImage2D> actual = converter.exportToCompressedImage(ImageView2D{PixelFormat::R8Unorm, {16, 8}, Containers::ArrayView<char>{nullptr, 24}}); Containers::Optional<CompressedImage2D> actual = converter.exportToCompressedImage(ImageView2D{PixelFormat::R8Unorm, {16, 8}, Containers::ArrayView<char>{nullptr, 128}});
CORRADE_VERIFY(actual); CORRADE_VERIFY(actual);
CORRADE_COMPARE(actual->data().size(), 64); CORRADE_COMPARE(actual->data().size(), 64);
CORRADE_COMPARE(actual->size(), (Vector2i{16, 8})); CORRADE_COMPARE(actual->size(), (Vector2i{16, 8}));
@ -216,7 +216,7 @@ void AbstractImageConverterTest::exportToCompressedImageNotSupported() {
Error redirectError{&out}; Error redirectError{&out};
Converter converter; Converter converter;
converter.exportToCompressedImage(ImageView2D{PixelFormat::R8Unorm, {16, 8}, Containers::ArrayView<char>{nullptr, 24}}); converter.exportToCompressedImage(ImageView2D{PixelFormat::R8Unorm, {16, 8}, Containers::ArrayView<char>{nullptr, 128}});
CORRADE_COMPARE(out.str(), "Trade::AbstractImageConverter::exportToCompressedImage(): feature not supported\n"); CORRADE_COMPARE(out.str(), "Trade::AbstractImageConverter::exportToCompressedImage(): feature not supported\n");
} }
@ -229,7 +229,7 @@ void AbstractImageConverterTest::exportToCompressedImageNotImplemented() {
Error redirectError{&out}; Error redirectError{&out};
Converter converter; Converter converter;
converter.exportToCompressedImage(ImageView2D{PixelFormat::R8Unorm, {16, 8}, Containers::ArrayView<char>{nullptr, 24}}); converter.exportToCompressedImage(ImageView2D{PixelFormat::R8Unorm, {16, 8}, Containers::ArrayView<char>{nullptr, 128}});
CORRADE_COMPARE(out.str(), "Trade::AbstractImageConverter::exportToCompressedImage(): feature advertised but not implemented\n"); CORRADE_COMPARE(out.str(), "Trade::AbstractImageConverter::exportToCompressedImage(): feature advertised but not implemented\n");
} }

Loading…
Cancel
Save