From 0abf5a60f990407a34d2faa04c28c84fb4a034a1 Mon Sep 17 00:00:00 2001 From: ExPikaPaka Date: Wed, 30 Sep 2026 08:49:34 +0200 Subject: [PATCH] Say what the code does, not what it replaced The timings and the runs that never finished belong in the commit messages, where they can be read against the change; a reader of the code cannot check them. Kept the cost that still explains the design. MultiPoint also spells out the consequence: a moved-from Polygon or Polyline is now really empty where it used to silently keep its points. The two wall spacing comments the parallel loop reindented are plain ASCII now, so the whole file is. --- src/libslic3r/GCode/OrderingStrategies.cpp | 10 +++++----- src/libslic3r/MultiPoint.hpp | 4 +++- src/libslic3r/PerimeterGenerator.cpp | 4 ++-- src/libslic3r/Support/TreeSupport.cpp | 10 +++++----- 4 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/libslic3r/GCode/OrderingStrategies.cpp b/src/libslic3r/GCode/OrderingStrategies.cpp index d52a326e2e..69a19954c3 100644 --- a/src/libslic3r/GCode/OrderingStrategies.cpp +++ b/src/libslic3r/GCode/OrderingStrategies.cpp @@ -137,8 +137,8 @@ bool tsp_remove_crossings(std::vector& path, const Points& centers) // For many islands, the same scan with the edges binned in a uniform grid over their boxes, so each edge is only tested against the edges sharing a // cell with it - two edges whose boxes overlap always do. It returns the same crossing as the all-pairs scan - // (smallest i, then smallest j), so the result is unchanged; with thousands of islands on a layer the all-pairs - // scan, repeated after every reversal, never finished. Rebuilding the grid costs more than it saves on small inputs. + // (smallest i, then smallest j), so the result is unchanged. The all-pairs scan is quadratic in the edge count and + // runs again after every reversal; rebuilding the grid costs more than it saves below the threshold. constexpr size_t grid_min_size = 500; BoundingBox extent; for (size_t idx : path) @@ -190,9 +190,9 @@ bool tsp_remove_crossings(std::vector& path, const Points& centers) // Cap iterations to prevent infinite loops on collinear/overlapping segments. int max_iters = static_cast(pn * pn); bool improved = false; - // Reversing between two segments that only touch or overlap along a line need not remove the intersection, and on - // islands laid out on a regular grid (a tiled texture, an array of parts) the loop cycled through the same orderings - // until the pn * pn cap - effectively forever. Stop as soon as an ordering repeats: until then this is the same loop. + // Reversing between two segments that only touch or overlap along a line need not remove the intersection, so on + // islands laid out on a regular grid (a tiled texture, an array of parts) the loop can cycle through the same + // orderings until the pn * pn cap. Stop as soon as an ordering repeats; up to that point this is the same loop. std::unordered_set seen_paths; const auto path_hash = [&path]() { uint64_t h = 1469598103934665603ull; // FNV-1a diff --git a/src/libslic3r/MultiPoint.hpp b/src/libslic3r/MultiPoint.hpp index b4f8af0871..19b55ceb5a 100644 --- a/src/libslic3r/MultiPoint.hpp +++ b/src/libslic3r/MultiPoint.hpp @@ -22,7 +22,9 @@ public: MultiPoint(MultiPoint &&other) noexcept : points(std::move(other.points)) {} MultiPoint(std::initializer_list list) : points(list) {} explicit MultiPoint(const Points &_points) : points(_points) {} - // Without it, the derived classes' move constructors passing std::move(points) here copied them. + // Without it, the derived classes' move constructors passing std::move(points) here copied them, which + // also means a moved-from Polygon or Polyline is now really empty where it used to silently keep its + // points: a use-after-move anywhere in the tree that happened to work before now sees nothing. explicit MultiPoint(Points &&_points) noexcept : points(std::move(_points)) {} MultiPoint& operator=(const MultiPoint &other) { points = other.points; return *this; } MultiPoint& operator=(MultiPoint &&other) noexcept { points = std::move(other.points); return *this; } diff --git a/src/libslic3r/PerimeterGenerator.cpp b/src/libslic3r/PerimeterGenerator.cpp index e99c458b2c..78cf7f9713 100644 --- a/src/libslic3r/PerimeterGenerator.cpp +++ b/src/libslic3r/PerimeterGenerator.cpp @@ -2762,10 +2762,10 @@ void PerimeterGenerator::process_arachne() // Get searching thresholds. For an external perimeter we take the external perimeter spacing/2 plus the internal perimeter spacing/2 and expand by the factor // rounding errors. When precise wall is enabled, the external perimeter full spacing is used. coord_t threshold_external = (apply_precise_outer_wall) - // Precise outer wall ⇒ use “full external spacing” + // Precise outer wall: use the full external spacing ? ( this->ext_perimeter_flow.scaled_spacing() + this->perimeter_flow.scaled_spacing()/2.0 ) - // Normal ⇒ half ext spacing + half int spacing + // Normal: half ext spacing plus half int spacing : ( this->ext_perimeter_flow.scaled_spacing()/2.0 + this->perimeter_flow.scaled_spacing()/2.0 ); diff --git a/src/libslic3r/Support/TreeSupport.cpp b/src/libslic3r/Support/TreeSupport.cpp index 174c5e4fce..aaeacf3d41 100644 --- a/src/libslic3r/Support/TreeSupport.cpp +++ b/src/libslic3r/Support/TreeSupport.cpp @@ -854,8 +854,8 @@ void TreeSupport::detect_overhangs(bool check_support_necessity/* = false*/) if (is_auto(stype) && config_detect_sharp_tails) { // BBS detect sharp tail - // Each island is tested only against the lower islands whose box meets its own: overlaps() tries every - // pair, which on a layer cut through a fine relief (thousands of islands above thousands) never ends. + // Each island is tested only against the lower islands whose box meets its own; overlaps() tries + // every pair, which is quadratic in the island counts of the two layers. std::vector lower_bboxes; lower_bboxes.reserve(lower_polys.size()); for (const ExPolygon &lower : lower_polys) @@ -870,9 +870,9 @@ void TreeSupport::detect_overhangs(bool check_support_necessity/* = false*/) for (size_t i = 0; i < lower_polys.size(); ++i) if (lower_bboxes[i].overlap(bbox)) lower_nearby.emplace_back(lower_polys[i]); - // As overlaps(expanded, lower_nearby), with each lower island cut to the island's box first: below - // a fine relief the lower layer is a few islands with thousands of holes, whose whole boundary - // was otherwise intersected again for every island above. + // As overlaps(expanded, lower_nearby), with each lower island cut to the island's box first: + // below a fine relief the lower layer is a few islands with thousands of holes, and the whole + // of that boundary would otherwise be intersected once per island above. const auto overlaps_nearby = [&]() { for (const ExPolygon &a : expanded) { if (a.empty())