diff --git a/doc/snippets/MagnumTrade.cpp b/doc/snippets/MagnumTrade.cpp index 06082a698..10f4e2fd5 100644 --- a/doc/snippets/MagnumTrade.cpp +++ b/doc/snippets/MagnumTrade.cpp @@ -190,6 +190,24 @@ importer->openFile("scene.gltf"); // memory-maps all files } #endif +#if defined(CORRADE_TARGET_UNIX) || (defined(CORRADE_TARGET_WINDOWS) && !defined(CORRADE_TARGET_WINDOWS_RT)) +{ +Containers::Pointer importer; +/* [AbstractImporter-usage-zerocopy] */ +importer->addFlags(Trade::ImporterFlag::ZeroCopy); + +Containers::Array memory; +if(!(memory = Utility::Directory::mapRead("huge-file.gltf")) || + !importer->openMemory(memory)) + Fatal{} << "Can't memory-map and open the file"; + +/* Depending on the importer, the actual vertex/index data will get paged from + the above file into the physical memory only once you actually access them */ +Containers::Optional mesh = importer->mesh("huge-cathedral"); +/* [AbstractImporter-usage-zerocopy] */ +} +#endif + { Containers::Pointer importer; /* [AbstractImporter-setFileCallback] */ diff --git a/src/Magnum/Trade/AbstractImporter.cpp b/src/Magnum/Trade/AbstractImporter.cpp index 65e669390..f2a4cf134 100644 --- a/src/Magnum/Trade/AbstractImporter.cpp +++ b/src/Magnum/Trade/AbstractImporter.cpp @@ -112,6 +112,30 @@ AbstractImporter::~AbstractImporter() = default; void AbstractImporter::setFlags(ImporterFlags flags) { CORRADE_ASSERT(!isOpened(), "Trade::AbstractImporter::setFlags(): can't be set while a file is opened", ); + #ifndef CORRADE_NO_ASSERT + /* Separating the common string prefix in a hope that the compiler + deduplicates the literals to reduce binary size. There's probably also a + fancy loop-ish way to do this if we'd have the same binary value for the + features and flags but it's Sunday evening and my brain capacity is + limited. */ + const ImporterFeatures features = this->features(); + CORRADE_ASSERT(!(flags >= ImporterFlag::ForceZeroCopyAnimations) || (features & ImporterFeature::ZeroCopyAnimations), + "Trade::AbstractImporter::setFlags(): importer doesn't support zero-copy" << "animations", ); + CORRADE_ASSERT(!(flags >= ImporterFlag::ForceZeroCopyImages) || (features & ImporterFeature::ZeroCopyImages), + "Trade::AbstractImporter::setFlags(): importer doesn't support zero-copy" << "images", ); + CORRADE_ASSERT(!(flags >= ImporterFlag::ForceZeroCopyMaterialAttributes) || (features & ImporterFeature::ZeroCopyMaterialAttributes), + "Trade::AbstractImporter::setFlags(): importer doesn't support zero-copy" << "material attributes", ); + CORRADE_ASSERT(!(flags >= ImporterFlag::ForceZeroCopyMaterialLayers) || (features & ImporterFeature::ZeroCopyMaterialLayers), + "Trade::AbstractImporter::setFlags(): importer doesn't support zero-copy" << "material layers", ); + CORRADE_ASSERT(!(flags >= ImporterFlag::ForceZeroCopyMeshIndices) || (features & ImporterFeature::ZeroCopyMeshIndices), + "Trade::AbstractImporter::setFlags(): importer doesn't support zero-copy" << "mesh indices", ); + CORRADE_ASSERT(!(flags >= ImporterFlag::ForceZeroCopyMeshVertices) || (features & ImporterFeature::ZeroCopyMeshVertices), + "Trade::AbstractImporter::setFlags(): importer doesn't support zero-copy" << "mesh vertices", ); + CORRADE_ASSERT(!(flags >= ImporterFlag::ForceZeroCopySkinJoints) || (features & ImporterFeature::ZeroCopySkinJoints), + "Trade::AbstractImporter::setFlags(): importer doesn't support zero-copy" << "skin joints", ); + CORRADE_ASSERT(!(flags >= ImporterFlag::ForceZeroCopySkinInverseBindMatrices) || (features & ImporterFeature::ZeroCopySkinInverseBindMatrices), + "Trade::AbstractImporter::setFlags(): importer doesn't support zero-copy" << "skin inverse bind matrices", ); + #endif _flags = flags; doSetFlags(flags); } @@ -137,7 +161,7 @@ void AbstractImporter::setFileCallback(Containers::Optional>(*)(const std::string&, InputFileCallbackPolicy, void*), void*) {} -bool AbstractImporter::openData(Containers::ArrayView data) { +bool AbstractImporter::openData(Containers::Array&& data, const DataFlags dataFlags) { CORRADE_ASSERT(features() & ImporterFeature::OpenData, "Trade::AbstractImporter::openData(): feature not supported", {}); @@ -145,10 +169,14 @@ bool AbstractImporter::openData(Containers::ArrayView data) { the check doesn't be done on the plugin side) because for some file formats it could be valid (e.g. OBJ or JSON-based formats). */ close(); - doOpenData(Containers::Array{const_cast(static_cast(data.data())), data.size(), Implementation::nonOwnedArrayDeleter}, {}); + doOpenData(std::move(data), dataFlags); return isOpened(); } +bool AbstractImporter::openData(Containers::ArrayView data) { + return openData(Containers::Array{const_cast(static_cast(data.data())), data.size(), Implementation::nonOwnedArrayDeleter}, {}); +} + #ifdef MAGNUM_BUILD_DEPRECATED void AbstractImporter::doOpenData(Containers::ArrayView) { CORRADE_ASSERT_UNREACHABLE("Trade::AbstractImporter::openData(): feature advertised but not implemented", ); @@ -1542,11 +1570,19 @@ Debug& operator<<(Debug& debug, const ImporterFeature value) { _c(OpenData) _c(OpenState) _c(FileCallback) + _c(ZeroCopyAnimations) + _c(ZeroCopyImages) + _c(ZeroCopyMaterialAttributes) + _c(ZeroCopyMaterialLayers) + _c(ZeroCopyMeshIndices) + _c(ZeroCopyMeshVertices) + _c(ZeroCopySkinJoints) + _c(ZeroCopySkinInverseBindMatrices) #undef _c /* LCOV_EXCL_STOP */ } - return debug << "(" << Debug::nospace << reinterpret_cast(UnsignedByte(value)) << Debug::nospace << ")"; + return debug << "(" << Debug::nospace << reinterpret_cast(UnsignedShort(value)) << Debug::nospace << ")"; } Debug& operator<<(Debug& debug, const ImporterFeatures value) { @@ -1563,16 +1599,34 @@ Debug& operator<<(Debug& debug, const ImporterFlag value) { /* LCOV_EXCL_START */ #define _c(v) case ImporterFlag::v: return debug << "::" #v; _c(Verbose) + _c(ZeroCopy) + _c(ForceZeroCopyAnimations) + _c(ForceZeroCopyImages) + _c(ForceZeroCopyMaterialAttributes) + _c(ForceZeroCopyMaterialLayers) + _c(ForceZeroCopyMeshIndices) + _c(ForceZeroCopyMeshVertices) + _c(ForceZeroCopySkinJoints) + _c(ForceZeroCopySkinInverseBindMatrices) #undef _c /* LCOV_EXCL_STOP */ } - return debug << "(" << Debug::nospace << reinterpret_cast(UnsignedByte(value)) << Debug::nospace << ")"; + return debug << "(" << Debug::nospace << reinterpret_cast(UnsignedShort(value)) << Debug::nospace << ")"; } Debug& operator<<(Debug& debug, const ImporterFlags value) { return Containers::enumSetDebugOutput(debug, value, "Trade::ImporterFlags{}", { - ImporterFlag::Verbose}); + ImporterFlag::Verbose, + ImporterFlag::ZeroCopy, + ImporterFlag::ForceZeroCopyAnimations, + ImporterFlag::ForceZeroCopyImages, + ImporterFlag::ForceZeroCopyMaterialAttributes, + ImporterFlag::ForceZeroCopyMaterialLayers, + ImporterFlag::ForceZeroCopyMeshIndices, + ImporterFlag::ForceZeroCopyMeshVertices, + ImporterFlag::ForceZeroCopySkinJoints, + ImporterFlag::ForceZeroCopySkinInverseBindMatrices}); } }} diff --git a/src/Magnum/Trade/AbstractImporter.h b/src/Magnum/Trade/AbstractImporter.h index f84002c0c..f8369d575 100644 --- a/src/Magnum/Trade/AbstractImporter.h +++ b/src/Magnum/Trade/AbstractImporter.h @@ -48,7 +48,7 @@ namespace Magnum { namespace Trade { @see @ref ImporterFeatures, @ref AbstractImporter::features() */ -enum class ImporterFeature: UnsignedByte { +enum class ImporterFeature: UnsignedShort { /** * Opening files from raw data or non-temporary memory using * @ref AbstractImporter::openData() or @@ -68,7 +68,121 @@ enum class ImporterFeature: UnsignedByte { * See @ref Trade-AbstractImporter-usage-callbacks and particular importer * documentation for more information. */ - FileCallback = 1 << 2 + FileCallback = 1 << 2, + + /** + * Importing animations without data copies. If the + * @ref AbstractImporter::openMemory() function is used, + * @ref ImporterFlag::ZeroCopy is set and the animation data doesn't need + * to be processed in any way during the import, + * @ref AbstractImporter::animation() will then return an + * @ref AnimationData that's a view on the memory passed to + * @relativeref{AbstractImporter,openMemory()}, indicated with + * @ref DataFlag::ExternallyOwned in @ref AnimationData::dataFlags(). + * @see @ref ImporterFlag::ForceZeroCopyAnimations + * @m_since_latest + */ + ZeroCopyAnimations = 1 << 3, + + /** + * Importing images without data copies. If the + * @ref AbstractImporter::openMemory() function is used, + * @ref ImporterFlag::ZeroCopy is set and the image data doesn't need to be + * processed in any way during the import, + * @ref AbstractImporter::image1D() / @relativeref{AbstractImporter,image2D()} + * / @relativeref{AbstractImporter,image3D()} will then return an + * @ref ImageData that's a view on the memory passed to + * @relativeref{AbstractImporter,openMemory()}, indicated with + * @ref DataFlag::ExternallyOwned in @ref ImageData::dataFlags(). + * @see @ref ImporterFlag::ForceZeroCopyImages + * @m_since_latest + */ + ZeroCopyImages = 1 << 4, + + /** + * Importing material attributes without data copies. If the + * @ref AbstractImporter::openMemory() function is used, + * @ref ImporterFlag::ZeroCopy is set and the material attribute data + * doesn't need to be processed in any way during the import, + * @ref AbstractImporter::material() will then return a + * @ref MaterialData with attributes being a view on the memory passed to + * @relativeref{AbstractImporter,openMemory()}, indicated with + * @ref DataFlag::ExternallyOwned in @ref MaterialData::attributeDataFlags(). + * @see @ref ImporterFlag::ForceZeroCopyMaterialAttributes + * @m_since_latest + */ + ZeroCopyMaterialAttributes = 1 << 5, + + /** + * Importing material layers without data copies. If the + * @ref AbstractImporter::openMemory() function is used, + * @ref ImporterFlag::ZeroCopy is set and the material layer data doesn't + * need to be processed in any way during the import, + * @ref AbstractImporter::material() will then return a + * @ref MaterialData with layers being a view on the memory passed to + * @relativeref{AbstractImporter,openMemory()}, indicated with + * @ref DataFlag::ExternallyOwned in @ref MaterialData::layerDataFlags(). + * @see @ref ImporterFlag::ForceZeroCopyMaterialLayers + * @m_since_latest + */ + ZeroCopyMaterialLayers = 1 << 6, + + /** + * Importing mesh indices without data copies. If the + * @ref AbstractImporter::openMemory() function is used, + * @ref ImporterFlag::ZeroCopy is set and the mesh index data doesn't need + * to be processed in any way during the import, + * @ref AbstractImporter::mesh() will then return a @ref MeshData with + * indices being a view on the memory passed to + * @relativeref{AbstractImporter,openMemory()}, indicated with + * @ref DataFlag::ExternallyOwned in @ref MeshData::indexDataFlags(). + * @see @ref ImporterFlag::ForceZeroCopyMeshIndices + * @m_since_latest + */ + ZeroCopyMeshIndices = 1 << 7, + + /** + * Importing mesh vertices without data copies. If the + * @ref AbstractImporter::openMemory() function is used, + * @ref ImporterFlag::ZeroCopy is set and the mesh vertex data doesn't need + * to be processed in any way during the import, + * @ref AbstractImporter::mesh() will then return a @ref MeshData with + * vertices being a view on the memory passed to + * @relativeref{AbstractImporter,openMemory()}, indicated with + * @ref DataFlag::ExternallyOwned in @ref MeshData::vertexDataFlags(). + * @see @ref ImporterFlag::ForceZeroCopyMeshVertices + * @m_since_latest + */ + ZeroCopyMeshVertices = 1 << 8, + + /** + * Importing skin joints without data copies. If the + * @ref AbstractImporter::openMemory() function is used, + * @ref ImporterFlag::ZeroCopy is set and the skin joint data doesn't need + * to be processed in any way during the import, + * @ref AbstractImporter::skin2D() / @relativeref{AbstractImporter,skin3D()} + * will then return a @ref SkinData with vertices being a view on the + * memory passed to @relativeref{AbstractImporter,openMemory()}, indicated + * with @ref DataFlag::ExternallyOwned in @ref SkinData::jointDataFlags(). + * @see @ref ImporterFlag::ForceZeroCopySkinJoints + * @m_since_latest + */ + ZeroCopySkinJoints = 1 << 9, + + /** + * Importing skin inverse bind matrices without data copies. If the + * @ref AbstractImporter::openMemory() function is used, + * @ref ImporterFlag::ZeroCopy is set and the skin joint data doesn't need + * to be processed in any way during the import, + * @ref AbstractImporter::skin2D() / @relativeref{AbstractImporter,skin3D()} + * will then return a @ref SkinData with vertices being a view on the + * memory passed to @relativeref{AbstractImporter,openMemory()}, indicated + * with @ref DataFlag::ExternallyOwned in + * @ref SkinData::inverseBindMatrixDataFlags(). + * @see @ref ImporterFlag::ForceZeroCopySkinInverseBindMatrices + * @m_since_latest + */ + ZeroCopySkinInverseBindMatrices = 1 << 10 }; /** @@ -100,7 +214,7 @@ typedef CORRADE_DEPRECATED("use InputFileCallbackPolicy instead") InputFileCallb @see @ref ImporterFlags, @ref AbstractImporter::setFlags() */ -enum class ImporterFlag: UnsignedByte { +enum class ImporterFlag: UnsignedShort { /** * Print verbose diagnostic during import. By default the importer only * prints messages on error or when some operation might cause unexpected @@ -113,6 +227,144 @@ enum class ImporterFlag: UnsignedByte { */ Verbose = 1 << 0, + /** + * Opt-in to zero-copy import, if possible. When this flag is set and + * @ref AbstractImporter::openMemory() is used, returned @ref AnimationData, + * @ref ImageData, @ref MaterialData, @ref MeshData and @ref SkinData + * instances may be views on memory passed to + * @relativeref{AbstractImporter,openMemory()} instead of allocated copies, + * indicated with @ref DataFlag::ExternallyOwned. + * + * Since it's not always possible to directly reference the input memory + * (for example because the input data may need to be parsed from text, + * gathered from an incompatible data layout or patched in some way), this + * flag doesn't put any requirement on the importer --- plugins that don't + * support zero-copy import will behave the same as if this flag was not + * set. + * + * Setting this flag however means you're responsible to keep the memory + * passed to @relativeref{AbstractImporter,openMemory()} in scope for as + * long as any `*Data` instances referencing it are alive. + * @m_since_latest + */ + ZeroCopy = 1 << 1, + + /** + * Force zero-copy import of animation data opened through + * @ref AbstractImporter::openMemory(). Implies + * @ref ImporterFlag::ZeroCopy, can be set only if the importer supports + * @ref ImporterFeature::ZeroCopyAnimations. + * + * By default, if the data has to be processed in some way, preventing + * zero-copy import, the importer will return a modified copy of the data. + * Setting this flag will cause @ref AbstractImporter::animation() to fail + * if it can't perform a zero-copy import. + * @m_since_latest + */ + ForceZeroCopyAnimations = ZeroCopy|(1 << 2), + + /** + * Force zero-copy import of image data opened through + * @ref AbstractImporter::openMemory(). Implies + * @ref ImporterFlag::ZeroCopy, can be set only if the importer supports + * @ref ImporterFeature::ZeroCopyImages. + * + * By default, if the data has to be processed in some way, preventing + * zero-copy import, the importer will return a modified copy of the data. + * Setting this flag will cause @ref AbstractImporter::image1D() / + * @relativeref{AbstractImporter,image2D()} / + * @relativeref{AbstractImporter,image3D()} to fail if it can't perform a + * zero-copy import. + * @m_since_latest + */ + ForceZeroCopyImages = ZeroCopy|(1 << 3), + + /** + * Force zero-copy import of material attribute data opened through + * @ref AbstractImporter::openMemory(). Implies + * @ref ImporterFlag::ZeroCopy, can be set only if the importer supports + * @ref ImporterFeature::ZeroCopyMaterialAttributes. + * + * By default, if the data has to be processed in some way, preventing + * zero-copy import, the importer will return a modified copy of the data. + * Setting this flag will cause @ref AbstractImporter::material() to fail + * if it can't perform a zero-copy import. + * @m_since_latest + */ + ForceZeroCopyMaterialAttributes = ZeroCopy|(1 << 4), + + /** + * Force zero-copy import of material layer data opened through + * @ref AbstractImporter::openMemory(). Implies + * @ref ImporterFlag::ZeroCopy, can be set only if the importer supports + * @ref ImporterFeature::ZeroCopyMaterialLayers. + * + * By default, if the data has to be processed in some way, preventing + * zero-copy import, the importer will return a modified copy of the data. + * Setting this flag will cause @ref AbstractImporter::material() to fail + * if it can't perform a zero-copy import. + * @m_since_latest + */ + ForceZeroCopyMaterialLayers = ZeroCopy|(1 << 5), + + /** + * Force zero-copy import of mesh index data opened through + * @ref AbstractImporter::openMemory(). Implies + * @ref ImporterFlag::ZeroCopy, can be set only if the importer supports + * @ref ImporterFeature::ZeroCopyMeshIndices. + * + * By default, if the data has to be processed in some way, preventing + * zero-copy import, the importer will return a modified copy of the data. + * Setting this flag will cause @ref AbstractImporter::mesh() to fail if it + * can't perform a zero-copy import. + * @m_since_latest + */ + ForceZeroCopyMeshIndices = ZeroCopy|(1 << 6), + + /** + * Force zero-copy import of mesh vertex data opened through + * @ref AbstractImporter::openMemory(). Implies + * @ref ImporterFlag::ZeroCopy, can be set only if the importer supports + * @ref ImporterFeature::ZeroCopyMeshVertices. + * + * By default, if the data has to be processed in some way, preventing + * zero-copy import, the importer will return a modified copy of the data. + * Setting this flag will cause @ref AbstractImporter::mesh() to fail if it + * can't perform a zero-copy import. + * @m_since_latest + */ + ForceZeroCopyMeshVertices = ZeroCopy|(1 << 7), + + /** + * Force zero-copy import of skin joint data opened through + * @ref AbstractImporter::openMemory(). Implies + * @ref ImporterFlag::ZeroCopy, can be set only if the importer supports + * @ref ImporterFeature::ZeroCopySkinJoints. + * + * By default, if the data has to be processed in some way, preventing + * zero-copy import, the importer will return a modified copy of the data. + * Setting this flag will cause @ref AbstractImporter::skin2D() / + * @relativeref{AbstractImporter,skin3D()} to fail if it can't perform a + * zero-copy import. + * @m_since_latest + */ + ForceZeroCopySkinJoints = ZeroCopy|(1 << 8), + + /** + * Force zero-copy import of skin inverse bind matrix data opened through + * @ref AbstractImporter::openMemory(). Implies + * @ref ImporterFlag::ZeroCopy, can be set only if the importer supports + * @ref ImporterFeature::ZeroCopySkinInverseBindMatrices. + * + * By default, if the data has to be processed in some way, preventing + * zero-copy import, the importer will return a modified copy of the data. + * Setting this flag will cause @ref AbstractImporter::skin2D() / + * @relativeref{AbstractImporter,skin3D()} to fail if it can't perform a + * zero-copy import. + * @m_since_latest + */ + ForceZeroCopySkinInverseBindMatrices = ZeroCopy|(1 << 9) + /** @todo ~~Y flip~~ Y up for images, "I want to import just once, don't copy" ... */ }; @@ -311,6 +563,41 @@ name doesn't exist. - Texture names using @ref textureName() & @ref textureForName(), imported with @ref texture(const std::string&) +@subsection Trade-AbstractImporter-usage-zerocopy Zero-copy data import + +Some file formats have the data structured in a way that allows them to be +loaded directly into memory or onto the GPU and used as-is. If you memory-map +such a a file and open it with a capable importer, it can give you a view on a +sub-range of the memory-mapped file instead of allocating a copy. Importers +advertise such capabilities with @ref ImporterFeature::ZeroCopyImages, +@relativeref{ImporterFeature,ZeroCopyMeshIndices}, +@relativeref{ImporterFeature,ZeroCopyMeshVertices} and related flags. Because +this puts additional constraints on data lifetime, you have to explicitly +enable the behavior with @ref ImporterFlag::ZeroCopy. Then use +@ref openMemory() to open the memory and ensure it stays in scope for as long +as you operate on the instances returned from the importer: + +@snippet MagnumTrade.cpp AbstractImporter-usage-zerocopy + +Returned instances that reference the original memory are indicated with a +presence of @ref DataFlag::ExternallyOwned. If you use the mutable +@ref openMemory(Containers::ArrayView) overload, the returned data will +have also @ref DataFlag::Mutable set, allowing you to do in-place modifications +on the original file. + +In some cases the importer might still need to process the data on import --- +for example converting image endianness or flipping image origin, and in that +case it'll still return a copy of the data, indicated with +@ref DataFlag::Owned instead. To enforce zero-copy behavior, enable one of the +@ref ImporterFlag::ForceZeroCopyImages, ... flags, providing the plugin +actually supports the corresponding feature. In that case import of particular +data will fail instead of returning a copy, which is useful when you want to, +for example, operate in-place on the imported file or when it's needed to avoid +accidental slowdowns. + +See documentation of a particular importer plugin for information about +provided zero-copy features and their limitations. + @subsection Trade-AbstractImporter-usage-state Internal importer state Some importers, especially ones that make use of well-known external libraries, @@ -644,7 +931,8 @@ class MAGNUM_TRADE_EXPORT AbstractImporter: public PluginManager::AbstractManagi * scope until the importer is destructed, @ref close() is called or * another file is opened. This allows the implementation to directly * operate on the provided memory, without having to allocate a local - * copy to extend its lifetime. + * copy to extend its lifetime. See also @ref ImporterFlag::ZeroCopy + * for a possibility of further optimizations. * @see @ref features(), @ref openFile(), @ref openState() */ bool openMemory(Containers::ArrayView memory); @@ -1697,6 +1985,34 @@ class MAGNUM_TRADE_EXPORT AbstractImporter: public PluginManager::AbstractManagi const void* importerState() const; protected: + /** + * @brief Open data that originated elsewhere + * @m_since_latest + * + * Closes previous file, if it was opened, and tries to open given raw + * data. Available only if @ref ImporterFeature::OpenData is supported. + * Returns @cpp true @ce on success, @cpp false @ce otherwise. + * + * Designed to be called instead of the public + * @ref openData(Containers::ArrayView) by importers that + * proxy loading to other plugins, with the intent of enabling + * zero-copy import in the proxied-to implementations as well. Possible + * scenarios: + * + * - Called from inside a @ref doOpenData() implementation that + * proxies loading to other plugins (for example based on file + * type). In this case it's meant to pass through the @p data and + * @p dataFlags unchanged. + * - Called from inside (for example) a @ref doImage2D() in a scene + * importer that delegates image loading to specialized plugins. + * Assuming the delegated-to importer receives a subrange of the + * data held by the originating importer and its lifetime doesn't + * exceed the originating importer lifetime, the @p data should + * have a no-op deleter and @p dataFlags should be + * @ref DataFlag::Owned. + */ + bool openData(Containers::Array&& data, DataFlags dataFlags); + /** * @brief Implementation for @ref openFile() * diff --git a/src/Magnum/Trade/Test/AbstractImporterTest.cpp b/src/Magnum/Trade/Test/AbstractImporterTest.cpp index 799885abb..71094bc71 100644 --- a/src/Magnum/Trade/Test/AbstractImporterTest.cpp +++ b/src/Magnum/Trade/Test/AbstractImporterTest.cpp @@ -31,6 +31,7 @@ #include #include #include +#include #include "Magnum/PixelFormat.h" #include "Magnum/FileCallback.h" @@ -74,6 +75,7 @@ struct AbstractImporterTest: TestSuite::Tester { void setFlags(); void setFlagsFileOpened(); + void setFlagsFeatureNotSupported(); void setFlagsNotImplemented(); void openData(); @@ -342,6 +344,23 @@ struct AbstractImporterTest: TestSuite::Tester { void debugFlags(); }; +const struct { + const char* message; + ImporterFlag flag; + ImporterFeatures features; +} SetFlagsFeatureNotSupportedData[] { + #define _c(name, enumName) {"zero-copy " name, ImporterFlag::ForceZeroCopy ## enumName, ~ImporterFeature::ZeroCopy ## enumName} + _c("animations", Animations), + _c("images", Images), + _c("material attributes", MaterialAttributes), + _c("material layers", MaterialLayers), + _c("mesh indices", MeshIndices), + _c("mesh vertices", MeshVertices), + _c("skin joints", SkinJoints), + _c("skin inverse bind matrices", SkinInverseBindMatrices) + #undef _c +}; + constexpr struct { const char* name; bool checkMessage; @@ -357,8 +376,12 @@ AbstractImporterTest::AbstractImporterTest() { &AbstractImporterTest::constructWithPluginManagerReference, &AbstractImporterTest::setFlags, - &AbstractImporterTest::setFlagsFileOpened, - &AbstractImporterTest::setFlagsNotImplemented, + &AbstractImporterTest::setFlagsFileOpened}); + + addInstancedTests({&AbstractImporterTest::setFlagsFeatureNotSupported}, + Containers::arraySize(SetFlagsFeatureNotSupportedData)); + + addTests({&AbstractImporterTest::setFlagsNotImplemented, &AbstractImporterTest::openData, #ifdef MAGNUM_BUILD_DEPRECATED @@ -681,14 +704,13 @@ void AbstractImporterTest::setFlags() { CORRADE_COMPARE(importer.flags(), ImporterFlag::Verbose); CORRADE_COMPARE(importer._flags, ImporterFlag::Verbose); - /** @todo use a real flag when we have more than one */ - importer.addFlags(ImporterFlag(4)); - CORRADE_COMPARE(importer.flags(), ImporterFlag::Verbose|ImporterFlag(4)); - CORRADE_COMPARE(importer._flags, ImporterFlag::Verbose|ImporterFlag(4)); + importer.addFlags(ImporterFlag::ZeroCopy); + CORRADE_COMPARE(importer.flags(), ImporterFlag::Verbose|ImporterFlag::ZeroCopy); + CORRADE_COMPARE(importer._flags, ImporterFlag::Verbose|ImporterFlag::ZeroCopy); importer.clearFlags(ImporterFlag::Verbose); - CORRADE_COMPARE(importer.flags(), ImporterFlag(4)); - CORRADE_COMPARE(importer._flags, ImporterFlag(4)); + CORRADE_COMPARE(importer.flags(), ImporterFlag::ZeroCopy); + CORRADE_COMPARE(importer._flags, ImporterFlag::ZeroCopy); } void AbstractImporterTest::setFlagsFileOpened() { @@ -714,6 +736,32 @@ void AbstractImporterTest::setFlagsFileOpened() { "Trade::AbstractImporter::setFlags(): can't be set while a file is opened\n"); } +void AbstractImporterTest::setFlagsFeatureNotSupported() { + auto&& data = SetFlagsFeatureNotSupportedData[testCaseInstanceId()]; + setTestCaseDescription(data.message); + + #ifdef CORRADE_NO_ASSERT + CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions"); + #endif + + struct Importer: AbstractImporter { + explicit Importer(ImporterFeatures features): _features{features} {} + + ImporterFeatures doFeatures() const override { return _features; } + bool doIsOpened() const override { return false; } + void doClose() override {} + + ImporterFeatures _features; + } importer{data.features}; + + std::ostringstream out; + Error redirectError{&out}; + importer.setFlags(data.flag); + CORRADE_COMPARE(out.str(), Utility::formatString( + "Trade::AbstractImporter::setFlags(): importer doesn't support {}\n", + data.message)); +} + void AbstractImporterTest::setFlagsNotImplemented() { struct: AbstractImporter { ImporterFeatures doFeatures() const override { return {}; } @@ -7224,8 +7272,8 @@ void AbstractImporterTest::importerStateNoFile() { void AbstractImporterTest::debugFeature() { std::ostringstream out; - Debug{&out} << ImporterFeature::OpenData << ImporterFeature(0xf0); - CORRADE_COMPARE(out.str(), "Trade::ImporterFeature::OpenData Trade::ImporterFeature(0xf0)\n"); + Debug{&out} << ImporterFeature::OpenData << ImporterFeature(0xfefe); + CORRADE_COMPARE(out.str(), "Trade::ImporterFeature::OpenData Trade::ImporterFeature(0xfefe)\n"); } void AbstractImporterTest::debugFeatures() { @@ -7238,15 +7286,15 @@ void AbstractImporterTest::debugFeatures() { void AbstractImporterTest::debugFlag() { std::ostringstream out; - Debug{&out} << ImporterFlag::Verbose << ImporterFlag(0xf0); - CORRADE_COMPARE(out.str(), "Trade::ImporterFlag::Verbose Trade::ImporterFlag(0xf0)\n"); + Debug{&out} << ImporterFlag::Verbose << ImporterFlag(0xbaba); + CORRADE_COMPARE(out.str(), "Trade::ImporterFlag::Verbose Trade::ImporterFlag(0xbaba)\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"); + Debug{&out} << (ImporterFlag::Verbose|ImporterFlag(0xf000)) << ImporterFlags{}; + CORRADE_COMPARE(out.str(), "Trade::ImporterFlag::Verbose|Trade::ImporterFlag(0xf000) Trade::ImporterFlags{}\n"); } }}}} diff --git a/src/MagnumPlugins/AnyImageImporter/AnyImageImporter.cpp b/src/MagnumPlugins/AnyImageImporter/AnyImageImporter.cpp index d910e4eaa..e5823f752 100644 --- a/src/MagnumPlugins/AnyImageImporter/AnyImageImporter.cpp +++ b/src/MagnumPlugins/AnyImageImporter/AnyImageImporter.cpp @@ -156,7 +156,7 @@ void AnyImageImporter::doOpenFile(const std::string& filename) { _in = std::move(importer); } -void AnyImageImporter::doOpenData(Containers::Array&& data, DataFlags) { +void AnyImageImporter::doOpenData(Containers::Array&& data, const DataFlags dataFlags) { using namespace Containers::Literals; CORRADE_INTERNAL_ASSERT(manager()); @@ -259,7 +259,7 @@ void AnyImageImporter::doOpenData(Containers::Array&& data, DataFlags) { /* Try to open the file (error output should be printed by the plugin itself) */ - if(!importer->openData(data)) return; + if(!importer->openData(std::move(data), dataFlags)) return; /* Success, save the instance */ _in = std::move(importer);