From 5e7443a5f8dc0bc17013a3402daeef20aa471bad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sun, 24 Oct 2021 19:58:25 +0200 Subject: [PATCH] Trade: add flags for forcing zero-copy import of certain data. Because there's no format ever that would support zero-copy everything (except the upcoming Magnum blobs, hah!), it makes no sense to force zero-copy globally. Instead, zero-copy can be forced just for particular data of interest. For example a glTF can zero-copy-import mesh and animation data, but not always image data (except if it embeds an uncompressed KTX), and never scene or material data, which are stored in the textual JSON. --- src/Magnum/Trade/AbstractImporter.cpp | 54 +++- src/Magnum/Trade/AbstractImporter.h | 236 +++++++++++++++++- .../Trade/Test/AbstractImporterTest.cpp | 65 ++++- 3 files changed, 341 insertions(+), 14 deletions(-) diff --git a/src/Magnum/Trade/AbstractImporter.cpp b/src/Magnum/Trade/AbstractImporter.cpp index 0a3fd2d05..533bee2d3 100644 --- a/src/Magnum/Trade/AbstractImporter.cpp +++ b/src/Magnum/Trade/AbstractImporter.cpp @@ -98,6 +98,30 @@ AbstractImporter::AbstractImporter(PluginManager::AbstractManager& manager, cons 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); } @@ -1155,11 +1179,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) { @@ -1177,17 +1209,33 @@ Debug& operator<<(Debug& debug, const ImporterFlag value) { #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::ZeroCopy}); + 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 9625d511d..369df8afc 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 @@ -135,6 +249,122 @@ enum class ImporterFlag: UnsignedByte { */ 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" ... */ }; diff --git a/src/Magnum/Trade/Test/AbstractImporterTest.cpp b/src/Magnum/Trade/Test/AbstractImporterTest.cpp index 17da10077..e03624b2f 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" @@ -67,6 +68,7 @@ struct AbstractImporterTest: TestSuite::Tester { void setFlags(); void setFlagsFileOpened(); + void setFlagsFeatureNotSupported(); void setFlagsNotImplemented(); void openData(); @@ -295,6 +297,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; @@ -308,8 +327,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 @@ -618,6 +641,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 {}; } @@ -5058,8 +5107,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() { @@ -5072,15 +5121,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"); } }}}}