From 9cd03baab3ae9ad2a2c71e41a0e745d36f0d8b14 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= <mosra@centrum.cz>
Date: Thu, 18 May 2023 12:44:14 +0200
Subject: [PATCH] Trade: allow calling setMeshAttributeName() on single-mesh
 converters.

It's useful there as well, for example the StanfordSceneConverter
implements just single-mesh conversion but is capable of having the
attributes named in a custom way (although it's not implemented at the
moment). Or a mesh optimization plugin can have specialized behavior for
custom attributes but only if it knows what they are.
---
 src/Magnum/Trade/AbstractSceneConverter.cpp   | 13 +++++--
 src/Magnum/Trade/AbstractSceneConverter.h     | 16 +++++----
 .../Trade/Test/AbstractSceneConverterTest.cpp | 36 ++++++++++++++-----
 3 files changed, 49 insertions(+), 16 deletions(-)

diff --git a/src/Magnum/Trade/AbstractSceneConverter.cpp b/src/Magnum/Trade/AbstractSceneConverter.cpp
index b363312a0..fe968e56d 100644
--- a/src/Magnum/Trade/AbstractSceneConverter.cpp
+++ b/src/Magnum/Trade/AbstractSceneConverter.cpp
@@ -855,9 +855,18 @@ bool AbstractSceneConverter::doAdd(UnsignedInt, const Containers::Iterable<const
 }
 
 void AbstractSceneConverter::setMeshAttributeName(const MeshAttribute attribute, const Containers::StringView name) {
-    CORRADE_ASSERT(features() & SceneConverterFeature::AddMeshes,
+    CORRADE_ASSERT(features() & (SceneConverterFeature::AddMeshes|
+                                 SceneConverterFeature::ConvertMesh|
+                                 SceneConverterFeature::ConvertMeshInPlace|
+                                 SceneConverterFeature::ConvertMeshToData|
+                                 SceneConverterFeature::ConvertMeshToFile),
         "Trade::AbstractSceneConverter::setMeshAttributeName(): feature not supported", );
-    CORRADE_ASSERT(_state,
+    /* Unless single mesh conversion is supported, allow this function to be
+       called only if begin*() was called before */
+    CORRADE_ASSERT(features() & (SceneConverterFeature::ConvertMesh|
+                                 SceneConverterFeature::ConvertMeshInPlace|
+                                 SceneConverterFeature::ConvertMeshToData|
+                                 SceneConverterFeature::ConvertMeshToFile) || _state,
         "Trade::AbstractSceneConverter::setMeshAttributeName(): no conversion in progress", );
     CORRADE_ASSERT(isMeshAttributeCustom(attribute),
         "Trade::AbstractSceneConverter::setMeshAttributeName():" << attribute << "is not custom", );
diff --git a/src/Magnum/Trade/AbstractSceneConverter.h b/src/Magnum/Trade/AbstractSceneConverter.h
index a05a42537..86267ab0f 100644
--- a/src/Magnum/Trade/AbstractSceneConverter.h
+++ b/src/Magnum/Trade/AbstractSceneConverter.h
@@ -1500,12 +1500,16 @@ class MAGNUM_TRADE_EXPORT AbstractSceneConverter: public PluginManager::Abstract
          * @brief Set name of a custom mesh attribute
          * @m_since_latest
          *
-         * Expects that a conversion is currently in progress,
-         * @ref SceneConverterFeature::AddMeshes is supported and @p attribute
-         * is a custom attribute. The attribute name will get used only for
-         * mesh data added after this function has been called. If the
-         * converter doesn't support custom mesh attributes or doesn't support
-         * naming them, the call is ignored.
+         * Expects that either a batch conversion is currently in progress and
+         * @ref SceneConverterFeature::AddMeshes is supported, or that at least
+         * one of @ref SceneConverterFeature::ConvertMesh,
+         * @relativeref{SceneConverterFeature,ConvertMeshInPlace},
+         * @relativeref{SceneConverterFeature,ConvertMeshToData} or
+         * @relativeref{SceneConverterFeature,ConvertMeshToFile} is supported.
+         * The @p attribute is expected to be custom. The name will get used
+         * only for mesh data added or converted after this function has been
+         * called. If the converter doesn't support custom mesh attributes or
+         * doesn't support naming them, the call is ignored.
          * @see @ref isConverting(), @ref features(),
          *      @ref isMeshAttributeCustom(), @ref setSceneFieldName(),
          *      @ref setAnimationTrackTargetName()
diff --git a/src/Magnum/Trade/Test/AbstractSceneConverterTest.cpp b/src/Magnum/Trade/Test/AbstractSceneConverterTest.cpp
index 450806710..f494918ef 100644
--- a/src/Magnum/Trade/Test/AbstractSceneConverterTest.cpp
+++ b/src/Magnum/Trade/Test/AbstractSceneConverterTest.cpp
@@ -318,6 +318,16 @@ struct AbstractSceneConverterTest: TestSuite::Tester {
 
 using namespace Containers::Literals;
 
+const struct {
+    const char* name;
+    SceneConverterFeatures features;
+} SetMeshAttributeData[]{
+    {"batch conversion supported", SceneConverterFeature::ConvertMultiple|SceneConverterFeature::AddMeshes},
+    {"single conversion supported", SceneConverterFeature::ConvertMesh},
+    {"single to file conversion supported", SceneConverterFeature::ConvertMeshToFile},
+    {"in-place conversion supported", SceneConverterFeature::ConvertMeshInPlace},
+};
+
 const struct {
     TestSuite::TestCaseDescriptionSourceLocation name;
     SceneContents contents;
@@ -763,10 +773,12 @@ AbstractSceneConverterTest::AbstractSceneConverterTest() {
               &AbstractSceneConverterTest::addMeshLevelsNoLevels,
               &AbstractSceneConverterTest::addMeshLevelsNotImplemented,
 
-              &AbstractSceneConverterTest::addMeshThroughLevels,
+              &AbstractSceneConverterTest::addMeshThroughLevels});
 
-              &AbstractSceneConverterTest::setMeshAttributeName,
-              &AbstractSceneConverterTest::setMeshAttributeNameNotImplemented,
+    addInstancedTests({&AbstractSceneConverterTest::setMeshAttributeName},
+        Containers::arraySize(SetMeshAttributeData));
+
+    addTests({&AbstractSceneConverterTest::setMeshAttributeNameNotImplemented,
               &AbstractSceneConverterTest::setMeshAttributeNameNotCustom,
 
               &AbstractSceneConverterTest::addMaterial,
@@ -4190,10 +4202,14 @@ void AbstractSceneConverterTest::addMeshThroughLevels() {
 }
 
 void AbstractSceneConverterTest::setMeshAttributeName() {
-    struct: AbstractSceneConverter {
+    auto&& data = SetMeshAttributeData[testCaseInstanceId()];
+    setTestCaseDescription(data.name);
+
+    struct Converter: AbstractSceneConverter {
+        explicit Converter(SceneConverterFeatures features): features{features} {}
+
         SceneConverterFeatures doFeatures() const override {
-            return SceneConverterFeature::ConvertMultiple|
-                   SceneConverterFeature::AddMeshes;
+            return features;
         }
 
         bool doBegin() override { return true; }
@@ -4204,12 +4220,16 @@ void AbstractSceneConverterTest::setMeshAttributeName() {
             setMeshAttributeNameCalled = true;
         }
 
+        SceneConverterFeatures features;
         bool setMeshAttributeNameCalled = false;
-    } converter;
+    } converter{data.features};
 
     CORRADE_VERIFY(true); /* capture correct function name */
 
-    CORRADE_VERIFY(converter.begin());
+    /* Only single-mesh conversion can call the API alone */
+    if(data.features & SceneConverterFeature::ConvertMultiple)
+        CORRADE_VERIFY(converter.begin());
+
     converter.setMeshAttributeName(meshAttributeCustom(1337), "hello!");
     CORRADE_VERIFY(converter.setMeshAttributeNameCalled);
 }