diff --git a/src/libslic3r/PrintConfig.cpp b/src/libslic3r/PrintConfig.cpp index 2a4eb8d7a7..17d7cee678 100644 --- a/src/libslic3r/PrintConfig.cpp +++ b/src/libslic3r/PrintConfig.cpp @@ -10091,6 +10091,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]; } @@ -10108,6 +10112,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]; } @@ -10302,6 +10308,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]; } @@ -10332,6 +10342,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]; } @@ -10362,6 +10376,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]; } @@ -10402,6 +10418,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()) @@ -10417,7 +10442,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()) @@ -10425,7 +10456,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]); } @@ -10441,7 +10472,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()) @@ -10449,7 +10486,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; } @@ -10465,15 +10502,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); + } +}