From 8f76bf9321d0c3d43f5489260207eae19c527667 Mon Sep 17 00:00:00 2001 From: Thomas Rahm <67757218+ThomasRahm@users.noreply.github.com> Date: Sat, 8 Jun 2024 16:23:00 +0200 Subject: [PATCH 1/7] Improve handling of small areas --- src/SkeletalTrapezoidation.cpp | 75 ++++++++++++++++++++++++++-------- src/WallToolPaths.cpp | 4 ++ 2 files changed, 62 insertions(+), 17 deletions(-) diff --git a/src/SkeletalTrapezoidation.cpp b/src/SkeletalTrapezoidation.cpp index 46454c3eb6..869f7a304b 100644 --- a/src/SkeletalTrapezoidation.cpp +++ b/src/SkeletalTrapezoidation.cpp @@ -2183,6 +2183,32 @@ void SkeletalTrapezoidation::connectJunctions(ptr_vector_t& edge_ void SkeletalTrapezoidation::generateLocalMaximaSingleBeads() { std::vector& generated_toolpaths = *p_generated_toolpaths; + constexpr bool is_odd = true; + + std::function addCircleToToolpath = [&](Point2LL center, coord_t width, size_t inset_index) + { + if (inset_index >= generated_toolpaths.size()) + { + generated_toolpaths.resize(inset_index + 1); + } + generated_toolpaths[inset_index].emplace_back(inset_index, is_odd); + ExtrusionLine& line = generated_toolpaths[inset_index].back(); + // total area to be extruded is pi*(w/2)^2 = pi*w*w/4 + // Width a constant extrusion width w, that would be a length of pi*w/4 + // If we make a small circle to fill up the hole, then that circle would have a circumference of 2*pi*r + // So our circle needs to be such that r=w/8 + const coord_t r = width / 8; + constexpr coord_t n_segments = 6; + for (coord_t segment = 0; segment < n_segments; segment++) + { + double a = 2.0 * std::numbers::pi / n_segments * segment; + line.junctions_.emplace_back(center + Point2LL(r * cos(a), r * sin(a)), width, inset_index); + } + }; + + Point2LL local_maxima_accumulator; + coord_t width_accumulator = 0; + size_t accumulator_ctr = 0; for (auto& node : graph_.nodes) { @@ -2191,32 +2217,47 @@ void SkeletalTrapezoidation::generateLocalMaximaSingleBeads() continue; } Beading& beading = node.data_.getBeading()->beading_; - if (beading.bead_widths.size() % 2 == 1 && node.isLocalMaximum(true) && ! node.isCentral()) + if(beading.bead_widths.size() % 2 == 1 && node.isLocalMaximum(true)) { const size_t inset_index = beading.bead_widths.size() / 2; - constexpr bool is_odd = true; - if (inset_index >= generated_toolpaths.size()) + const coord_t width = beading.bead_widths[inset_index]; + local_maxima_accumulator += node.p_; + width_accumulator += width; + accumulator_ctr++; + if (! node.isCentral()) { - generated_toolpaths.resize(inset_index + 1); + addCircleToToolpath(node.p_, width, inset_index); } - generated_toolpaths[inset_index].emplace_back(inset_index, is_odd); - ExtrusionLine& line = generated_toolpaths[inset_index].back(); - const coord_t width = beading.bead_widths[inset_index]; - // total area to be extruded is pi*(w/2)^2 = pi*w*w/4 - // Width a constant extrusion width w, that would be a length of pi*w/4 - // If we make a small circle to fill up the hole, then that circle would have a circumference of 2*pi*r - // So our circle needs to be such that r=w/8 - const coord_t r = width / 8; - constexpr coord_t n_segments = 6; - for (coord_t segment = 0; segment < n_segments; segment++) + } + } + + if(accumulator_ctr > 0) + { + bool replace_with_local_maxima = generated_toolpaths.empty() || generated_toolpaths[0].empty(); + coord_t total_path_length = 0; + if(! replace_with_local_maxima) + { + coord_t min_width = std::numeric_limits::max(); + + for(auto line: generated_toolpaths[0]) { - double a = 2.0 * std::numbers::pi / n_segments * segment; - line.junctions_.emplace_back(node.p_ + Point2LL(r * cos(a), r * sin(a)), width, inset_index); + total_path_length += line.length(); + for (const ExtrusionJunction& j : line) + { + min_width = std::min(min_width, j.w_); + } } + replace_with_local_maxima |= total_path_length <= min_width / 2; + } + if(replace_with_local_maxima) + { + const coord_t width = width_accumulator / accumulator_ctr; + local_maxima_accumulator = Point2LL(local_maxima_accumulator.X / accumulator_ctr, local_maxima_accumulator.Y / accumulator_ctr); + generated_toolpaths[0].clear(); + addCircleToToolpath(local_maxima_accumulator, width, 0); } } } - // // ^^^^^^^^^^^^^^^^^^^^^ // TOOLPATH GENERATION diff --git a/src/WallToolPaths.cpp b/src/WallToolPaths.cpp index b144d0ebfb..92b013dd17 100644 --- a/src/WallToolPaths.cpp +++ b/src/WallToolPaths.cpp @@ -281,6 +281,10 @@ void WallToolPaths::removeSmallLines(std::vector& toolpaths) for (size_t line_idx = 0; line_idx < inset.size(); line_idx++) { ExtrusionLine& line = inset[line_idx]; + if(line.is_outer_wall()) + { + continue; + } coord_t min_width = std::numeric_limits::max(); for (const ExtrusionJunction& j : line) { From 893ae7eeab91304a4840aad462038b7a5e82c936 Mon Sep 17 00:00:00 2001 From: Remco Burema Date: Wed, 12 Jun 2024 10:52:59 +0200 Subject: [PATCH 2/7] Better reflect current use of function (up'd docs and renamed). In the current branch (suggested changes to fix the inconsistant small areas w.r.t. innaccurate points of sharp objects), only non-outer walls are removed. This aligns more closely with the originally intended (and at the moment, only) use-case of this function (as specified in the commit where it was added). However, the original function name and documentation/comment, still specified what it used to do, not what it was supposed to, and now does. (Which is removing small lines _from gap filling_ instead of just everywhere.) part of CURA-9399 --- include/WallToolPaths.h | 4 ++-- src/WallToolPaths.cpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/WallToolPaths.h b/include/WallToolPaths.h index ff1df0a63a..1ef9db161b 100644 --- a/include/WallToolPaths.h +++ b/include/WallToolPaths.h @@ -110,9 +110,9 @@ class WallToolPaths static void stitchToolPaths(std::vector& toolpaths, const Settings& settings); /*! - * Remove polylines shorter than half the smallest line width along that polyline. + * Remove polylines shorter than half the smallest line width along that polyline, if that polyline isn't part of an outer wall. */ - static void removeSmallLines(std::vector& toolpaths); + static void removeSmallFillLines(std::vector& toolpaths); /*! * Simplifies the variable-width toolpaths by calling the simplify on every line in the toolpath using the provided diff --git a/src/WallToolPaths.cpp b/src/WallToolPaths.cpp index 92b013dd17..104fb9ea06 100644 --- a/src/WallToolPaths.cpp +++ b/src/WallToolPaths.cpp @@ -181,7 +181,7 @@ const std::vector& WallToolPaths::generate() scripta::PointVDI{ "width", &ExtrusionJunction::w_ }, scripta::PointVDI{ "perimeter_index", &ExtrusionJunction::perimeter_index_ }); - removeSmallLines(toolpaths_); + removeSmallFillLines(toolpaths_); scripta::log( "toolpaths_2", toolpaths_, @@ -274,7 +274,7 @@ void WallToolPaths::stitchToolPaths(std::vector& toolpaths, } } -void WallToolPaths::removeSmallLines(std::vector& toolpaths) +void WallToolPaths::removeSmallFillLines(std::vector& toolpaths) { for (VariableWidthLines& inset : toolpaths) { From f6de85bd024bfa13b3a7d133ab883fc2e8b94054 Mon Sep 17 00:00:00 2001 From: Remco Burema Date: Wed, 12 Jun 2024 11:50:08 +0200 Subject: [PATCH 3/7] Small refactors. None of this should change how it works, mostly just renaming and shuffeling things around. part of CURA-9399 --- src/SkeletalTrapezoidation.cpp | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/SkeletalTrapezoidation.cpp b/src/SkeletalTrapezoidation.cpp index 869f7a304b..a35cc49dbe 100644 --- a/src/SkeletalTrapezoidation.cpp +++ b/src/SkeletalTrapezoidation.cpp @@ -2183,14 +2183,14 @@ void SkeletalTrapezoidation::connectJunctions(ptr_vector_t& edge_ void SkeletalTrapezoidation::generateLocalMaximaSingleBeads() { std::vector& generated_toolpaths = *p_generated_toolpaths; - constexpr bool is_odd = true; - std::function addCircleToToolpath = [&](Point2LL center, coord_t width, size_t inset_index) + const auto addCircleToToolpath = [&](Point2LL center, coord_t width, size_t inset_index) { if (inset_index >= generated_toolpaths.size()) { generated_toolpaths.resize(inset_index + 1); } + constexpr bool is_odd = true; generated_toolpaths[inset_index].emplace_back(inset_index, is_odd); ExtrusionLine& line = generated_toolpaths[inset_index].back(); // total area to be extruded is pi*(w/2)^2 = pi*w*w/4 @@ -2208,7 +2208,7 @@ void SkeletalTrapezoidation::generateLocalMaximaSingleBeads() Point2LL local_maxima_accumulator; coord_t width_accumulator = 0; - size_t accumulator_ctr = 0; + size_t accumulator_count = 0; for (auto& node : graph_.nodes) { @@ -2223,7 +2223,7 @@ void SkeletalTrapezoidation::generateLocalMaximaSingleBeads() const coord_t width = beading.bead_widths[inset_index]; local_maxima_accumulator += node.p_; width_accumulator += width; - accumulator_ctr++; + ++accumulator_count; if (! node.isCentral()) { addCircleToToolpath(node.p_, width, inset_index); @@ -2231,14 +2231,13 @@ void SkeletalTrapezoidation::generateLocalMaximaSingleBeads() } } - if(accumulator_ctr > 0) + if(accumulator_count > 0) { bool replace_with_local_maxima = generated_toolpaths.empty() || generated_toolpaths[0].empty(); coord_t total_path_length = 0; if(! replace_with_local_maxima) { coord_t min_width = std::numeric_limits::max(); - for(auto line: generated_toolpaths[0]) { total_path_length += line.length(); @@ -2251,8 +2250,8 @@ void SkeletalTrapezoidation::generateLocalMaximaSingleBeads() } if(replace_with_local_maxima) { - const coord_t width = width_accumulator / accumulator_ctr; - local_maxima_accumulator = Point2LL(local_maxima_accumulator.X / accumulator_ctr, local_maxima_accumulator.Y / accumulator_ctr); + const coord_t width = width_accumulator / accumulator_count; + local_maxima_accumulator = local_maxima_accumulator / accumulator_count; generated_toolpaths[0].clear(); addCircleToToolpath(local_maxima_accumulator, width, 0); } From a15471fb88ff6868e0230d9e9e6259b0e7126fdb Mon Sep 17 00:00:00 2001 From: rburema Date: Wed, 12 Jun 2024 09:51:35 +0000 Subject: [PATCH 4/7] Applied clang-format. --- src/SkeletalTrapezoidation.cpp | 10 +++++----- src/WallToolPaths.cpp | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/SkeletalTrapezoidation.cpp b/src/SkeletalTrapezoidation.cpp index a35cc49dbe..e87fa428a3 100644 --- a/src/SkeletalTrapezoidation.cpp +++ b/src/SkeletalTrapezoidation.cpp @@ -2217,7 +2217,7 @@ void SkeletalTrapezoidation::generateLocalMaximaSingleBeads() continue; } Beading& beading = node.data_.getBeading()->beading_; - if(beading.bead_widths.size() % 2 == 1 && node.isLocalMaximum(true)) + if (beading.bead_widths.size() % 2 == 1 && node.isLocalMaximum(true)) { const size_t inset_index = beading.bead_widths.size() / 2; const coord_t width = beading.bead_widths[inset_index]; @@ -2231,14 +2231,14 @@ void SkeletalTrapezoidation::generateLocalMaximaSingleBeads() } } - if(accumulator_count > 0) + if (accumulator_count > 0) { bool replace_with_local_maxima = generated_toolpaths.empty() || generated_toolpaths[0].empty(); coord_t total_path_length = 0; - if(! replace_with_local_maxima) + if (! replace_with_local_maxima) { coord_t min_width = std::numeric_limits::max(); - for(auto line: generated_toolpaths[0]) + for (auto line : generated_toolpaths[0]) { total_path_length += line.length(); for (const ExtrusionJunction& j : line) @@ -2248,7 +2248,7 @@ void SkeletalTrapezoidation::generateLocalMaximaSingleBeads() } replace_with_local_maxima |= total_path_length <= min_width / 2; } - if(replace_with_local_maxima) + if (replace_with_local_maxima) { const coord_t width = width_accumulator / accumulator_count; local_maxima_accumulator = local_maxima_accumulator / accumulator_count; diff --git a/src/WallToolPaths.cpp b/src/WallToolPaths.cpp index 104fb9ea06..c34aaf8d69 100644 --- a/src/WallToolPaths.cpp +++ b/src/WallToolPaths.cpp @@ -281,7 +281,7 @@ void WallToolPaths::removeSmallFillLines(std::vector& toolpa for (size_t line_idx = 0; line_idx < inset.size(); line_idx++) { ExtrusionLine& line = inset[line_idx]; - if(line.is_outer_wall()) + if (line.is_outer_wall()) { continue; } From 3c5b736b2a226dd242fec4a11da2a259d7def373 Mon Sep 17 00:00:00 2001 From: Remco Burema Date: Tue, 18 Jun 2024 09:19:01 +0200 Subject: [PATCH 5/7] Review comments: Const correctness. done as part of CURA-9399 --- include/SkeletalTrapezoidationJoint.h | 2 +- src/SkeletalTrapezoidation.cpp | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/include/SkeletalTrapezoidationJoint.h b/include/SkeletalTrapezoidationJoint.h index 059f6d50d1..3aa2ff88e3 100644 --- a/include/SkeletalTrapezoidationJoint.h +++ b/include/SkeletalTrapezoidationJoint.h @@ -51,7 +51,7 @@ class SkeletalTrapezoidationJoint { beading_ = storage; } - std::shared_ptr getBeading() + std::shared_ptr getBeading() const { return beading_.lock(); } diff --git a/src/SkeletalTrapezoidation.cpp b/src/SkeletalTrapezoidation.cpp index e87fa428a3..f8f53f7f76 100644 --- a/src/SkeletalTrapezoidation.cpp +++ b/src/SkeletalTrapezoidation.cpp @@ -2184,7 +2184,7 @@ void SkeletalTrapezoidation::generateLocalMaximaSingleBeads() { std::vector& generated_toolpaths = *p_generated_toolpaths; - const auto addCircleToToolpath = [&](Point2LL center, coord_t width, size_t inset_index) + const auto addCircleToToolpath = [&](const Point2LL& center, coord_t width, size_t inset_index) { if (inset_index >= generated_toolpaths.size()) { @@ -2210,13 +2210,13 @@ void SkeletalTrapezoidation::generateLocalMaximaSingleBeads() coord_t width_accumulator = 0; size_t accumulator_count = 0; - for (auto& node : graph_.nodes) + for (const auto& node : graph_.nodes) { if (! node.data_.hasBeading()) { continue; } - Beading& beading = node.data_.getBeading()->beading_; + const Beading& beading = node.data_.getBeading()->beading_; if (beading.bead_widths.size() % 2 == 1 && node.isLocalMaximum(true)) { const size_t inset_index = beading.bead_widths.size() / 2; @@ -2238,7 +2238,7 @@ void SkeletalTrapezoidation::generateLocalMaximaSingleBeads() if (! replace_with_local_maxima) { coord_t min_width = std::numeric_limits::max(); - for (auto line : generated_toolpaths[0]) + for (const auto& line : generated_toolpaths[0]) { total_path_length += line.length(); for (const ExtrusionJunction& j : line) From 251a977bb843b5ed8600e23fd8e69ae2b0cffc8b Mon Sep 17 00:00:00 2001 From: Remco Burema Date: Tue, 18 Jun 2024 11:54:43 +0200 Subject: [PATCH 6/7] 'Use' (read: adapt ... to) 'makeCircle' instead (DRY). Since a junction is a bit more than just a point, I needed to bring in the varargs to make the emplace still work properly in order to make it work in all cases (both points in a polygon and junctions in a vector). done as part of CURA-9399 --- include/utils/polygonUtils.h | 11 ++++++++++- src/SkeletalTrapezoidation.cpp | 8 +++----- src/utils/polygonUtils.cpp | 10 ---------- 3 files changed, 13 insertions(+), 16 deletions(-) diff --git a/include/utils/polygonUtils.h b/include/utils/polygonUtils.h index 825bc62e79..7ed9d92787 100644 --- a/include/utils/polygonUtils.h +++ b/include/utils/polygonUtils.h @@ -674,7 +674,16 @@ class PolygonUtils * \param a_step The angle between segments of the circle. * \return A new Polygon containing the circle. */ - static Polygon makeCircle(const Point2LL mid, const coord_t radius, const AngleRadians a_step = std::numbers::pi / 8); + template + static T makeCircle(const Point2LL& mid, const coord_t radius, const AngleRadians a_step = std::numbers::pi / 8, VA... args) + { + T circle; + for (double a = 0; a < 2 * std::numbers::pi; a += a_step) + { + circle.emplace_back(mid + Point2LL(radius * cos(a), radius * sin(a)), args...); + } + return circle; + } /*! * Create a "wheel" shape. diff --git a/src/SkeletalTrapezoidation.cpp b/src/SkeletalTrapezoidation.cpp index f8f53f7f76..5551f81155 100644 --- a/src/SkeletalTrapezoidation.cpp +++ b/src/SkeletalTrapezoidation.cpp @@ -17,6 +17,7 @@ #include "utils/VoronoiUtils.h" #include "utils/linearAlg2D.h" #include "utils/macros.h" +#include "utils/polygonUtils.h" #define SKELETAL_TRAPEZOIDATION_BEAD_SEARCH_MAX \ 1000 // A limit to how long it'll keep searching for adjacent beads. Increasing will re-use beadings more often (saving performance), but search longer for beading (costing @@ -2199,11 +2200,8 @@ void SkeletalTrapezoidation::generateLocalMaximaSingleBeads() // So our circle needs to be such that r=w/8 const coord_t r = width / 8; constexpr coord_t n_segments = 6; - for (coord_t segment = 0; segment < n_segments; segment++) - { - double a = 2.0 * std::numbers::pi / n_segments * segment; - line.junctions_.emplace_back(center + Point2LL(r * cos(a), r * sin(a)), width, inset_index); - } + const auto circle = PolygonUtils::makeCircle>(center, r, 2 * std::numbers::pi / n_segments, width, inset_index); + line.junctions_.insert(line.junctions_.end(), circle.begin(), circle.end()); }; Point2LL local_maxima_accumulator; diff --git a/src/utils/polygonUtils.cpp b/src/utils/polygonUtils.cpp index 84f367c586..3890055558 100644 --- a/src/utils/polygonUtils.cpp +++ b/src/utils/polygonUtils.cpp @@ -1386,16 +1386,6 @@ double PolygonUtils::relativeHammingDistance(const Shape& poly_a, const Shape& p return hamming_distance / total_area; } -Polygon PolygonUtils::makeCircle(const Point2LL mid, const coord_t radius, const AngleRadians a_step) -{ - Polygon circle; - for (double a = 0; a < 2 * std::numbers::pi; a += a_step) - { - circle.emplace_back(mid + Point2LL(radius * cos(a), radius * sin(a))); - } - return circle; -} - Polygon PolygonUtils::makeWheel(const Point2LL& mid, const coord_t inner_radius, const coord_t outer_radius, const size_t semi_nb_spokes, const size_t arc_angle_resolution) { Polygon wheel; From 5c24075c5561a5709bebd2cff506b88b8e97bf6e Mon Sep 17 00:00:00 2001 From: Remco Burema Date: Wed, 19 Jun 2024 17:02:45 +0200 Subject: [PATCH 7/7] Workaround for when the toolpaths are empty at this point. done as part of CURA-9399 --- src/SkeletalTrapezoidation.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/SkeletalTrapezoidation.cpp b/src/SkeletalTrapezoidation.cpp index 5551f81155..2bb4f2ccca 100644 --- a/src/SkeletalTrapezoidation.cpp +++ b/src/SkeletalTrapezoidation.cpp @@ -2250,7 +2250,14 @@ void SkeletalTrapezoidation::generateLocalMaximaSingleBeads() { const coord_t width = width_accumulator / accumulator_count; local_maxima_accumulator = local_maxima_accumulator / accumulator_count; - generated_toolpaths[0].clear(); + if (generated_toolpaths.empty()) + { + generated_toolpaths.emplace_back(); + } + else + { + generated_toolpaths[0].clear(); + } addCircleToToolpath(local_maxima_accumulator, width, 0); } }