diff --git a/src/libslic3r/PrintConfig.cpp b/src/libslic3r/PrintConfig.cpp index 3f977647ce..634164a005 100644 --- a/src/libslic3r/PrintConfig.cpp +++ b/src/libslic3r/PrintConfig.cpp @@ -10238,6 +10238,10 @@ int DynamicPrintConfig::update_values_from_single_to_multi(DynamicPrintConfig& m for (int index = 0; index < variant_count; index++) { + //variant_count is the variant column width, src_opt the value array; + //they disagree when the source was authored at a different width + if (index >= (int)src_opt->values.size()) + break; if (opt->values[index] > src_opt->values[index]) opt->values[index] = src_opt->values[index]; } @@ -10255,6 +10259,8 @@ int DynamicPrintConfig::update_values_from_single_to_multi(DynamicPrintConfig& m for (int index = 0; index < variant_count; index++) { + if (index >= (int)src_opt->values.size()) + break; if (opt->values[index].value > src_opt->values[index].value) opt->values[index] = src_opt->values[index]; } @@ -10449,6 +10455,10 @@ int DynamicPrintConfig::update_values_from_multi_to_multi(DynamicPrintConfig& ne for(auto idx : variant_indices){ assert(idx < old_count); + //the counts come from the variant columns, the arrays from the options; + //they disagree when a config was authored at a different variant width + if (idx >= old_count || new_variant_index >= (int)opt->values.size()) + continue; if (old_values[idx] < opt->values[new_variant_index]) opt->values[new_variant_index] = old_values[idx]; } @@ -10479,6 +10489,10 @@ int DynamicPrintConfig::update_values_from_multi_to_multi(DynamicPrintConfig& ne for(auto idx : variant_indices){ assert(idx < old_count); + //the counts come from the variant columns, the arrays from the options; + //they disagree when a config was authored at a different variant width + if (idx >= old_count || new_variant_index >= (int)opt->values.size()) + continue; if (old_values[idx] < opt->values[new_variant_index]) opt->values[new_variant_index] = old_values[idx]; } @@ -10509,6 +10523,8 @@ int DynamicPrintConfig::update_values_from_multi_to_multi(DynamicPrintConfig& ne for(auto idx : variant_indices){ assert(idx < old_count); + if (idx >= old_count || new_variant_index >= (int)opt->values.size()) + continue; if (old_values[idx]) //enabled opt->values[new_variant_index] = old_values[idx]; } @@ -10549,6 +10565,15 @@ int DynamicPrintConfig::update_values_from_multi_to_multi_2(const std::vectorkeys(); for(auto& key : keys){ if(key_sets.find(key) == key_sets.end()) @@ -10564,7 +10589,13 @@ int DynamicPrintConfig::update_values_from_multi_to_multi_2(const std::vectoroption(key); auto src_values = opt->values; - auto dst_values = dst_config.option(key) ->values; + const auto* dst_opt = dst_config.option(key); + if(!dst_opt){ + BOOST_LOG_TRIVIAL(warning) << __FUNCTION__ << boost::format(", Line %1%: %2% missing from destination config")%__LINE__%key; + break; + } + auto dst_values = dst_opt->values; + dst_values.resize(dst_variant_count, ConfigOptionFloatsNullable::nil_value()); for(size_t dst_idx =0; dst_idx < same_variant_indices.size(); ++dst_idx){ auto& indices = same_variant_indices[dst_idx]; if(indices.empty()) @@ -10572,7 +10603,7 @@ int DynamicPrintConfig::update_values_from_multi_to_multi_2(const std::vector::max(); for(auto idx : indices){ - if(opt && idx < opt->values.size() && !opt->is_nil(idx)){ + if(idx < (int)opt->values.size() && !opt->is_nil(idx)){ has_value = true; target_value = std::min(target_value, src_values[idx]); } @@ -10588,7 +10619,13 @@ int DynamicPrintConfig::update_values_from_multi_to_multi_2(const std::vectoroption(key); auto src_values = opt->values; - auto dst_values = dst_config.option(key) ->values; + const auto* dst_opt = dst_config.option(key); + if(!dst_opt){ + BOOST_LOG_TRIVIAL(warning) << __FUNCTION__ << boost::format(", Line %1%: %2% missing from destination config")%__LINE__%key; + break; + } + auto dst_values = dst_opt->values; + dst_values.resize(dst_variant_count, ConfigOptionFloatsOrPercentsNullable::nil_value()); for(size_t dst_idx =0; dst_idx < same_variant_indices.size(); ++dst_idx){ auto& indices = same_variant_indices[dst_idx]; if(indices.empty()) @@ -10596,7 +10633,7 @@ int DynamicPrintConfig::update_values_from_multi_to_multi_2(const std::vectoris_nil(idx)){ + if(idx < (int)opt->values.size() && !opt->is_nil(idx)){ has_value = true; target_value = src_values[idx].value < target_value.value ? src_values[idx] : target_value; } @@ -10612,15 +10649,21 @@ int DynamicPrintConfig::update_values_from_multi_to_multi_2(const std::vectoroption(key); auto src_values = opt->values; - auto dst_values = dst_config.option(key) ->values; + const auto* dst_opt = dst_config.option(key); + if(!dst_opt){ + BOOST_LOG_TRIVIAL(warning) << __FUNCTION__ << boost::format(", Line %1%: %2% missing from destination config")%__LINE__%key; + break; + } + auto dst_values = dst_opt->values; + dst_values.resize(dst_variant_count, ConfigOptionBoolsNullable::nil_value()); for(size_t dst_idx =0; dst_idx < same_variant_indices.size(); ++dst_idx){ auto indices = same_variant_indices[dst_idx]; if(indices.empty()) continue; bool has_value = false; - bool target_value; + bool target_value = false; for(auto idx : indices){ - if(opt && !opt->is_nil(idx)){ + if(idx < (int)opt->values.size() && !opt->is_nil(idx)){ has_value = true; target_value = src_values[idx]; break; diff --git a/tests/libslic3r/test_config_variant_expansion.cpp b/tests/libslic3r/test_config_variant_expansion.cpp index dd7de2af44..28901702cc 100644 --- a/tests/libslic3r/test_config_variant_expansion.cpp +++ b/tests/libslic3r/test_config_variant_expansion.cpp @@ -479,3 +479,84 @@ TEST_CASE("update_values_to_printer_extruders_for_multiple_filaments resolves pe REQUIRE(config.option("filament_self_index")->values == std::vector({1, 2})); } } + +// update_values_from_multi_to_multi_2 walks the DESTINATION PRINTER's variant list while writing +// into a row taken from the destination PRINT preset, whose arrays are sized to its own +// print_extruder_variant. Those two widths disagree until the print preset is re-selected for the +// new printer -- Tab::load_current_preset() runs this migration first -- so a project authored on +// a single-variant printer, opened and switched to a wider one, wrote past the end of the row. +TEST_CASE("update_values_from_multi_to_multi_2 sizes the destination row to the variant count", + "[Config][VariantExpansion]") +{ + const std::vector src_variants{"Direct Drive Standard"}; + const std::vector dst_variants{"Direct Drive Standard", "Direct Drive High Flow", + "Direct Drive Standard", "Direct Drive High Flow"}; + const std::set keys{"outer_wall_speed"}; + + // The per-object override as authored on the single-variant printer. + const auto object_override = [] { + DynamicPrintConfig c; + c.option("outer_wall_speed", true)->values = {42.}; + return c; + }; + + SECTION("a row narrower than the variant list is grown, not overrun") { + DynamicPrintConfig object_config = object_override(); + DynamicPrintConfig dst; + dst.option("outer_wall_speed", true)->values = {200.}; + + REQUIRE(object_config.update_values_from_multi_to_multi_2(src_variants, dst_variants, dst, keys) == 0); + + const auto& out = object_config.option("outer_wall_speed")->values; + REQUIRE(out.size() == dst_variants.size()); + // Both "Direct Drive Standard" columns match the source variant, so they take the override. + CHECK(out[0] == Catch::Approx(42.)); + CHECK(out[2] == Catch::Approx(42.)); + // The High Flow columns have no matching source variant: nil, so the destination keeps + // tracking the print preset rather than being pinned to another variant's value. + CHECK(std::isnan(out[1])); + CHECK(std::isnan(out[3])); + } + + // The regression guard: where the row already matches the variant list -- every case that was + // not corrupting the heap -- the resize is a no-op and the output is unchanged. + SECTION("a correctly sized row is untouched") { + DynamicPrintConfig object_config = object_override(); + DynamicPrintConfig dst; + dst.option("outer_wall_speed", true)->values = {200., 500., 210., 510.}; + + REQUIRE(object_config.update_values_from_multi_to_multi_2(src_variants, dst_variants, dst, keys) == 0); + + const auto& out = object_config.option("outer_wall_speed")->values; + REQUIRE(out.size() == 4); + CHECK(out[0] == Catch::Approx(42.)); // matched -> override + CHECK(out[1] == Catch::Approx(500.)); // unmatched -> preset value preserved + CHECK(out[2] == Catch::Approx(42.)); + CHECK(out[3] == Catch::Approx(510.)); + } + + // is_nil(idx) indexes values[idx] with no bounds check, so a source shorter than its own + // variant list read out of range before the guard was added. + SECTION("a source shorter than its variant list is read in range") { + DynamicPrintConfig object_config = object_override(); // one value... + DynamicPrintConfig dst; + dst.option("outer_wall_speed", true)->values = {200., 500.}; + + REQUIRE(object_config.update_values_from_multi_to_multi_2( + {"Direct Drive Standard", "Direct Drive Standard"}, // ...but two source variants + {"Direct Drive Standard", "Direct Drive High Flow"}, dst, keys) == 0); + + const auto& out = object_config.option("outer_wall_speed")->values; + REQUIRE(out.size() == 2); + CHECK(out[0] == Catch::Approx(42.)); + CHECK(out[1] == Catch::Approx(500.)); + } + + SECTION("an empty destination variant list is refused") { + DynamicPrintConfig object_config = object_override(); + DynamicPrintConfig dst; + dst.option("outer_wall_speed", true)->values = {200.}; + + CHECK(object_config.update_values_from_multi_to_multi_2(src_variants, {}, dst, keys) == -1); + } +}