diff --git a/src/libslic3r/Config.hpp b/src/libslic3r/Config.hpp index 9d76262ec6..fd1745d7cc 100644 --- a/src/libslic3r/Config.hpp +++ b/src/libslic3r/Config.hpp @@ -841,8 +841,12 @@ public: { UNUSED(append); std::istringstream iss(str); - iss >> this->value; - return !iss.fail(); + double value; + iss >> value; + if (iss.fail()) + return false; + this->value = value; + return true; } bool operator==(const ConfigOption &rhs) const override @@ -1033,8 +1037,12 @@ public: { UNUSED(append); std::istringstream iss(str); - iss >> this->value; - return !iss.fail(); + int value; + iss >> value; + if (iss.fail()) + return false; + this->value = value; + return true; } ConfigOptionInt& operator=(const ConfigOption *opt) @@ -1252,8 +1260,12 @@ public: UNUSED(append); // don't try to parse the trailing % since it's optional std::istringstream iss(str); - iss >> this->value; - return !iss.fail(); + double value; + iss >> value; + if (iss.fail()) + return false; + this->value = value; + return true; } private: @@ -1365,10 +1377,14 @@ public: bool deserialize(const std::string &str, bool append = false) override { UNUSED(append); - this->percent = str.find_first_of("%") != std::string::npos; std::istringstream iss(str); - iss >> this->value; - return !iss.fail(); + double value; + iss >> value; + if (iss.fail()) + return false; + this->value = value; + this->percent = str.find_first_of("%") != std::string::npos; + return true; } private: @@ -1538,8 +1554,12 @@ public: { UNUSED(append); char dummy; - return sscanf(str.data(), " %lf , %lf %c", &this->value(0), &this->value(1), &dummy) == 2 || - sscanf(str.data(), " %lf x %lf %c", &this->value(0), &this->value(1), &dummy) == 2; + Vec2d value; + if (sscanf(str.data(), " %lf , %lf %c", &value(0), &value(1), &dummy) != 2 && + sscanf(str.data(), " %lf x %lf %c", &value(0), &value(1), &dummy) != 2) + return false; + this->value = value; + return true; } private: @@ -1658,8 +1678,12 @@ public: { UNUSED(append); char dummy; - return sscanf(str.data(), " %lf , %lf , %lf %c", &this->value(0), &this->value(1), &this->value(2), &dummy) == 3 || - sscanf(str.data(), " %lf x %lf x %lf %c", &this->value(0), &this->value(1), &this->value(2), &dummy) == 3; + Vec3d value; + if (sscanf(str.data(), " %lf , %lf , %lf %c", &value(0), &value(1), &value(2), &dummy) != 3 && + sscanf(str.data(), " %lf x %lf x %lf %c", &value(0), &value(1), &value(2), &dummy) != 3) + return false; + this->value = value; + return true; } private: diff --git a/tests/libslic3r/test_config.cpp b/tests/libslic3r/test_config.cpp index 20177bcc07..914aeb7f68 100644 --- a/tests/libslic3r/test_config.cpp +++ b/tests/libslic3r/test_config.cpp @@ -1263,6 +1263,55 @@ TEST_CASE("read_cli rejects an invalid scalar numeric value", "[Config]") { REQUIRE_FALSE(config.read_cli(3, argv, &extra, &keys)); } +TEST_CASE("A scalar option keeps its value when deserializing fails", "[Config]") { + using Catch::Matchers::WithinAbs; + + SECTION("float") { + ConfigOptionFloat opt(7.5); + REQUIRE_FALSE(opt.deserialize("abc")); + CHECK_THAT(opt.value, WithinAbs(7.5, 1e-9)); + } + SECTION("int") { + ConfigOptionInt opt(3); + REQUIRE_FALSE(opt.deserialize("abc")); + CHECK(opt.value == 3); + } + SECTION("percent") { + ConfigOptionPercent opt(40.); + REQUIRE_FALSE(opt.deserialize("abc%")); + CHECK_THAT(opt.value, WithinAbs(40., 1e-9)); + } + SECTION("float or percent") { + ConfigOptionFloatOrPercent opt(0.4, false); + REQUIRE_FALSE(opt.deserialize("abc%")); + CHECK_THAT(opt.value, WithinAbs(0.4, 1e-9)); + CHECK_FALSE(opt.percent); + } + SECTION("point") { + ConfigOptionPoint opt(Vec2d(1., 2.)); + REQUIRE_FALSE(opt.deserialize("5,abc")); + CHECK(opt.value == Vec2d(1., 2.)); + } + SECTION("point3") { + ConfigOptionPoint3 opt(Vec3d(1., 2., 3.)); + REQUIRE_FALSE(opt.deserialize("5,6,abc")); + CHECK(opt.value == Vec3d(1., 2., 3.)); + } +} + +TEST_CASE("A scalar option still takes a valid value", "[Config]") { + using Catch::Matchers::WithinAbs; + + ConfigOptionFloatOrPercent opt(0.4, false); + REQUIRE(opt.deserialize("50%")); + CHECK_THAT(opt.value, WithinAbs(50., 1e-9)); + CHECK(opt.percent); + + ConfigOptionPoint point; + REQUIRE(point.deserialize("3x4")); + CHECK(point.value == Vec2d(3., 4.)); +} + TEST_CASE("read_cli appends values when a vector option is repeated", "[Config]") { Slic3r::DynamicPrintConfig config; t_config_option_keys extra, keys;