diff --git a/doc/snippets/MagnumTrade.cpp b/doc/snippets/MagnumTrade.cpp index 14c392c62..a5318d486 100644 --- a/doc/snippets/MagnumTrade.cpp +++ b/doc/snippets/MagnumTrade.cpp @@ -71,20 +71,22 @@ struct Data { } data; importer->setFileCallback([](const std::string& filename, - Trade::ImporterFileCallbackPolicy policy, Data& data) { + Trade::ImporterFileCallbackPolicy policy, Data& data) + -> Containers::Optional> + { auto found = data.files.find(filename); /* Discard the memory mapping, if not needed anymore */ if(policy == Trade::ImporterFileCallbackPolicy::Close) { if(found != data.files.end()) data.files.erase(found); - return Containers::ArrayView{}; + return {}; } /* Load if not there yet */ if(found == data.files.end()) found = data.files.emplace( filename, Utility::Directory::mapRead(filename)).first; - return Containers::ArrayView{found->second}; + return Containers::arrayView(found->second); }, data); importer->openFile("scene.gltf"); // memory-maps all files @@ -113,7 +115,7 @@ std::unique_ptr importer; importer->setFileCallback([](const std::string& filename, Trade::ImporterFileCallbackPolicy, void*) { Utility::Resource rs("data"); - return rs.getRaw(filename); + return Containers::optional(rs.getRaw(filename)); }); /* [AbstractImporter-setFileCallback] */ } @@ -126,10 +128,15 @@ struct Data { } data; importer->setFileCallback([](const std::string& filename, - Trade::ImporterFileCallbackPolicy, Data& data) { + Trade::ImporterFileCallbackPolicy, Data& data) + -> Containers::Optional> + { auto found = data.files.find(filename); - if(found == data.files.end()) found = data.files.emplace( - filename, Utility::Directory::read(filename)).first; + if(found == data.files.end()) { + if(!Utility::Directory::fileExists(filename)) + return Containers::NullOpt; + found = data.files.emplace(filename, Utility::Directory::read(filename)).first; + } return Containers::ArrayView{found->second}; }, data); /* [AbstractImporter-setFileCallback-template] */ diff --git a/src/Magnum/Trade/AbstractImporter.cpp b/src/Magnum/Trade/AbstractImporter.cpp index c5c406211..c27b704b7 100644 --- a/src/Magnum/Trade/AbstractImporter.cpp +++ b/src/Magnum/Trade/AbstractImporter.cpp @@ -73,19 +73,16 @@ AbstractImporter::AbstractImporter(PluginManager::Manager& man AbstractImporter::AbstractImporter(PluginManager::AbstractManager& manager, const std::string& plugin): PluginManager::AbstractManagingPlugin{manager, plugin} {} -void AbstractImporter::setFileCallback(Containers::ArrayView(*callback)(const std::string&, ImporterFileCallbackPolicy, void*), void* const userData) { +void AbstractImporter::setFileCallback(Containers::Optional>(*callback)(const std::string&, ImporterFileCallbackPolicy, void*), void* const userData) { CORRADE_ASSERT(!isOpened(), "Trade::AbstractImporter::setFileCallback(): can't be set while a file is opened", ); - if(!(features() & (Feature::FileCallback|Feature::OpenData))) { - Warning{} << "Trade::AbstractImporter::setFileCallback(): importer supports neither loading from data nor via callbacks, ignoring"; - return; - } + CORRADE_ASSERT(features() & (Feature::FileCallback|Feature::OpenData), "Trade::AbstractImporter::setFileCallback(): importer supports neither loading from data nor via callbacks, callbacks can't be used", ); _fileCallback = callback; _fileCallbackUserData = userData; doSetFileCallback(callback, userData); } -void AbstractImporter::doSetFileCallback(Containers::ArrayView(*)(const std::string&, ImporterFileCallbackPolicy, void*), void*) {} +void AbstractImporter::doSetFileCallback(Containers::Optional>(*)(const std::string&, ImporterFileCallbackPolicy, void*), void*) {} bool AbstractImporter::openData(Containers::ArrayView data) { CORRADE_ASSERT(features() & Feature::OpenData, @@ -116,15 +113,35 @@ void AbstractImporter::doOpenState(const void*, const std::string&) { bool AbstractImporter::openFile(const std::string& filename) { close(); - /* If file loading callbacks are set and the importer can open data, do - that. Mark the file as ready to be closed once opening is finished. */ - if((doFeatures() & Feature::OpenData) && _fileCallback) { - doOpenData(_fileCallback(filename, ImporterFileCallbackPolicy::LoadTemporary, _fileCallbackUserData)); + /* If file loading callbacks are not set or the importer supports handling + them directly, call into the implementation */ + if(!_fileCallback || (doFeatures() & Feature::FileCallback)) { + doOpenFile(filename); + + /* Otherwise, if loading from data is supported, use the callback and pass + the data through to openData(). Mark the file as ready to be closed once + opening is finished. */ + } else if(doFeatures() & Feature::OpenData) { + /* This needs to be duplicated here and in the doOpenFile() + implementation in order to support both following cases: + - plugins that don't support FileCallback but have their own + doOpenFile() implementation (callback needs to be used here, + because the base doOpenFile() implementation might never get + called) + - plugins that support FileCallback but want to delegate the actual + file loading to the default implementation (callback used in the + base doOpenFile() implementation, because this branch is never + taken in that case) */ + const Containers::Optional> data = _fileCallback(filename, ImporterFileCallbackPolicy::LoadTemporary, _fileCallbackUserData); + if(!data) { + Error() << "Trade::AbstractImporter::openFile(): cannot open file" << filename; + return isOpened(); + } + doOpenData(*data); _fileCallback(filename, ImporterFileCallbackPolicy::Close, _fileCallbackUserData); - /* Otherwise (either no callbacks set or opening data is not supported) - just call the default implementation */ - } else doOpenFile(filename); + /* Shouldn't get here, the assert is fired already in setFileCallback() */ + } else CORRADE_ASSERT_UNREACHABLE(); /* LCOV_EXCL_LINE */ return isOpened(); } @@ -132,13 +149,26 @@ bool AbstractImporter::openFile(const std::string& filename) { void AbstractImporter::doOpenFile(const std::string& filename) { CORRADE_ASSERT(features() & Feature::OpenData, "Trade::AbstractImporter::openFile(): not implemented", ); - /* Open file */ - if(!Utility::Directory::fileExists(filename)) { - Error() << "Trade::AbstractImporter::openFile(): cannot open file" << filename; - return; - } + /* If callbacks are set, use them. This is the same implementation as in + openFile(), see the comment there for details. */ + if(_fileCallback) { + const Containers::Optional> data = _fileCallback(filename, ImporterFileCallbackPolicy::LoadTemporary, _fileCallbackUserData); + if(!data) { + Error() << "Trade::AbstractImporter::openFile(): cannot open file" << filename; + return; + } + doOpenData(*data); + _fileCallback(filename, ImporterFileCallbackPolicy::Close, _fileCallbackUserData); - doOpenData(Utility::Directory::read(filename)); + /* Otherwise open the file directly */ + } else { + if(!Utility::Directory::fileExists(filename)) { + Error() << "Trade::AbstractImporter::openFile(): cannot open file" << filename; + return; + } + + doOpenData(Utility::Directory::read(filename)); + } } void AbstractImporter::close() { diff --git a/src/Magnum/Trade/AbstractImporter.h b/src/Magnum/Trade/AbstractImporter.h index 856032063..862afbb63 100644 --- a/src/Magnum/Trade/AbstractImporter.h +++ b/src/Magnum/Trade/AbstractImporter.h @@ -123,13 +123,23 @@ you may want to intercept those references and load them in a custom way as well. For importers that advertise support for this with @ref Feature::FileCallback this is done by specifying a file loading callback using @ref setFileCallback(). The callback gets a filename, @ref ImporterFileCallbackPolicy and an user -pointer as parameters. For example, loading a scene from memory-mapped files -could look like this. Note that the file loading callback affects @ref openFile() -as well --- you don't have to load the top-level file manually and pass it to -@ref openData(), it's done implicitly by the importer: +pointer as parameters; returns a non-owning view on the loaded data or a +@ref Corrade::Containers::NullOpt "Containers::NullOpt" to indicate the file +loading failed. For example, loading a scene from memory-mapped files could +look like below. Note that the file loading callback affects @ref openFile() as +well --- you don't have to load the top-level file manually and pass it to +@ref openData(), any importer supporting the callback feature handles that +correctly. @snippet MagnumTrade.cpp AbstractImporter-usage-callbacks +For importers that don't support @ref Feature::FileCallback directly, the base +@ref openFile() implementation will use the file callback to pass the loaded +data through to @ref openData(), in case the importer supports at least +@ref Feature::OpenData. If the importer supports neither @ref Feature::FileCallback +nor @ref Feature::OpenData, @ref setFileCallback() doesn't allow the callbacks +to be set. + @subsection Trade-AbstractImporter-usage-state Internal importer state Some importers, especially ones that make use of well-known external libraries, @@ -178,10 +188,16 @@ into* a @ref std::shared_ptr instance and that might not be desirable. The plugin needs to implement the @ref doFeatures(), @ref doIsOpened() functions, at least one of @ref doOpenData() / @ref doOpenFile() / -@ref doOpenState() functions, function @ref doClose(), function -@ref doSetFileCallback() in case it's desired to respond on file loading -callback setup and one or more tuples of data access functions, based on what -features are supported in given format. +@ref doOpenState() functions, function @ref doClose() and one or more tuples of +data access functions, based on what features are supported in given format. + +In order to support @ref Feature::FileCallback, the importer needs to properly +use the callbacks to both load the top-level file in @ref doOpenFile() and also +load any external files when needed. The @ref doOpenFile() can delegate back +into the base implementation, but it should remember at least the base file +path to pass correct paths to subsequent file callbacks. The +@ref doSetFileCallback() can be overriden in case it's desired to respond to +file loading callback setup, but doesn't have to be. For multi-data formats the file opening shouldn't take long and all parsing should be done in the data parsing functions instead, because the user might @@ -296,7 +312,7 @@ class MAGNUM_TRADE_EXPORT AbstractImporter: public PluginManager::AbstractManagi * * @see @ref Trade-AbstractImporter-usage-callbacks */ - auto fileCallback() const -> Containers::ArrayView(*)(const std::string&, ImporterFileCallbackPolicy, void*) { return _fileCallback; } + auto fileCallback() const -> Containers::Optional>(*)(const std::string&, ImporterFileCallbackPolicy, void*) { return _fileCallback; } /** * @brief File opening callback user data @@ -308,35 +324,41 @@ class MAGNUM_TRADE_EXPORT AbstractImporter: public PluginManager::AbstractManagi /** * @brief Set file opening callback * - * In case a scene file opened using @ref openData() references other - * files such as images and @ref Feature::FileCallback is supported by - * the importer, @p callback will be used to load file data. The - * callback function gets a filename, @ref ImporterFileCallbackPolicy - * and the @p userData pointer as input and returns a non-owning view - * on the loaded data as output. - * - * In case @ref openFile() is used and at least @ref Feature::OpenData - * is supported, the callback is used for loading the top-level file. - * First the file is loaded with @ref ImporterFileCallbackPolicy::LoadTemporary - * passed to the callback, then the returned memory view is passed to + * In case the importer supports @ref Feature::FileCallback, files + * opened through @ref openFile() will be loaded through the provided + * callback. Besides that, all external files referenced by the + * top-level file will be loaded through the callback function as well, + * usually on demand. The callback function gets a filename, + * @ref ImporterFileCallbackPolicy and the @p userData pointer as input + * and returns a non-owning view on the loaded data as output or a + * @ref Corrade::Containers::NullOpt if loading failed --- because + * empty files might also be valid in some circumstances, @cpp nullptr @ce + * can't be used to indicate a failure. + * + * In case the importer doesn't support @ref Feature::FileCallback but + * supports at least @ref Feature::OpenData, a file opened through + * @ref openFile() will be internally loaded through the provided + * callback and then passed to @ref openData(). First the file is + * loaded with @ref ImporterFileCallbackPolicy::LoadTemporary passed to + * the callback, then the returned memory view is passed to * @ref openData() (sidestepping the potential @ref openFile() - * implementation of that particular plugin) and after that the + * implementation of that particular importer) and after that the * callback is called again with @ref ImporterFileCallbackPolicy::Close * because the semantics of @ref openData() don't require the data to * be alive after. In case you need a different behavior, use * @ref openData() directly. * - * In case the importer supports neither @ref Feature::FileCallback nor - * @ref Feature::OpenData, the callback won't have a chance to be used - * in any of the above scenarios and thus the function prints a warning - * and returns without setting anything. In case @p callback is - * @cpp nullptr @ce, the current callback (if any) is reset. + * In case @p callback is @cpp nullptr @ce, the current callback (if + * any) is reset. This function expects that the importer supports + * either @ref Feature::FileCallback or @ref Feature::OpenData. If an + * importer supports neither, callbacks can't be used. * * It's expected that this function is called *before* a file is * opened. It's also expected that the loaded data are kept in scope * for as long as the importer needs them, based on the value of - * @ref ImporterFileCallbackPolicy. Particular importer documentation - * provides * more information about the behavior. + * @ref ImporterFileCallbackPolicy. Documentation of particular + * importers provides more information about the expected callback + * behavior. * * Following is an example of setting up a file loading callback for * fetching compiled-in resources from @ref Corrade::Utility::Resource. @@ -347,7 +369,7 @@ class MAGNUM_TRADE_EXPORT AbstractImporter: public PluginManager::AbstractManagi * * @see @ref Trade-AbstractImporter-usage-callbacks */ - void setFileCallback(Containers::ArrayView(*callback)(const std::string&, ImporterFileCallbackPolicy, void*), void* userData = nullptr); + void setFileCallback(Containers::Optional>(*callback)(const std::string&, ImporterFileCallbackPolicy, void*), void* userData = nullptr); /** * @brief Set file opening callback @@ -361,7 +383,7 @@ class MAGNUM_TRADE_EXPORT AbstractImporter: public PluginManager::AbstractManagi * @see @ref Trade-AbstractImporter-usage-callbacks */ #ifdef DOXYGEN_GENERATING_OUTPUT - template void setFileCallback(Containers::ArrayView(*callback)(const std::string&, ImporterFileCallbackPolicy, T&), T& userData); + template void setFileCallback(Containers::Optional>(*callback)(const std::string&, ImporterFileCallbackPolicy, T&), T& userData); #else /* Otherwise the user would be forced to use the + operator to convert a lambda to a function pointer and (besides being weird and @@ -808,7 +830,13 @@ class MAGNUM_TRADE_EXPORT AbstractImporter: public PluginManager::AbstractManagi * the file and calls @ref Magnum::Trade::AbstractImporter::doOpenData() "doOpenData()" * with its contents. It is allowed to call this function from your * @ref Magnum::Trade::AbstractImporter::doOpenFile() "doOpenFile()" - * implementation. + * implementation --- in particular, this implementation will also + * correctly handle callbacks set through @ref setFileCallback(). + * + * This function is not called when file callbacks are set through + * @ref setFileCallback() and @ref Feature::FileCallback is not + * supported --- instead, file is loaded though the callback and data + * passed through to @ref Magnum::Trade::AbstractImporter::doOpenData() "doOpenData()". */ virtual void doOpenFile(const std::string& filename); @@ -829,7 +857,7 @@ class MAGNUM_TRADE_EXPORT AbstractImporter: public PluginManager::AbstractManagi * and user data pointer are available through @ref fileCallback() and * @ref fileCallbackUserData(). */ - virtual void doSetFileCallback(Containers::ArrayView(*callback)(const std::string&, ImporterFileCallbackPolicy, void*), void* userData); + virtual void doSetFileCallback(Containers::Optional>(*callback)(const std::string&, ImporterFileCallbackPolicy, void*), void* userData); /** @brief Implementation for @ref isOpened() */ virtual bool doIsOpened() const = 0; @@ -1166,7 +1194,7 @@ class MAGNUM_TRADE_EXPORT AbstractImporter: public PluginManager::AbstractManagi virtual const void* doImporterState() const; private: - Containers::ArrayView(*_fileCallback)(const std::string&, ImporterFileCallbackPolicy, void*){}; + Containers::Optional>(*_fileCallback)(const std::string&, ImporterFileCallbackPolicy, void*){}; void* _fileCallbackUserData{}; /* Used by the templated version only */ @@ -1180,13 +1208,13 @@ class MAGNUM_TRADE_EXPORT AbstractImporter: public PluginManager::AbstractManagi template void AbstractImporter::setFileCallback(Callback callback, T& userData) { /* Don't try to wrap a null function pointer. Need to cast first because MSVC (even 2017) can't apply ! to a lambda. Ugh. */ - const auto callbackPtr = static_cast(*)(const std::string&, ImporterFileCallbackPolicy, T&)>(callback); + const auto callbackPtr = static_cast>(*)(const std::string&, ImporterFileCallbackPolicy, T&)>(callback); if(!callbackPtr) return setFileCallback(nullptr); _fileCallbackTemplate = { reinterpret_cast(callbackPtr), &userData }; setFileCallback([](const std::string& filename, const ImporterFileCallbackPolicy flags, void* const userData) { auto& s = *reinterpret_cast(userData); - return reinterpret_cast(*)(const std::string&, ImporterFileCallbackPolicy, T&)>(s.callback)(filename, flags, *static_cast(s.userData)); + return reinterpret_cast>(*)(const std::string&, ImporterFileCallbackPolicy, T&)>(s.callback)(filename, flags, *static_cast(s.userData)); }, &_fileCallbackTemplate); } #endif diff --git a/src/Magnum/Trade/Test/AbstractImporterTest.cpp b/src/Magnum/Trade/Test/AbstractImporterTest.cpp index 5c44ae951..ec7eabe68 100644 --- a/src/Magnum/Trade/Test/AbstractImporterTest.cpp +++ b/src/Magnum/Trade/Test/AbstractImporterTest.cpp @@ -67,8 +67,11 @@ class AbstractImporterTest: public TestSuite::Tester { void setFileCallbackFileOpened(); void setFileCallbackNotImplemented(); void setFileCallbackNotSupported(); + void setFileCallbackOpenFileDirectly(); + void setFileCallbackOpenFileThroughBaseImplementation(); + void setFileCallbackOpenFileThroughBaseImplementationFailed(); void setFileCallbackOpenFileAsData(); - void setFileCallbackOpenFileAsDataNotSupported(); + void setFileCallbackOpenFileAsDataFailed(); void defaultScene(); void defaultSceneNotImplemented(); @@ -258,8 +261,11 @@ AbstractImporterTest::AbstractImporterTest() { &AbstractImporterTest::setFileCallbackFileOpened, &AbstractImporterTest::setFileCallbackNotImplemented, &AbstractImporterTest::setFileCallbackNotSupported, + &AbstractImporterTest::setFileCallbackOpenFileDirectly, + &AbstractImporterTest::setFileCallbackOpenFileThroughBaseImplementation, + &AbstractImporterTest::setFileCallbackOpenFileThroughBaseImplementationFailed, &AbstractImporterTest::setFileCallbackOpenFileAsData, - &AbstractImporterTest::setFileCallbackOpenFileAsDataNotSupported, + &AbstractImporterTest::setFileCallbackOpenFileAsDataFailed, &AbstractImporterTest::defaultScene, &AbstractImporterTest::defaultSceneNotImplemented, @@ -586,7 +592,7 @@ void AbstractImporterTest::setFileCallback() { Features doFeatures() const override { return Feature::OpenData|Feature::FileCallback; } bool doIsOpened() const override { return false; } void doClose() override {} - void doSetFileCallback(Containers::ArrayView(*)(const std::string&, ImporterFileCallbackPolicy, void*), void* userData) override { + void doSetFileCallback(Containers::Optional>(*)(const std::string&, ImporterFileCallbackPolicy, void*), void* userData) override { *static_cast(userData) = 1337; } }; @@ -594,7 +600,7 @@ void AbstractImporterTest::setFileCallback() { int a = 0; Importer importer; auto lambda = [](const std::string&, ImporterFileCallbackPolicy, void*) { - return Containers::ArrayView{}; + return Containers::Optional>{}; }; importer.setFileCallback(lambda, &a); CORRADE_COMPARE(importer.fileCallback(), lambda); @@ -607,7 +613,7 @@ void AbstractImporterTest::setFileCallbackTemplate() { Features doFeatures() const override { return Feature::OpenData|Feature::FileCallback; } bool doIsOpened() const override { return false; } void doClose() override {} - void doSetFileCallback(Containers::ArrayView(*)(const std::string&, ImporterFileCallbackPolicy, void*), void*) override { + void doSetFileCallback(Containers::Optional>(*)(const std::string&, ImporterFileCallbackPolicy, void*), void*) override { called = true; } @@ -616,7 +622,7 @@ void AbstractImporterTest::setFileCallbackTemplate() { int a = 0; auto lambda = [](const std::string&, ImporterFileCallbackPolicy, int&) { - return Containers::ArrayView{}; + return Containers::Optional>{}; }; Importer importer; importer.setFileCallback(lambda, a); @@ -625,7 +631,7 @@ void AbstractImporterTest::setFileCallbackTemplate() { CORRADE_VERIFY(importer.called); /* The data pointers should be wrapped, thus not the same */ - CORRADE_VERIFY(reinterpret_cast(importer.fileCallback()) != reinterpret_cast(static_cast(*)(const std::string&, ImporterFileCallbackPolicy, int&)>(lambda))); + CORRADE_VERIFY(reinterpret_cast(importer.fileCallback()) != reinterpret_cast(static_cast>(*)(const std::string&, ImporterFileCallbackPolicy, int&)>(lambda))); CORRADE_VERIFY(importer.fileCallbackUserData() != &a); } @@ -634,7 +640,7 @@ void AbstractImporterTest::setFileCallbackTemplateNull() { Features doFeatures() const override { return Feature::OpenData|Feature::FileCallback; } bool doIsOpened() const override { return false; } void doClose() override {} - void doSetFileCallback(Containers::ArrayView(*callback)(const std::string&, ImporterFileCallbackPolicy, void*), void* userData) override { + void doSetFileCallback(Containers::Optional>(*callback)(const std::string&, ImporterFileCallbackPolicy, void*), void* userData) override { called = !callback && !userData; } @@ -643,7 +649,7 @@ void AbstractImporterTest::setFileCallbackTemplateNull() { int a = 0; Importer importer; - importer.setFileCallback(static_cast(*)(const std::string&, ImporterFileCallbackPolicy, int&)>(nullptr), a); + importer.setFileCallback(static_cast>(*)(const std::string&, ImporterFileCallbackPolicy, int&)>(nullptr), a); CORRADE_VERIFY(!importer.fileCallback()); CORRADE_VERIFY(!importer.fileCallbackUserData()); CORRADE_VERIFY(importer.called); @@ -661,7 +667,7 @@ void AbstractImporterTest::setFileCallbackFileOpened() { Importer importer; importer.setFileCallback([](const std::string&, ImporterFileCallbackPolicy, void*) { - return Containers::ArrayView{}; + return Containers::Optional>{}; }); CORRADE_COMPARE(out.str(), "Trade::AbstractImporter::setFileCallback(): can't be set while a file is opened\n"); } @@ -675,7 +681,7 @@ void AbstractImporterTest::setFileCallbackNotImplemented() { int a; auto lambda = [](const std::string&, ImporterFileCallbackPolicy, void*) { - return Containers::ArrayView{}; + return Containers::Optional>{}; }; Importer importer; importer.setFileCallback(lambda, &a); @@ -692,29 +698,65 @@ void AbstractImporterTest::setFileCallbackNotSupported() { }; std::ostringstream out; - Warning redirectWarning{&out}; + Error redirectError{&out}; int a; Importer importer; importer.setFileCallback([](const std::string&, ImporterFileCallbackPolicy, void*) { - return Containers::ArrayView{}; + return Containers::Optional>{}; }, &a); - CORRADE_COMPARE(out.str(), "Trade::AbstractImporter::setFileCallback(): importer supports neither loading from data nor via callbacks, ignoring\n"); - CORRADE_VERIFY(!importer.fileCallback()); - CORRADE_VERIFY(!importer.fileCallbackUserData()); + CORRADE_COMPARE(out.str(), "Trade::AbstractImporter::setFileCallback(): importer supports neither loading from data nor via callbacks, callbacks can't be used\n"); } -void AbstractImporterTest::setFileCallbackOpenFileAsData() { +void AbstractImporterTest::setFileCallbackOpenFileDirectly() { class Importer: public Trade::AbstractImporter { - Features doFeatures() const override { return Feature::OpenData; } + Features doFeatures() const override { return Feature::FileCallback|Feature::OpenData; } + bool doIsOpened() const override { return _opened; } + void doClose() override { _opened = false; } + + void doOpenFile(const std::string& filename) override { + /* Called because FileCallback is supported */ + _opened = filename == "file.dat" && fileCallback() && fileCallbackUserData(); + } + + void doOpenData(Containers::ArrayView) override { + /* Shouldn't be called because FileCallback is supported */ + openDataCalledNotSureWhy = true; + } + + bool _opened = false; + public: bool openDataCalledNotSureWhy = false; + }; + + bool calledNotSureWhy = false; + Importer importer; + importer.setFileCallback([](const std::string&, ImporterFileCallbackPolicy, bool& calledNotSureWhy) -> Containers::Optional> { + calledNotSureWhy = true; + return {}; + }, calledNotSureWhy); + + CORRADE_VERIFY(importer.openFile("file.dat")); + CORRADE_VERIFY(!calledNotSureWhy); + CORRADE_VERIFY(!importer.openDataCalledNotSureWhy); +} + +void AbstractImporterTest::setFileCallbackOpenFileThroughBaseImplementation() { + class Importer: public Trade::AbstractImporter { + Features doFeatures() const override { return Feature::FileCallback|Feature::OpenData; } bool doIsOpened() const override { return _opened; } void doClose() override { _opened = false; } + void doOpenFile(const std::string& filename) override { + openFileCalled = filename == "file.dat" && fileCallback() && fileCallbackUserData(); + AbstractImporter::doOpenFile(filename); + } + void doOpenData(Containers::ArrayView data) override { _opened = (data.size() == 1 && data[0] == '\xb0'); } bool _opened = false; + public: bool openFileCalled = false; }; struct State { @@ -724,49 +766,126 @@ void AbstractImporterTest::setFileCallbackOpenFileAsData() { bool calledNotSureWhy = false; } state; Importer importer; - importer.setFileCallback([](const std::string& filename, ImporterFileCallbackPolicy policy, State& state) { + importer.setFileCallback([](const std::string& filename, ImporterFileCallbackPolicy policy, State& state) -> Containers::Optional> { if(filename == "file.dat" && policy == ImporterFileCallbackPolicy::LoadTemporary) { state.loaded = true; - return Containers::ArrayView{&state.data, 1}; + return Containers::arrayView(&state.data, 1); } if(filename == "file.dat" && policy == ImporterFileCallbackPolicy::Close) { state.closed = true; - return Containers::ArrayView{}; + return {}; } state.calledNotSureWhy = true; - return Containers::ArrayView{}; + return {}; }, state); CORRADE_VERIFY(importer.openFile("file.dat")); + CORRADE_VERIFY(importer.openFileCalled); CORRADE_VERIFY(state.loaded); CORRADE_VERIFY(state.closed); CORRADE_VERIFY(!state.calledNotSureWhy); } -void AbstractImporterTest::setFileCallbackOpenFileAsDataNotSupported() { +void AbstractImporterTest::setFileCallbackOpenFileThroughBaseImplementationFailed() { class Importer: public Trade::AbstractImporter { - Features doFeatures() const override { return Feature::FileCallback; } + Features doFeatures() const override { return Feature::FileCallback|Feature::OpenData; } + bool doIsOpened() const override { return false; } + void doClose() override {} + + void doOpenFile(const std::string& filename) override { + openFileCalled = true; + AbstractImporter::doOpenFile(filename); + } + + public: bool openFileCalled = false; + }; + + Importer importer; + importer.setFileCallback([](const std::string&, ImporterFileCallbackPolicy, void*) -> Containers::Optional> { + return {}; + }); + + std::ostringstream out; + Error redirectError{&out}; + + CORRADE_VERIFY(!importer.openFile("file.dat")); + CORRADE_VERIFY(importer.openFileCalled); + CORRADE_COMPARE(out.str(), "Trade::AbstractImporter::openFile(): cannot open file file.dat\n"); +} + +void AbstractImporterTest::setFileCallbackOpenFileAsData() { + class Importer: public Trade::AbstractImporter { + Features doFeatures() const override { return Feature::OpenData; } bool doIsOpened() const override { return _opened; } void doClose() override { _opened = false; } - void doOpenFile(const std::string& filename) override { - _opened = filename == "file.dat"; + void doOpenFile(const std::string&) override { + openFileCalled = true; + } + + void doOpenData(Containers::ArrayView data) override { + _opened = (data.size() == 1 && data[0] == '\xb0'); } bool _opened = false; + public: bool openFileCalled = false; }; - bool calledNotSureWhy = false; + struct State { + const char data = '\xb0'; + bool loaded = false; + bool closed = false; + bool calledNotSureWhy = false; + } state; Importer importer; - importer.setFileCallback([](const std::string&, ImporterFileCallbackPolicy, bool& calledNotSureWhy) { - calledNotSureWhy = true; - return Containers::ArrayView{}; - }, calledNotSureWhy); + importer.setFileCallback([](const std::string& filename, ImporterFileCallbackPolicy policy, State& state) -> Containers::Optional> { + if(filename == "file.dat" && policy == ImporterFileCallbackPolicy::LoadTemporary) { + state.loaded = true; + return Containers::arrayView(&state.data, 1); + } + + if(filename == "file.dat" && policy == ImporterFileCallbackPolicy::Close) { + state.closed = true; + return {}; + } + + state.calledNotSureWhy = true; + return {}; + }, state); CORRADE_VERIFY(importer.openFile("file.dat")); - CORRADE_VERIFY(!calledNotSureWhy); + CORRADE_VERIFY(!importer.openFileCalled); + CORRADE_VERIFY(state.loaded); + CORRADE_VERIFY(state.closed); + CORRADE_VERIFY(!state.calledNotSureWhy); +} + +void AbstractImporterTest::setFileCallbackOpenFileAsDataFailed() { + class Importer: public Trade::AbstractImporter { + Features doFeatures() const override { return Feature::OpenData; } + bool doIsOpened() const override { return false; } + void doClose() override {} + + void doOpenFile(const std::string&) override { + openFileCalled = true; + } + + public: bool openFileCalled = false; + }; + + Importer importer; + importer.setFileCallback([](const std::string&, ImporterFileCallbackPolicy, void*) { + return Containers::Optional>{}; + }); + + std::ostringstream out; + Error redirectError{&out}; + + CORRADE_VERIFY(!importer.openFile("file.dat")); + CORRADE_VERIFY(!importer.openFileCalled); + CORRADE_COMPARE(out.str(), "Trade::AbstractImporter::openFile(): cannot open file file.dat\n"); } void AbstractImporterTest::defaultScene() { diff --git a/src/MagnumPlugins/AnyImageImporter/Test/AnyImageImporterTest.cpp b/src/MagnumPlugins/AnyImageImporter/Test/AnyImageImporterTest.cpp index 4b774aa6b..128ac30da 100644 --- a/src/MagnumPlugins/AnyImageImporter/Test/AnyImageImporterTest.cpp +++ b/src/MagnumPlugins/AnyImageImporter/Test/AnyImageImporterTest.cpp @@ -53,7 +53,7 @@ struct AnyImageImporterTest: TestSuite::Tester { namespace { -Containers::ArrayView fileCallback(const std::string& filename, Trade::ImporterFileCallbackPolicy, Containers::Array& storage) { +Containers::Optional> fileCallback(const std::string& filename, Trade::ImporterFileCallbackPolicy, Containers::Array& storage) { storage = Utility::Directory::read(filename); return Containers::ArrayView{storage}; } @@ -61,7 +61,7 @@ Containers::ArrayView fileCallback(const std::string& filename, Trad constexpr struct { const char* name; const char* filename; - Containers::ArrayView(*callback)(const std::string&, Trade::ImporterFileCallbackPolicy, Containers::Array&); + Containers::Optional>(*callback)(const std::string&, Trade::ImporterFileCallbackPolicy, Containers::Array&); } LoadData[]{ {"TGA", TGA_FILE, nullptr}, {"TGA data", TGA_FILE, fileCallback} @@ -70,7 +70,7 @@ constexpr struct { constexpr struct { const char* name; const char* filename; - Containers::ArrayView(*callback)(const std::string&, Trade::ImporterFileCallbackPolicy, Containers::Array&); + Containers::Optional>(*callback)(const std::string&, Trade::ImporterFileCallbackPolicy, Containers::Array&); const char* plugin; } DetectData[]{ {"PNG", "rgb.png", nullptr, "PngImporter"},