From 0bdb70fcf6a53c33fa51f5095e37cd2adaacf272 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Thu, 23 Apr 2020 12:15:29 +0200 Subject: [PATCH] Trade: add flags to AbstractImporter and AbstractImageConverter. --- doc/changelog.dox | 3 + src/Magnum/Trade/AbstractImageConverter.cpp | 26 +++++++ src/Magnum/Trade/AbstractImageConverter.h | 72 +++++++++++++++++ src/Magnum/Trade/AbstractImporter.cpp | 28 +++++++ src/Magnum/Trade/AbstractImporter.h | 73 ++++++++++++++++++ .../Trade/Test/AbstractImageConverterTest.cpp | 54 ++++++++++++- .../Trade/Test/AbstractImporterTest.cpp | 77 ++++++++++++++++++- 7 files changed, 331 insertions(+), 2 deletions(-) diff --git a/doc/changelog.dox b/doc/changelog.dox index 8ed59f4c8..46567f3ed 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -276,6 +276,9 @@ See also: - @ref Trade::PhongMaterialData now supports both color and texture instead of just one or the other, can reference normal textures as well and specfify texture coordinate transform +- Added @ref Trade::AbstractImporter::setFlags() and + @ref Trade::AbstractImageConverter::setFlags() for configuring common + plugin behavior such as output verbosity level - @ref magnum-imageconverter "magnum-imageconverter" has a new `--info` option for printing detailed info about a particular file - RLE compression support in @ref Trade::TgaImporter "TgaImporter" diff --git a/src/Magnum/Trade/AbstractImageConverter.cpp b/src/Magnum/Trade/AbstractImageConverter.cpp index 739759d74..74948441c 100644 --- a/src/Magnum/Trade/AbstractImageConverter.cpp +++ b/src/Magnum/Trade/AbstractImageConverter.cpp @@ -74,6 +74,13 @@ AbstractImageConverter::AbstractImageConverter(PluginManager::Manager{manager, plugin} {} +void AbstractImageConverter::setFlags(ImageConverterFlags flags) { + _flags = flags; + doSetFlags(flags); +} + +void AbstractImageConverter::doSetFlags(ImageConverterFlags) {} + Containers::Optional AbstractImageConverter::exportToImage(const ImageView2D& image) { CORRADE_ASSERT(features() & ImageConverterFeature::ConvertImage, "Trade::AbstractImageConverter::exportToImage(): feature not supported", {}); @@ -208,4 +215,23 @@ Debug& operator<<(Debug& debug, const ImageConverterFeatures value) { ImageConverterFeature::ConvertCompressedFile}); } +Debug& operator<<(Debug& debug, const ImageConverterFlag value) { + debug << "Trade::ImageConverterFlag" << Debug::nospace; + + switch(value) { + /* LCOV_EXCL_START */ + #define _c(v) case ImageConverterFlag::v: return debug << "::" #v; + _c(Verbose) + #undef _c + /* LCOV_EXCL_STOP */ + } + + return debug << "(" << Debug::nospace << reinterpret_cast(UnsignedByte(value)) << Debug::nospace << ")"; +} + +Debug& operator<<(Debug& debug, const ImageConverterFlags value) { + return Containers::enumSetDebugOutput(debug, value, "Trade::ImageConverterFlags{}", { + ImageConverterFlag::Verbose}); +} + }} diff --git a/src/Magnum/Trade/AbstractImageConverter.h b/src/Magnum/Trade/AbstractImageConverter.h index 4ad3f6534..66fb2973a 100644 --- a/src/Magnum/Trade/AbstractImageConverter.h +++ b/src/Magnum/Trade/AbstractImageConverter.h @@ -99,6 +99,45 @@ MAGNUM_TRADE_EXPORT Debug& operator<<(Debug& debug, ImageConverterFeature value) /** @debugoperatorenum{ImageConverterFeatures} */ MAGNUM_TRADE_EXPORT Debug& operator<<(Debug& debug, ImageConverterFeatures value); +/** +@brief Image converter flag +@m_since_latest + +@see @ref ImageConverterFlags, @ref AbstractImageConverter::setFlags() +*/ +enum class ImageConverterFlag: UnsignedByte { + /** + * Print verbose diagnostic during import. By default the importer only + * prints messages on error or when some operation might cause unexpected + * data modification or loss. + */ + Verbose = 1 << 0 + + /** @todo Y flip */ +}; + +/** +@brief Image converter flags +@m_since_latest + +@see @ref AbstractImporter::setFlags() +*/ +typedef Containers::EnumSet ImageConverterFlags; + +CORRADE_ENUMSET_OPERATORS(ImageConverterFlags) + +/** +@debugoperatorenum{ImageConverterFlag} +@m_since_latest +*/ +MAGNUM_TRADE_EXPORT Debug& operator<<(Debug& debug, ImageConverterFlag value); + +/** +@debugoperatorenum{ImageConverterFlags} +@m_since_latest +*/ +MAGNUM_TRADE_EXPORT Debug& operator<<(Debug& debug, ImageConverterFlags value); + /** @brief Base for image converter plugins @@ -202,6 +241,22 @@ class MAGNUM_TRADE_EXPORT AbstractImageConverter: public PluginManager::Abstract /** @brief Features supported by this converter */ ImageConverterFeatures features() const { return doFeatures(); } + /** + * @brief Converter flags + * @m_since_latest + */ + ImageConverterFlags flags() const { return _flags; } + + /** + * @brief Set converter flags + * @m_since_latest + * + * Some flags can be set only if the converter supports particular + * features, see documentation of each @ref ImageConverterFlag for more + * information. By default no flags are set. + */ + void setFlags(ImageConverterFlags flags); + /** * @brief Convert image to different format * @@ -288,6 +343,21 @@ class MAGNUM_TRADE_EXPORT AbstractImageConverter: public PluginManager::Abstract /** @brief Implementation of @ref features() */ virtual ImageConverterFeatures doFeatures() const = 0; + /** + * @brief Implementation for @ref setFlags() + * + * Useful when the converter needs to modify some internal state on + * flag setup. Default implementation does nothing and this + * function doesn't need to be implemented --- the flags are available + * through @ref flags(). + * + * To reduce the amount of error checking on user side, this function + * isn't expected to fail --- if a flag combination is invalid / + * unsuported, error reporting should be delayed to various conversion + * functions, where the user is expected to do error handling anyway. + */ + virtual void doSetFlags(ImageConverterFlags flags); + /** @brief Implementation of @ref exportToImage() */ virtual Containers::Optional doExportToImage(const ImageView2D& image); @@ -317,6 +387,8 @@ class MAGNUM_TRADE_EXPORT AbstractImageConverter: public PluginManager::Abstract * and saves the result to given file. */ virtual bool doExportToFile(const CompressedImageView2D& image, const std::string& filename); + + ImageConverterFlags _flags; }; }} diff --git a/src/Magnum/Trade/AbstractImporter.cpp b/src/Magnum/Trade/AbstractImporter.cpp index d7d853ef9..cec16c185 100644 --- a/src/Magnum/Trade/AbstractImporter.cpp +++ b/src/Magnum/Trade/AbstractImporter.cpp @@ -90,6 +90,15 @@ AbstractImporter::AbstractImporter(PluginManager::Manager& man AbstractImporter::AbstractImporter(PluginManager::AbstractManager& manager, const std::string& plugin): PluginManager::AbstractManagingPlugin{manager, plugin} {} +void AbstractImporter::setFlags(ImporterFlags flags) { + CORRADE_ASSERT(!isOpened(), + "Trade::AbstractImporter::setFlags(): can't be set while a file is opened", ); + _flags = flags; + doSetFlags(flags); +} + +void AbstractImporter::doSetFlags(ImporterFlags) {} + void AbstractImporter::setFileCallback(Containers::Optional>(*callback)(const std::string&, InputFileCallbackPolicy, void*), void* const userData) { CORRADE_ASSERT(!isOpened(), "Trade::AbstractImporter::setFileCallback(): can't be set while a file is opened", ); CORRADE_ASSERT(features() & (ImporterFeature::FileCallback|ImporterFeature::OpenData), "Trade::AbstractImporter::setFileCallback(): importer supports neither loading from data nor via callbacks, callbacks can't be used", ); @@ -955,4 +964,23 @@ Debug& operator<<(Debug& debug, const ImporterFeatures value) { ImporterFeature::FileCallback}); } +Debug& operator<<(Debug& debug, const ImporterFlag value) { + debug << "Trade::ImporterFlag" << Debug::nospace; + + switch(value) { + /* LCOV_EXCL_START */ + #define _c(v) case ImporterFlag::v: return debug << "::" #v; + _c(Verbose) + #undef _c + /* LCOV_EXCL_STOP */ + } + + return debug << "(" << Debug::nospace << reinterpret_cast(UnsignedByte(value)) << Debug::nospace << ")"; +} + +Debug& operator<<(Debug& debug, const ImporterFlags value) { + return Containers::enumSetDebugOutput(debug, value, "Trade::ImporterFlags{}", { + ImporterFlag::Verbose}); +} + }} diff --git a/src/Magnum/Trade/AbstractImporter.h b/src/Magnum/Trade/AbstractImporter.h index 35105d1f5..be6326286 100644 --- a/src/Magnum/Trade/AbstractImporter.h +++ b/src/Magnum/Trade/AbstractImporter.h @@ -92,6 +92,45 @@ MAGNUM_TRADE_EXPORT Debug& operator<<(Debug& debug, ImporterFeatures value); typedef CORRADE_DEPRECATED("use InputFileCallbackPolicy instead") InputFileCallbackPolicy ImporterFileCallbackPolicy; #endif +/** +@brief Importer flag +@m_since_latest + +@see @ref ImporterFlags, @ref AbstractImporter::setFlags() +*/ +enum class ImporterFlag: UnsignedByte { + /** + * Print verbose diagnostic during import. By default the importer only + * prints messages on error or when some operation might cause unexpected + * data modification or loss. + */ + Verbose = 1 << 0, + + /** @todo Y flip for images, "I want to import just once, don't copy" ... */ +}; + +/** +@brief Importer flags +@m_since_latest + +@see @ref AbstractImporter::setFlags() +*/ +typedef Containers::EnumSet ImporterFlags; + +CORRADE_ENUMSET_OPERATORS(ImporterFlags) + +/** +@debugoperatorenum{ImporterFlag} +@m_since_latest +*/ +MAGNUM_TRADE_EXPORT Debug& operator<<(Debug& debug, ImporterFlag value); + +/** +@debugoperatorenum{ImporterFlags} +@m_since_latest +*/ +MAGNUM_TRADE_EXPORT Debug& operator<<(Debug& debug, ImporterFlags value); + /** @brief Base for importer plugins @@ -322,6 +361,23 @@ class MAGNUM_TRADE_EXPORT AbstractImporter: public PluginManager::AbstractManagi /** @brief Features supported by this importer */ ImporterFeatures features() const { return doFeatures(); } + /** + * @brief Importer flags + * @m_since_latest + */ + ImporterFlags flags() const { return _flags; } + + /** + * @brief Set importer flags + * @m_since_latest + * + * It's expected that this function is called *before* a file is + * opened. Some flags can be set only if the importer supports + * particular features, see documentation of each @ref ImporterFlag for + * more information. By default no flags are set. + */ + void setFlags(ImporterFlags flags); + /** * @brief File opening callback function * @@ -1255,6 +1311,21 @@ class MAGNUM_TRADE_EXPORT AbstractImporter: public PluginManager::AbstractManagi /** @brief Implementation for @ref features() */ virtual ImporterFeatures doFeatures() const = 0; + /** + * @brief Implementation for @ref setFlags() + * + * Useful when the importer needs to modify some internal state on + * flag setup. Default implementation does nothing and this + * function doesn't need to be implemented --- the flags are available + * through @ref flags(). + * + * To reduce the amount of error checking on user side, this function + * isn't expected to fail --- if a flag combination is invalid / + * unsuported, error reporting should be delayed to @ref openFile() and + * others, where the user is expected to do error handling anyway. + */ + virtual void doSetFlags(ImporterFlags flags); + /** * @brief Implementation for @ref setFileCallback() * @@ -1761,6 +1832,8 @@ class MAGNUM_TRADE_EXPORT AbstractImporter: public PluginManager::AbstractManagi /** @brief Implementation for @ref importerState() */ virtual const void* doImporterState() const; + ImporterFlags _flags; + Containers::Optional>(*_fileCallback)(const std::string&, InputFileCallbackPolicy, void*){}; void* _fileCallbackUserData{}; diff --git a/src/Magnum/Trade/Test/AbstractImageConverterTest.cpp b/src/Magnum/Trade/Test/AbstractImageConverterTest.cpp index ffa3a44af..25d3181c7 100644 --- a/src/Magnum/Trade/Test/AbstractImageConverterTest.cpp +++ b/src/Magnum/Trade/Test/AbstractImageConverterTest.cpp @@ -48,6 +48,9 @@ struct AbstractImageConverterTest: TestSuite::Tester { void construct(); void constructWithPluginManagerReference(); + void setFlags(); + void setFlagsNotImplemented(); + void thingNotSupported(); void exportToImage(); @@ -82,12 +85,17 @@ struct AbstractImageConverterTest: TestSuite::Tester { void debugFeature(); void debugFeatures(); + void debugFlag(); + void debugFlags(); }; AbstractImageConverterTest::AbstractImageConverterTest() { addTests({&AbstractImageConverterTest::construct, &AbstractImageConverterTest::constructWithPluginManagerReference, + &AbstractImageConverterTest::setFlags, + &AbstractImageConverterTest::setFlagsNotImplemented, + &AbstractImageConverterTest::thingNotSupported, &AbstractImageConverterTest::exportToImage, @@ -121,7 +129,9 @@ AbstractImageConverterTest::AbstractImageConverterTest() { &AbstractImageConverterTest::exportImageDataToFile, &AbstractImageConverterTest::debugFeature, - &AbstractImageConverterTest::debugFeatures}); + &AbstractImageConverterTest::debugFeatures, + &AbstractImageConverterTest::debugFlag, + &AbstractImageConverterTest::debugFlags}); /* Create testing dir */ Utility::Directory::mkpath(TRADE_TEST_OUTPUT_DIR); @@ -147,6 +157,34 @@ void AbstractImageConverterTest::constructWithPluginManagerReference() { CORRADE_COMPARE(converter.features(), ImageConverterFeatures{}); } +void AbstractImageConverterTest::setFlags() { + struct: AbstractImageConverter { + ImageConverterFeatures doFeatures() const override { return {}; } + void doSetFlags(ImageConverterFlags flags) override { + _flags = flags; + } + + ImageConverterFlags _flags; + } converter; + + CORRADE_COMPARE(converter.flags(), ImageConverterFlags{}); + CORRADE_COMPARE(converter._flags, ImageConverterFlags{}); + converter.setFlags(ImageConverterFlag::Verbose); + CORRADE_COMPARE(converter.flags(), ImageConverterFlag::Verbose); + CORRADE_COMPARE(converter._flags, ImageConverterFlag::Verbose); +} + +void AbstractImageConverterTest::setFlagsNotImplemented() { + struct: AbstractImageConverter { + ImageConverterFeatures doFeatures() const override { return {}; } + } converter; + + CORRADE_COMPARE(converter.flags(), ImageConverterFlags{}); + converter.setFlags(ImageConverterFlag::Verbose); + CORRADE_COMPARE(converter.flags(), ImageConverterFlag::Verbose); + /* Should just work, no need to implement the function */ +} + void AbstractImageConverterTest::thingNotSupported() { #ifdef CORRADE_NO_ASSERT CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions"); @@ -554,6 +592,20 @@ void AbstractImageConverterTest::debugFeatures() { CORRADE_COMPARE(out.str(), "Trade::ImageConverterFeature::ConvertData|Trade::ImageConverterFeature::ConvertCompressedFile Trade::ImageConverterFeatures{}\n"); } +void AbstractImageConverterTest::debugFlag() { + std::ostringstream out; + + Debug{&out} << ImageConverterFlag::Verbose << ImageConverterFlag(0xf0); + CORRADE_COMPARE(out.str(), "Trade::ImageConverterFlag::Verbose Trade::ImageConverterFlag(0xf0)\n"); +} + +void AbstractImageConverterTest::debugFlags() { + std::ostringstream out; + + Debug{&out} << (ImageConverterFlag::Verbose|ImageConverterFlag(0xf0)) << ImageConverterFlags{}; + CORRADE_COMPARE(out.str(), "Trade::ImageConverterFlag::Verbose|Trade::ImageConverterFlag(0xf0) Trade::ImageConverterFlags{}\n"); +} + }}}} CORRADE_TEST_MAIN(Magnum::Trade::Test::AbstractImageConverterTest) diff --git a/src/Magnum/Trade/Test/AbstractImporterTest.cpp b/src/Magnum/Trade/Test/AbstractImporterTest.cpp index b898c215b..b63ae4855 100644 --- a/src/Magnum/Trade/Test/AbstractImporterTest.cpp +++ b/src/Magnum/Trade/Test/AbstractImporterTest.cpp @@ -62,6 +62,10 @@ struct AbstractImporterTest: TestSuite::Tester { void construct(); void constructWithPluginManagerReference(); + void setFlags(); + void setFlagsFileOpened(); + void setFlagsNotImplemented(); + void openData(); void openFileAsData(); void openFileAsDataNotFound(); @@ -241,6 +245,8 @@ struct AbstractImporterTest: TestSuite::Tester { void debugFeature(); void debugFeatures(); + void debugFlag(); + void debugFlags(); }; constexpr struct { @@ -255,6 +261,10 @@ AbstractImporterTest::AbstractImporterTest() { addTests({&AbstractImporterTest::construct, &AbstractImporterTest::constructWithPluginManagerReference, + &AbstractImporterTest::setFlags, + &AbstractImporterTest::setFlagsFileOpened, + &AbstractImporterTest::setFlagsNotImplemented, + &AbstractImporterTest::openData, &AbstractImporterTest::openFileAsData, &AbstractImporterTest::openFileAsDataNotFound, @@ -436,7 +446,9 @@ AbstractImporterTest::AbstractImporterTest() { &AbstractImporterTest::importerStateNoFile, &AbstractImporterTest::debugFeature, - &AbstractImporterTest::debugFeatures}); + &AbstractImporterTest::debugFeatures, + &AbstractImporterTest::debugFlag, + &AbstractImporterTest::debugFlags}); } void AbstractImporterTest::construct() { @@ -467,6 +479,55 @@ void AbstractImporterTest::constructWithPluginManagerReference() { CORRADE_VERIFY(!importer.isOpened()); } +void AbstractImporterTest::setFlags() { + struct: AbstractImporter { + ImporterFeatures doFeatures() const override { return {}; } + void doSetFlags(ImporterFlags flags) override { + _flags = flags; + } + bool doIsOpened() const override { return false; } + void doClose() override {} + + ImporterFlags _flags; + } importer; + + CORRADE_COMPARE(importer.flags(), ImporterFlags{}); + CORRADE_COMPARE(importer._flags, ImporterFlags{}); + importer.setFlags(ImporterFlag::Verbose); + CORRADE_COMPARE(importer.flags(), ImporterFlag::Verbose); + CORRADE_COMPARE(importer._flags, ImporterFlag::Verbose); +} + +void AbstractImporterTest::setFlagsNotImplemented() { + struct: AbstractImporter { + ImporterFeatures doFeatures() const override { return {}; } + bool doIsOpened() const override { return false; } + void doClose() override {} + } importer; + + CORRADE_COMPARE(importer.flags(), ImporterFlags{}); + importer.setFlags(ImporterFlag::Verbose); + CORRADE_COMPARE(importer.flags(), ImporterFlag::Verbose); + /* Should just work, no need to implement the function */ +} + +void AbstractImporterTest::setFlagsFileOpened() { + #ifdef CORRADE_NO_ASSERT + CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions"); + #endif + + struct: AbstractImporter { + ImporterFeatures doFeatures() const override { return {}; } + bool doIsOpened() const override { return true; } + void doClose() override {} + } importer; + + std::ostringstream out; + Error redirectError{&out}; + importer.setFlags(ImporterFlag::Verbose); + CORRADE_COMPARE(out.str(), "Trade::AbstractImporter::setFlags(): can't be set while a file is opened\n"); +} + void AbstractImporterTest::openData() { struct: AbstractImporter { ImporterFeatures doFeatures() const override { return ImporterFeature::OpenData; } @@ -3965,6 +4026,20 @@ void AbstractImporterTest::debugFeatures() { CORRADE_COMPARE(out.str(), "Trade::ImporterFeature::OpenData|Trade::ImporterFeature::OpenState Trade::ImporterFeatures{}\n"); } +void AbstractImporterTest::debugFlag() { + std::ostringstream out; + + Debug{&out} << ImporterFlag::Verbose << ImporterFlag(0xf0); + CORRADE_COMPARE(out.str(), "Trade::ImporterFlag::Verbose Trade::ImporterFlag(0xf0)\n"); +} + +void AbstractImporterTest::debugFlags() { + std::ostringstream out; + + Debug{&out} << (ImporterFlag::Verbose|ImporterFlag(0xf0)) << ImporterFlags{}; + CORRADE_COMPARE(out.str(), "Trade::ImporterFlag::Verbose|Trade::ImporterFlag(0xf0) Trade::ImporterFlags{}\n"); +} + }}}} CORRADE_TEST_MAIN(Magnum::Trade::Test::AbstractImporterTest)