diff --git a/src/Magnum/Trade/AbstractSceneConverter.cpp b/src/Magnum/Trade/AbstractSceneConverter.cpp index 58409e146..d0c676c28 100644 --- a/src/Magnum/Trade/AbstractSceneConverter.cpp +++ b/src/Magnum/Trade/AbstractSceneConverter.cpp @@ -188,6 +188,8 @@ void AbstractSceneConverter::clearFlags(SceneConverterFlags flags) { } Containers::Optional AbstractSceneConverter::convert(const MeshData& mesh) { + abort(); + CORRADE_ASSERT(features() & SceneConverterFeature::ConvertMesh, "Trade::AbstractSceneConverter::convert(): mesh conversion not supported", {}); @@ -205,6 +207,8 @@ Containers::Optional AbstractSceneConverter::doConvert(const MeshData& } bool AbstractSceneConverter::convertInPlace(MeshData& mesh) { + abort(); + CORRADE_ASSERT(features() & SceneConverterFeature::ConvertMeshInPlace, "Trade::AbstractSceneConverter::convertInPlace(): mesh conversion not supported", {}); @@ -221,6 +225,8 @@ Containers::Optional> Implementation::SceneConverterOptionalButAlsoArray #endif AbstractSceneConverter::convertToData(const MeshData& mesh) { + abort(); + if(features() >= SceneConverterFeature::ConvertMeshToData) { Containers::Optional> out = doConvertToData(mesh); CORRADE_ASSERT(!out || !out->deleter() || out->deleter() == static_cast(Implementation::nonOwnedArrayDeleter) || out->deleter() == ArrayAllocator::deleter, @@ -251,6 +257,8 @@ Containers::Optional> AbstractSceneConverter::doConvertT } bool AbstractSceneConverter::convertToFile(const MeshData& mesh, const Containers::StringView filename) { + abort(); + if(features() >= SceneConverterFeature::ConvertMeshToFile) { return doConvertToFile(mesh, filename); @@ -302,7 +310,7 @@ void AbstractSceneConverter::abort() { void AbstractSceneConverter::doAbort() {} bool AbstractSceneConverter::begin() { - if(_state) abort(); + abort(); _state.emplace(State::Type::Convert); @@ -385,7 +393,7 @@ Containers::Pointer AbstractSceneConverter::doEnd() { } bool AbstractSceneConverter::beginData() { - if(_state) abort(); + abort(); _state.emplace(State::Type::ConvertToData); @@ -442,7 +450,7 @@ Containers::Optional> AbstractSceneConverter::doEndData( } bool AbstractSceneConverter::beginFile(const Containers::StringView filename) { - if(_state) abort(); + abort(); _state.emplace(State::Type::ConvertToFile); _state->filename = Containers::String::nullTerminatedGlobalView(filename); diff --git a/src/Magnum/Trade/AbstractSceneConverter.h b/src/Magnum/Trade/AbstractSceneConverter.h index 3d25bb31b..40973ca0a 100644 --- a/src/Magnum/Trade/AbstractSceneConverter.h +++ b/src/Magnum/Trade/AbstractSceneConverter.h @@ -511,6 +511,10 @@ checked by the implementation: @ref doAdd() functions are called only if a corresponding @ref begin(), @ref beginData() or @ref beginFile() was called before and @ref abort() wasn't called in the meantime. +- The @ref doConvert(), @ref doConvertInPlace(), @ref doConvertToData(), + @ref doConvertToFile(), @ref doBegin(), @ref doBeginData() and + @ref doBeginFile() functions are called only after the previous conversion + (if any) was aborted with @ref doAbort(). - The @ref doAdd() and various `doSet*()` functions are called only if a corresponding @ref SceneConverterFeature is supported. - The @ref doAdd((UnsignedInt, Containers::Iterable, Containers::StringView) @@ -652,7 +656,8 @@ class MAGNUM_TRADE_EXPORT AbstractSceneConverter: public PluginManager::Abstract /** * @brief Convert a mesh * - * On failure prints a message to @relativeref{Magnum,Error} and + * If a (batch) conversion is currently in progress, calls @ref abort() + * first. On failure prints a message to @relativeref{Magnum,Error} and * returns @ref Containers::NullOpt. * * Expects that @ref SceneConverterFeature::ConvertMesh is supported. @@ -661,26 +666,30 @@ class MAGNUM_TRADE_EXPORT AbstractSceneConverter: public PluginManager::Abstract * and retrieve the output from the importer returned by @ref end() --- * in such case the process can also return zero or more than one mesh * instead of always exactly one. - * @see @ref features(), @ref convertInPlace(MeshData&) + * @see @ref isConverting(), @ref features(), + * @ref convertInPlace(MeshData&) */ Containers::Optional convert(const MeshData& mesh); /** * @brief Convert a mesh in-place * - * On failure prints a message to @relativeref{Magnum,Error} and + * If a (batch) conversion is currently in progress, calls @ref abort() + * first. On failure prints a message to @relativeref{Magnum,Error} and * returns @cpp false @ce, @p mesh is guaranteed to stay unchanged. * * Expects that @ref SceneConverterFeature::ConvertMeshInPlace is * supported. - * @see @ref features(), @ref convert(const MeshData&) + * @see @ref isConverting(), @ref features(), + * @ref convert(const MeshData&) */ bool convertInPlace(MeshData& mesh); /** * @brief Convert a mesh to a raw data * - * On failure prints a message to @relativeref{Magnum,Error} and + * If (batch) conversion is currently in progress, calls @ref abort() + * first. On failure prints a message to @relativeref{Magnum,Error} and * returns @ref Containers::NullOpt. * * Expects that @ref SceneConverterFeature::ConvertMeshToData is @@ -689,7 +698,7 @@ class MAGNUM_TRADE_EXPORT AbstractSceneConverter: public PluginManager::Abstract * @ref SceneConverterFeature::AddMeshes is supported instead, * delegates to a sequence of @ref beginData(), * @ref add(const MeshData&, Containers::StringView) and @ref endData(). - * @see @ref features(), @ref convertToFile() + * @see @ref isConverting(), @ref features(), @ref convertToFile() */ #if !defined(MAGNUM_BUILD_DEPRECATED) || defined(DOXYGEN_GENERATING_OUTPUT) Containers::Optional> @@ -702,7 +711,8 @@ class MAGNUM_TRADE_EXPORT AbstractSceneConverter: public PluginManager::Abstract * @brief Convert a mesh to a file * @m_since_latest * - * On failure prints a message to @relativeref{Magnum,Error} and + * If a (batch) conversion is currently in progress, calls @ref abort() + * first. On failure prints a message to @relativeref{Magnum,Error} and * returns @cpp false @ce. * * Expects that @ref SceneConverterFeature::ConvertMeshToFile is @@ -712,7 +722,7 @@ class MAGNUM_TRADE_EXPORT AbstractSceneConverter: public PluginManager::Abstract * delegates to a sequence of @ref beginFile(), * @ref add(const MeshData&, Containers::StringView) and * @ref endFile(). - * @see @ref features(), @ref convertToData() + * @see @ref isConverting(), @ref features(), @ref convertToData() */ bool convertToFile(const MeshData& mesh, Containers::StringView filename); @@ -762,7 +772,8 @@ class MAGNUM_TRADE_EXPORT AbstractSceneConverter: public PluginManager::Abstract * @ref add(const MeshData&, Containers::StringView) to * @ref convert(const MeshData&) and return the result from * @ref end(). - * @see @ref features(), @ref beginData(), @ref beginFile() + * @see @ref isConverting(), @ref features(), @ref beginData(), + * @ref beginFile() */ bool begin(); @@ -817,7 +828,8 @@ class MAGNUM_TRADE_EXPORT AbstractSceneConverter: public PluginManager::Abstract * @ref add(const MeshData&, Containers::StringView) to * @ref convertToData(const MeshData&) and return the result from * @ref endData(). - * @see @ref features(), @ref begin(), @ref beginFile() + * @see @ref isConverting(), @ref features(), @ref begin(), + * @ref beginFile() */ bool beginData(); @@ -855,7 +867,8 @@ class MAGNUM_TRADE_EXPORT AbstractSceneConverter: public PluginManager::Abstract * @ref add(const MeshData&, Containers::StringView) to * @ref convertToFile(const MeshData&, Containers::StringView) and * return the result from @ref endFile(). - * @see @ref features(), @ref begin(), @ref beginData() + * @see @ref isConverting(), @ref features(), @ref begin(), + * @ref beginData() */ bool beginFile(Containers::StringView filename); diff --git a/src/Magnum/Trade/Test/AbstractSceneConverterTest.cpp b/src/Magnum/Trade/Test/AbstractSceneConverterTest.cpp index 6b20c2aad..4b171c762 100644 --- a/src/Magnum/Trade/Test/AbstractSceneConverterTest.cpp +++ b/src/Magnum/Trade/Test/AbstractSceneConverterTest.cpp @@ -126,6 +126,13 @@ struct AbstractSceneConverterTest: TestSuite::Tester { void abort(); void abortNotImplemented(); + void abortImplicitlyConvertMesh(); + void abortImplicitlyConvertMeshInPlace(); + void abortImplicitlyConvertMeshToData(); + void abortImplicitlyConvertMeshToFile(); + void abortImplicitlyBegin(); + void abortImplicitlyBeginData(); + void abortImplicitlyBeginFile(); void thingNoBegin(); void endMismatchedBegin(); @@ -342,6 +349,13 @@ AbstractSceneConverterTest::AbstractSceneConverterTest() { &AbstractSceneConverterTest::abort, &AbstractSceneConverterTest::abortNotImplemented, + &AbstractSceneConverterTest::abortImplicitlyConvertMesh, + &AbstractSceneConverterTest::abortImplicitlyConvertMeshInPlace, + &AbstractSceneConverterTest::abortImplicitlyConvertMeshToData, + &AbstractSceneConverterTest::abortImplicitlyConvertMeshToFile, + &AbstractSceneConverterTest::abortImplicitlyBegin, + &AbstractSceneConverterTest::abortImplicitlyBeginData, + &AbstractSceneConverterTest::abortImplicitlyBeginFile, &AbstractSceneConverterTest::thingNoBegin, &AbstractSceneConverterTest::endMismatchedBegin, @@ -1807,6 +1821,236 @@ void AbstractSceneConverterTest::abortNotImplemented() { CORRADE_VERIFY(!converter.isConverting()); } +void AbstractSceneConverterTest::abortImplicitlyConvertMesh() { + struct: AbstractSceneConverter { + SceneConverterFeatures doFeatures() const override { + return SceneConverterFeature::ConvertMesh| + SceneConverterFeature::ConvertMultiple; + } + + Containers::Optional doConvert(const MeshData&) override { + return MeshData{MeshPrimitive::Lines, 2}; + } + + void doAbort() override { + CORRADE_VERIFY(!abortCalled); + abortCalled = true; + } + + bool doBegin() override { return true; } + + bool abortCalled = false; + } converter; + + /* Shouldn't be called if there's no previous conversion happening */ + CORRADE_VERIFY(!converter.abortCalled); + CORRADE_VERIFY(converter.begin()); + CORRADE_VERIFY(converter.isConverting()); + CORRADE_VERIFY(!converter.abortCalled); + + /* Should be called if there's a batch conversion happening and the + immediate APIs are used */ + CORRADE_VERIFY(converter.convert(MeshData{MeshPrimitive::Triangles, 6})); + CORRADE_VERIFY(!converter.isConverting()); + CORRADE_VERIFY(converter.abortCalled); +} + +void AbstractSceneConverterTest::abortImplicitlyConvertMeshInPlace() { + struct: AbstractSceneConverter { + SceneConverterFeatures doFeatures() const override { + return SceneConverterFeature::ConvertMeshInPlace| + SceneConverterFeature::ConvertMultiple; + } + + bool doConvertInPlace(MeshData&) override { + return true; + } + + void doAbort() override { + CORRADE_VERIFY(!abortCalled); + abortCalled = true; + } + + bool doBegin() override { return true; } + + bool abortCalled = false; + } converter; + + /* Shouldn't be called if there's no previous conversion happening */ + CORRADE_VERIFY(!converter.abortCalled); + CORRADE_VERIFY(converter.begin()); + CORRADE_VERIFY(converter.isConverting()); + CORRADE_VERIFY(!converter.abortCalled); + + /* Should be called if there's a batch conversion happening and the + immediate APIs are used */ + MeshData mesh{MeshPrimitive::Triangles, 6}; + CORRADE_VERIFY(converter.convertInPlace(mesh)); + CORRADE_VERIFY(!converter.isConverting()); + CORRADE_VERIFY(converter.abortCalled); +} + +void AbstractSceneConverterTest::abortImplicitlyConvertMeshToData() { + struct: AbstractSceneConverter { + SceneConverterFeatures doFeatures() const override { + return SceneConverterFeature::ConvertMeshToData| + SceneConverterFeature::ConvertMultiple; + } + + Containers::Optional> doConvertToData(const MeshData&) override { + return Containers::Array{}; + } + + void doAbort() override { + CORRADE_VERIFY(!abortCalled); + abortCalled = true; + } + + bool doBegin() override { return true; } + + bool abortCalled = false; + } converter; + + /* Shouldn't be called if there's no previous conversion happening */ + CORRADE_VERIFY(!converter.abortCalled); + CORRADE_VERIFY(converter.begin()); + CORRADE_VERIFY(converter.isConverting()); + CORRADE_VERIFY(!converter.abortCalled); + + /* Should be called if there's a batch conversion happening and the + immediate APIs are used */ + CORRADE_VERIFY(converter.convertToData(MeshData{MeshPrimitive::Triangles, 6})); + CORRADE_VERIFY(!converter.isConverting()); + CORRADE_VERIFY(converter.abortCalled); +} + +void AbstractSceneConverterTest::abortImplicitlyConvertMeshToFile() { + struct: AbstractSceneConverter { + SceneConverterFeatures doFeatures() const override { + return SceneConverterFeature::ConvertMeshToFile| + SceneConverterFeature::ConvertMultiple; + } + + bool doConvertToFile(const MeshData&, Containers::StringView) override { + return true; + } + + void doAbort() override { + CORRADE_VERIFY(!abortCalled); + abortCalled = true; + } + + bool doBegin() override { return true; } + + bool abortCalled = false; + } converter; + + /* Shouldn't be called if there's no previous conversion happening */ + CORRADE_VERIFY(!converter.abortCalled); + CORRADE_VERIFY(converter.begin()); + CORRADE_VERIFY(converter.isConverting()); + CORRADE_VERIFY(!converter.abortCalled); + + /* Should be called if there's a batch conversion happening and the + immediate APIs are used */ + CORRADE_VERIFY(converter.convertToFile(MeshData{MeshPrimitive::Triangles, 6}, Utility::Path::join(TRADE_TEST_OUTPUT_DIR, "mesh.out"))); + CORRADE_VERIFY(!converter.isConverting()); + CORRADE_VERIFY(converter.abortCalled); +} + +void AbstractSceneConverterTest::abortImplicitlyBegin() { + struct: AbstractSceneConverter { + SceneConverterFeatures doFeatures() const override { + return SceneConverterFeature::ConvertMultiple; + } + + void doAbort() override { + CORRADE_VERIFY(!abortCalled); + abortCalled = true; + } + + bool doBegin() override { return true; } + + bool abortCalled = false; + } converter; + + /* Shouldn't be called if there's no previous conversion happening */ + CORRADE_VERIFY(!converter.abortCalled); + CORRADE_VERIFY(converter.begin()); + CORRADE_VERIFY(converter.isConverting()); + CORRADE_VERIFY(!converter.abortCalled); + + /* Should be called if calling begin() while another conversion is already + happening. Then, what is happening is the new conversion. */ + CORRADE_VERIFY(converter.begin()); + CORRADE_VERIFY(converter.isConverting()); + CORRADE_VERIFY(converter.abortCalled); +} + +void AbstractSceneConverterTest::abortImplicitlyBeginData() { + struct: AbstractSceneConverter { + SceneConverterFeatures doFeatures() const override { + return SceneConverterFeature::ConvertMultiple| + SceneConverterFeature::ConvertMultipleToData; + } + + void doAbort() override { + CORRADE_VERIFY(!abortCalled); + abortCalled = true; + } + + bool doBegin() override { return true; } + + bool doBeginData() override { return true; } + + bool abortCalled = false; + } converter; + + /* Shouldn't be called if there's no previous conversion happening */ + CORRADE_VERIFY(!converter.abortCalled); + CORRADE_VERIFY(converter.begin()); + CORRADE_VERIFY(converter.isConverting()); + CORRADE_VERIFY(!converter.abortCalled); + + /* Should be called if calling beginData() while another conversion is + already happening. Then, what is happening is the new conversion. */ + CORRADE_VERIFY(converter.beginData()); + CORRADE_VERIFY(converter.isConverting()); + CORRADE_VERIFY(converter.abortCalled); +} + +void AbstractSceneConverterTest::abortImplicitlyBeginFile() { + struct: AbstractSceneConverter { + SceneConverterFeatures doFeatures() const override { + return SceneConverterFeature::ConvertMultiple| + SceneConverterFeature::ConvertMultipleToFile; + } + + void doAbort() override { + CORRADE_VERIFY(!abortCalled); + abortCalled = true; + } + + bool doBegin() override { return true; } + + bool doBeginFile(Containers::StringView) override { return true; } + + bool abortCalled = false; + } converter; + + /* Shouldn't be called if there's no previous conversion happening */ + CORRADE_VERIFY(!converter.abortCalled); + CORRADE_VERIFY(converter.begin()); + CORRADE_VERIFY(converter.isConverting()); + CORRADE_VERIFY(!converter.abortCalled); + + /* Should be called if calling beginData() while another conversion is + already happening. Then, what is happening is the new conversion. */ + CORRADE_VERIFY(converter.beginFile(Utility::Path::join(TRADE_TEST_OUTPUT_DIR, "mesh.out"))); + CORRADE_VERIFY(converter.isConverting()); + CORRADE_VERIFY(converter.abortCalled); +} + void AbstractSceneConverterTest::thingNoBegin() { CORRADE_SKIP_IF_NO_ASSERT();