Browse Source

Trade: restrict multi-level image conversion to same flags as well.

They are required to have the same format already, so flags make sense
as well -- what's the point of saving a multi-level cube map with one of
the levels being just a 2D array, anyway?
pull/578/head
Vladimír Vondruš 4 years ago
parent
commit
667e8c1ca9
  1. 6
      src/Magnum/Trade/AbstractImageConverter.cpp
  2. 110
      src/Magnum/Trade/AbstractImageConverter.h
  3. 44
      src/Magnum/Trade/Test/AbstractImageConverterTest.cpp

6
src/Magnum/Trade/AbstractImageConverter.cpp

@ -537,6 +537,7 @@ template<UnsignedInt dimensions> bool checkImageValidity(const char* const messa
const PixelFormat format = imageLevels[0].format();
const UnsignedInt formatExtra = imageLevels[0].formatExtra();
const ImageFlags<dimensions> flags = imageLevels[0].flags();
/* Going through *all* levels although the format assertion is never fired
in the first iteration in order to properly check also the first one for
zero size / nullptr. */
@ -549,6 +550,8 @@ template<UnsignedInt dimensions> bool checkImageValidity(const char* const messa
messagePrefix << "levels don't have the same format, expected" << format << "but got" << imageLevels[i].format() << "for image" << i, false);
CORRADE_ASSERT(imageLevels[i].formatExtra() == formatExtra,
messagePrefix << "levels don't have the same extra format field, expected" << formatExtra << "but got" << imageLevels[i].formatExtra() << "for image" << i, false);
CORRADE_ASSERT(imageLevels[i].flags() == flags,
messagePrefix << "levels don't have the same flags, expected" << flags << "but got" << imageLevels[i].flags() << "for image" << i, false);
}
return true;
@ -559,6 +562,7 @@ template<UnsignedInt dimensions> bool checkImageValidity(const char* const messa
messagePrefix << "at least one image has to be specified", false);
const CompressedPixelFormat format = imageLevels[0].format();
const ImageFlags<dimensions> flags = imageLevels[0].flags();
/* Going through *all* levels although the format assertion is never fired
in the first iteration in order to properly check also the first one for
zero size / nullptr. */
@ -569,6 +573,8 @@ template<UnsignedInt dimensions> bool checkImageValidity(const char* const messa
messagePrefix << "can't convert image" << i << "with a nullptr view", false);
CORRADE_ASSERT(imageLevels[i].format() == format,
messagePrefix << "levels don't have the same format, expected" << format << "but got" << imageLevels[i].format() << "for image" << i, false);
CORRADE_ASSERT(imageLevels[i].flags() == flags,
messagePrefix << "levels don't have the same flags, expected" << flags << "but got" << imageLevels[i].flags() << "for image" << i, false);
}
return true;

110
src/Magnum/Trade/AbstractImageConverter.h

@ -612,10 +612,10 @@ checked by the implementation:
- All @ref doConvertToData() and @ref doConvertToFile() functions taking
multiple (compressed) images are called only if the list has at least one
image, each of the images has a non-zero size, the views are not
@cpp nullptr @ce and additionally all views have the same pixel format.
Since file formats have varying requirements on image level sizes and their
order and some don't impose any requirements at all, the plugin
implementation is expected to check the sizes on its own.
@cpp nullptr @ce and additionally all views have the same pixel format and
layout flags. Since file formats have varying requirements on image level
sizes and their order and some don't impose any requirements at all, the
plugin implementation is expected to check the sizes on its own.
@m_class{m-block m-warning}
@ -1082,11 +1082,11 @@ class MAGNUM_TRADE_EXPORT AbstractImageConverter: public PluginManager::Abstract
* Available only if @ref ImageConverterFeature::ConvertLevels1DToData
* is supported. The function expects at least one image to be passed,
* with each view expected to not be @cpp nullptr @ce, to have a
* non-zero size, and all of them sharing the same pixel format. Note
* that certain converters may impose additional size and order
* restrictions on the images, see documentation of a particular plugin
* for more information. On failure prints a message to
* @relativeref{Magnum,Error} and returns @ref Containers::NullOpt.
* non-zero size, and all of them sharing the same pixel format and
* layout flags. Note that certain converters may impose additional
* size and order restrictions on the images, see documentation of a
* particular plugin for more information. On failure prints a message
* to @relativeref{Magnum,Error} and returns @ref Containers::NullOpt.
* @see @ref features(), @ref convertToData(Containers::ArrayView<const CompressedImageView1D>),
* @ref convert(), @ref convertToFile()
*/
@ -1115,10 +1115,11 @@ class MAGNUM_TRADE_EXPORT AbstractImageConverter: public PluginManager::Abstract
* is supported. The function expects at least one image to be passed,
* with each view expected to not be @cpp nullptr @ce, to have a
* non-zero size in all dimensions, and all views sharing the same
* pixel format. Note that certain converters may impose additional
* size and order restrictions on the images, see documentation of a
* particular plugin for more information. On failure prints a message
* to @relativeref{Magnum,Error} and returns @ref Containers::NullOpt.
* pixel format and layout flags. Note that certain converters may
* impose additional size and order restrictions on the images, see
* documentation of a particular plugin for more information. On
* failure prints a message to @relativeref{Magnum,Error} and returns
* @ref Containers::NullOpt.
* @see @ref features(), @ref convertToData(Containers::ArrayView<const CompressedImageView2D>),
* @ref convert(), @ref convertToFile()
*/
@ -1147,10 +1148,11 @@ class MAGNUM_TRADE_EXPORT AbstractImageConverter: public PluginManager::Abstract
* is supported. The function expects at least one image to be passed,
* with each view expected to not be @cpp nullptr @ce, to have a
* non-zero size in all dimensions, and all views sharing the same
* pixel format. Note that certain converters may impose additional
* size and order restrictions on the images, see documentation of a
* particular plugin for more information. On failure prints a message
* to @relativeref{Magnum,Error} and returns @ref Containers::NullOpt.
* pixel format and layout flags. Note that certain converters may
* impose additional size and order restrictions on the images, see
* documentation of a particular plugin for more information. On
* failure prints a message to @relativeref{Magnum,Error} and returns
* @ref Containers::NullOpt.
* @see @ref features(), @ref convertToData(Containers::ArrayView<const CompressedImageView3D>),
* @ref convert(), @ref convertToFile()
*/
@ -1178,11 +1180,11 @@ class MAGNUM_TRADE_EXPORT AbstractImageConverter: public PluginManager::Abstract
* Available only if @ref ImageConverterFeature::ConvertCompressedLevels1DToData
* is supported. The function expects at least one image to be passed,
* with each view expected to not be @cpp nullptr @ce, to have a
* non-zero size, and all views sharing the same pixel format. Note
* that certain converters may impose additional size and order
* restrictions on the images, see documentation of a particular plugin
* for more information. On failure prints a message to
* @relativeref{Magnum,Error} and returns @ref Containers::NullOpt.
* non-zero size, and all views sharing the same pixel format and
* layout flags. Note that certain converters may impose additional
* size and order restrictions on the images, see documentation of a
* particular plugin for more information. On failure prints a message
* to @relativeref{Magnum,Error} and returns @ref Containers::NullOpt.
* @see @ref features(), @ref convertToData(Containers::ArrayView<const ImageView1D>),
* @ref convert(), @ref convertToFile()
*/
@ -1211,10 +1213,11 @@ class MAGNUM_TRADE_EXPORT AbstractImageConverter: public PluginManager::Abstract
* is supported. The function expects at least one image to be passed,
* with each view expected to not be @cpp nullptr @ce, to have a
* non-zero size in all dimensions, and all views sharing the same
* pixel format. Note that certain converters may impose additional
* size and order restrictions on the images, see documentation of a
* particular plugin for more information. On failure prints a message
* to @relativeref{Magnum,Error} and returns @ref Containers::NullOpt.
* pixel format and layout flags. Note that certain converters may
* impose additional size and order restrictions on the images, see
* documentation of a particular plugin for more information. On
* failure prints a message to @relativeref{Magnum,Error} and returns
* @ref Containers::NullOpt.
* @see @ref features(), @ref convertToData(Containers::ArrayView<const ImageView2D>),
* @ref convert(), @ref convertToFile()
*/
@ -1243,10 +1246,11 @@ class MAGNUM_TRADE_EXPORT AbstractImageConverter: public PluginManager::Abstract
* is supported. The function expects at least one image to be passed,
* with each view expected to not be @cpp nullptr @ce, to have a
* non-zero size in all dimensions, and all views sharing the same
* pixel format. Note that certain converters may impose additional
* size and order restrictions on the images, see documentation of a
* particular plugin for more information. On failure prints a message
* to @relativeref{Magnum,Error} and returns @ref Containers::NullOpt.
* pixel format and layout flags. Note that certain converters may
* impose additional size and order restrictions on the images, see
* documentation of a particular plugin for more information. On
* failure prints a message to @relativeref{Magnum,Error} and returns
* @ref Containers::NullOpt.
* @see @ref features(), @ref convertToData(Containers::ArrayView<const ImageView3D>),
* @ref convert(), @ref convertToFile()
*/
@ -1465,10 +1469,11 @@ class MAGNUM_TRADE_EXPORT AbstractImageConverter: public PluginManager::Abstract
* is supported. The function expects at least one image to be passed,
* with each view expected to not be @cpp nullptr @ce, to have a
* non-zero size in all dimensions, and all views sharing the same
* pixel format. Note that certain converters may impose additional
* size and order restrictions on the images, see documentation of a
* particular plugin for more information. On failure prints a message
* to @relativeref{Magnum,Error} and returns @cpp false @ce.
* pixel format and layout flags. Note that certain converters may
* impose additional size and order restrictions on the images, see
* documentation of a particular plugin for more information. On
* failure prints a message to @relativeref{Magnum,Error} and returns
* @cpp false @ce.
* @see @ref features(), @ref convertToFile(Containers::ArrayView<const CompressedImageView2D>, Containers::StringView),
* @ref convert(), @ref convertToData()
*/
@ -1487,10 +1492,11 @@ class MAGNUM_TRADE_EXPORT AbstractImageConverter: public PluginManager::Abstract
* is supported. The function expects at least one image to be passed,
* with each view expected to not be @cpp nullptr @ce, to have a
* non-zero size in all dimensions, and all views sharing the same
* pixel format. Note that certain converters may impose additional
* size and order restrictions on the images, see documentation of a
* particular plugin for more information. On failure prints a message
* to @relativeref{Magnum,Error} and returns @cpp false @ce.
* pixel format and layout flags. Note that certain converters may
* impose additional size and order restrictions on the images, see
* documentation of a particular plugin for more information. On
* failure prints a message to @relativeref{Magnum,Error} and returns
* @cpp false @ce.
* @see @ref features(), @ref convertToFile(Containers::ArrayView<const CompressedImageView3D>, Containers::StringView),
* @ref convert(), @ref convertToData()
*/
@ -1508,11 +1514,11 @@ class MAGNUM_TRADE_EXPORT AbstractImageConverter: public PluginManager::Abstract
* Available only if @ref ImageConverterFeature::ConvertCompressedLevels1DToFile
* is supported. The function expects at least one image to be passed,
* with each view expected to not be @cpp nullptr @ce, to have a
* non-zero size, and all views sharing the same pixel format. Note
* that certain converters may impose additional size and order
* restrictions on the images, see documentation of a particular plugin
* for more information. On failure prints a message to
* @relativeref{Magnum,Error} and returns @cpp false @ce.
* non-zero size, and all views sharing the same pixel format and
* layout flags. Note that certain converters may impose additional
* size and order restrictions on the images, see documentation of a
* particular plugin for more information. On failure prints a message
* to @relativeref{Magnum,Error} and returns @cpp false @ce.
* @see @ref features(), @ref convertToFile(Containers::ArrayView<const ImageView1D>, Containers::StringView),
* @ref convert(), @ref convertToData()
*/
@ -1531,10 +1537,11 @@ class MAGNUM_TRADE_EXPORT AbstractImageConverter: public PluginManager::Abstract
* is supported. The function expects at least one image to be passed,
* with each view expected to not be @cpp nullptr @ce, to have a
* non-zero size in all dimensions, and all views sharing the same
* pixel format. Note that certain converters may impose additional
* size and order restrictions on the images, see documentation of a
* particular plugin for more information. On failure prints a message
* to @relativeref{Magnum,Error} and returns @cpp false @ce.
* pixel format and layout flags. Note that certain converters may
* impose additional size and order restrictions on the images, see
* documentation of a particular plugin for more information. On
* failure prints a message to @relativeref{Magnum,Error} and returns
* @cpp false @ce.
* @see @ref features(), @ref convertToFile(Containers::ArrayView<const ImageView2D>, Containers::StringView),
* @ref convert(), @ref convertToData()
*/
@ -1553,10 +1560,11 @@ class MAGNUM_TRADE_EXPORT AbstractImageConverter: public PluginManager::Abstract
* is supported. The function expects at least one image to be passed,
* with each view expected to not be @cpp nullptr @ce, to have a
* non-zero size in all dimensions, and all views sharing the same
* pixel format. Note that certain converters may impose additional
* size and order restrictions on the images, see documentation of a
* particular plugin for more information. On failure prints a message
* to @relativeref{Magnum,Error} and returns @cpp false @ce.
* pixel format and layout flags. Note that certain converters may
* impose additional size and order restrictions on the images, see
* documentation of a particular plugin for more information. On
* failure prints a message to @relativeref{Magnum,Error} and returns
* @cpp false @ce.
* @see @ref features(), @ref convertToFile(Containers::ArrayView<const ImageView3D>, Containers::StringView),
* @ref convert(), @ref convertToData()
*/

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

@ -142,6 +142,7 @@ struct AbstractImageConverterTest: TestSuite::Tester {
void convertLevels2DToDataNullptr();
void convertLevels2DToDataInconsistentFormat();
void convertLevels2DToDataInconsistentFormatExtra();
void convertLevels2DToDataInconsistentFlags();
void convertLevels3DToDataInvalidImage();
void convertLevels1DToDataNotImplemented();
void convertLevels2DToDataNotImplemented();
@ -165,6 +166,7 @@ struct AbstractImageConverterTest: TestSuite::Tester {
void convertCompressedLevels2DToDataZeroSize();
void convertCompressedLevels2DToDataNullptr();
void convertCompressedLevels2DToDataInconsistentFormat();
void convertCompressedLevels2DToDataInconsistentFlags();
void convertCompressedLevels3DToDataInvalidImage();
void convertCompressedLevels1DToDataNotImplemented();
void convertCompressedLevels2DToDataNotImplemented();
@ -384,6 +386,7 @@ AbstractImageConverterTest::AbstractImageConverterTest() {
&AbstractImageConverterTest::convertLevels2DToDataNullptr,
&AbstractImageConverterTest::convertLevels2DToDataInconsistentFormat,
&AbstractImageConverterTest::convertLevels2DToDataInconsistentFormatExtra,
&AbstractImageConverterTest::convertLevels2DToDataInconsistentFlags,
&AbstractImageConverterTest::convertLevels3DToDataInvalidImage,
&AbstractImageConverterTest::convertLevels1DToDataNotImplemented,
&AbstractImageConverterTest::convertLevels2DToDataNotImplemented,
@ -403,6 +406,7 @@ AbstractImageConverterTest::AbstractImageConverterTest() {
&AbstractImageConverterTest::convertCompressedLevels2DToDataZeroSize,
&AbstractImageConverterTest::convertCompressedLevels2DToDataNullptr,
&AbstractImageConverterTest::convertCompressedLevels2DToDataInconsistentFormat,
&AbstractImageConverterTest::convertCompressedLevels2DToDataInconsistentFlags,
&AbstractImageConverterTest::convertCompressedLevels3DToDataInvalidImage,
&AbstractImageConverterTest::convertCompressedLevels1DToDataNotImplemented,
&AbstractImageConverterTest::convertCompressedLevels2DToDataNotImplemented,
@ -1986,6 +1990,26 @@ void AbstractImageConverterTest::convertLevels2DToDataInconsistentFormatExtra()
CORRADE_COMPARE(out.str(), "Trade::AbstractImageConverter::convertToData(): levels don't have the same extra format field, expected 1037 but got 4467 for image 2\n");
}
void AbstractImageConverterTest::convertLevels2DToDataInconsistentFlags() {
#ifdef CORRADE_NO_ASSERT
CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");
#endif
struct: AbstractImageConverter {
ImageConverterFeatures doFeatures() const override { return ImageConverterFeature::ConvertLevels2DToData; }
} converter;
const char data[16]{};
std::ostringstream out;
Error redirectError{&out};
converter.convertToData({
ImageView2D{PixelFormat::RGBA8Unorm, {2, 2}, data, ImageFlag2D::Array},
ImageView2D{PixelFormat::RGBA8Unorm, {1, 1}, data, ImageFlag2D::Array},
ImageView2D{PixelFormat::RGBA8Unorm, {4, 1}, data}
});
CORRADE_COMPARE(out.str(), "Trade::AbstractImageConverter::convertToData(): levels don't have the same flags, expected ImageFlag2D::Array but got ImageFlags2D{} for image 2\n");
}
void AbstractImageConverterTest::convertLevels3DToDataInvalidImage() {
#ifdef CORRADE_NO_ASSERT
CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");
@ -2308,6 +2332,26 @@ void AbstractImageConverterTest::convertCompressedLevels2DToDataInconsistentForm
CORRADE_COMPARE(out.str(), "Trade::AbstractImageConverter::convertToData(): levels don't have the same format, expected CompressedPixelFormat::Bc1RGBAUnorm but got CompressedPixelFormat::Bc1RGBASrgb for image 2\n");
}
void AbstractImageConverterTest::convertCompressedLevels2DToDataInconsistentFlags() {
#ifdef CORRADE_NO_ASSERT
CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");
#endif
struct: AbstractImageConverter {
ImageConverterFeatures doFeatures() const override { return ImageConverterFeature::ConvertCompressedLevels2DToData; }
} converter;
const char data[16]{};
std::ostringstream out;
Error redirectError{&out};
converter.convertToData({
CompressedImageView2D{CompressedPixelFormat::Bc1RGBAUnorm, {8, 4}, data, ImageFlag2D::Array},
CompressedImageView2D{CompressedPixelFormat::Bc1RGBAUnorm, {4, 4}, data, ImageFlag2D::Array},
CompressedImageView2D{CompressedPixelFormat::Bc1RGBAUnorm, {4, 4}, data},
});
CORRADE_COMPARE(out.str(), "Trade::AbstractImageConverter::convertToData(): levels don't have the same flags, expected ImageFlag2D::Array but got ImageFlags2D{} for image 2\n");
}
void AbstractImageConverterTest::convertCompressedLevels3DToDataInvalidImage() {
#ifdef CORRADE_NO_ASSERT
CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");

Loading…
Cancel
Save