From 0da3ca37e3b74ba378a5faa9daad03a88a7a79c3 Mon Sep 17 00:00:00 2001 From: Christos Tsolakis Date: Sat, 16 May 2026 13:08:00 +0300 Subject: [PATCH 1/6] Store options as custom variant instead of plain strings This allows to store either a string or a vector of strings. The latter becomes useful for options that support multiple values. --- application/F3DOptionsTools.cxx | 70 ++++++++++++++++++++++++--------- application/F3DOptionsTools.h | 23 ++++++++--- application/F3DStarter.cxx | 30 +++++++++----- 3 files changed, 89 insertions(+), 34 deletions(-) diff --git a/application/F3DOptionsTools.cxx b/application/F3DOptionsTools.cxx index 039ec3e64b..88e50279d0 100644 --- a/application/F3DOptionsTools.cxx +++ b/application/F3DOptionsTools.cxx @@ -318,7 +318,9 @@ F3DOptionsTools::OptionsDict F3DOptionsTools::ParseCLIOptions( auto appIter = F3DOptionsTools::DefaultAppOptions.find(longName); if (appIter != F3DOptionsTools::DefaultAppOptions.end()) { - defaultValue = appIter->second; + // defaults are always stored as strings + assert(std::holds_alternative(appIter->second)); + defaultValue = std::get(appIter->second); } else { @@ -438,7 +440,7 @@ F3DOptionsTools::OptionsDict F3DOptionsTools::ParseCLIOptions( throw F3DExFailure("unknown options"); } - // Add each CLI options into a vector of string/string and return it + // Add each CLI options into a OptionsDict and return it F3DOptionsTools::OptionsDict cliOptionsDict; for (const auto& res : result) { @@ -491,10 +493,11 @@ void F3DOptionsTools::PrintHelpPair( } //---------------------------------------------------------------------------- -std::vector> F3DOptionsTools::ConvertToLibf3dOptions( - const std::string& key, const std::string& value) +std::vector> +F3DOptionsTools::ConvertToLibf3dOptions(const std::string& key, const OptionValue& value) { - std::vector> libf3dOptions; + std::vector> libf3dOptions; + const bool is_single_valued = std::holds_alternative(value); // Simple one-to-one case auto libf3dIter = F3DOptionsTools::LibOptionsNames.find(key); @@ -506,20 +509,22 @@ std::vector> F3DOptionsTools::ConvertToLibf3 // anti-aliasing is handled in two options in the lib else if (key == "anti-aliasing") { - if (value != "none") + assert(is_single_valued); + const std::string& value_str = std::get(value); + if (value_str != "none") { // Handle deprecated boolean option bool deprecatedBooleanOption; - if (F3DOptionsTools::Parse(value, deprecatedBooleanOption)) + if (F3DOptionsTools::Parse(value_str, deprecatedBooleanOption)) { f3d::log::warn("--anti-aliasing is a now a string, please specify the type of " "anti-aliasing or use the implicit default"); - libf3dOptions.emplace_back(std::make_pair("render.effect.antialiasing.enable", value)); + libf3dOptions.emplace_back(std::make_pair("render.effect.antialiasing.enable", value_str)); } else { libf3dOptions.emplace_back(std::make_pair("render.effect.antialiasing.enable", "true")); - libf3dOptions.emplace_back(std::make_pair("render.effect.antialiasing.mode", value)); + libf3dOptions.emplace_back(std::make_pair("render.effect.antialiasing.mode", value_str)); } } else @@ -531,17 +536,21 @@ std::vector> F3DOptionsTools::ConvertToLibf3 // handle deprecated anti-aliasing option else if (key == "anti-aliasing-mode") { + assert(is_single_valued); + const std::string& value_str = std::get(value); f3d::log::warn("--anti-aliasing-mode is deprecated"); - libf3dOptions.emplace_back(std::make_pair("render.effect.antialiasing.mode", value)); + libf3dOptions.emplace_back(std::make_pair("render.effect.antialiasing.mode", value_str)); } // blending is handled in two options in the lib else if (key == "blending") { - if (value != "none") + assert(is_single_valued); + const std::string& value_str = std::get(value); + if (value_str != "none") { libf3dOptions.emplace_back(std::make_pair("render.effect.blending.enable", "true")); - libf3dOptions.emplace_back(std::make_pair("render.effect.blending.mode", value)); + libf3dOptions.emplace_back(std::make_pair("render.effect.blending.mode", value_str)); } else { @@ -552,14 +561,18 @@ std::vector> F3DOptionsTools::ConvertToLibf3 // handle deprecated translucency support else if (key == "translucency-support") { + assert(is_single_valued); + const std::string& value_str = std::get(value); f3d::log::warn("--translucency-support is deprecated, please use --blending instead"); - libf3dOptions.emplace_back(std::make_pair("render.effect.blending.enable", value)); + libf3dOptions.emplace_back(std::make_pair("render.effect.blending.enable", value_str)); } // point sprites is handled in two options in the lib else if (key == "point-sprites") { - if (value != "none") + assert(is_single_valued); + const std::string& value_str = std::get(value); + if (value_str != "none") { // Handle deprecated boolean option bool deprecatedBooleanOption; @@ -567,12 +580,12 @@ std::vector> F3DOptionsTools::ConvertToLibf3 { f3d::log::warn("--point-sprites is a now a string, please specify the type of " "point sprites to use or use the implicit default"); - libf3dOptions.emplace_back(std::make_pair("model.point_sprites.enable", value)); + libf3dOptions.emplace_back(std::make_pair("model.point_sprites.enable", value_str)); } else { libf3dOptions.emplace_back(std::make_pair("model.point_sprites.enable", "true")); - libf3dOptions.emplace_back(std::make_pair("model.point_sprites.type", value)); + libf3dOptions.emplace_back(std::make_pair("model.point_sprites.type", value_str)); } } else @@ -584,19 +597,22 @@ std::vector> F3DOptionsTools::ConvertToLibf3 // handle deprecated point-sprites-type option else if (key == "point-sprites-type") { + assert(is_single_valued); + const std::string& value_str = std::get(value); f3d::log::warn("--point-sprites-type is deprecated"); - libf3dOptions.emplace_back(std::make_pair("model.point_sprites.type", value)); + libf3dOptions.emplace_back(std::make_pair("model.point_sprites.type", value_str)); } // handle deprecated interaction-trackball option else if (key == "interaction-trackball") { + const std::string& value_str = std::get(value); f3d::log::warn( "--interaction-trackball is deprecated, please use --interaction-style=trackball instead"); bool trackball; - if (!F3DOptionsTools::Parse(value, trackball)) + if (!F3DOptionsTools::Parse(value_str, trackball)) { - f3d::log::error("Cannot parse --interaction-trackball value: " + value); + f3d::log::error("Cannot parse --interaction-trackball value: " + value_str); } else if (trackball) { @@ -612,3 +628,19 @@ std::vector> F3DOptionsTools::ConvertToLibf3 return libf3dOptions; } + +std::string F3DOptionsTools::ConvertToString(const F3DOptionsTools::OptionValue& optionValue) +{ + if (std::holds_alternative(optionValue)) + { + return std::get(optionValue); + } + else if (std::holds_alternative>(optionValue)) + { + const auto& vec = std::get>(optionValue); + auto concatenate = [](const std::string& result, const std::string& value) + { return result + " , " + value; }; + return std::accumulate(vec.cbegin(), vec.cend(), std::string{ "" }, concatenate); + } + return ""; +} diff --git a/application/F3DOptionsTools.h b/application/F3DOptionsTools.h index edb8746979..f272d9f2c9 100644 --- a/application/F3DOptionsTools.h +++ b/application/F3DOptionsTools.h @@ -9,13 +9,15 @@ #include #include +#include #include #include #include namespace F3DOptionsTools { -using OptionsDict = std::map; +using OptionValue = std::variant, std::string>; +using OptionsDict = std::map; using OptionsEntry = std::tuple; using OptionsEntries = std::vector; @@ -27,7 +29,7 @@ using OptionsEntries = std::vector; * and ParseCLIOptions */ static inline const OptionsDict DefaultAppOptions = { - { "input", "" }, + { "input", { "" } }, { "output", "" }, { "list-bindings", "false" }, { "no-background", "false" }, @@ -167,11 +169,16 @@ static inline const std::map CustomMappingOp { "point-sprites-type", "" }, }; +/** + * List of CLI option names that can accept multiple values + */ +static inline const std::set MultiValueOptions = { "define", "input", "reset" }; + /** * Convert a CLI options key/value to a vector of libf3d options key/value */ -std::vector> ConvertToLibf3dOptions( - const std::string& key, const std::string& value); +std::vector> ConvertToLibf3dOptions( + const std::string& key, const OptionValue& value); /** * Browse through all possible option names to find one that have the smallest distance to the @@ -196,13 +203,19 @@ F3DOptionsTools::OptionsDict ParseCLIOptions( void PrintHelpPair( std::string_view key, std::string_view help, int keyWidth = 10, int helpWidth = 70); +/** + * Convert the variant-based OptionValue to a string. Useful for debugging + */ +std::string ConvertToString(const OptionValue& optionValue); + /** * Parse provided string into provided typed var. * Return true if successful, false otherwise. */ template -bool Parse(const std::string& optionString, T& option) +bool Parse(const OptionValue& optionValue, T& option) { + const std::string optionString = ConvertToString(optionValue); try { option = f3d::options::parse(optionString); diff --git a/application/F3DStarter.cxx b/application/F3DStarter.cxx index bc65a16705..11f5d9ec76 100644 --- a/application/F3DStarter.cxx +++ b/application/F3DStarter.cxx @@ -76,7 +76,8 @@ class F3DStarter::F3DInternals public: F3DInternals() = default; - using log_entry_t = std::tuple; + using log_entry_t = + std::tuple; // XXX: The values in the following two structs // are left uninitialized as the will all be initialized from @@ -570,7 +571,8 @@ class F3DStarter::F3DInternals { const auto& [bindStr, source, matchType, match, commands] = tuple; const std::string origin = F3DInternals::FormatOrigin(source, matchType, match); - f3d::log::debug(" '", bindStr, "' ", sep, " '", commands, "' from ", origin); + f3d::log::debug(" '", bindStr, "' ", sep, " '", F3DOptionsTools::ConvertToString(commands), + "' from ", origin); } f3d::log::debug(""); } @@ -668,7 +670,10 @@ class F3DStarter::F3DInternals if (std::ranges::find(readerOptionNames, libf3dOptionName) != readerOptionNames.end()) { - f3d::engine::setReaderOption(libf3dOptionName, libf3dOptionValue); + assert(std::holds_alternative(libf3dOptionValue)); + std::string libf3dOptionValueStr = std::get(libf3dOptionValue); + + f3d::engine::setReaderOption(libf3dOptionName, libf3dOptionValueStr); continue; } @@ -681,7 +686,10 @@ class F3DStarter::F3DInternals } else { - libOptions.setAsString(libf3dOptionName, libf3dOptionValue); + assert(std::holds_alternative(libf3dOptionValue)); + const std::string libf3dOptionValueStr = + std::get(libf3dOptionValue); + libOptions.setAsString(libf3dOptionName, libf3dOptionValueStr); } // Log the option if needed @@ -696,8 +704,9 @@ class F3DStarter::F3DInternals if (!quiet) { const std::string origin = F3DInternals::FormatOrigin(source, matchType, match); - f3d::log::warn("Could not set '", keyForLog, "' to '", libf3dOptionValue, - "' from ", origin, " because: ", ex.what()); + f3d::log::warn("Could not set '", keyForLog, "' to '", + F3DOptionsTools::ConvertToString(libf3dOptionValue), "' from ", origin, + " because: ", ex.what()); } } catch (const f3d::options::inexistent_exception&) @@ -749,7 +758,8 @@ class F3DStarter::F3DInternals { if (!F3DOptionsTools::Parse(appOptions.at(name), option)) { - f3d::log::warn("Could not parse '" + appOptions.at(name) + "' into '" + name + "' option"); + f3d::log::warn("Could not parse '" + F3DOptionsTools::ConvertToString(appOptions.at(name)) + + "' into '" + name + "' option"); } } @@ -761,7 +771,7 @@ class F3DStarter::F3DInternals void ParseOption(const F3DOptionsTools::OptionsDict& appOptions, const std::string& name, std::optional& option) { - const std::string& optStr = appOptions.at(name); + const std::string& optStr = F3DOptionsTools::ConvertToString(appOptions.at(name)); if (optStr.empty()) { option = std::nullopt; @@ -1069,8 +1079,8 @@ int F3DStarter::Start(int argc, char** argv) { if (!F3DOptionsTools::Parse(iter->second, noConfig)) { - f3d::log::warn( - "Could not parse '" + iter->second + "' into 'no-config' option, assuming false"); + f3d::log::warn("Could not parse '" + F3DOptionsTools::ConvertToString(iter->second) + + "' into 'no-config' option, assuming false"); } } From fdd9a242d1def406cd63f276100484a6b96b9d2b Mon Sep 17 00:00:00 2001 From: Christos Tsolakis Date: Sat, 16 May 2026 13:14:47 +0300 Subject: [PATCH 2/6] Update reset option to use multi-value infrastructure --- application/CMakeLists.txt | 2 +- application/F3DOptionsTools.cxx | 44 +++++++++++++------ application/F3DStarter.cxx | 76 ++++++++++++++++++++++----------- resources/cli-options.json | 3 +- 4 files changed, 83 insertions(+), 42 deletions(-) diff --git a/application/CMakeLists.txt b/application/CMakeLists.txt index 2a9f85e36a..f097732bd4 100644 --- a/application/CMakeLists.txt +++ b/application/CMakeLists.txt @@ -191,7 +191,7 @@ f3d_generate_cli_options( F3D_SOURCE_DIR "${F3D_SOURCE_DIR}" F3D_BINARY_DIR "${F3D_BINARY_DIR}" ENABLED_CONDITIONALS ${f3d_enabled_conditionals} # Do NOT quote so this stays a list - CPP_CLI_OPTIONS_TO_SKIP input;define;reset # Do NOT quote so this stays a list + CPP_CLI_OPTIONS_TO_SKIP input;define # Do NOT quote so this stays a list ) set(F3D_SOURCE_FILES diff --git a/application/F3DOptionsTools.cxx b/application/F3DOptionsTools.cxx index 88e50279d0..9112c59a67 100644 --- a/application/F3DOptionsTools.cxx +++ b/application/F3DOptionsTools.cxx @@ -26,11 +26,10 @@ namespace { /** * True boolean options need to be filtered out in ParseCLIOptions - * Also filter out special options like `define` and `reset` * This is the easiest, compile time way to do it */ constexpr std::array CLIBooleans = { "version", "help", "list-readers", "scan-plugins", - "list-rendering-backends", "define", "reset" }; + "list-rendering-backends" }; //---------------------------------------------------------------------------- /** @@ -278,9 +277,6 @@ F3DOptionsTools::OptionsDict F3DOptionsTools::ParseCLIOptions( std::vector defines; auto cxxoptsDefines = cxxopts::value>(defines); - std::vector resets; - auto cxxoptsResets = cxxopts::value>(resets); - try { cxxopts::Options cxxOptions(execName, F3D::AppTitle); @@ -295,7 +291,6 @@ F3DOptionsTools::OptionsDict F3DOptionsTools::ParseCLIOptions( { group("input", "Input files", cxxoptsInputPositionals, ""); group("D,define", "Define libf3d options", cxxoptsDefines, "libf3d.option=value"); - group("R,reset", "Reset libf3d options", cxxoptsResets, "libf3d.option"); } // Add each option to cxxopts @@ -313,6 +308,7 @@ F3DOptionsTools::OptionsDict F3DOptionsTools::ParseCLIOptions( std::string defaultValue; std::string helpText(cliOption.HelpText); std::string longName(cliOption.LongName); + std::string valueHelper(cliOption.ValueHelper); // Recover default value from app options auto appIter = F3DOptionsTools::DefaultAppOptions.find(longName); @@ -353,7 +349,14 @@ F3DOptionsTools::OptionsDict F3DOptionsTools::ParseCLIOptions( } // Recover the implicit value and set it if any - cxxoptsValues.emplace_back(cxxopts::value()); + if (valueHelper == "") + { + cxxoptsValues.emplace_back(cxxopts::value>()); + } + else + { + cxxoptsValues.emplace_back(cxxopts::value()); + } auto& val = cxxoptsValues.back(); if (!cliOption.ImplicitValue.empty()) { @@ -445,10 +448,29 @@ F3DOptionsTools::OptionsDict F3DOptionsTools::ParseCLIOptions( for (const auto& res : result) { // Discard boolean option like `--version` or `--help` - if (std::ranges::find(::CLIBooleans, res.key()) == ::CLIBooleans.end()) + if (std::ranges::find(::CLIBooleans, res.key()) != ::CLIBooleans.end()) + { + continue; + } + auto iter = cliOptionsDict.find(res.key()); + if(iter == cliOptionsDict.end()) { cliOptionsDict[res.key()] = res.value(); } + else + { + // key already exists. Create or append to vector + if(std::holds_alternative>(iter->second)) + { + auto& vec = std::get>(iter->second); + vec.push_back(res.value()); + } + else + { + auto& item = std::get(iter->second); + cliOptionsDict[res.key()] = std::vector{item,res.value()}; + } + } } // Handle defines and add them as proper options @@ -463,12 +485,6 @@ F3DOptionsTools::OptionsDict F3DOptionsTools::ParseCLIOptions( cliOptionsDict[define.substr(0, sepIdx)] = define.substr(sepIdx + 1); } - // Handles reset using the dedicated syntax - for (const std::string& reset : resets) - { - cliOptionsDict["reset-" + reset] = ""; - } - return cliOptionsDict; } catch (const cxxopts::exceptions::exception& ex) diff --git a/application/F3DStarter.cxx b/application/F3DStarter.cxx index 11f5d9ec76..81043142bc 100644 --- a/application/F3DStarter.cxx +++ b/application/F3DStarter.cxx @@ -71,6 +71,39 @@ f3d::interactor* GlobalInteractor = nullptr; constexpr std::string_view F3D_PIPED = "-"; +namespace +{ +// helper to apply Callable to all items of OptionValue no matter if it is a +// single valued string or a vector of strings +// based on example at https://en.cppreference.com/cpp/utility/variant/visit2 + +template +void apply_to_all(F3DOptionsTools::OptionValue& option, Callable f) +{ + std::visit( + [f](auto&& value) + { + using T = std::decay_t; + if constexpr (std::is_same_v) + { + f(value); + } + else if constexpr (std::is_same_v>) + { + for (auto& item : value) + { + f(item); + } + } + else + { + static_assert(false, "non-exhaustive visitor!"); + } + }, + option); +} +} + class F3DStarter::F3DInternals { public: @@ -645,26 +678,6 @@ class F3DStarter::F3DInternals { auto [libf3dOptionName, libf3dOptionValue] = libf3dOption; - bool reset = false; - - // Handle options reset - if (libf3dOptionName.starts_with("reset-")) - { - if (libf3dOptionName.size() > 6) - { - reset = true; - libf3dOptionName = libf3dOptionName.substr(6); - keyForLog = libf3dOptionName; - libf3dOptionValue = "reset"; - } - else - { - f3d::log::warn("Invalid option: 'reset' must be followed by a valid option " - "name, ignoring entry"); - continue; - } - } - // Handle reader options std::vector readerOptionNames = f3d::engine::getAllReaderOptionNames(); if (std::ranges::find(readerOptionNames, libf3dOptionName) != @@ -679,10 +692,20 @@ class F3DStarter::F3DInternals try { - // Assume this is a libf3d option and set/reset the value - if (reset) + if (libf3dOptionName == "reset") { - libOptions.reset(libf3dOptionName); + apply_to_all(libf3dOptionValue, + [&libOptions, &keyForLog](const std::string& value) + { + keyForLog = value; + if (value.empty()) + { + f3d::log::warn("Invalid option: 'reset' must be followed by a valid " + "option name, ignoring entry"); + return; + } + libOptions.reset(value); + }); } else { @@ -714,8 +737,7 @@ class F3DStarter::F3DInternals if (!quiet) { const std::string origin = F3DInternals::FormatOrigin(source, matchType, match); - auto [closestName, dist] = - F3DOptionsTools::GetClosestOption(libf3dOptionName, true); + auto [closestName, dist] = F3DOptionsTools::GetClosestOption(keyForLog, true); f3d::log::warn("'", keyForLog, "' option from ", origin, " does not exists , did you mean '", closestName, "'?"); } @@ -1578,6 +1600,7 @@ void F3DStarter::LoadFileGroupInternal( // options names are shared between options instance F3DOptionsTools::OptionsDict dynamicOptionsDict; std::vector optionNames = dynamicOptions.getAllNames(); + std::vector resets; for (const auto& name : optionNames) { if (!dynamicOptions.isSame(this->Internals->LibOptions, name)) @@ -1586,7 +1609,7 @@ void F3DStarter::LoadFileGroupInternal( { // If a dynamic option has been changed and does not have value, it means it was reset using // the command line reset it using the dedicated syntax - dynamicOptionsDict["reset-" + name] = ""; + resets.push_back(name); } else { @@ -1596,6 +1619,7 @@ void F3DStarter::LoadFileGroupInternal( } } } + dynamicOptionsDict["reset"] = resets; // Detect interactively changed verbose level and add it to dynamic options f3d::log::VerboseLevel currentVerboseLevel = f3d::log::getVerboseLevel(); diff --git a/resources/cli-options.json b/resources/cli-options.json index caac810241..27bdf3d216 100644 --- a/resources/cli-options.json +++ b/resources/cli-options.json @@ -17,7 +17,8 @@ { "longName": "reset", "shortName": "R", - "helpText": "Reset libf3d options" + "helpText": "Reset libf3d options", + "valueHelper": "" }, { "longName": "output", From 43b0c72da86ef1ddc8b1dac1212df2e2afb25ace Mon Sep 17 00:00:00 2001 From: Christos Tsolakis Date: Sat, 16 May 2026 23:39:02 +0300 Subject: [PATCH 3/6] Update --define to use multi-value infrastructure --- application/CMakeLists.txt | 2 +- application/F3DOptionsTools.cxx | 30 +++++++----------------------- application/F3DStarter.cxx | 21 +++++++++++++++++++++ resources/cli-options.json | 3 ++- 4 files changed, 31 insertions(+), 25 deletions(-) diff --git a/application/CMakeLists.txt b/application/CMakeLists.txt index f097732bd4..1a27a0ad48 100644 --- a/application/CMakeLists.txt +++ b/application/CMakeLists.txt @@ -191,7 +191,7 @@ f3d_generate_cli_options( F3D_SOURCE_DIR "${F3D_SOURCE_DIR}" F3D_BINARY_DIR "${F3D_BINARY_DIR}" ENABLED_CONDITIONALS ${f3d_enabled_conditionals} # Do NOT quote so this stays a list - CPP_CLI_OPTIONS_TO_SKIP input;define # Do NOT quote so this stays a list + CPP_CLI_OPTIONS_TO_SKIP input # Do NOT quote so this stays a list ) set(F3D_SOURCE_FILES diff --git a/application/F3DOptionsTools.cxx b/application/F3DOptionsTools.cxx index 9112c59a67..c1fd0f8a4b 100644 --- a/application/F3DOptionsTools.cxx +++ b/application/F3DOptionsTools.cxx @@ -274,9 +274,6 @@ F3DOptionsTools::OptionsDict F3DOptionsTools::ParseCLIOptions( std::vector> cxxoptsValues; auto cxxoptsInputPositionals = cxxopts::value>(positionals); - std::vector defines; - auto cxxoptsDefines = cxxopts::value>(defines); - try { cxxopts::Options cxxOptions(execName, F3D::AppTitle); @@ -290,7 +287,6 @@ F3DOptionsTools::OptionsDict F3DOptionsTools::ParseCLIOptions( if (std::string(optionGroup.GroupName) == "Applicative") { group("input", "Input files", cxxoptsInputPositionals, ""); - group("D,define", "Define libf3d options", cxxoptsDefines, "libf3d.option=value"); } // Add each option to cxxopts @@ -453,38 +449,26 @@ F3DOptionsTools::OptionsDict F3DOptionsTools::ParseCLIOptions( continue; } auto iter = cliOptionsDict.find(res.key()); - if(iter == cliOptionsDict.end()) + if (iter == cliOptionsDict.end()) { cliOptionsDict[res.key()] = res.value(); } - else + else { // key already exists. Create or append to vector - if(std::holds_alternative>(iter->second)) + if (std::holds_alternative>(iter->second)) { - auto& vec = std::get>(iter->second); + auto& vec = std::get>(iter->second); vec.push_back(res.value()); } - else + else { - auto& item = std::get(iter->second); - cliOptionsDict[res.key()] = std::vector{item,res.value()}; + auto& item = std::get(iter->second); + cliOptionsDict[res.key()] = std::vector{ item, res.value() }; } } } - // Handle defines and add them as proper options - for (const std::string& define : defines) - { - std::string::size_type sepIdx = define.find_first_of('='); - if (sepIdx == std::string::npos) - { - f3d::log::warn("Could not parse a define '", define, "'"); - continue; - } - cliOptionsDict[define.substr(0, sepIdx)] = define.substr(sepIdx + 1); - } - return cliOptionsDict; } catch (const cxxopts::exceptions::exception& ex) diff --git a/application/F3DStarter.cxx b/application/F3DStarter.cxx index 81043142bc..85d596abf1 100644 --- a/application/F3DStarter.cxx +++ b/application/F3DStarter.cxx @@ -102,6 +102,18 @@ void apply_to_all(F3DOptionsTools::OptionValue& option, Callable f) }, option); } +void HandleDefine(f3d::options& libOptions, const std::string& optionValue) +{ + std::string::size_type sepidx = optionValue.find_first_of('='); + if (sepidx == std::string::npos) + { + f3d::log::warn("Could not parse a define '", optionValue, "'"); + return; + } + const auto key = optionValue.substr(0, sepidx); + const auto value = optionValue.substr(sepidx + 1); + libOptions.setAsString(key, value); +} } class F3DStarter::F3DInternals @@ -707,6 +719,15 @@ class F3DStarter::F3DInternals libOptions.reset(value); }); } + else if (libf3dOptionName == "define") + { + apply_to_all(libf3dOptionValue, + [&libOptions, &keyForLog](const std::string& value) + { + keyForLog = value; + HandleDefine(libOptions, value); + }); + } else { assert(std::holds_alternative(libf3dOptionValue)); diff --git a/resources/cli-options.json b/resources/cli-options.json index 27bdf3d216..6e2ac2a179 100644 --- a/resources/cli-options.json +++ b/resources/cli-options.json @@ -12,7 +12,8 @@ { "longName": "define", "shortName": "D", - "helpText": "Define libf3d options" + "helpText": "Define libf3d options", + "valueHelper": "" }, { "longName": "reset", From 5706f75c57e5a171ce61843b042b5bd05836da46 Mon Sep 17 00:00:00 2001 From: Christos Tsolakis Date: Sun, 17 May 2026 00:00:28 +0300 Subject: [PATCH 4/6] Add new tests for multivalue --define and --reset --- application/testing/tests.features.cmake | 3 +++ testing/baselines/TestConfigResetMultiple.png | 3 +++ testing/baselines/TestDefineAndReset.png | 3 +++ testing/baselines/TestDefinesMultiple.png | 3 +++ 4 files changed, 12 insertions(+) create mode 100644 testing/baselines/TestConfigResetMultiple.png create mode 100644 testing/baselines/TestDefineAndReset.png create mode 100644 testing/baselines/TestDefinesMultiple.png diff --git a/application/testing/tests.features.cmake b/application/testing/tests.features.cmake index 4a8b00ae74..0cdb462fbb 100644 --- a/application/testing/tests.features.cmake +++ b/application/testing/tests.features.cmake @@ -232,13 +232,16 @@ f3d_test(NAME TestFontColor DATA suzanne.ply ARGS -n --font-color=Orange UI) ## Special CLI syntax f3d_test(NAME TestDefines DATA dragon.vtu ARGS -Dscene.up_direction=+Z --define=model.color.rgb=red) +f3d_test(NAME TestDefinesMultiple DATA dragon.vtu ARGS --define=scene.up_direction=+Z --define=model.color.rgb=red) f3d_test(NAME TestDefinesInvalid DATA dragon.vtu ARGS -Dscene.up_direction+Z REGEXP "Could not parse a define" NO_BASELINE) f3d_test(NAME TestDefinesInexistent DATA dragon.vtu ARGS -Dscene.up_director=+Z REGEXP "option from CLI options does not exists" NO_BASELINE) f3d_test(NAME TestAlternativeOptionSyntax DATA WaterBottle.glb ARGS --max-size 0.2 REGEXP "file is bigger than max size" NO_BASELINE) f3d_test(NAME TestCustomOptionsNone DATA red_translucent_monkey.gltf ARGS --blending=none --anti-aliasing=none --point-sprites=none) +f3d_test(NAME TestDefineAndReset DATA dragon.vtu ARGS -Drender.show_edges=1 --define=render.background.color=1,1,1 -Rrender.show_edges --reset=render.background.color) ## Config f3d_test(NAME TestConfigReset DATA suzanne.stl ARGS -Rrender.grid.enable --reset=ui.axis CONFIG ${F3D_SOURCE_DIR}/testing/configs/complex.json) +f3d_test(NAME TestConfigResetMultiple DATA suzanne.stl ARGS --reset=render.grid.enable --reset=ui.axis CONFIG ${F3D_SOURCE_DIR}/testing/configs/complex.json) f3d_test(NAME TestConfigResetInexistent DATA suzanne.stl ARGS -Rrender.glid.enable REGEXP "option from CLI options does not exists" NO_BASELINE) f3d_test(NAME TestConfigOrder DATA suzanne.ply ARGS CONFIG ${F3D_SOURCE_DIR}/testing/configs/config_order.json) # `.+` > `.*` alphabetically but overridden by the order # Needs https://gitlab.kitware.com/vtk/vtk/-/merge_requests/12489 diff --git a/testing/baselines/TestConfigResetMultiple.png b/testing/baselines/TestConfigResetMultiple.png new file mode 100644 index 0000000000..ceb11bd00f --- /dev/null +++ b/testing/baselines/TestConfigResetMultiple.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:2e6e2b95e9ad761851cc0c767bcc4c04d0e9a38687350bf5401fd4f4da630789 +size 23695 diff --git a/testing/baselines/TestDefineAndReset.png b/testing/baselines/TestDefineAndReset.png new file mode 100644 index 0000000000..70da184380 --- /dev/null +++ b/testing/baselines/TestDefineAndReset.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:64feff09fbb315240054a0c163dbd97e18f00ab8732cb1f9719bc39496baacf5 +size 22986 diff --git a/testing/baselines/TestDefinesMultiple.png b/testing/baselines/TestDefinesMultiple.png new file mode 100644 index 0000000000..ebb3f7d0f2 --- /dev/null +++ b/testing/baselines/TestDefinesMultiple.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:ed55b928b8bfd5d027840d84487eaaca5945a5d8fcacc3b792204f92b0529f53 +size 26830 From a2bbccbceafe650c88fd1621866ce839c6f3d9f8 Mon Sep 17 00:00:00 2001 From: Christos Tsolakis Date: Wed, 20 May 2026 19:44:09 +0300 Subject: [PATCH 5/6] Fix unused variable warning fixes warning emited due to [-Werror=unused-variable] --- application/F3DOptionsTools.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/application/F3DOptionsTools.cxx b/application/F3DOptionsTools.cxx index c1fd0f8a4b..d58042777f 100644 --- a/application/F3DOptionsTools.cxx +++ b/application/F3DOptionsTools.cxx @@ -497,7 +497,7 @@ std::vector> F3DOptionsTools::ConvertToLibf3dOptions(const std::string& key, const OptionValue& value) { std::vector> libf3dOptions; - const bool is_single_valued = std::holds_alternative(value); + [[maybe_unused]] const bool is_single_valued = std::holds_alternative(value); // Simple one-to-one case auto libf3dIter = F3DOptionsTools::LibOptionsNames.find(key); From 970844211cbbb4e6a79732fffabd3d6e041522ec Mon Sep 17 00:00:00 2001 From: Christos Tsolakis Date: Wed, 20 May 2026 19:55:12 +0300 Subject: [PATCH 6/6] Fix shadowing warning fixes warning emitted due to [-Werror=shadow] --- application/F3DStarter.cxx | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/application/F3DStarter.cxx b/application/F3DStarter.cxx index 85d596abf1..5b57eca021 100644 --- a/application/F3DStarter.cxx +++ b/application/F3DStarter.cxx @@ -707,25 +707,25 @@ class F3DStarter::F3DInternals if (libf3dOptionName == "reset") { apply_to_all(libf3dOptionValue, - [&libOptions, &keyForLog](const std::string& value) + [&libOptions, &keyForLog](const std::string& optionValue) { - keyForLog = value; - if (value.empty()) + keyForLog = optionValue; + if (optionValue.empty()) { f3d::log::warn("Invalid option: 'reset' must be followed by a valid " "option name, ignoring entry"); return; } - libOptions.reset(value); + libOptions.reset(optionValue); }); } else if (libf3dOptionName == "define") { apply_to_all(libf3dOptionValue, - [&libOptions, &keyForLog](const std::string& value) + [&libOptions, &keyForLog](const std::string& optionValue) { - keyForLog = value; - HandleDefine(libOptions, value); + keyForLog = optionValue; + HandleDefine(libOptions, optionValue); }); } else