sceneconverter: propagate names and custom attributes of processed meshes.

It's done by addSupportedImporterContents() only if they aren't
processed explicitly and passed directly.

src/Magnum/SceneTools/Test/CMakeLists.txt

@@ -79,6 +79,10 @@ corrade_add_test(SceneToolsSceneConverterTest SceneConverterTest.cpp
         SceneConverterTestFiles/quad-duplicates-fuzzy.obj
         SceneConverterTestFiles/quad-duplicates.obj
         SceneConverterTestFiles/quad-duplicates.ply
+        SceneConverterTestFiles/quad-name-custom-attributes-duplicates.bin
+        SceneConverterTestFiles/quad-name-custom-attributes-duplicates.gltf
+        SceneConverterTestFiles/quad-name-custom-attributes.bin
+        SceneConverterTestFiles/quad-name-custom-attributes.gltf
         SceneConverterTestFiles/quad-normals-texcoords.obj
         SceneConverterTestFiles/quad-strip.bin
         SceneConverterTestFiles/quad-strip.gltf

src/Magnum/SceneTools/Test/SceneConverterTest.cpp

@@ -390,6 +390,30 @@ const struct {
         "    65536 -> 65536 covered pixels\n"
         "    overdraw 1 -> 1\n"
         "Trade::AnySceneConverter::beginFile(): using StanfordSceneConverter\n"},
+    {"implicit custom-processed mesh with a name and custom attributes", Containers::array<Containers::String>({
+        /* Removing the generator identifier to have the file closer to the
+           original */
+        "--remove-duplicate-vertices", "-c", "generator=",
+        Utility::Path::join(SCENETOOLS_TEST_DIR, "SceneConverterTestFiles/quad-name-custom-attributes-duplicates.gltf"), Utility::Path::join(SCENETOOLS_TEST_OUTPUT_DIR, "SceneConverterTestFiles/quad-name-custom-attributes.gltf")}),
+        "GltfImporter", "GltfSceneConverter",
+        /* The output should be mostly the same, except that there's now only 4
+           vertices instead of 6. The code that adds meshes manually instead of
+           using addSupportedImporterContents() should take care of propagating
+           mesh names and custom attributes as well. */
+        "quad-name-custom-attributes.gltf", "quad-name-custom-attributes.bin",
+        {}},
+    {"selected custom-processed mesh with a name and custom attributes", Containers::array<Containers::String>({
+        /* Removing the generator identifier to have the file closer to the
+           original */
+        "--mesh", "0", "--remove-duplicate-vertices", "-c", "generator=",
+        Utility::Path::join(SCENETOOLS_TEST_DIR, "SceneConverterTestFiles/quad-name-custom-attributes-duplicates.gltf"), Utility::Path::join(SCENETOOLS_TEST_OUTPUT_DIR, "SceneConverterTestFiles/quad-name-custom-attributes.gltf")}),
+        "GltfImporter", "GltfSceneConverter",
+        /* The output should be mostly the same, except that there's now only 4
+           vertices instead of 6. The code that adds meshes manually instead of
+           using addSupportedImporterContents() should take care of propagating
+           mesh names and custom attributes as well. */
+        "quad-name-custom-attributes.gltf", "quad-name-custom-attributes.bin",
+        {}},
 };
 
 const struct {

src/Magnum/SceneTools/Test/SceneConverterTestFiles/quad-name-custom-attributes-duplicates.bin.in

@@ -0,0 +1,19 @@
+type = "6H 3f3f3f 3f3f3f"
+input = [
+    # 2 3--5
+    # |\ \ |
+    # | \ \|
+    # 0--1 4
+
+    0, 1, 2, 3, 4, 5,
+
+    -1, -1, 0,
+     1, -1, 0,
+    -1,  1, 0,
+
+    -1,  1, 0,
+     1, -1, 0,
+     1,  1, 0,
+]
+
+# kate: hl python

src/Magnum/SceneTools/Test/SceneConverterTestFiles/quad-name-custom-attributes-duplicates.gltf

@@ -0,0 +1,50 @@
+{
+  "asset": {
+    "version": "2.0"
+  },
+  "buffers": [
+    {
+      "uri": "quad-name-custom-attributes-duplicates.bin",
+      "byteLength": 72
+    }
+  ],
+  "bufferViews": [
+    {
+      "buffer": 0,
+      "byteOffset": 0,
+      "byteLength": 12
+    },
+    {
+      "buffer": 0,
+      "byteOffset": 12,
+      "byteLength": 72
+    }
+  ],
+  "accessors": [
+    {
+      "bufferView": 0,
+      "componentType": 5123,
+      "count": 6,
+      "type": "SCALAR"
+    },
+    {
+      "bufferView": 1,
+      "componentType": 5126,
+      "count": 6,
+      "type": "VEC3"
+    }
+  ],
+  "meshes": [
+    {
+      "primitives": [
+        {
+          "indices": 0,
+          "attributes": {
+            "_CUSTOM_ATTRIBUTE": 1
+          }
+        }
+      ],
+      "name": "A mesh where POSITION is _CUSTOM_ATTRIBUTE for some reason"
+    }
+  ]
+}

src/Magnum/SceneTools/Test/SceneConverterTestFiles/quad-name-custom-attributes.gltf

@@ -0,0 +1,50 @@
+{
+  "asset": {
+    "version": "2.0"
+  },
+  "buffers": [
+    {
+      "uri": "quad-name-custom-attributes.bin",
+      "byteLength": 60
+    }
+  ],
+  "bufferViews": [
+    {
+      "buffer": 0,
+      "byteOffset": 0,
+      "byteLength": 12
+    },
+    {
+      "buffer": 0,
+      "byteOffset": 12,
+      "byteLength": 48
+    }
+  ],
+  "accessors": [
+    {
+      "bufferView": 0,
+      "componentType": 5123,
+      "count": 6,
+      "type": "SCALAR"
+    },
+    {
+      "bufferView": 1,
+      "componentType": 5126,
+      "count": 4,
+      "type": "VEC3"
+    }
+  ],
+  "meshes": [
+    {
+      "primitives": [
+        {
+          "indices": 0,
+          "attributes": {
+            "_CUSTOM_ATTRIBUTE": 1
+          }
+        }
+      ],
+      "name": "A mesh where POSITION is _CUSTOM_ATTRIBUTE for some reason"
+    }
+  ]
+}

src/Magnum/SceneTools/sceneconverter.cpp

@@ -513,25 +513,59 @@ the first mesh.)")
                 vertexCount};
         }
 
-        /* Create an importer instance that contains just the single mesh for
+        /* Create an importer instance that contains just the single mesh and
-           further step, without any other data. Simpler than having to
+           related metadata for further steps, without anything else */
-           special-case the single-mesh case in all following steps. */
+        /** @todo might be useful to have this split out of the file and tested
+            directly if the complexity grows even further */
         struct SingleMeshImporter: Trade::AbstractImporter {
-            explicit SingleMeshImporter(Trade::MeshData&& mesh): mesh{std::move(mesh)} {}
+            explicit SingleMeshImporter(Trade::MeshData&& mesh_, Containers::String&& name, Trade::AbstractImporter& original): mesh{std::move(mesh_)}, name{std::move(name)} {
+                for(UnsignedInt i = 0; i != mesh.attributeCount(); ++i) {
+                    const Trade::MeshAttribute name = mesh.attributeName(i);
+                    if(!isMeshAttributeCustom(name)) continue;
+                    /* Appending even empty ones so we don't have to
+                       special-case "not found" in doMeshAttributeName() */
+                    arrayAppend(attributeNames, InPlaceInit, meshAttributeCustom(name), original.meshAttributeName(name));
+                }
+            }
 
             Trade::ImporterFeatures doFeatures() const override { return {}; } /* LCOV_EXCL_LINE */
             bool doIsOpened() const override { return true; }
             void doClose() override {} /* LCOV_EXCL_LINE */
 
             UnsignedInt doMeshCount() const override { return 1; }
+            Containers::String doMeshName(UnsignedInt) override {
+                return name;
+            }
+            Containers::String doMeshAttributeName(UnsignedShort name) override {
+                for(const Containers::Pair<UnsignedShort, Containers::String>& i: attributeNames)
+                    if(i.first() == name) return i.second();
+                /* All custom attributes, including the unnamed, are in the
+                   attributeNames array and both our attribute name propagation
+                   here and addSupportedImporterContents() call
+                   meshAttributeName() only with attributes present in the
+                   actual mesh, so this should never be reached */
+                CORRADE_INTERNAL_ASSERT_UNREACHABLE();
+            }
             Containers::Optional<Trade::MeshData> doMesh(UnsignedInt, UnsignedInt) override {
                 return MeshTools::reference(mesh);
             }
 
             Trade::MeshData mesh;
+            Containers::String name;
+            Containers::Array<Containers::Pair<UnsignedShort, Containers::String>> attributeNames;
         };
 
-        importer.emplace<SingleMeshImporter>(*std::move(mesh));
+        /* Save the previous importer so we can pass it to the constructor in
+           emplace(), otherwise the constructor would access a deleted pointer */
+        /** @todo would it make sense for emplace() to first construct and only
+            then delete so I wouldn't need to juggle the previous value
+            manually? */
+        Containers::Pointer<Trade::AbstractImporter> previousImporter = std::move(importer);
+        importer.emplace<SingleMeshImporter>(*std::move(mesh),
+            /* Propagate the name only in case of a single mesh, for
+               concatenation it wouldn't make sense */
+            args.value<Containers::StringView>("mesh") ? previousImporter->meshName(args.value<UnsignedInt>("mesh")) : Containers::String{},
+            *previousImporter);
     }
 
     /* Operations to perform on all meshes in the importer. If there are any,
@@ -658,7 +692,28 @@ the first mesh.)")
                 Warning{} << "Ignoring" << meshes.size() << "meshes not supported by the converter";
             } else for(UnsignedInt i = 0; i != meshes.size(); ++i) {
                 Trade::Implementation::Duration d{conversionTime};
-                if(!converter->add(meshes[i])) {
+
+                const Trade::MeshData& mesh = meshes[i];
+
+                /* Propagate custom attribute names, skip ones that are empty.
+                   Compared to data names this is done always to avoid
+                   information loss. */
+                for(UnsignedInt j = 0; j != mesh.attributeCount(); ++j) {
+                    /** @todo have some kind of a map to not have to query the
+                        same custom attribute again for each mesh */
+                    const Trade::MeshAttribute name = mesh.attributeName(j);
+                    if(!isMeshAttributeCustom(name)) continue;
+                    /* The expectation here is that the meshes are coming from
+                       the importer instance. If --mesh or --concatenate-meshes
+                       was used, the original importer is replaced a new one
+                       containing just one mesh, so in that case it works
+                       too. */
+                    if(const Containers::String nameString = importer->meshAttributeName(name)) {
+                        converter->setMeshAttributeName(name, nameString);
+                    }
+                }
+
+                if(!converter->add(mesh, contents & Trade::SceneContent::Names ? importer->meshName(i) : Containers::String{})) {
                     Error{} << "Cannot add mesh" << i;
                     return 1;
                 }