From 4446300d18856c0dd86afbf6d20057aef16fd578 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Mon, 25 Oct 2021 13:00:25 +0200 Subject: [PATCH] [wip] Trade: add a protected openData() overload taking a rvalue Array. Meant to be used from within plugin implementations to avoid copies in the delegated importers. TODO: the heck, why can't I call a protected function from a subclass but another instance? TODO: figure out a way to test this TODO: update docs and behavior for the ZeroCopy flag TODO: adapt AnySceneImporter also --- src/Magnum/Trade/AbstractImporter.cpp | 8 ++++-- src/Magnum/Trade/AbstractImporter.h | 28 +++++++++++++++++++ .../AnyImageImporter/AnyImageImporter.cpp | 4 +-- 3 files changed, 36 insertions(+), 4 deletions(-) diff --git a/src/Magnum/Trade/AbstractImporter.cpp b/src/Magnum/Trade/AbstractImporter.cpp index 533bee2d3..8fbb4b696 100644 --- a/src/Magnum/Trade/AbstractImporter.cpp +++ b/src/Magnum/Trade/AbstractImporter.cpp @@ -147,7 +147,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", {}); @@ -155,10 +155,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", ); diff --git a/src/Magnum/Trade/AbstractImporter.h b/src/Magnum/Trade/AbstractImporter.h index 21ab1f081..fb8368b15 100644 --- a/src/Magnum/Trade/AbstractImporter.h +++ b/src/Magnum/Trade/AbstractImporter.h @@ -1900,6 +1900,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/MagnumPlugins/AnyImageImporter/AnyImageImporter.cpp b/src/MagnumPlugins/AnyImageImporter/AnyImageImporter.cpp index d177ad43f..b7158f6b5 100644 --- a/src/MagnumPlugins/AnyImageImporter/AnyImageImporter.cpp +++ b/src/MagnumPlugins/AnyImageImporter/AnyImageImporter.cpp @@ -153,7 +153,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()); @@ -253,7 +253,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);