From 13d0b0b1c7f9ef3b2a8000096e96f683d1fba94b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Tue, 16 May 2023 15:30:19 +0200 Subject: [PATCH] sceneconverter: add a --prefer option for plugin overrides. The main use case is being able to specify what concrete plugin gets used for a particular alias, e.g. to be able to use SpngImporter instead of PngImporter for faster PNG image loading. So far the option is implemented only here, as the imageconverter, shaderconverter and other tools don't really deal with plugins that delegate to other plugins. Yet. --- doc/changelog.dox | 3 + src/Magnum/SceneTools/Test/CMakeLists.txt | 2 + .../SceneTools/Test/SceneConverterTest.cpp | 107 +++++++++++++++++- .../info-preferred-importer-plugin.txt | 4 + .../Test/SceneConverterTestFiles/rgba.png | Bin 0 -> 92 bytes src/Magnum/SceneTools/sceneconverter.cpp | 59 +++++++++- 6 files changed, 172 insertions(+), 3 deletions(-) create mode 100644 src/Magnum/SceneTools/Test/SceneConverterTestFiles/info-preferred-importer-plugin.txt create mode 100644 src/Magnum/SceneTools/Test/SceneConverterTestFiles/rgba.png diff --git a/doc/changelog.dox b/doc/changelog.dox index bf3582592..791d2b81f 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -666,6 +666,9 @@ See also: `--info-materials`, `--info-meshes`, `--info-skins` and `--info-textures` for printing information just about particular data type, with `--info` being a shortcut for all specified together +- Added a `--prefer` option to @ref magnum-sceneconverter "magnum-sceneconverter", + allowing to specify what plugins should be preferred for particular import + and conversion plugin aliases @subsubsection changelog-latest-changes-shaders Shaders library diff --git a/src/Magnum/SceneTools/Test/CMakeLists.txt b/src/Magnum/SceneTools/Test/CMakeLists.txt index be7c6c71a..7ce43e1f2 100644 --- a/src/Magnum/SceneTools/Test/CMakeLists.txt +++ b/src/Magnum/SceneTools/Test/CMakeLists.txt @@ -124,6 +124,7 @@ if(CORRADE_TARGET_UNIX AND NOT CORRADE_PLUGINMANAGER_NO_DYNAMIC_PLUGIN_SUPPORT) SceneConverterTestFiles/info-image-converter.txt SceneConverterTestFiles/info-importer.txt SceneConverterTestFiles/info-importer-ignored-input-output.txt + SceneConverterTestFiles/info-preferred-importer-plugin.txt SceneConverterTestFiles/materials-3d.gltf SceneConverterTestFiles/materials-pbr.gltf SceneConverterTestFiles/materials-phong.mtl @@ -149,6 +150,7 @@ if(CORRADE_TARGET_UNIX AND NOT CORRADE_PLUGINMANAGER_NO_DYNAMIC_PLUGIN_SUPPORT) SceneConverterTestFiles/red2x2.png # magnum-imageconverter --layers red2x2.png --array red2x2x1.ktx2 -c writerName= SceneConverterTestFiles/red2x2x1.ktx2 + SceneConverterTestFiles/rgba.png # copied from PngImporter tests SceneConverterTestFiles/two-quads-duplicates-fuzzy.bin SceneConverterTestFiles/two-quads-duplicates-fuzzy.gltf SceneConverterTestFiles/two-quads-duplicates.bin diff --git a/src/Magnum/SceneTools/Test/SceneConverterTest.cpp b/src/Magnum/SceneTools/Test/SceneConverterTest.cpp index f16189344..46237a40f 100644 --- a/src/Magnum/SceneTools/Test/SceneConverterTest.cpp +++ b/src/Magnum/SceneTools/Test/SceneConverterTest.cpp @@ -108,7 +108,16 @@ const struct { "-I", "ObjImporter", "--info", Utility::Path::join(SCENETOOLS_TEST_DIR, "SceneConverterTestFiles/point.obj"), "whatever.ply" }}, "ObjImporter", nullptr, nullptr, - "info-data-ignored-output.txt"} + "info-data-ignored-output.txt"}, + {"data, preferred importer plugin", {InPlaceInit, { + "-I", "AnyImageImporter", "--info", + /* Tested thoroughly in convert(preferred importer plugin), here it + just verifies that the option has an effect on --info as well */ + "--prefer", "PngImporter:StbImageImporter", "-v", + Utility::Path::join(SCENETOOLS_TEST_DIR, "SceneConverterTestFiles/blue4x4.png") + }}, + "StbImageImporter", nullptr, nullptr, + "info-preferred-importer-plugin.txt"}, }; const struct { @@ -786,6 +795,77 @@ const struct { /* 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"}, + {"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. */ + "--prefer", "PngImporter:Sdl3ImageImporter,StbImageImporter,SpngImporter,", + "-v", + Utility::Path::join(SCENETOOLS_TEST_DIR, "SceneConverterTestFiles/images-2d.gltf"), + Utility::Path::join(SCENETOOLS_TEST_OUTPUT_DIR, "SceneConverterTestFiles/images-2d.gltf") + }}, + "GltfImporter", "StbImageImporter", "GltfSceneConverter", + {"PngImageConverter", nullptr}, nullptr, + /* Not checking either of the files, the verbose output is enough to + verify */ + nullptr, nullptr, + "Trade::AnySceneImporter::openFile(): using GltfImporter\n" + "Trade::AnySceneConverter::beginFile(): using GltfSceneConverter\n" + "Trade::AbstractSceneConverter::addImporterContents(): adding 2D image 0 out of 2\n" + "Trade::AnyImageImporter::openFile(): using PngImporter (provided by StbImageImporter)\n" + "Trade::AbstractSceneConverter::addImporterContents(): adding 2D image 1 out of 2\n" + "Trade::AnyImageImporter::openFile(): using PngImporter (provided by StbImageImporter)\n"}, + {"preferred image converter plugin", {InPlaceInit, { + /* The main logic was tested in "preferred importer plugin" above, + this just verifies that it works for image converters too. The + converter doesn't use AnyImageConverter so we can't rely on + verbose output, instead we convert a RGBA image to a JPEG + (embedded in a glTF) and check the warning message. */ + "--prefer", "JpegImageConverter:StbImageConverter", + "-I", "PngImporter", + "-c", "imageConverter=JpegImageConverter", + Utility::Path::join(SCENETOOLS_TEST_DIR, "SceneConverterTestFiles/rgba.png"), + Utility::Path::join(SCENETOOLS_TEST_OUTPUT_DIR, "SceneConverterTestFiles/rgba.gltf") + }}, + "PngImporter", nullptr, "GltfSceneConverter", + {"StbImageConverter", nullptr}, nullptr, + /* Not checking either of the files, the warning output is enough to + verify */ + nullptr, nullptr, + "Trade::StbImageConverter::convertToData(): ignoring alpha channel for BMP/JPEG output\n"}, + {"preferred scene converter plugin", {InPlaceInit, { + /* There aren't any alternative implementations for any scene + converters so can only ensure the code doesn't blow up if scene + converters are passed to --prefer */ + "--prefer", "GltfSceneConverter:", + /* Removing the generator identifier for a roundtrip */ + "-c", "generator=", + Utility::Path::join(SCENETOOLS_TEST_DIR, "SceneConverterTestFiles/empty.gltf"), + Utility::Path::join(SCENETOOLS_TEST_OUTPUT_DIR, "SceneConverterTestFiles/empty.gltf") + }}, + "GltfImporter", nullptr, "GltfSceneConverter", + {}, nullptr, + /* It should give back the same file */ + "empty.gltf", nullptr, + {}}, + {"multiple --prefer options", {InPlaceInit, { + /* Basically a combination of "preferred importer plugin" and + "preferred image converter plugin" cases */ + "--prefer", "PngImporter:StbImageImporter", + "--prefer", "JpegImageConverter:StbImageConverter", + "-I", "AnyImageImporter", + "-c", "imageConverter=JpegImageConverter", "-v", + Utility::Path::join(SCENETOOLS_TEST_DIR, "SceneConverterTestFiles/rgba.png"), + Utility::Path::join(SCENETOOLS_TEST_OUTPUT_DIR, "SceneConverterTestFiles/rgba.gltf") + }}, + "PngImporter", nullptr, "GltfSceneConverter", + {"StbImageConverter", nullptr}, nullptr, + /* Not checking either of the files, the warning output is enough to + verify */ + nullptr, nullptr, + "Trade::AnyImageImporter::openFile(): using PngImporter (provided by StbImageImporter)\n" + "Trade::AnySceneConverter::beginFile(): using GltfSceneConverter\n" + "Trade::AbstractSceneConverter::addImporterContents(): adding 2D image 0 out of 1\n" + "Trade::StbImageConverter::convertToData(): ignoring alpha channel for BMP/JPEG output\n"}, }; const struct { @@ -820,6 +900,27 @@ const struct { }}, nullptr, nullptr, nullptr, nullptr, "The --only-mesh-attributes option can only be used with --mesh or --concatenate-meshes\n"}, + {"--prefer without a colon", {InPlaceInit, { + "--prefer", "PngImporter=StbImageImporter", "a", "b", + }}, + nullptr, nullptr, nullptr, nullptr, + "Invalid --prefer option PngImporter=StbImageImporter\n"}, + {"--prefer alias suffix unknown", {InPlaceInit, { + "--prefer", "TrueTypeFont:HarfBuzzFont", "a", "b", + }}, + nullptr, nullptr, nullptr, nullptr, + "Alias TrueTypeFont not recognized for a --prefer option\n"}, + {"--prefer alias name not found", {InPlaceInit, { + "--prefer", "FbxSceneConverter:UfbxSceneConverter", "a", "b", + }}, + nullptr, nullptr, nullptr, nullptr, + "Alias FbxSceneConverter not found for a --prefer option\n"}, + {"--prefer plugin doesn't provide alias", {InPlaceInit, { + "--prefer", "GltfImporter:UfbxImporter", "a", "b", + }}, + /* UfbxImporter is not really an image importer but it works here */ + "GltfImporter", "UfbxImporter", nullptr, nullptr, + "UfbxImporter doesn't provide GltfImporter for a --prefer option\n"}, {"can't load importer plugin", {InPlaceInit, { /* Override also the plugin directory for consistent output */ "--plugin-dir", "nonexistent", "-I", "NonexistentImporter", "whatever.obj", @@ -1221,7 +1322,9 @@ void SceneConverterTest::convert() { CORRADE_COMPARE(output.second(), data.message); CORRADE_VERIFY(output.first()); - CORRADE_COMPARE_AS(Utility::Path::join({SCENETOOLS_TEST_OUTPUT_DIR, "SceneConverterTestFiles", data.expected}), + /* In some cases the test verifies only the printed output and doesn't + check any file */ + if(data.expected) CORRADE_COMPARE_AS(Utility::Path::join({SCENETOOLS_TEST_OUTPUT_DIR, "SceneConverterTestFiles", data.expected}), Utility::Path::join({SCENETOOLS_TEST_DIR, "SceneConverterTestFiles", data.expected}), TestSuite::Compare::File); if(data.expected2) CORRADE_COMPARE_AS(Utility::Path::join({SCENETOOLS_TEST_OUTPUT_DIR, "SceneConverterTestFiles", data.expected2}), diff --git a/src/Magnum/SceneTools/Test/SceneConverterTestFiles/info-preferred-importer-plugin.txt b/src/Magnum/SceneTools/Test/SceneConverterTestFiles/info-preferred-importer-plugin.txt new file mode 100644 index 000000000..6153110ef --- /dev/null +++ b/src/Magnum/SceneTools/Test/SceneConverterTestFiles/info-preferred-importer-plugin.txt @@ -0,0 +1,4 @@ +Trade::AnyImageImporter::openFile(): using PngImporter (provided by StbImageImporter) +2D image 0: + Level 0: {4, 4} @ RGB8Unorm (0.0 kB) +Total image data size: 0.0 kB diff --git a/src/Magnum/SceneTools/Test/SceneConverterTestFiles/rgba.png b/src/Magnum/SceneTools/Test/SceneConverterTestFiles/rgba.png new file mode 100644 index 0000000000000000000000000000000000000000..e65f0bbe49efc6b6b8bf6c5659d17024574a0277 GIT binary patch literal 92 zcmeAS@N?(olHy`uVBq!ia0vp^%s|YO=#}JO|$rfMN|FLIgW("prefer", i); + const Containers::Array3 aliasNames = value.partition(':'); + if(!aliasNames[1]) { + Error{} << "Invalid --prefer option" << value; + return 1; + } + + /* Figure out manager name */ + PluginManager::AbstractManager* manager; + if(aliasNames[0].hasSuffix("Importer"_s)) + manager = &importerManager; + else if(aliasNames[0].hasSuffix("ImageConverter"_s)) + manager = &imageConverterManager; + else if(aliasNames[0].hasSuffix("SceneConverter"_s)) + manager = &converterManager; + else { + Error{} << "Alias" << aliasNames[0] << "not recognized for a --prefer option"; + return 1; + } + + /* The alias has to be found, otherwise it'd assert */ + if(manager->loadState(aliasNames[0]) == PluginManager::LoadState::NotFound) { + Error{} << "Alias" << aliasNames[0] << "not found for a --prefer option"; + return 1; + } + + /* Check that the names actually provide given alias, otherwise it'd + assert */ + const Containers::Array names = aliasNames[2].splitWithoutEmptyParts(','); + for(const Containers::StringView name: names) { + /* Not found plugins are allowed in the list */ + const PluginManager::PluginMetadata* const metadata = manager->metadata(name); + if(!metadata) + continue; + + bool found = false; + for(const Containers::StringView provides: metadata->provides()) { + if(provides == aliasNames[0]) { + found = true; + break; + } + } + if(!found) { + Error{} << name << "doesn't provide" << aliasNames[0] << "for a --prefer option"; + return 1; + } + } + + manager->setPreferredPlugins(aliasNames[0], names); + } + /* Print plugin info, if requested */ /** @todo these all duplicate plugin loading & option setting, move to some helpers (shared among all command-line tools)? */