From b91b0b24cde81a4eb9029cb2202b3a652c9bd0e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sun, 26 Nov 2023 21:16:05 +0100 Subject: [PATCH] sceneconverter: add --remove-duplicate-materials. Amazing, I haven't been in this corner of the codebase for over six months but the test setup and code coverage still makes it quite straightforward to add a rather complex new feature without breaking unrelated cases or leaving certain areas completely untested. --- src/Magnum/SceneTools/Test/CMakeLists.txt | 6 +- .../SceneTools/Test/SceneConverterTest.cpp | 88 +++++++++++- .../broken-mesh-with-scene.gltf | 24 ++++ .../ignoring-unsupported.gltf | 3 + .../materials-duplicate-removed.gltf | 129 ++++++++++++++++++ .../materials-duplicate.gltf | 103 ++++++++++++++ .../SceneConverterTestFiles/two-scenes.gltf | 16 +++ src/Magnum/SceneTools/sceneconverter.cpp | 121 +++++++++++++++- 8 files changed, 480 insertions(+), 10 deletions(-) create mode 100644 src/Magnum/SceneTools/Test/SceneConverterTestFiles/broken-mesh-with-scene.gltf create mode 100644 src/Magnum/SceneTools/Test/SceneConverterTestFiles/materials-duplicate-removed.gltf create mode 100644 src/Magnum/SceneTools/Test/SceneConverterTestFiles/materials-duplicate.gltf create mode 100644 src/Magnum/SceneTools/Test/SceneConverterTestFiles/two-scenes.gltf diff --git a/src/Magnum/SceneTools/Test/CMakeLists.txt b/src/Magnum/SceneTools/Test/CMakeLists.txt index dae7aa355..c13c0dac7 100644 --- a/src/Magnum/SceneTools/Test/CMakeLists.txt +++ b/src/Magnum/SceneTools/Test/CMakeLists.txt @@ -109,6 +109,7 @@ if(CORRADE_TARGET_UNIX AND NOT CORRADE_PLUGINMANAGER_NO_DYNAMIC_PLUGIN_SUPPORT) SceneConverterTestFiles/broken-image-3d.gltf SceneConverterTestFiles/broken-material.gltf SceneConverterTestFiles/broken-mesh.obj + SceneConverterTestFiles/broken-mesh-with-scene.gltf SceneConverterTestFiles/broken-scene.gltf SceneConverterTestFiles/dxt1.dds SceneConverterTestFiles/empty.gltf @@ -133,6 +134,8 @@ if(CORRADE_TARGET_UNIX AND NOT CORRADE_PLUGINMANAGER_NO_DYNAMIC_PLUGIN_SUPPORT) SceneConverterTestFiles/info-importer-ignored-input-output.txt SceneConverterTestFiles/info-preferred-importer-plugin.txt SceneConverterTestFiles/materials-3d.gltf + SceneConverterTestFiles/materials-duplicate.gltf + SceneConverterTestFiles/materials-duplicate-removed.gltf SceneConverterTestFiles/materials-pbr.gltf SceneConverterTestFiles/materials-phong.mtl SceneConverterTestFiles/materials-phong.obj @@ -167,7 +170,8 @@ if(CORRADE_TARGET_UNIX AND NOT CORRADE_PLUGINMANAGER_NO_DYNAMIC_PLUGIN_SUPPORT) SceneConverterTestFiles/two-triangles-transformed.bin SceneConverterTestFiles/two-triangles-transformed.gltf SceneConverterTestFiles/two-triangles-transformed-no-default-scene.gltf - SceneConverterTestFiles/two-triangles.obj) + SceneConverterTestFiles/two-triangles.obj + SceneConverterTestFiles/two-scenes.gltf) target_include_directories(SceneToolsSceneConverterTest PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/$) if(MAGNUM_WITH_SCENECONVERTER) add_dependencies(SceneToolsSceneConverterTest magnum-sceneconverter) diff --git a/src/Magnum/SceneTools/Test/SceneConverterTest.cpp b/src/Magnum/SceneTools/Test/SceneConverterTest.cpp index da1418bf2..5b6050102 100644 --- a/src/Magnum/SceneTools/Test/SceneConverterTest.cpp +++ b/src/Magnum/SceneTools/Test/SceneConverterTest.cpp @@ -791,6 +791,24 @@ const struct { "Trade::AbstractSceneConverter::addImporterContents(): adding mesh 0 out of 2\n" "Trade::AbstractSceneConverter::addImporterContents(): adding mesh 1 out of 2\n" "Trade::AbstractSceneConverter::addImporterContents(): adding scene 0 out of 1\n"}, + {"remove duplicate materials", {InPlaceInit, { + "-I" "GltfImporter", "-C", "GltfSceneConverter", "--remove-duplicate-materials", "-c", "generator=", + Utility::Path::join(SCENETOOLS_TEST_DIR, "SceneConverterTestFiles/materials-duplicate.gltf"), Utility::Path::join(SCENETOOLS_TEST_OUTPUT_DIR, "SceneConverterTestFiles/materials-duplicate-removed.gltf") + }}, + "GltfImporter", nullptr, "GltfSceneConverter", {}, nullptr, + "materials-duplicate-removed.gltf", nullptr, + {}}, + {"remove duplicate materials, verbose", {InPlaceInit, { + /* Same as above, just with -v added */ + "-I" "GltfImporter", "-C", "GltfSceneConverter", "--remove-duplicate-materials", "-c", "generator=", "-v", + Utility::Path::join(SCENETOOLS_TEST_DIR, "SceneConverterTestFiles/materials-duplicate.gltf"), Utility::Path::join(SCENETOOLS_TEST_OUTPUT_DIR, "SceneConverterTestFiles/materials-duplicate-removed.gltf") + }}, + "GltfImporter", nullptr, "GltfSceneConverter", {}, nullptr, + "materials-duplicate-removed.gltf", nullptr, + "Duplicate material removal: 3 -> 2 materials\n" + "Trade::AbstractSceneConverter::addImporterContents(): adding mesh 0 out of 3\n" + "Trade::AbstractSceneConverter::addImporterContents(): adding mesh 1 out of 3\n" + "Trade::AbstractSceneConverter::addImporterContents(): adding mesh 2 out of 3\n"}, {"data unsupported by the converter", {InPlaceInit, { "-I", "GltfImporter", "-i", "experimentalKhrTextureKtx", "-C", "StanfordSceneConverter", @@ -803,7 +821,8 @@ const struct { "Trade::AbstractSceneConverter::addSupportedImporterContents(): ignoring 1 2D images not supported by the converter\n" "Trade::AbstractSceneConverter::addSupportedImporterContents(): ignoring 1 3D images not supported by the converter\n" "Trade::AbstractSceneConverter::addSupportedImporterContents(): ignoring 2 textures not supported by the converter\n" - "Trade::AbstractSceneConverter::addSupportedImporterContents(): ignoring 1 materials not supported by the converter\n"}, + "Trade::AbstractSceneConverter::addSupportedImporterContents(): ignoring 1 materials not supported by the converter\n" + "Trade::AbstractSceneConverter::addSupportedImporterContents(): ignoring 1 scenes not supported by the converter\n"}, {"per-image processed images unsupported by the converter", {InPlaceInit, { "-I", "GltfImporter", "-i", "experimentalKhrTextureKtx", "-P", "StbResizeImageConverter", "-p", "size=\"1 1\"", @@ -819,7 +838,8 @@ const struct { "Ignoring 1 2D images not supported by the converter\n" "Ignoring 1 3D images not supported by the converter\n" "Trade::AbstractSceneConverter::addSupportedImporterContents(): ignoring 2 textures not supported by the converter\n" - "Trade::AbstractSceneConverter::addSupportedImporterContents(): ignoring 1 materials not supported by the converter\n"}, + "Trade::AbstractSceneConverter::addSupportedImporterContents(): ignoring 1 materials not supported by the converter\n" + "Trade::AbstractSceneConverter::addSupportedImporterContents(): ignoring 1 scenes not supported by the converter\n"}, {"per-material processed materials unsupported by the converter", {InPlaceInit, { "-I", "GltfImporter", "-i", "experimentalKhrTextureKtx", "--phong-to-pbr", "-C", "StanfordSceneConverter", @@ -834,7 +854,28 @@ const struct { "Trade::AbstractSceneConverter::addSupportedImporterContents(): ignoring 2 textures not supported by the converter\n" /* Compared to "data unsupported by the converter" this message is printed by sceneconverter itself, not the converter interface */ - "Ignoring 1 materials not supported by the converter\n"}, + "Ignoring 1 materials not supported by the converter\n" + "Trade::AbstractSceneConverter::addSupportedImporterContents(): ignoring 1 scenes not supported by the converter\n"}, + {"per-scene processed scenes unsupported by the converter", {InPlaceInit, { + "-I", "GltfImporter", "-i", "experimentalKhrTextureKtx", + "--remove-duplicate-materials", "-C", "StanfordSceneConverter", + Utility::Path::join(SCENETOOLS_TEST_DIR, "SceneConverterTestFiles/ignoring-unsupported.gltf"), + Utility::Path::join(SCENETOOLS_TEST_OUTPUT_DIR, "SceneConverterTestFiles/quad.ply") + }}, + "GltfImporter", "KtxImporter", "StanfordSceneConverter", + {"StbResizeImageConverter", nullptr}, nullptr, + "quad.ply", nullptr, + "Trade::AbstractSceneConverter::addSupportedImporterContents(): ignoring 1 2D images not supported by the converter\n" + "Trade::AbstractSceneConverter::addSupportedImporterContents(): ignoring 1 3D images not supported by the converter\n" + "Trade::AbstractSceneConverter::addSupportedImporterContents(): ignoring 2 textures not supported by the converter\n" + /* Compared to "data unsupported by the converter" these messages are + printed by sceneconverter itself, not the converter interface. Both + scenes and materials get imported directly for duplicate material + removal. */ + /** @todo switch to something that only touches scenes and not + materials once it exists */ + "Ignoring 1 materials not supported by the converter\n" + "Ignoring 1 scenes not supported by the converter\n"}, {"preferred importer plugin", {InPlaceInit, { /* First is not found, second should be always found, third might be also but shouldn't be picked. The trailing comma should be allowed, using the plugin itself in the list should work too. */ @@ -1323,7 +1364,46 @@ const struct { "UfbxImporter", "PngImporter", "GltfSceneConverter", nullptr, "Trade::GltfSceneConverter::add(): unsupported R/R packing of a metallic/roughness texture\n" - "Cannot add material 1\n"} + "Cannot add material 1\n"}, + {"can't load a scene for material deduplication", {InPlaceInit, { + "-I", "GltfImporter", "--remove-duplicate-materials", + Utility::Path::join(SCENETOOLS_TEST_DIR, "SceneConverterTestFiles/broken-scene.gltf"), + Utility::Path::join(SCENETOOLS_TEST_OUTPUT_DIR, "SceneConverterTestFiles/whatever.gltf") + }}, + "GltfImporter", nullptr, nullptr, + nullptr, + "Trade::GltfImporter::scene(): mesh index 1 in node 0 out of range for 1 meshes\n" + "Cannot import scene 0\n"}, + {"can't add scene dependencies", {InPlaceInit, { + /* --remove-duplicate-materials is a no-op because the input has no + materials, we just need something that causes the scenes to be + added directly */ + "-I", "GltfImporter", "--remove-duplicate-materials", + "-C", "GltfSceneConverter", + Utility::Path::join(SCENETOOLS_TEST_DIR, "SceneConverterTestFiles/broken-mesh-with-scene.gltf"), + Utility::Path::join(SCENETOOLS_TEST_OUTPUT_DIR, "SceneConverterTestFiles/whatever.gltf") + }}, + "GltfImporter", nullptr, "GltfSceneConverter", + nullptr, + "Trade::GltfImporter::mesh(): accessor index 0 out of range for 0 accessors\n" + "Cannot add scene dependencies\n"}, + {"can't add processed scene", {InPlaceInit, { + /* --remove-duplicate-materials is a no-op because the input has no + materials, we just need something that causes the scenes to be + added directly */ + "-I", "GltfImporter", "--remove-duplicate-materials", + "-C", "GltfSceneConverter", + /* The input file makes no sense for PrimitiveImporter, it just has + to be something that exists */ + Utility::Path::join(SCENETOOLS_TEST_DIR, "SceneConverterTestFiles/two-scenes.gltf"), + Utility::Path::join(SCENETOOLS_TEST_OUTPUT_DIR, "SceneConverterTestFiles/whatever.gltf") + }}, + "PrimitiveImporter", nullptr, "GltfSceneConverter", + nullptr, + /** @todo find some better case for this, this will pass once + GltfSceneConverter has multi-scene support */ + "Trade::GltfSceneConverter::add(): only one scene is supported at the moment\n" + "Cannot add scene 1\n"}, }; SceneConverterTest::SceneConverterTest() { diff --git a/src/Magnum/SceneTools/Test/SceneConverterTestFiles/broken-mesh-with-scene.gltf b/src/Magnum/SceneTools/Test/SceneConverterTestFiles/broken-mesh-with-scene.gltf new file mode 100644 index 000000000..66ee03a02 --- /dev/null +++ b/src/Magnum/SceneTools/Test/SceneConverterTestFiles/broken-mesh-with-scene.gltf @@ -0,0 +1,24 @@ +{ + "asset": { + "version": "2.0" + }, + "meshes": [ + { + "primitives": [ + { + "indices": 0 + } + ] + } + ], + "nodes": [ + { + "mesh": 0 + } + ], + "scenes": [ + { + "nodes": [0] + } + ] +} diff --git a/src/Magnum/SceneTools/Test/SceneConverterTestFiles/ignoring-unsupported.gltf b/src/Magnum/SceneTools/Test/SceneConverterTestFiles/ignoring-unsupported.gltf index a97dd2d1a..e04654fd3 100644 --- a/src/Magnum/SceneTools/Test/SceneConverterTestFiles/ignoring-unsupported.gltf +++ b/src/Magnum/SceneTools/Test/SceneConverterTestFiles/ignoring-unsupported.gltf @@ -82,5 +82,8 @@ } } } + ], + "scenes": [ + {} ] } diff --git a/src/Magnum/SceneTools/Test/SceneConverterTestFiles/materials-duplicate-removed.gltf b/src/Magnum/SceneTools/Test/SceneConverterTestFiles/materials-duplicate-removed.gltf new file mode 100644 index 000000000..98ddada58 --- /dev/null +++ b/src/Magnum/SceneTools/Test/SceneConverterTestFiles/materials-duplicate-removed.gltf @@ -0,0 +1,129 @@ +{ + "asset": { + "version": "2.0" + }, + "extensionsUsed": [ + "KHR_materials_unlit" + ], + "buffers": [ + { + "uri": "materials-duplicate-removed.bin", + "byteLength": 108 + } + ], + "bufferViews": [ + { + "buffer": 0, + "byteOffset": 0, + "byteLength": 36, + "byteStride": 12, + "target": 34962 + }, + { + "buffer": 0, + "byteOffset": 36, + "byteLength": 36, + "byteStride": 12, + "target": 34962 + }, + { + "buffer": 0, + "byteOffset": 72, + "byteLength": 36, + "byteStride": 12, + "target": 34962 + } + ], + "accessors": [ + { + "bufferView": 0, + "componentType": 5126, + "count": 3, + "type": "VEC3", + "min": [-1, -1, 0], + "max": [1, 1, 0] + }, + { + "bufferView": 1, + "componentType": 5126, + "count": 3, + "type": "VEC3", + "min": [-1, -1, 0], + "max": [1, 1, 0] + }, + { + "bufferView": 2, + "componentType": 5126, + "count": 3, + "type": "VEC3", + "min": [-1, -1, 0], + "max": [1, 1, 0] + } + ], + "meshes": [ + { + "primitives": [ + { + "attributes": { + "POSITION": 0 + }, + "material": 1 + } + ], + "name": "First mesh" + }, + { + "primitives": [ + { + "attributes": { + "POSITION": 2 + }, + "material": 0 + } + ], + "name": "Third mesh that should get a material same as second" + }, + { + "primitives": [ + { + "attributes": { + "POSITION": 1 + }, + "material": 0 + } + ], + "name": "Second mesh, unfortunately the shared verted data get duplicated" + } + ], + "materials": [ + { + "pbrMetallicRoughness": { + "baseColorFactor": [0.5, 0.25, 0.75, 1] + }, + "name": "Duplicate material, this name is preserved" + }, + { + "extensions": { + "KHR_materials_unlit": {} + }, + "name": "Unique material" + } + ], + "nodes": [ + { + "mesh": 0 + }, + { + "mesh": 1 + }, + { + "mesh": 2 + } + ], + "scenes": [ + { + "nodes": [0, 2, 1], + "name": "Scene with three nodes" + } + ] +} diff --git a/src/Magnum/SceneTools/Test/SceneConverterTestFiles/materials-duplicate.gltf b/src/Magnum/SceneTools/Test/SceneConverterTestFiles/materials-duplicate.gltf new file mode 100644 index 000000000..ec24e81b8 --- /dev/null +++ b/src/Magnum/SceneTools/Test/SceneConverterTestFiles/materials-duplicate.gltf @@ -0,0 +1,103 @@ +{ + "asset": { + "version": "2.0" + }, + "buffers": [ + { + "uri": "quad.bin", + "byteLength": 72 + } + ], + "bufferViews": [ + { + "buffer": 0, + "byteOffset": 24, + "byteLength": 36, + "byteStride": 12, + "target": 34962 + } + ], + "accessors": [ + { + "bufferView": 0, + "componentType": 5126, + "count": 3, + "type": "VEC3", + "min": [-1, -1, 0], + "max": [1, 1, 0] + } + ], + "meshes": [ + { + "name": "First mesh", + "primitives": [ + { + "attributes": { + "POSITION": 0 + }, + "material": 1 + } + ] + }, + { + "name": "Second mesh, unfortunately the shared verted data get duplicated", + "primitives": [ + { + "attributes": { + "POSITION": 0 + }, + "material": 0 + } + ] + }, + { + "name": "Third mesh that should get a material same as second", + "primitives": [ + { + "attributes": { + "POSITION": 0 + }, + "material": 2 + } + ] + } + ], + "materials": [ + { + "name": "Duplicate material, this name is preserved", + "pbrMetallicRoughness": { + "baseColorFactor": [0.5, 0.25, 0.75, 1] + } + }, + { + "name": "Unique material", + "extensions": { + "KHR_materials_unlit": {} + } + }, + { + "name": "Duplicate material, this name in't preserved", + "pbrMetallicRoughness": { + "baseColorFactor": [0.5, 0.25, 0.75, 1] + } + } + ], + "nodes": [ + { + "mesh": 0, + "name": "First" + }, + { + "mesh": 2 + }, + { + "mesh": 1 + } + ], + "scenes": [ + { + "nodes": [0, 2, 1], + "name": "Scene with three nodes" + } + ] +} diff --git a/src/Magnum/SceneTools/Test/SceneConverterTestFiles/two-scenes.gltf b/src/Magnum/SceneTools/Test/SceneConverterTestFiles/two-scenes.gltf new file mode 100644 index 000000000..e0e16004f --- /dev/null +++ b/src/Magnum/SceneTools/Test/SceneConverterTestFiles/two-scenes.gltf @@ -0,0 +1,16 @@ +{ + "asset": { + "version": "2.0" + }, + "nodes": [ + {} + ], + "scenes": [ + { + "nodes": [0] + }, + { + "nodes": [0] + } + ] +} diff --git a/src/Magnum/SceneTools/sceneconverter.cpp b/src/Magnum/SceneTools/sceneconverter.cpp index c9d90f60d..06730b69e 100644 --- a/src/Magnum/SceneTools/sceneconverter.cpp +++ b/src/Magnum/SceneTools/sceneconverter.cpp @@ -34,11 +34,13 @@ #include /* parseNumberSequence() */ #include "Magnum/MaterialTools/PhongToPbrMetallicRoughness.h" +#include "Magnum/MaterialTools/RemoveDuplicates.h" #include "Magnum/MeshTools/Concatenate.h" #include "Magnum/MeshTools/Copy.h" #include "Magnum/MeshTools/RemoveDuplicates.h" #include "Magnum/MeshTools/Transform.h" #include "Magnum/SceneTools/Hierarchy.h" +#include "Magnum/SceneTools/Map.h" #include "Magnum/Trade/AbstractImporter.h" #include "Magnum/Trade/MeshData.h" #include "Magnum/Trade/AbstractImageConverter.h" @@ -183,6 +185,7 @@ magnum-sceneconverter [-h|--help] [-I|--importer PLUGIN] [--prefer alias:plugin1,plugin2,…]... [--set plugin:key=val,key2=val2,…]... [--map] [--only-mesh-attributes N1,N2-N3…] [--remove-duplicate-vertices] [--remove-duplicate-vertices-fuzzy EPSILON] [--phong-to-pbr] + [--remove-duplicate-materials] [-i|--importer-options key=val,key2=val2,…] [-c|--converter-options key=val,key2=val2,…]... [-p|--image-converter-options key=val,key2=val2,…]... @@ -226,6 +229,8 @@ Arguments: in all meshes after import - `--phong-to-pbr` --- convert Phong materials to PBR metallic/roughness using @ref MaterialTools::phongToPbrMetallicRoughness() +- `--remove-duplicate-materials` --- remove duplicate materials using + @ref MaterialTools::removeDuplicatesInPlace() - `-i`, `--importer-options key=val,key2=val2,…` --- configuration options to pass to the importer - `-c`, `--converter-options key=val,key2=val2,…` --- configuration options @@ -307,8 +312,9 @@ feature for given image dimensions, all mesh converters in the chain have to support the ConvertMesh feature. If no `-P` / `-M` is specified, the imported images / meshes are passed directly to the scene converter. -The `--remove-duplicate-vertices` and `--phong-to-pbr` operations are performed -on meshes and materials before passing them to any converter. +The `--remove-duplicate-vertices`, `--phong-to-pbr` and +`--remove-duplicate-materials` operations are performed on meshes and materials +before passing them to any converter. If `--concatenate-meshes` is given, all meshes of the input file are first concatenated into a single mesh using @ref MeshTools::concatenate(), with @@ -430,6 +436,7 @@ int main(int argc, char** argv) { .addBooleanOption("remove-duplicate-vertices").setHelp("remove-duplicate-vertices", "remove duplicate vertices in all meshes after import") .addOption("remove-duplicate-vertices-fuzzy").setHelp("remove-duplicate-vertices-fuzzy", "remove duplicate vertices with fuzzy comparison in all meshes after import", "EPSILON") .addBooleanOption("phong-to-pbr").setHelp("phong-to-pbr", "convert Phong materials to PBR metallic/roughness") + .addBooleanOption("remove-duplicate-materials").setHelp("remove-duplicate-materials", "remove duplicate materials") .addOption('i', "importer-options").setHelp("importer-options", "configuration options to pass to the importer", "key=val,key2=val2,…") .addArrayOption('c', "converter-options").setHelp("converter-options", "configuration options to pass to the converter(s)", "key=val,key2=val2,…") .addArrayOption('p', "image-converter-options").setHelp("image-converter-options", "configuration options to pass to the image converter(s)", "key=val,key2=val2,…") @@ -508,8 +515,9 @@ feature for given image dimensions, all mesh converters in the chain have to support the ConvertMesh feature. If no -P / -M is specified, the imported images / meshes are passed directly to the scene converter. -The --remove-duplicate-vertices and --phong-to-pbr operations are performed on -meshes and materials before passing them to any converter. +The --remove-duplicate-vertices, --phong-to-pbr and +--remove-duplicate-materials operations are performed on meshes and materials +before passing them to any converter. If --concatenate-meshes is given, all meshes of the input file are first concatenated into a single mesh, with the scene hierarchy transformation baked @@ -783,6 +791,28 @@ well, the IDs reference attributes of the first mesh.)") /* Wow, C++, you suck. This implicitly initializes to random shit?! */ std::chrono::high_resolution_clock::duration conversionTime{}; + /* Import all scenes, in case something later needs to modify them. There's + currently no other operations done on those. */ + Containers::Array scenes; + if(args.isSet("remove-duplicate-materials")) { + arrayReserve(scenes, importer->sceneCount()); + + for(UnsignedInt i = 0; i != importer->sceneCount(); ++i) { + Containers::Optional scene; + { + Trade::Implementation::Duration d{importConversionTime}; + if(!(scene = importer->scene(i))) { + Error{} << "Cannot import scene" << i; + return 1; + } + } + + /* There's currently no operations done on scenes directly */ + + arrayAppend(scenes, *Utility::move(scene)); + } + } + /* Take a single mesh or concatenate all meshes together, if requested. After that, the importer is changed to one that contains just a single mesh... */ @@ -1099,7 +1129,9 @@ well, the IDs reference attributes of the first mesh.)") any, materials are supplied manually to the converter from the array below. */ Containers::Array materials; - if(args.isSet("phong-to-pbr")) { + if(args.isSet("phong-to-pbr") || + args.isSet("remove-duplicate-materials")) + { arrayReserve(materials, importer->materialCount()); for(UnsignedInt i = 0; i != importer->materialCount(); ++i) { @@ -1126,6 +1158,33 @@ well, the IDs reference attributes of the first mesh.)") arrayAppend(materials, *Utility::move(material)); } + + /* Duplicate removal */ + if(args.isSet("remove-duplicate-materials")) { + Trade::Implementation::Duration d{conversionTime}; + + Containers::Pair, std::size_t> mapping = MaterialTools::removeDuplicatesInPlace(materials); + if(args.isSet("verbose")) + Debug{} << "Duplicate material removal:" << materials.size() << "->" << mapping.second() << "materials"; + + arrayRemoveSuffix(materials, materials.size() - mapping.second()); + + /* Remap scene material references. The scenes should have been + imported for --remove-duplicate-materials above already. */ + CORRADE_INTERNAL_ASSERT(scenes.size() == importer->sceneCount()); + for(Trade::SceneData& scene: scenes) { + if(const Containers::Optional materialFieldId = scene.findFieldId(Trade::SceneField::MeshMaterial)) { + /** @todo handle a case with immutable scene data, once it + exists (PrimitiveImporter is closest, but it doesn't + have materials so it never enters this branch) */ + + /* Deduplication makes the material index range smaller, so + we can map them in-place without having to worry that + the new indices won't fit into existing packed types */ + SceneTools::mapIndexFieldInPlace(scene, *materialFieldId, mapping.first()); + } + } + } } /* Assume there's always one passed --converter option less, and the last @@ -1354,6 +1413,58 @@ well, the IDs reference attributes of the first mesh.)") materials = {}; } + /* If there are any loose scenes from previous conversion steps, add + them directly, and clear the array so the next iteration (if any) + takes them from the importer instead */ + if(scenes) { + /* Scenes may reference almost everything else except skins an + animations (which reference scenes instead), thus we need to add + all that first */ + { + const Trade::SceneContents sceneDependencies = contents & + ~(Trade::SceneContent::Skins2D| + Trade::SceneContent::Skins3D| + Trade::SceneContent::Scenes| + Trade::SceneContent::Animations); + + Trade::Implementation::Duration d{importConversionTime}; + if(!converter->addSupportedImporterContents(*importer, sceneDependencies)) { + Error{} << "Cannot add scene dependencies"; + return 5; + } + + /* Ensure these are not added by addSupportedImporterContents() + again below, except for names -- those should be added as + long as they were in the contents originally. */ + contents &= ~(sceneDependencies & ~Trade::SceneContent::Names); + } + + if(!(Trade::sceneContentsFor(*converter) & Trade::SceneContent::Scenes)) { + Warning{} << "Ignoring" << scenes.size() << "scenes not supported by the converter"; + } else for(UnsignedInt i = 0; i != scenes.size(); ++i) { + Trade::Implementation::Duration d{conversionTime}; + + if(!converter->add(scenes[i], contents & Trade::SceneContent::Names ? importer->sceneName(i) : Containers::String{})) { + Error{} << "Cannot add scene" << i; + return 1; + } + } + + /* Ensure the scenes are not added by + addSupportedImporterContents() below. Do this also in case the + converter actually doesn't support scene addition, as it would + otherwise cause two warnings about the same "not supported" + thing being printed. */ + contents &= ~Trade::SceneContent::Scenes; + + /* Delete the list to avoid adding them again for the next + converter (at which point they would be stale) */ + /** @todo this line is untested, needs two chained conversion steps + that each change the output to verify the old scenes don't get + reused in the next step again */ + scenes = {}; + } + { Trade::Implementation::Duration d{importConversionTime}; if(!converter->addSupportedImporterContents(*importer, contents)) {