From b00237807dc76bec2c967d5330497c934e95e2ce Mon Sep 17 00:00:00 2001 From: Matias Alejandro Yocca Date: Thu, 15 Oct 2026 18:13:28 -0300 Subject: [PATCH] Fix crash when dragging objects in sequential printing mode (#16029) The clearance preview shown while dragging sorted instances with a comparator that is not a strict weak ordering, which aborts with libc++ debug hardening. The preview now uses object-list order, with each object's copies in their last validated print order, which is the order print validation uses. The height check is now in SequentialPrintClearance.hpp, with unit tests. --- src/slic3r/CMakeLists.txt | 1 + src/slic3r/GUI/GLCanvas3D.cpp | 106 ++---------- src/slic3r/GUI/SequentialPrintClearance.hpp | 63 +++++++ tests/slic3rutils/CMakeLists.txt | 1 + .../slic3rutils/test_sequential_clearance.cpp | 162 ++++++++++++++++++ 5 files changed, 242 insertions(+), 91 deletions(-) create mode 100644 src/slic3r/GUI/SequentialPrintClearance.hpp create mode 100644 tests/slic3rutils/test_sequential_clearance.cpp diff --git a/src/slic3r/CMakeLists.txt b/src/slic3r/CMakeLists.txt index 05650d2275..367aadaa27 100644 --- a/src/slic3r/CMakeLists.txt +++ b/src/slic3r/CMakeLists.txt @@ -222,6 +222,7 @@ set(SLIC3R_GUI_SOURCES GUI/Gizmos/GLGizmoUtils.hpp GUI/GLCanvas3D.cpp GUI/GLCanvas3D.hpp + GUI/SequentialPrintClearance.hpp GUI/GLModel.cpp GUI/GLModel.hpp GUI/GLSelectionRectangle.cpp diff --git a/src/slic3r/GUI/GLCanvas3D.cpp b/src/slic3r/GUI/GLCanvas3D.cpp index bf87241fb6..95944a8da5 100644 --- a/src/slic3r/GUI/GLCanvas3D.cpp +++ b/src/slic3r/GUI/GLCanvas3D.cpp @@ -65,6 +65,7 @@ #include #include "slic3r/GUI/Selection.hpp" #include "GLCanvas3D.hpp" +#include "SequentialPrintClearance.hpp" #ifdef SLIC3R_CAD #include "slic3r/GUI/CAD/DesignSketchTool.hpp" // Design tab: interactive 2D sketch tool #endif @@ -6397,17 +6398,16 @@ void GLCanvas3D::update_sequential_clearance() //BBS: add the height logic PartPlate* plate = wxGetApp().plater()->get_partplate_list().get_curr_plate(); Polygons polygons; - std::vector> height_polygons; polygons.reserve(instances_count); - height_polygons.reserve(instances_count); - std::vector convex_and_bounding_boxes; - struct height_info - { - double instance_height; - BoundingBox bounding_box; - Polygon hull_polygon; - }; + std::vector convex_and_bounding_boxes; + convex_and_bounding_boxes.reserve(instances_count); + std::map print_order; + for (const PrintObject* print_object : fff_print()->objects()) + for (const PrintInstance& instance : print_object->instances()) + print_order.emplace(instance.model_instance->id(), instance.model_instance->arrange_order); + for (size_t i = 0; i < instance_transforms.size(); ++i) { + const size_t first_instance = convex_and_bounding_boxes.size(); const auto& instances = instance_transforms[i]; double rotation_z0 = instances.front()->get_rotation().z(); int index = 0; @@ -6427,96 +6427,20 @@ void GLCanvas3D::update_sequential_clearance() Polygon convex_hull(std::move(inst_pts)); BoundingBox bouding_box = convex_hull.bounding_box(); BoundingBox plate_bb = plate->get_bounding_box_crd(); + const ObjectID instance_id = m_model->objects[i]->instances[index]->id(); double instance_height = m_model->objects[i]->get_instance_max_z(index++); //skip the object for not current plate if (!plate_bb.overlap(bouding_box)) continue; - convex_and_bounding_boxes.push_back({instance_height, bouding_box, convex_hull}); + convex_and_bounding_boxes.push_back({instance_height, bouding_box, convex_hull, instance_id}); polygons.emplace_back(std::move(convex_hull)); } + sort_sequential_clearance_instances(convex_and_bounding_boxes.begin() + first_instance, convex_and_bounding_boxes.end(), print_order); } - //sort the print instance - std::sort(convex_and_bounding_boxes.begin(), convex_and_bounding_boxes.end(), - [](auto &l, auto &r) { - auto ly1 = l.bounding_box.min.y(); - auto ly2 = l.bounding_box.max.y(); - auto ry1 = r.bounding_box.min.y(); - auto ry2 = r.bounding_box.max.y(); - auto inter_min = std::max(ly1, ry1); - auto inter_max = std::min(ly2, ry2); - auto lx = l.bounding_box.min.x(); - auto rx = r.bounding_box.min.x(); - if (inter_max - inter_min > 0) - return (lx < rx) || ((lx == rx)&&(ly1 < ry1)); - else - return (ly1 < ry1); - }); - - /*bool has_interlaced_objects = false; - for (int k = 0; k < bounding_box_count; k++) - { - Polygon& convex = convex_and_bounding_boxes[k].hull_polygon; - BoundingBox& bbox = convex_and_bounding_boxes[k].bounding_box; - auto iy1 = bbox.min.y(); - auto iy2 = bbox.max.y(); - - for (int i = k+1; i < bounding_box_count; i++) - { - Polygon& next_convex = convex_and_bounding_boxes[i].hull_polygon; - BoundingBox& next_bbox = convex_and_bounding_boxes[i].bounding_box; - auto py1 = next_bbox.min.y(); - auto py2 = next_bbox.max.y(); - auto inter_min = std::max(iy1, py1); // min y of intersection - auto inter_max = std::min(iy2, py2); // max y of intersection. length=max_y-min_y>0 means intersection exists - if (inter_max - inter_min > 0) { - has_interlaced_objects = true; - break; - } - } - if (has_interlaced_objects) - break; - }*/ - - int bounding_box_count = convex_and_bounding_boxes.size(); - double printable_height = fff_print()->config().printable_height; - double hc1 = fff_print()->config().extruder_clearance_height_to_lid; - double hc2 = fff_print()->config().extruder_clearance_height_to_rod; - for (int k = 0; k < bounding_box_count; k++) - { - Polygon& convex = convex_and_bounding_boxes[k].hull_polygon; - BoundingBox& bbox = convex_and_bounding_boxes[k].bounding_box; - auto iy1 = bbox.min.y(); - auto iy2 = bbox.max.y(); - double height = (k == (bounding_box_count - 1))?printable_height:hc1; - - /*if (has_interlaced_objects) { - if ((k < (bounding_box_count - 1)) && (convex_and_bounding_boxes[k].instance_height > hc2)) { - height_polygons.emplace_back(std::make_pair(convex, hc2)); - } - } - else { - if ((k < (bounding_box_count - 1)) && (convex_and_bounding_boxes[k].instance_height > hc1)) { - height_polygons.emplace_back(std::make_pair(convex, hc1)); - } - }*/ - - for (int i = k+1; i < bounding_box_count; i++) - { - Polygon& next_convex = convex_and_bounding_boxes[i].hull_polygon; - BoundingBox& next_bbox = convex_and_bounding_boxes[i].bounding_box; - auto py1 = next_bbox.min.y(); - auto py2 = next_bbox.max.y(); - auto inter_min = std::max(iy1, py1); // min y of intersection - auto inter_max = std::min(iy2, py2); // max y of intersection. length=max_y-min_y>0 means intersection exists - if (inter_max - inter_min > 0) { - height = hc2; - break; - } - } - if (height < convex_and_bounding_boxes[k].instance_height) - height_polygons.emplace_back(std::make_pair(convex, height)); - } + const auto& config = fff_print()->config(); + const auto height_polygons = sequential_clearance_height_polygons(convex_and_bounding_boxes, config.printable_height, + config.extruder_clearance_height_to_lid, config.extruder_clearance_height_to_rod); // sends instances 2d hulls to be rendered set_sequential_print_clearance_visible(true); diff --git a/src/slic3r/GUI/SequentialPrintClearance.hpp b/src/slic3r/GUI/SequentialPrintClearance.hpp new file mode 100644 index 0000000000..0f5d79be9c --- /dev/null +++ b/src/slic3r/GUI/SequentialPrintClearance.hpp @@ -0,0 +1,63 @@ +#pragma once + +#include "libslic3r/BoundingBox.hpp" +#include "libslic3r/ObjectID.hpp" +#include "libslic3r/Polygon.hpp" + +#include +#include +#include +#include +#include + +namespace Slic3r::GUI { + +struct SequentialClearanceInstance +{ + double instance_height; + BoundingBox bounding_box; + Polygon hull_polygon; + ObjectID instance_id; +}; + +// PrintApply may group rotated copies in a different order from ModelObject::instances. +// Reorder one object's instances only when validation has assigned all of them an order. +inline void sort_sequential_clearance_instances( + std::vector::iterator first, std::vector::iterator last, + const std::map& print_order) +{ + if (!std::all_of(first, last, [&print_order](const SequentialClearanceInstance& instance) { + const auto order = print_order.find(instance.instance_id); + return order != print_order.end() && order->second > 0; + })) + return; + + std::stable_sort(first, last, [&print_order](const SequentialClearanceInstance& lhs, const SequentialClearanceInstance& rhs) { + return print_order.at(lhs.instance_id) < print_order.at(rhs.instance_id); + }); +} + +// Input follows object-list order with validated print order within each object. +inline std::vector> sequential_clearance_height_polygons( + const std::vector& instances, double printable_height, double height_to_lid, double height_to_rod) +{ + std::vector> height_polygons; + height_polygons.reserve(instances.size()); + for (size_t k = 0; k < instances.size(); ++k) { + const auto& instance = instances[k]; + double height = k + 1 == instances.size() ? printable_height : height_to_lid; + for (size_t i = k + 1; i < instances.size(); ++i) { + const auto& next_bbox = instances[i].bounding_box; + if (std::min(instance.bounding_box.max.y(), next_bbox.max.y()) > + std::max(instance.bounding_box.min.y(), next_bbox.min.y())) { + height = height_to_rod; + break; + } + } + if (height < instance.instance_height) + height_polygons.emplace_back(instance.hull_polygon, float(height)); + } + return height_polygons; +} + +} // namespace Slic3r::GUI diff --git a/tests/slic3rutils/CMakeLists.txt b/tests/slic3rutils/CMakeLists.txt index ac774899e7..83632601f7 100644 --- a/tests/slic3rutils/CMakeLists.txt +++ b/tests/slic3rutils/CMakeLists.txt @@ -47,6 +47,7 @@ add_executable(${_TEST_NAME}_tests test_file_url.cpp test_imgui_wrapper.cpp test_user_manager.cpp + test_sequential_clearance.cpp ../fff_print/test_helpers.cpp ) diff --git a/tests/slic3rutils/test_sequential_clearance.cpp b/tests/slic3rutils/test_sequential_clearance.cpp new file mode 100644 index 0000000000..8068212a32 --- /dev/null +++ b/tests/slic3rutils/test_sequential_clearance.cpp @@ -0,0 +1,162 @@ +#include +#include +#include + +#include "catch2/matchers/catch_matchers.hpp" +#include "fff_print/test_helpers.hpp" +#include "slic3r/GUI/SequentialPrintClearance.hpp" + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +using namespace Slic3r; +using namespace Slic3r::GUI; +using Catch::Matchers::WithinAbs; + +namespace { + +SequentialClearanceInstance instance(coord_t x, coord_t y, double height) +{ + return {height, BoundingBox(Point(x, y), Point(x + 5, y + 10)), + Polygon{Point(x, y), Point(x + 5, y), Point(x + 5, y + 10), Point(x, y + 10)}}; +} + +} // namespace + +TEST_CASE("Sequential clearance preserves model print order when objects move", "[SequentialClearance][Regression]") +{ + const coord_t last_y = GENERATE(-20, 20); + std::vector instances{instance(20, 0, 50.), instance(0, last_y, 50.)}; + const auto warnings = sequential_clearance_height_polygons(instances, 200., 30., 10.); + + REQUIRE(warnings.size() == 1); + CHECK(warnings.front().first == instance(20, 0, 50.).hull_polygon); + CHECK_THAT(warnings.front().second, WithinAbs(30., 1e-6)); +} + +TEST_CASE("Sequential clearance handles chains of overlapping Y ranges", "[SequentialClearance][Regression]") +{ + // A overlaps B, B overlaps C, but A does not overlap C. Combining X order + // for overlaps with Y order otherwise formed the cycle B < A < C < B. + std::vector instances{ + instance(20, 0, 50.), instance(10, 8, 50.), instance(0, 16, 50.)}; + const auto warnings = sequential_clearance_height_polygons(instances, 200., 30., 10.); + + REQUIRE(warnings.size() == 2); + CHECK(warnings[0].first == instance(20, 0, 50.).hull_polygon); + CHECK(warnings[1].first == instance(10, 8, 50.).hull_polygon); + CHECK_THAT(warnings[0].second, WithinAbs(10., 1e-6)); + CHECK_THAT(warnings[1].second, WithinAbs(10., 1e-6)); +} + +TEST_CASE("Sequential clearance applies rod height only to positive Y overlap", "[SequentialClearance]") +{ + const coord_t next_y = GENERATE(9, 10, 11); + std::vector instances{instance(0, 0, 50.), instance(20, next_y, 50.)}; + const auto warnings = sequential_clearance_height_polygons(instances, 200., 30., 10.); + + REQUIRE(warnings.size() == 1); + CHECK(warnings.front().first == instances.front().hull_polygon); + CHECK_THAT(warnings.front().second, WithinAbs(next_y < 10 ? 10. : 30., 1e-6)); +} + +TEST_CASE("Sequential clearance allows the last object up to printable height", "[SequentialClearance]") +{ + std::vector instances; + CHECK(sequential_clearance_height_polygons(instances, 200., 30., 10.).empty()); + + instances.push_back(instance(0, 0, 200.)); + CHECK(sequential_clearance_height_polygons(instances, 200., 30., 10.).empty()); + + instances.front().instance_height = 201.; + const auto warnings = sequential_clearance_height_polygons(instances, 200., 30., 10.); + REQUIRE(warnings.size() == 1); + CHECK_THAT(warnings.front().second, WithinAbs(200., 1e-6)); +} + +TEST_CASE("Sequential clearance warns the same rotated copy as print validation", "[SequentialClearance][Regression]") +{ + constexpr double rod_height = 10.; + constexpr double lid_height = 30.; + constexpr double printable_height = 200.; + Model model = Test::model("rotated copies", make_cube(10., 10., 50.)); + ModelObject* object = model.objects.front(); + object->instances.front()->set_offset(Vec3d(150., 80., 0.)); + ModelInstance* rotated = object->add_instance(); + rotated->set_offset(Vec3d(50., 80., 0.)); + rotated->set_rotation(Z, PI / 2.); + + DynamicPrintConfig config = DynamicPrintConfig::full_print_config(); + config.set_deserialize_strict({{"print_sequence", "by object"}, {"extruder_clearance_radius", 20.}, + {"extruder_clearance_height_to_lid", lid_height}, {"extruder_clearance_height_to_rod", rod_height}, + {"printable_height", printable_height}}); + Print print; + print.auto_assign_extruders(object); + print.apply(model, config); + Polygons polygons; + std::vector> validation_warnings; + Print::sequential_print_clearance_valid(print, &polygons, &validation_warnings); + + std::map print_order; + for (const PrintObject* print_object : print.objects()) + for (const PrintInstance& print_instance : print_object->instances()) + print_order.emplace(print_instance.model_instance->id(), print_instance.model_instance->arrange_order); + + std::vector instances; + for (size_t i = 0; i < object->instances.size(); ++i) { + Polygon hull = object->convex_hull_2d(object->instances[i]->get_matrix()); + instances.push_back({object->get_instance_max_z(i), hull.bounding_box(), hull, object->instances[i]->id()}); + } + const Polygon expected_warning = instances[1].hull_polygon; + REQUIRE(validation_warnings.size() == 1); + CHECK(validation_warnings.front().first.bounding_box().contains(instances[1].bounding_box.center())); + CHECK_FALSE(validation_warnings.front().first.bounding_box().contains(instances[0].bounding_box.center())); + + sort_sequential_clearance_instances(instances.begin(), instances.end(), print_order); + const auto warnings = sequential_clearance_height_polygons(instances, printable_height, lid_height, rod_height); + REQUIRE(warnings.size() == 1); + CHECK(warnings.front().first == expected_warning); + CHECK_THAT(warnings.front().second, WithinAbs(rod_height, 1e-6)); +} + +TEST_CASE("Sequential clearance keeps copy order when print order is unavailable", "[SequentialClearance]") +{ + std::vector instances{instance(20, 0, 50.), instance(0, 0, 50.)}; + instances[0].instance_id = ObjectID(1); + instances[1].instance_id = ObjectID(2); + std::map print_order{{ObjectID(1), 2}, {ObjectID(2), 1}}; + + SECTION("No validation order") { print_order.clear(); } + SECTION("New copy missing from print") { print_order.erase(ObjectID(2)); } + SECTION("Copy has not been ordered") { print_order[ObjectID(2)] = 0; } + SECTION("Copy has an invalid order") { print_order[ObjectID(2)] = -1; } + + sort_sequential_clearance_instances(instances.begin(), instances.end(), print_order); + CHECK(instances[0].instance_id == ObjectID(1)); + CHECK(instances[1].instance_id == ObjectID(2)); +} + +TEST_CASE("Sequential clearance keeps object order when sorting copies", "[SequentialClearance]") +{ + std::vector instances(4, instance(0, 0, 50.)); + for (size_t i = 0; i < instances.size(); ++i) + instances[i].instance_id = ObjectID(instances.size() - i); + const std::map print_order{{ObjectID(1), 1}, {ObjectID(2), 2}, {ObjectID(3), 3}, {ObjectID(4), 4}}; + + // The two objects were reordered in the model since the last validation. + sort_sequential_clearance_instances(instances.begin(), instances.begin() + 2, print_order); + sort_sequential_clearance_instances(instances.begin() + 2, instances.end(), print_order); + CHECK(instances[0].instance_id == ObjectID(3)); + CHECK(instances[1].instance_id == ObjectID(4)); + CHECK(instances[2].instance_id == ObjectID(1)); + CHECK(instances[3].instance_id == ObjectID(2)); +}