From 74cf1483841b0421282ee5eb9ff5f617d3e1a79d Mon Sep 17 00:00:00 2001 From: Kris Austin Date: Fri, 11 Sep 2026 19:22:16 -0500 Subject: [PATCH] fix: sequential-print arrange settings are ignored and never persisted (#15425) --- src/slic3r/GUI/GLCanvas3D.cpp | 138 ++++++++++--------------------- src/slic3r/GUI/GLCanvas3D.hpp | 19 +---- src/slic3r/GUI/GUI_App.cpp | 2 +- tests/libslic3r/test_arrange.cpp | 98 +++++++++++++++++++++- tests/libslic3r/test_config.cpp | 70 ++++++++++++++++ 5 files changed, 213 insertions(+), 114 deletions(-) diff --git a/src/slic3r/GUI/GLCanvas3D.cpp b/src/slic3r/GUI/GLCanvas3D.cpp index 676d310f7b..e63501eec1 100644 --- a/src/slic3r/GUI/GLCanvas3D.cpp +++ b/src/slic3r/GUI/GLCanvas3D.cpp @@ -1078,56 +1078,36 @@ const double GLCanvas3D::DefaultCameraZoomToPlateMarginFactor = 1.25; void GLCanvas3D::load_arrange_settings() { - std::string dist_fff_str = - wxGetApp().app_config->get("arrange", "min_object_distance_fff"); + // Each key must match what _render_arrange_menu writes, which appends a per-mode + // postfix to the base name. + auto load_float = [](const char *key, float &out) { + // The menu writes these with float_to_string_decimal_point, so parse them back + // the same way rather than with anything locale-dependent. + std::string value = wxGetApp().app_config->get("arrange", key); + size_t parsed = 0; + double number = string_to_double_decimal_point(value, &parsed); + if (parsed > 0) + out = float(number); + }; + auto load_bool = [](const char *key, bool &out) { + std::string value = wxGetApp().app_config->get("arrange", key); + if (!value.empty()) + out = (value == "1" || value == "true"); + }; - std::string dist_fff_seq_print_str = - wxGetApp().app_config->get("arrange", "min_object_distance_seq_print_fff"); + load_float("min_object_distance_fff", m_arrange_settings_fff.distance); + load_float("min_object_distance_fff_seq_print", m_arrange_settings_fff_seq_print.distance); + load_float("min_object_distance_sla", m_arrange_settings_sla.distance); - std::string dist_sla_str = - wxGetApp().app_config->get("arrange", "min_object_distance_sla"); + load_bool("enable_rotation_fff", m_arrange_settings_fff.enable_rotation); + load_bool("enable_rotation_fff_seq_print", m_arrange_settings_fff_seq_print.enable_rotation); + load_bool("enable_rotation_sla", m_arrange_settings_sla.enable_rotation); - std::string en_rot_fff_str = - wxGetApp().app_config->get("arrange", "enable_rotation_fff"); - - std::string en_rot_fff_seqp_str = - wxGetApp().app_config->get("arrange", "enable_rotation_seq_print"); - - std::string en_rot_sla_str = - wxGetApp().app_config->get("arrange", "enable_rotation_sla"); - - std::string en_allow_multiple_materials_str = - wxGetApp().app_config->get("arrange", "allow_multi_materials_on_same_plate"); - - std::string en_avoid_region_str = - wxGetApp().app_config->get("arrange", "avoid_extrusion_cali_region"); - - - - if (!dist_fff_str.empty()) - m_arrange_settings_fff.distance = std::stof(dist_fff_str); - - if (!dist_fff_seq_print_str.empty()) - m_arrange_settings_fff_seq_print.distance = std::stof(dist_fff_seq_print_str); - - if (!dist_sla_str.empty()) - m_arrange_settings_sla.distance = std::stof(dist_sla_str); - - if (!en_rot_fff_str.empty()) - m_arrange_settings_fff.enable_rotation = (en_rot_fff_str == "1" || en_rot_fff_str == "true"); - - if (!en_allow_multiple_materials_str.empty()) - m_arrange_settings_fff.allow_multi_materials_on_same_plate = (en_allow_multiple_materials_str == "1" || en_allow_multiple_materials_str == "true"); - - - if (!en_rot_fff_seqp_str.empty()) - m_arrange_settings_fff_seq_print.enable_rotation = (en_rot_fff_seqp_str == "1" || en_rot_fff_seqp_str == "true"); - - if(!en_avoid_region_str.empty()) - m_arrange_settings_fff.avoid_extrusion_cali_region = (en_avoid_region_str == "1" || en_avoid_region_str == "true"); - - if (!en_rot_sla_str.empty()) - m_arrange_settings_sla.enable_rotation = (en_rot_sla_str == "1" || en_rot_sla_str == "true"); + // These two keys carry no postfix, so the one stored value covers both FFF modes. + load_bool("allow_multi_materials_on_same_plate", m_arrange_settings_fff.allow_multi_materials_on_same_plate); + load_bool("allow_multi_materials_on_same_plate", m_arrange_settings_fff_seq_print.allow_multi_materials_on_same_plate); + load_bool("avoid_extrusion_cali_region", m_arrange_settings_fff.avoid_extrusion_cali_region); + load_bool("avoid_extrusion_cali_region", m_arrange_settings_fff_seq_print.avoid_extrusion_cali_region); //BBS: add specific arrange settings m_arrange_settings_fff_seq_print.is_seq_print = true; @@ -5959,7 +5939,7 @@ bool GLCanvas3D::_render_orient_menu(float left, float right, float bottom, floa } //BBS: GUI refactor: adjust main toolbar position -bool GLCanvas3D::_render_arrange_menu(float left, float right, float bottom, float top) +void GLCanvas3D::_render_arrange_menu(float left, float right, float bottom, float top) { ImGuiWrapper *imgui = wxGetApp().imgui(); @@ -5984,7 +5964,6 @@ bool GLCanvas3D::_render_arrange_menu(float left, float right, float bottom, flo imgui->begin(_L("Arrange options"), ImGuiWindowFlags_NoMove | ImGuiWindowFlags_AlwaysAutoResize | ImGuiWindowFlags_NoCollapse | ImGuiWindowFlags_NoTitleBar); - ArrangeSettings settings = get_arrange_settings(); ArrangeSettings &settings_out = get_arrange_settings(); const float slider_icon_width = imgui->get_slider_icon_size().x; const float cursor_slider_left = imgui->calc_text_size(_L("Spacing")).x + imgui->scaled(1.5f); @@ -5993,13 +5972,9 @@ bool GLCanvas3D::_render_arrange_menu(float left, float right, float bottom, flo auto &appcfg = wxGetApp().app_config; PrinterTechnology ptech = current_printer_technology(); - bool settings_changed = false; - float dist_min = 0.f; // 0 means auto std::string dist_key = "min_object_distance", rot_key = "enable_rotation"; - std::string bed_shrink_x_key = "bed_shrink_x", bed_shrink_y_key = "bed_shrink_y"; std::string multi_material_key = "allow_multi_materials_on_same_plate"; std::string avoid_extrusion_key = "avoid_extrusion_cali_region"; - std::string align_to_y_axis_key = "align_to_y_axis"; std::string postfix; //BBS: bool seq_print = false; @@ -6007,59 +5982,41 @@ bool GLCanvas3D::_render_arrange_menu(float left, float right, float bottom, flo if (ptech == ptSLA) { postfix = "_sla"; } else if (ptech == ptFFF) { - seq_print = &settings == &m_arrange_settings_fff_seq_print; - if (seq_print) { - postfix = "_fff_seq_print"; - } else { - postfix = "_fff"; - } + seq_print = wxGetApp().global_print_sequence() == PrintSequence::ByObject; + postfix = seq_print ? "_fff_seq_print" : "_fff"; } dist_key += postfix; rot_key += postfix; - bed_shrink_x_key += postfix; - bed_shrink_y_key += postfix; ImGui::AlignTextToFramePadding(); imgui->text(_L("Spacing")); ImGui::SameLine(1.2 * cursor_slider_left); ImGui::PushItemWidth(window_width - slider_icon_width); - bool b_Spacing = imgui->bbl_slider_float_style("##Spacing", &settings.distance, dist_min, 100.0f, "%5.2f") || dist_min > settings.distance; + bool b_Spacing = imgui->bbl_slider_float_style("##Spacing", &settings_out.distance, 0.f, 100.0f, "%5.2f", 1.0f, /*clamp=*/false); ImGui::SameLine(window_width - slider_icon_width + 1.3 * cursor_slider_left); ImGui::PushItemWidth(1.5 * slider_icon_width); - bool b_spacing_input = ImGui::BBLDragFloat("##spacing_input", &settings.distance, 0.05f, 0.0f, 0.0f, "%.2f"); - if (b_Spacing || b_spacing_input) - { - settings.distance = std::max(dist_min, settings.distance); - settings_out.distance = settings.distance; + bool b_spacing_input = ImGui::BBLDragFloat("##spacing_input", &settings_out.distance, 0.05f, 0.0f, 0.0f, "%.2f"); + if (b_Spacing || b_spacing_input) { + settings_out.distance = std::max(0.f, settings_out.distance); appcfg->set("arrange", dist_key.c_str(), float_to_string_decimal_point(settings_out.distance)); - settings_changed = true; } imgui->text(_L("0 means auto spacing.")); ImGui::Separator(); - if (imgui->bbl_checkbox(_L("Auto rotate for arrangement"), settings.enable_rotation)) { - settings_out.enable_rotation = settings.enable_rotation; + if (imgui->bbl_checkbox(_L("Auto rotate for arrangement"), settings_out.enable_rotation)) appcfg->set("arrange", rot_key.c_str(), settings_out.enable_rotation); - settings_changed = true; - } - if (imgui->bbl_checkbox(_L("Allow multiple materials on same plate"), settings.allow_multi_materials_on_same_plate)) { - settings_out.allow_multi_materials_on_same_plate = settings.allow_multi_materials_on_same_plate; - appcfg->set("arrange", multi_material_key.c_str(), settings_out.allow_multi_materials_on_same_plate ); - settings_changed = true; - } + if (imgui->bbl_checkbox(_L("Allow multiple materials on same plate"), settings_out.allow_multi_materials_on_same_plate)) + appcfg->set("arrange", multi_material_key.c_str(), settings_out.allow_multi_materials_on_same_plate); // only show this option if the printer has micro Lidar and can do first layer scan DynamicPrintConfig ¤t_config = wxGetApp().preset_bundle->printers.get_edited_preset().config; const bool has_lidar = wxGetApp().preset_bundle->is_bbl_vendor(); auto op = current_config.option("scan_first_layer"); if (has_lidar && op && op->getBool()) { - if (imgui->bbl_checkbox(_L("Avoid extrusion calibration region"), settings.avoid_extrusion_cali_region)) { - settings_out.avoid_extrusion_cali_region = settings.avoid_extrusion_cali_region; - appcfg->set("arrange", avoid_extrusion_key.c_str(), settings_out.avoid_extrusion_cali_region ? "1" : "0"); - settings_changed = true; - } + if (imgui->bbl_checkbox(_L("Avoid extrusion calibration region"), settings_out.avoid_extrusion_cali_region)) + appcfg->set("arrange", avoid_extrusion_key.c_str(), settings_out.avoid_extrusion_cali_region); } else { settings_out.avoid_extrusion_cali_region = false; } @@ -6071,11 +6028,7 @@ bool GLCanvas3D::_render_arrange_menu(float left, float right, float bottom, flo settings_out.align_to_y_axis = false; } - if (imgui->bbl_checkbox(_L("Align to Y axis"), settings.align_to_y_axis)) { - settings_out.align_to_y_axis = settings.align_to_y_axis; - appcfg->set("arrange", align_to_y_axis_key, settings_out.align_to_y_axis ? "1" : "0"); - settings_changed = true; - } + imgui->bbl_checkbox(_L("Align to Y axis"), settings_out.align_to_y_axis); if (settings_out.enable_rotation == true) { imgui->disabled_end(); } } @@ -6091,7 +6044,6 @@ bool GLCanvas3D::_render_arrange_menu(float left, float right, float bottom, flo if (imgui->button(_L("Reset"))) { settings_out = ArrangeSettings{}; - settings_out.distance = std::max(dist_min, settings_out.distance); //BBS: add specific arrange settings if (seq_print) settings_out.is_seq_print = true; @@ -6101,18 +6053,16 @@ bool GLCanvas3D::_render_arrange_menu(float left, float right, float bottom, flo else settings_out.align_to_y_axis = false; - appcfg->set("arrange", dist_key, float_to_string_decimal_point(settings_out.distance)); - appcfg->set("arrange", rot_key, settings_out.enable_rotation ? "1" : "0"); - appcfg->set("arrange", align_to_y_axis_key, settings_out.align_to_y_axis ? "1" : "0"); - settings_changed = true; + appcfg->erase("arrange", dist_key); + appcfg->erase("arrange", rot_key); + appcfg->erase("arrange", multi_material_key); + appcfg->erase("arrange", avoid_extrusion_key); } ImGui::PopStyleVar(1); imgui->end(); //BBS ImGuiWrapper::pop_toolbar_style(); - - return settings_changed; } static const float cameraProjection[16] = {1.f, 0.f, 0.f, 0.f, 0.f, 1.f, 0.f, 0.f, 0.f, 0.f, 1.f, 0.f, 0.f, 0.f, 0.f, 1.f}; diff --git a/src/slic3r/GUI/GLCanvas3D.hpp b/src/slic3r/GUI/GLCanvas3D.hpp index 84dbd5d652..b1dd674d96 100644 --- a/src/slic3r/GUI/GLCanvas3D.hpp +++ b/src/slic3r/GUI/GLCanvas3D.hpp @@ -656,11 +656,7 @@ public: } void load_arrange_settings(); - ArrangeSettings& get_arrange_settings();// { return get_arrange_settings(this); } - ArrangeSettings& get_arrange_settings(PrintSequence print_seq) { - return (print_seq == PrintSequence::ByObject) ? m_arrange_settings_fff_seq_print - : m_arrange_settings_fff; - } + ArrangeSettings& get_arrange_settings(); class SequentialPrintClearance { @@ -1163,17 +1159,6 @@ public: void highlight_toolbar_item(const std::string& item_name); void highlight_gizmo(const std::string& gizmo_name); - ArrangeSettings get_arrange_settings() const { - const ArrangeSettings &settings = get_arrange_settings(); - ArrangeSettings ret = settings; - if (&settings == &m_arrange_settings_fff_seq_print) { - ret.distance = std::max(ret.distance, - float(min_object_distance(*m_config))); - } - - return ret; - } - // Timestamp for FPS calculation and notification fade-outs. static int64_t timestamp_now() { #ifdef _WIN32 @@ -1308,7 +1293,7 @@ private: void _render_selection_sidebar_hints() { m_selection.render_sidebar_hints(m_sidebar_field, m_gizmos.get_uniform_scaling()); } //BBS: GUI refactor: adjust main toolbar position bool _render_orient_menu(float left, float right, float bottom, float top); - bool _render_arrange_menu(float left, float right, float bottom, float top); + void _render_arrange_menu(float left, float right, float bottom, float top); void _render_3d_navigator(); void _update_volumes_hover_state(); diff --git a/src/slic3r/GUI/GUI_App.cpp b/src/slic3r/GUI/GUI_App.cpp index df2d1fccc0..fee18b4799 100644 --- a/src/slic3r/GUI/GUI_App.cpp +++ b/src/slic3r/GUI/GUI_App.cpp @@ -9197,7 +9197,7 @@ int GUI_App::filaments_cnt() const PrintSequence GUI_App::global_print_sequence() const { PrintSequence global_print_seq = PrintSequence::ByDefault; - auto curr_preset_config = preset_bundle->prints.get_edited_preset().config; + const auto &curr_preset_config = preset_bundle->prints.get_edited_preset().config; if (curr_preset_config.has("print_sequence")) global_print_seq = curr_preset_config.option>("print_sequence")->value; return global_print_seq; diff --git a/tests/libslic3r/test_arrange.cpp b/tests/libslic3r/test_arrange.cpp index a9fb51e352..3906cba8ba 100644 --- a/tests/libslic3r/test_arrange.cpp +++ b/tests/libslic3r/test_arrange.cpp @@ -4,6 +4,8 @@ #include "libslic3r/BoundingBox.hpp" #include "libslic3r/ClipperUtils.hpp" #include "libslic3r/ExPolygon.hpp" +#include "libslic3r/Print.hpp" +#include "libslic3r/PrintConfig.hpp" using namespace Slic3r; using namespace Slic3r::arrangement; @@ -24,11 +26,13 @@ ArrangePolygon make_square(coord_t side) return ap; } -ArrangePolygons squares(int n, double side_mm) +ArrangePolygons squares(int n, double side_mm, double height_mm = 0.) { ArrangePolygons items; - for (int i = 0; i < n; ++i) + for (int i = 0; i < n; ++i) { items.emplace_back(make_square(scaled(side_mm))); + items.back().height = height_mm; + } return items; } @@ -82,6 +86,38 @@ void require_no_overlap(const ArrangePolygons &items) REQUIRE(disjoint(placed_shapes(items))); } +// The sequential-print floor is chosen by comparing object height against the nozzle, +// so the two are defined together and every expectation is derived from them. +constexpr double NOZZLE_HEIGHT_MM = 2.5; +constexpr double CLEARANCE_MM = 30.; +constexpr double NOZZLE_FLOOR_MM = MAX_OUTER_NOZZLE_DIAMETER / 2.; + +ArrangeParams seq_print_params(coord_t min_dist) +{ + ArrangeParams p = quiet_params(min_dist); + p.is_seq_print = true; + p.clearance_radius = float(CLEARANCE_MM); + p.nozzle_height = float(NOZZLE_HEIGHT_MM); + p.object_skirt_offset = 0.f; + return p; +} + +// update_selected_items_inflation reads the bed out of the config to cap inflation. +DynamicPrintConfig bed_config() +{ + DynamicPrintConfig c; + c.set_key_value("printable_area", new ConfigOptionPoints{{0, 0}, {200, 0}, {200, 200}, {0, 200}}); + return c; +} + +ArrangePolygons squares_of_heights(const std::vector &heights_mm) +{ + ArrangePolygons items; + for (double height_mm : heights_mm) + items.push_back(squares(1, 20., height_mm).front()); + return items; +} + } // namespace // Prove the overlap check the other tests rely on actually detects overlap. @@ -222,3 +258,61 @@ TEST_CASE("Arrange aligns the pile to a custom center", "[Arrange]") REQUIRE(ap.bed_idx == 0); require_no_overlap(items); } + +TEST_CASE("Sequential print floors the object distance by object height", "[Arrange]") +{ + // The only place sequential-print clearance is enforced. The arrange menu offers + // no floor of its own, so a stored 0 has to be raised here or not at all. + struct Case + { + std::string description; + std::vector heights; + double skirt_offset_mm; + double expected_floor_mm; + }; + + auto c = GENERATE(values({ + {"objects taller than the nozzle need the full clearance", {NOZZLE_HEIGHT_MM * 2, NOZZLE_HEIGHT_MM * 2}, 0., CLEARANCE_MM}, + {"an object exactly at the nozzle height counts as tall", {NOZZLE_HEIGHT_MM, NOZZLE_HEIGHT_MM}, 0., CLEARANCE_MM}, + {"one tall object among short ones is enough", {NOZZLE_HEIGHT_MM / 2, NOZZLE_HEIGHT_MM * 2}, 0., CLEARANCE_MM}, + {"objects the nozzle clears keep only the nozzle-width floor", {NOZZLE_HEIGHT_MM / 2, NOZZLE_HEIGHT_MM / 2}, 0., NOZZLE_FLOOR_MM}, + {"a wide skirt raises the floor for short objects", {NOZZLE_HEIGHT_MM / 2, NOZZLE_HEIGHT_MM / 2}, 3., 6.}, + })); + + DYNAMIC_SECTION(c.description) + { + ArrangePolygons items = squares_of_heights(c.heights); + DynamicPrintConfig cfg = bed_config(); + ArrangeParams p = seq_print_params(0); + p.object_skirt_offset = float(c.skirt_offset_mm); + + update_selected_items_inflation(items, &cfg, p); + + CHECK(p.min_obj_distance >= scaled(c.expected_floor_mm)); + CHECK(p.min_obj_distance <= scaled(c.expected_floor_mm + 0.01)); + // Half each, so a pair ends up a full min_obj_distance apart. + CHECK(items.front().inflation == p.min_obj_distance / 2); + } +} + +TEST_CASE("Sequential print keeps an object distance already above the floor", "[Arrange]") +{ + const coord_t stored = scaled(CLEARANCE_MM * 2); + ArrangePolygons items = squares_of_heights({NOZZLE_HEIGHT_MM * 2, NOZZLE_HEIGHT_MM * 2}); + DynamicPrintConfig cfg = bed_config(); + ArrangeParams p = seq_print_params(stored); + + update_selected_items_inflation(items, &cfg, p); + CHECK(p.min_obj_distance == stored); +} + +TEST_CASE("Layered printing does not floor the object distance", "[Arrange]") +{ + ArrangePolygons items = squares_of_heights({NOZZLE_HEIGHT_MM * 2, NOZZLE_HEIGHT_MM * 2}); + DynamicPrintConfig cfg = bed_config(); + ArrangeParams p = seq_print_params(0); + p.is_seq_print = false; + + update_selected_items_inflation(items, &cfg, p); + CHECK(p.min_obj_distance == 0); +} diff --git a/tests/libslic3r/test_config.cpp b/tests/libslic3r/test_config.cpp index bd147b5881..9a70ecbaeb 100644 --- a/tests/libslic3r/test_config.cpp +++ b/tests/libslic3r/test_config.cpp @@ -1091,3 +1091,73 @@ TEST_CASE("get_filament_type treats empty vector options as absent", "[Config][F REQUIRE(displayed == "Sup.PLA"); } } + +namespace { + +// min_object_distance reads exactly these three options. +DynamicPrintConfig spacing_config(PrinterTechnology tech, PrintSequence seq, double clearance_radius) +{ + DynamicPrintConfig c; + c.set_key_value("printer_technology", new ConfigOptionEnum(tech)); + c.set_key_value("print_sequence", new ConfigOptionEnum(seq)); + c.set_key_value("extruder_clearance_radius", new ConfigOptionFloat(clearance_radius)); + return c; +} + +} // namespace + +TEST_CASE("min_object_distance floors object spacing per print sequence", "[Config]") +{ + struct Case + { + std::string description; + PrinterTechnology tech; + PrintSequence sequence; + double clearance_radius; + double expected; + }; + + auto c = GENERATE(values({ + {"sequential FFF takes a clearance radius above the floor", ptFFF, PrintSequence::ByObject, 12., 12.}, + {"sequential FFF holds the floor at the radius", ptFFF, PrintSequence::ByObject, 6., 6.}, + {"sequential FFF holds the floor below the radius", ptFFF, PrintSequence::ByObject, 4., 6.}, + {"layered FFF ignores the clearance radius", ptFFF, PrintSequence::ByLayer, 12., 6.}, + {"SLA is a flat 6mm", ptSLA, PrintSequence::ByObject, 12., 6.}, + {"SLA ignores the print sequence too", ptSLA, PrintSequence::ByLayer, 12., 6.}, + })); + + DYNAMIC_SECTION(c.description) + { + CHECK_THAT(min_object_distance(spacing_config(c.tech, c.sequence, c.clearance_radius)), + Catch::Matchers::WithinAbs(c.expected, 1e-9)); + } +} + +TEST_CASE("min_object_distance yields no floor when an FFF config lacks the options", "[Config]") +{ + // Missing options yield 0 rather than an error, so a caller gets no floor at all. + SECTION("no clearance radius") { + DynamicPrintConfig c; + c.set_key_value("printer_technology", new ConfigOptionEnum(ptFFF)); + c.set_key_value("print_sequence", new ConfigOptionEnum(PrintSequence::ByObject)); + CHECK_THAT(min_object_distance(c), Catch::Matchers::WithinAbs(0., 1e-9)); + } + + SECTION("no print sequence") { + DynamicPrintConfig c; + c.set_key_value("printer_technology", new ConfigOptionEnum(ptFFF)); + c.set_key_value("extruder_clearance_radius", new ConfigOptionFloat(12.)); + CHECK_THAT(min_object_distance(c), Catch::Matchers::WithinAbs(0., 1e-9)); + } + + SECTION("nothing at all") { + CHECK_THAT(min_object_distance(DynamicPrintConfig{}), Catch::Matchers::WithinAbs(0., 1e-9)); + } + + SECTION("an unset printer technology is treated as FFF") { + DynamicPrintConfig c; + c.set_key_value("print_sequence", new ConfigOptionEnum(PrintSequence::ByObject)); + c.set_key_value("extruder_clearance_radius", new ConfigOptionFloat(12.)); + CHECK_THAT(min_object_distance(c), Catch::Matchers::WithinAbs(12., 1e-9)); + } +}