From c5d2472a03e14bb2d86003b3019f1c4f78f31807 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Tue, 16 May 2023 16:19:44 +0200 Subject: [PATCH] sceneconverter: add a --set option for setting arbitrary plugin config. This is now the preferrable way to set options to plugins that get delegated to from other plugins, instead of Until now there wasn't a general command line interface to pass options to plugins that get delegated to from other plugins, such as image converters used by scene converters. Due to that limitation, e.g. GltfSceneConverter had to add an [imageConverter] configuration group that it then copied over to the chosen image converter. But such approach obviously doesn't scale -- every converter would have to do the same, would have to repeat the whole testing process, and basically the same would need to be done for all importers delegating to image plugins. Nightmare. So there's now --set, which allows arbitrary options to be set for arbitrary plugins, and that's the preferrable way now. To avoid having to maintain two ways to do the same, the [imageConverter] group will get removed eventually. --- doc/changelog.dox | 2 + .../Implementation/converterUtilities.h | 12 ++- src/Magnum/SceneTools/Test/CMakeLists.txt | 1 + .../SceneTools/Test/SceneConverterTest.cpp | 80 +++++++++++++++++++ .../info-global-plugin-options.txt | 3 + src/Magnum/SceneTools/sceneconverter.cpp | 36 +++++++++ 6 files changed, 130 insertions(+), 4 deletions(-) create mode 100644 src/Magnum/SceneTools/Test/SceneConverterTestFiles/info-global-plugin-options.txt diff --git a/doc/changelog.dox b/doc/changelog.dox index 791d2b81f..173b8b645 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -669,6 +669,8 @@ See also: - 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 +- Added a `--set` option to @ref magnum-sceneconverter "magnum-sceneconverter", + allowing to set configuration options to arbitrary plugins @subsubsection changelog-latest-changes-shaders Shaders library diff --git a/src/Magnum/Implementation/converterUtilities.h b/src/Magnum/Implementation/converterUtilities.h index 43408e93b..f5d013c95 100644 --- a/src/Magnum/Implementation/converterUtilities.h +++ b/src/Magnum/Implementation/converterUtilities.h @@ -38,7 +38,7 @@ namespace Magnum { namespace Implementation { /* Used only in executables where we don't want it to be exported */ namespace { -void setOptions(PluginManager::AbstractPlugin& plugin, const Containers::StringView anyPluginName, const Containers::StringView options) { +void setOptions(const Containers::StringView pluginName, Utility::ConfigurationGroup& configuration, const Containers::StringView anyPluginName, const Containers::StringView options) { std::unordered_set emptySubgroups; for(const Containers::StringView option: options.splitWithoutEmptyParts(',')) { auto keyValue = option.partition('='); @@ -47,7 +47,7 @@ void setOptions(PluginManager::AbstractPlugin& plugin, const Containers::StringV const Containers::Array keyParts = keyValue[0].split('/'); CORRADE_INTERNAL_ASSERT(!keyParts.isEmpty()); - Utility::ConfigurationGroup* group = &plugin.configuration(); + Utility::ConfigurationGroup* group = &configuration; bool groupNotRecognized = false; for(std::size_t i = 0; i != keyParts.size() - 1; ++i) { Utility::ConfigurationGroup* subgroup = group->group(keyParts[i]); @@ -82,9 +82,9 @@ void setOptions(PluginManager::AbstractPlugin& plugin, const Containers::StringV /* The warning isn't printed in case a value is added into an empty subgroup, see above */ emptySubgroups.find(group) == emptySubgroups.end() - )) && plugin.plugin() != anyPluginName) + )) && pluginName != anyPluginName) { - Warning{} << "Option" << keyValue[0] << "not recognized by" << plugin.plugin(); + Warning{} << "Option" << keyValue[0] << "not recognized by" << pluginName; } /* If the option doesn't have an =, treat it as a boolean flag that's @@ -97,6 +97,10 @@ void setOptions(PluginManager::AbstractPlugin& plugin, const Containers::StringV } } +void setOptions(PluginManager::AbstractPlugin& plugin, const Containers::StringView anyPluginName, const Containers::StringView options) { + setOptions(plugin.plugin(), plugin.configuration(), anyPluginName, options); +} + } }} diff --git a/src/Magnum/SceneTools/Test/CMakeLists.txt b/src/Magnum/SceneTools/Test/CMakeLists.txt index 7ce43e1f2..83fd2ad83 100644 --- a/src/Magnum/SceneTools/Test/CMakeLists.txt +++ b/src/Magnum/SceneTools/Test/CMakeLists.txt @@ -121,6 +121,7 @@ if(CORRADE_TARGET_UNIX AND NOT CORRADE_PLUGINMANAGER_NO_DYNAMIC_PLUGIN_SUPPORT) SceneConverterTestFiles/info-data.txt SceneConverterTestFiles/info-data-ignored-output.txt SceneConverterTestFiles/info-converter.txt + SceneConverterTestFiles/info-global-plugin-options.txt SceneConverterTestFiles/info-image-converter.txt SceneConverterTestFiles/info-importer.txt SceneConverterTestFiles/info-importer-ignored-input-output.txt diff --git a/src/Magnum/SceneTools/Test/SceneConverterTest.cpp b/src/Magnum/SceneTools/Test/SceneConverterTest.cpp index 46237a40f..23f76b651 100644 --- a/src/Magnum/SceneTools/Test/SceneConverterTest.cpp +++ b/src/Magnum/SceneTools/Test/SceneConverterTest.cpp @@ -118,6 +118,15 @@ const struct { }}, "StbImageImporter", nullptr, nullptr, "info-preferred-importer-plugin.txt"}, + {"data, global plugin options", {InPlaceInit, { + "-I", "StbImageImporter", "--info", + /* Tested thoroughly in convert(global importer options), here it + just verifies that the option has an effect on --info as well */ + "--set", "StbImageImporter:forceChannelCount=1", "-v", + Utility::Path::join(SCENETOOLS_TEST_DIR, "SceneConverterTestFiles/blue4x4.png") + }}, + "StbImageImporter", nullptr, nullptr, + "info-global-plugin-options.txt"}, }; const struct { @@ -866,6 +875,62 @@ const struct { "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"}, + {"global importer options", {InPlaceInit, { + /* The image is RGB, but we import it as RGBA to trigger a warning + inside the JPEG converter plugin */ + "--set", "StbImageImporter:forceChannelCount=4,unrecognizedOption=yes", + "-I", "StbImageImporter", + "-c", "imageConverter=StbJpegImageConverter", + Utility::Path::join(SCENETOOLS_TEST_DIR, "SceneConverterTestFiles/blue4x4.png"), + Utility::Path::join(SCENETOOLS_TEST_OUTPUT_DIR, "SceneConverterTestFiles/rgba.gltf") + }}, + "StbImageImporter", nullptr, "GltfSceneConverter", + {"StbImageConverter", nullptr}, nullptr, + /* Not checking either of the files, the warning output is enough to + verify the option got properly set */ + nullptr, nullptr, + "Option unrecognizedOption not recognized by StbImageImporter\n" + "Trade::StbImageConverter::convertToData(): ignoring alpha channel for BMP/JPEG output\n"}, + {"global image converter options", {InPlaceInit, { + /* This produces the same output as "2D image converter, two images" + above, it's just that the options are specified through --set */ + "--set", "StbResizeImageConverter:size=\"1 1\"", + "-P", "StbResizeImageConverter", + /* Removing the generator identifier for a smaller file, bundling + the images to avoid having too many files */ + "-c", "bundleImages,generator=", + Utility::Path::join(SCENETOOLS_TEST_DIR, "SceneConverterTestFiles/images-2d.gltf"), + Utility::Path::join(SCENETOOLS_TEST_OUTPUT_DIR, "SceneConverterTestFiles/images-2d-1x1.gltf") + }}, + "GltfImporter", "PngImporter", "GltfSceneConverter", + {"StbResizeImageConverter", "PngImageConverter"}, nullptr, + "images-2d-1x1.gltf", "images-2d-1x1.bin", + {}}, + {"global scene converter options", {InPlaceInit, { + /* Same as -c generator= */ + "--set", "GltfSceneConverter:generator=", + Utility::Path::join(SCENETOOLS_TEST_DIR, "SceneConverterTestFiles/empty.gltf"), + Utility::Path::join(SCENETOOLS_TEST_OUTPUT_DIR, "SceneConverterTestFiles/empty.gltf") + }}, + "GltfImporter", nullptr, "GltfSceneConverter", + {}, nullptr, + "empty.gltf", nullptr, + {}}, + {"multiple --set options", {InPlaceInit, { + /* Another variant of "2D image converter, two images", this time + with everything specified through --set */ + "--set", "StbResizeImageConverter:size=\"1 1\"", + /* Removing the generator identifier for a smaller file, bundling + the images to avoid having too many files */ + "--set", "GltfSceneConverter:bundleImages,generator=", + "-P", "StbResizeImageConverter", + Utility::Path::join(SCENETOOLS_TEST_DIR, "SceneConverterTestFiles/images-2d.gltf"), + Utility::Path::join(SCENETOOLS_TEST_OUTPUT_DIR, "SceneConverterTestFiles/images-2d-1x1.gltf") + }}, + "GltfImporter", "PngImporter", "GltfSceneConverter", + {"StbResizeImageConverter", "PngImageConverter"}, nullptr, + "images-2d-1x1.gltf", "images-2d-1x1.bin", + {}}, }; const struct { @@ -921,6 +986,21 @@ const struct { /* 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"}, + {"--set without a colon", {InPlaceInit, { + "--set", "StbImageImporter=forceChannelCount=3", "a", "b", + }}, + nullptr, nullptr, nullptr, nullptr, + "Invalid --set option StbImageImporter=forceChannelCount=3\n"}, + {"--set plugin suffix unknown", {InPlaceInit, { + "--set", "TrueTypeFont:hinting=slight", "a", "b", + }}, + nullptr, nullptr, nullptr, nullptr, + "Plugin TrueTypeFont not recognized for a --set option\n"}, + {"--set plugin name not found", {InPlaceInit, { + "--set", "FbxSceneConverter:binary=true", "a", "b", + }}, + nullptr, nullptr, nullptr, nullptr, + "Plugin FbxSceneConverter not found for a --set option\n"}, {"can't load importer plugin", {InPlaceInit, { /* Override also the plugin directory for consistent output */ "--plugin-dir", "nonexistent", "-I", "NonexistentImporter", "whatever.obj", diff --git a/src/Magnum/SceneTools/Test/SceneConverterTestFiles/info-global-plugin-options.txt b/src/Magnum/SceneTools/Test/SceneConverterTestFiles/info-global-plugin-options.txt new file mode 100644 index 000000000..7f92c5119 --- /dev/null +++ b/src/Magnum/SceneTools/Test/SceneConverterTestFiles/info-global-plugin-options.txt @@ -0,0 +1,3 @@ +2D image 0: + Level 0: {4, 4} @ R8Unorm (0.0 kB) +Total image data size: 0.0 kB diff --git a/src/Magnum/SceneTools/sceneconverter.cpp b/src/Magnum/SceneTools/sceneconverter.cpp index 1bd92ed0d..7dfbbf71c 100644 --- a/src/Magnum/SceneTools/sceneconverter.cpp +++ b/src/Magnum/SceneTools/sceneconverter.cpp @@ -417,6 +417,7 @@ int main(int argc, char** argv) { .addArrayOption('M', "mesh-converter").setHelp("mesh-converter", "converter plugin(s) to apply to each mesh in the scene", "PLUGIN") .addOption("plugin-dir").setHelp("plugin-dir", "override base plugin dir", "DIR") .addArrayOption("prefer").setHelp("prefer", "prefer particular plugins for given alias(es)", "alias:plugin1,plugin2,…") + .addArrayOption("set").setHelp("set", "set global plugin option(s)", "plugin:key=val,key2=val2,…") #if defined(CORRADE_TARGET_UNIX) || (defined(CORRADE_TARGET_WINDOWS) && !defined(CORRADE_TARGET_WINDOWS_RT)) .addBooleanOption("map").setHelp("map", "memory-map the input for zero-copy import (works only for standalone files)") #endif @@ -635,6 +636,41 @@ well, the IDs reference attributes of the first mesh.)") manager->setPreferredPlugins(aliasNames[0], names); } + /* Set global plugin options */ + for(std::size_t i = 0, iMax = args.arrayValueCount("set"); i != iMax; ++i) { + const auto value = args.arrayValue("set", i); + const Containers::Array3 nameOptions = value.partition(':'); + if(!nameOptions[1]) { + Error{} << "Invalid --set option" << value; + return 1; + } + + /* Figure out manager name */ + PluginManager::AbstractManager* manager; + if(nameOptions[0].hasSuffix("Importer"_s)) + manager = &importerManager; + else if(nameOptions[0].hasSuffix("ImageConverter"_s)) + manager = &imageConverterManager; + else if(nameOptions[0].hasSuffix("SceneConverter"_s)) + manager = &converterManager; + else { + Error{} << "Plugin" << nameOptions[0] << "not recognized for a --set option"; + return 1; + } + + /* Get the metadata to access global configuration */ + PluginManager::PluginMetadata* const metadata = manager->metadata(nameOptions[0]); + if(!metadata) { + Error{} << "Plugin" << nameOptions[0] << "not found for a --set option"; + return 1; + } + + /* Set options. Doing things like --set AnyImageImporter:foo=bar makes + no sense, so this isn't excluding any "Any*" plugins from the + unrecognized option warnings */ + Implementation::setOptions(nameOptions[0], metadata->configuration(), {}, nameOptions[2]); + } + /* Print plugin info, if requested */ /** @todo these all duplicate plugin loading & option setting, move to some helpers (shared among all command-line tools)? */