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

@@ -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
 

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<const Utility::ConfigurationGroup*> 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<Containers::StringView> 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);
+}
+
 }
 
 }}

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

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",

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

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<Containers::StringView>("set", i);
+        const Containers::Array3<Containers::StringView> 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)? */