Trade: implicitly abort previous conversion in all scene converter APIs.

And properly test the behavior as well. While it's rare that a batch
and immediate conversions would be mixed on a single converter instance,
if it happens by accident it should not lead to unexpected internal
state.

src/Magnum/Trade/AbstractSceneConverter.cpp

@@ -188,6 +188,8 @@ void AbstractSceneConverter::clearFlags(SceneConverterFlags flags) {
 }
 
 Containers::Optional<MeshData> AbstractSceneConverter::convert(const MeshData& mesh) {
+    abort();
+
     CORRADE_ASSERT(features() & SceneConverterFeature::ConvertMesh,
         "Trade::AbstractSceneConverter::convert(): mesh conversion not supported", {});
 
@@ -205,6 +207,8 @@ Containers::Optional<MeshData> 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<Containers::Array<char>>
 Implementation::SceneConverterOptionalButAlsoArray<char>
 #endif
 AbstractSceneConverter::convertToData(const MeshData& mesh) {
+    abort();
+
     if(features() >= SceneConverterFeature::ConvertMeshToData) {
         Containers::Optional<Containers::Array<char>> out = doConvertToData(mesh);
         CORRADE_ASSERT(!out || !out->deleter() || out->deleter() == static_cast<void(*)(char*, std::size_t)>(Implementation::nonOwnedArrayDeleter) || out->deleter() == ArrayAllocator<char>::deleter,
@@ -251,6 +257,8 @@ Containers::Optional<Containers::Array<char>> 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<AbstractImporter> AbstractSceneConverter::doEnd() {
 }
 
 bool AbstractSceneConverter::beginData() {
-    if(_state) abort();
+    abort();
 
     _state.emplace(State::Type::ConvertToData);
 
@@ -442,7 +450,7 @@ Containers::Optional<Containers::Array<char>> AbstractSceneConverter::doEndData(
 }
 
 bool AbstractSceneConverter::beginFile(const Containers::StringView filename) {
-    if(_state) abort();
+    abort();
 
     _state.emplace(State::Type::ConvertToFile);
     _state->filename = Containers::String::nullTerminatedGlobalView(filename);

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<const MeshData>, 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<MeshData> 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<Containers::Array<char>>
@@ -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);
 

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<MeshData> 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<Containers::Array<char>> doConvertToData(const MeshData&) override {
+            return Containers::Array<char>{};
+        }
+
+        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();