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 01/14] 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 02/14] 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 03/14] 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 04/14] 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 05/14] 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 06/14] '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 1ae51c93cbf1624fd1372bd95c9849ab132f23ff Mon Sep 17 00:00:00 2001 From: Remco Burema Date: Wed, 19 Jun 2024 14:13:23 +0200 Subject: [PATCH 07/14] Don't get closest point to polygon, but intersection with line-seg to center. In isolation, you'd really want the actual closest point on the polygon, however, that'd make the seams not fit together for polygons with different sizes and shapes, especially if rounding is involved. start of CURA-11974 (split off from CURA-9474). --- include/utils/linearAlg2D.h | 2 ++ src/InsetOrderOptimizer.cpp | 34 ++++++++++++++++++++++++++++------ src/utils/linearAlg2D.cpp | 25 +++++++++++++++++++++++++ 3 files changed, 55 insertions(+), 6 deletions(-) diff --git a/include/utils/linearAlg2D.h b/include/utils/linearAlg2D.h index b9f08fb45b..71cd4d3e80 100644 --- a/include/utils/linearAlg2D.h +++ b/include/utils/linearAlg2D.h @@ -67,6 +67,8 @@ class LinearAlg2D return -1; } + static bool lineSegmentLineSegmentIntersection(const Point2LL& p1, const Point2LL& p2, const Point2LL& p3, const Point2LL& p4, float* t, float* u); + static bool lineLineIntersection(const Point2LL& a, const Point2LL& b, const Point2LL& c, const Point2LL& d, Point2LL& output); /*! diff --git a/src/InsetOrderOptimizer.cpp b/src/InsetOrderOptimizer.cpp index c093aae5d2..49857a56b0 100644 --- a/src/InsetOrderOptimizer.cpp +++ b/src/InsetOrderOptimizer.cpp @@ -186,23 +186,45 @@ void InsetOrderOptimizer::insertSeamPoint(ExtrusionLine& closed_line) return; } - // NOTE: Maybe rewrite this once we can use C++23 ranges::views::adjacent + // Get the 'origin' point of the ray we're going to trace from there to the request_point. This is the center of the bounding box of the polygon. + AABB aabb; + for (const auto& junction : closed_line.junctions_) + { + aabb.include(junction.p_); + } + const Point2LL ray_origin{ aabb.getMiddle() }; + request_point = ray_origin + (request_point - ray_origin) * 10; + + // Find the 'closest' point on the polygon to the request_point. + // This isn't actually the closest, since that'd make for pertty messy seams on 'round' objects, but instead on a ray from the ray-origin to the request_point. + Point2LL closest_point; size_t closest_junction_idx = 0; coord_t closest_distance_sqd = std::numeric_limits::max(); for (const auto& [i, junction] : closed_line.junctions_ | ranges::views::enumerate) { + // NOTE: Maybe rewrite this once we can use C++23 ranges::views::adjacent const auto& next_junction = closed_line.junctions_[(i + 1) % closed_line.junctions_.size()]; - const coord_t distance_sqd = LinearAlg2D::getDist2FromLineSegment(junction.p_, request_point, next_junction.p_); - if (distance_sqd < closest_distance_sqd) + + float t, u; + if (LinearAlg2D::lineSegmentLineSegmentIntersection(ray_origin, request_point, junction.p_, next_junction.p_, &t, &u)) { - closest_distance_sqd = distance_sqd; - closest_junction_idx = i; + const Point2LL intersection = ray_origin + (request_point - ray_origin) * t; + const coord_t distance_sqd = vSize2(request_point - intersection); + if (distance_sqd < closest_distance_sqd) + { + closest_point = intersection; + closest_distance_sqd = distance_sqd; + closest_junction_idx = i; + } } } + if (closest_distance_sqd >= std::numeric_limits::max()) + { + return; + } const auto& start_pt = closed_line.junctions_[closest_junction_idx]; const auto& end_pt = closed_line.junctions_[(closest_junction_idx + 1) % closed_line.junctions_.size()]; - const auto closest_point = LinearAlg2D::getClosestOnLineSegment(request_point, start_pt.p_, end_pt.p_); constexpr coord_t smallest_dist_sqd = 25; if (vSize2(closest_point - start_pt.p_) <= smallest_dist_sqd || vSize2(closest_point - end_pt.p_) <= smallest_dist_sqd) { diff --git a/src/utils/linearAlg2D.cpp b/src/utils/linearAlg2D.cpp index 64b50cee10..e6093e0af0 100644 --- a/src/utils/linearAlg2D.cpp +++ b/src/utils/linearAlg2D.cpp @@ -272,6 +272,31 @@ Point3Matrix LinearAlg2D::rotateAround(const Point2LL& middle, double rotation) return Point3Matrix::translate(middle).compose(rotation_matrix_homogeneous).compose(Point3Matrix::translate(-middle)); } +// A single-shot line-segment/line-segment intersection that returns the parameters and doesn't require a grid-calculation beforehand. +bool LinearAlg2D::lineSegmentLineSegmentIntersection(const Point2LL& p1, const Point2LL& p2, const Point2LL& p3, const Point2LL& p4, float* t, float* u) +{ + const float x1mx2 = p1.X - p2.X; + const float x1mx3 = p1.X - p3.X; + const float x3mx4 = p3.X - p4.X; + const float y1my2 = p1.Y - p2.Y; + const float y1my3 = p1.Y - p3.Y; + const float y3my4 = p3.Y - p4.Y; + + t[0] = x1mx3 * y3my4 - y1my3 * x3mx4; + u[0] = x1mx3 * y1my2 - y1my3 * x1mx2; + const float div = x1mx2 * y3my4 - y1my2 * x3mx4; + if (div == 0.0f) + { + return false; + } + + // NOTE: In theory the comparison 0 <= par <= 1 can now done without division for each parameter (as an early-out), + // but this is easier & when the intersection _does_ happen and we want the normalized parameters returned anyway. + t[0] /= div; + u[0] /= div; + return t[0] >= 0.0f && u[0] >= 0.0f && t[0] <= 1.0f && u[0] <= 1.0f; +} + bool LinearAlg2D::lineLineIntersection(const Point2LL& a, const Point2LL& b, const Point2LL& c, const Point2LL& d, Point2LL& output) { // Adapted from Apex: https://github.com/Ghostkeeper/Apex/blob/eb75f0d96e36c7193d1670112826842d176d5214/include/apex/line_segment.hpp#L91 From 3dadc9da15bf1ee4ee4e69f488018379b7676c00 Mon Sep 17 00:00:00 2001 From: Remco Burema Date: Wed, 19 Jun 2024 15:17:25 +0200 Subject: [PATCH 08/14] Try user-spec. seam on line from model center (instead of polygon) first, and disable for shortest. Since as polygons shift over the players, their center also changes, we need to be careful to get the same point each time (or at least try to, since if it's not the polygon center anymore but that of the entire model, the intersection with the line from that center to the requested point might not exist). Also disable _any_ of this new behaviour (also the previously introduced) for the 'shortest.' part of CURA-11974 --- include/InsetOrderOptimizer.h | 2 ++ include/sliceDataStorage.h | 2 ++ src/FffGcodeWriter.cpp | 28 ++++++++++++------ src/InsetOrderOptimizer.cpp | 56 ++++++++++++++++++++++------------- src/TopSurface.cpp | 3 +- src/sliceDataStorage.cpp | 10 +++++++ 6 files changed, 70 insertions(+), 31 deletions(-) diff --git a/include/InsetOrderOptimizer.h b/include/InsetOrderOptimizer.h index 3b242d8900..bc76920154 100644 --- a/include/InsetOrderOptimizer.h +++ b/include/InsetOrderOptimizer.h @@ -56,6 +56,7 @@ class InsetOrderOptimizer const size_t wall_x_extruder_nr, const ZSeamConfig& z_seam_config, const std::vector& paths, + const Point2LL& center_point, const Shape& disallowed_areas_for_seams = {}); /*! @@ -107,6 +108,7 @@ class InsetOrderOptimizer const ZSeamConfig& z_seam_config_; const std::vector& paths_; const LayerIndex layer_nr_; + const Point2LL center_point_; Shape disallowed_areas_for_seams_; std::vector> inset_polys_; // vector of vectors holding the inset polygons diff --git a/include/sliceDataStorage.h b/include/sliceDataStorage.h index 669e6829d0..6e126feee4 100644 --- a/include/sliceDataStorage.h +++ b/include/sliceDataStorage.h @@ -425,6 +425,8 @@ class SliceDataStorage : public NoCopy const int extruder_nr = -1, const bool include_models = true) const; + AABB3D getModelBoundingBox() const; + /*! * Get the extruders used. * diff --git a/src/FffGcodeWriter.cpp b/src/FffGcodeWriter.cpp index b9c873a023..0b3d71df86 100644 --- a/src/FffGcodeWriter.cpp +++ b/src/FffGcodeWriter.cpp @@ -709,7 +709,8 @@ void FffGcodeWriter::processRaft(const SliceDataStorage& storage) base_extruder_nr, base_extruder_nr, z_seam_config, - raft_paths); + raft_paths, + storage.getModelBoundingBox().flatten().getMiddle()); wall_orderer.addToLayer(); } @@ -864,7 +865,8 @@ void FffGcodeWriter::processRaft(const SliceDataStorage& storage) interface_extruder_nr, interface_extruder_nr, z_seam_config, - raft_paths); + raft_paths, + storage.getModelBoundingBox().flatten().getMiddle()); wall_orderer.addToLayer(); } @@ -1027,7 +1029,8 @@ void FffGcodeWriter::processRaft(const SliceDataStorage& storage) surface_extruder_nr, surface_extruder_nr, z_seam_config, - raft_paths); + raft_paths, + storage.getModelBoundingBox().flatten().getMiddle()); wall_orderer.addToLayer(); } @@ -2320,7 +2323,8 @@ bool FffGcodeWriter::processSingleLayerInfill( extruder_nr, extruder_nr, z_seam_config, - tool_paths); + tool_paths, + storage.getModelBoundingBox().flatten().getMiddle()); added_something |= wall_orderer.addToLayer(); } } @@ -2789,7 +2793,8 @@ bool FffGcodeWriter::processInsets( mesh.settings.get("wall_0_extruder_nr").extruder_nr_, mesh.settings.get("wall_x_extruder_nr").extruder_nr_, z_seam_config, - part.wall_toolpaths); + part.wall_toolpaths, + storage.getModelBoundingBox().flatten().getMiddle()); added_something |= wall_orderer.addToLayer(); } return added_something; @@ -3214,7 +3219,8 @@ void FffGcodeWriter::processSkinPrintFeature( skin_extruder_nr, skin_extruder_nr, z_seam_config, - skin_paths); + skin_paths, + storage.getModelBoundingBox().flatten().getMiddle()); added_something |= wall_orderer.addToLayer(); } } @@ -3499,6 +3505,7 @@ bool FffGcodeWriter::processSupportInfill(const SliceDataStorage& storage, Layer extruder_nr, z_seam_config, wall_toolpaths, + storage.getModelBoundingBox().flatten().getMiddle(), disallowed_area_for_seams); added_something |= wall_orderer.addToLayer(); } @@ -3677,7 +3684,8 @@ bool FffGcodeWriter::processSupportInfill(const SliceDataStorage& storage, Layer extruder_nr, extruder_nr, z_seam_config, - wall_toolpaths_here); + wall_toolpaths_here, + storage.getModelBoundingBox().flatten().getMiddle()); added_something |= wall_orderer.addToLayer(); } } @@ -3812,7 +3820,8 @@ bool FffGcodeWriter::addSupportRoofsToGCode(const SliceDataStorage& storage, con roof_extruder_nr, roof_extruder_nr, z_seam_config, - roof_paths); + roof_paths, + storage.getModelBoundingBox().flatten().getMiddle()); wall_orderer.addToLayer(); } gcode_layer.addLinesByOptimizer(roof_lines, current_roof_config, (pattern == EFillMethod::ZIG_ZAG) ? SpaceFillType::PolyLines : SpaceFillType::Lines); @@ -3925,7 +3934,8 @@ bool FffGcodeWriter::addSupportBottomsToGCode(const SliceDataStorage& storage, L bottom_extruder_nr, bottom_extruder_nr, z_seam_config, - bottom_paths); + bottom_paths, + storage.getModelBoundingBox().flatten().getMiddle()); wall_orderer.addToLayer(); } gcode_layer.addLinesByOptimizer( diff --git a/src/InsetOrderOptimizer.cpp b/src/InsetOrderOptimizer.cpp index 49857a56b0..4a2aabacc4 100644 --- a/src/InsetOrderOptimizer.cpp +++ b/src/InsetOrderOptimizer.cpp @@ -51,6 +51,7 @@ InsetOrderOptimizer::InsetOrderOptimizer( const size_t wall_x_extruder_nr, const ZSeamConfig& z_seam_config, const std::vector& paths, + const Point2LL& center_point, const Shape& disallowed_areas_for_seams) : gcode_writer_(gcode_writer) , storage_(storage) @@ -71,6 +72,7 @@ InsetOrderOptimizer::InsetOrderOptimizer( , z_seam_config_(z_seam_config) , paths_(paths) , layer_nr_(gcode_layer.getLayerNr()) + , center_point_(center_point) , disallowed_areas_for_seams_{ disallowed_areas_for_seams } { } @@ -186,45 +188,57 @@ void InsetOrderOptimizer::insertSeamPoint(ExtrusionLine& closed_line) return; } - // Get the 'origin' point of the ray we're going to trace from there to the request_point. This is the center of the bounding box of the polygon. - AABB aabb; - for (const auto& junction : closed_line.junctions_) - { - aabb.include(junction.p_); - } - const Point2LL ray_origin{ aabb.getMiddle() }; - request_point = ray_origin + (request_point - ray_origin) * 10; - // Find the 'closest' point on the polygon to the request_point. // This isn't actually the closest, since that'd make for pertty messy seams on 'round' objects, but instead on a ray from the ray-origin to the request_point. Point2LL closest_point; size_t closest_junction_idx = 0; coord_t closest_distance_sqd = std::numeric_limits::max(); - for (const auto& [i, junction] : closed_line.junctions_ | ranges::views::enumerate) + bool should_reclaculate_closest = false; + if (z_seam_config_.type_ == EZSeamType::USER_SPECIFIED) { - // NOTE: Maybe rewrite this once we can use C++23 ranges::views::adjacent - const auto& next_junction = closed_line.junctions_[(i + 1) % closed_line.junctions_.size()]; + const Point2LL ray_origin = center_point_; + request_point = ray_origin + (request_point - ray_origin) * 10; - float t, u; - if (LinearAlg2D::lineSegmentLineSegmentIntersection(ray_origin, request_point, junction.p_, next_junction.p_, &t, &u)) + for (const auto& [i, junction] : closed_line.junctions_ | ranges::views::enumerate) { - const Point2LL intersection = ray_origin + (request_point - ray_origin) * t; - const coord_t distance_sqd = vSize2(request_point - intersection); - if (distance_sqd < closest_distance_sqd) + // NOTE: Maybe rewrite this once we can use C++23 ranges::views::adjacent + const auto& next_junction = closed_line.junctions_[(i + 1) % closed_line.junctions_.size()]; + + float t, u; + if (LinearAlg2D::lineSegmentLineSegmentIntersection(ray_origin, request_point, junction.p_, next_junction.p_, &t, &u)) { - closest_point = intersection; - closest_distance_sqd = distance_sqd; - closest_junction_idx = i; + const Point2LL intersection = ray_origin + (request_point - ray_origin) * t; + const coord_t distance_sqd = vSize2(request_point - intersection); + if (distance_sqd < closest_distance_sqd) + { + closest_point = intersection; + closest_distance_sqd = distance_sqd; + closest_junction_idx = i; + } } } } if (closest_distance_sqd >= std::numeric_limits::max()) { - return; + for (const auto& [i, junction] : closed_line.junctions_ | ranges::views::enumerate) + { + const auto& next_junction = closed_line.junctions_[(i + 1) % closed_line.junctions_.size()]; + const coord_t distance_sqd = LinearAlg2D::getDist2FromLineSegment(junction.p_, request_point, next_junction.p_); + if (distance_sqd < closest_distance_sqd) + { + closest_distance_sqd = distance_sqd; + closest_junction_idx = i; + } + } + should_reclaculate_closest = true; } const auto& start_pt = closed_line.junctions_[closest_junction_idx]; const auto& end_pt = closed_line.junctions_[(closest_junction_idx + 1) % closed_line.junctions_.size()]; + if (should_reclaculate_closest) + { + closest_point = LinearAlg2D::getClosestOnLineSegment(request_point, start_pt.p_, end_pt.p_); + } constexpr coord_t smallest_dist_sqd = 25; if (vSize2(closest_point - start_pt.p_) <= smallest_dist_sqd || vSize2(closest_point - end_pt.p_) <= smallest_dist_sqd) { diff --git a/src/TopSurface.cpp b/src/TopSurface.cpp index 8203ccb2ce..15a6b03cf8 100644 --- a/src/TopSurface.cpp +++ b/src/TopSurface.cpp @@ -185,7 +185,8 @@ bool TopSurface::ironing(const SliceDataStorage& storage, const SliceMeshStorage extruder_nr, extruder_nr, z_seam_config, - ironing_paths); + ironing_paths, + storage.getModelBoundingBox().flatten().getMiddle()); wall_orderer.addToLayer(); added = true; } diff --git a/src/sliceDataStorage.cpp b/src/sliceDataStorage.cpp index b50bcc5c26..d89edfbceb 100644 --- a/src/sliceDataStorage.cpp +++ b/src/sliceDataStorage.cpp @@ -376,6 +376,16 @@ Shape SliceDataStorage::getLayerOutlines( } } +AABB3D SliceDataStorage::getModelBoundingBox() const +{ + AABB3D bounding_box; + for (const auto& mesh : meshes) + { + bounding_box.include(mesh->bounding_box); + } + return bounding_box; +} + std::vector SliceDataStorage::getExtrudersUsed() const { std::vector ret; From 00785da2a0b3ecedb367a7dd51218d0c227ecc4a Mon Sep 17 00:00:00 2001 From: Remco Burema Date: Wed, 19 Jun 2024 15:41:11 +0200 Subject: [PATCH 09/14] Force seam to adhere to chosen/'best' position when 'not on vertex'. Otherwise the algo would still occasionally choose other vertices, despite there being only one that we actually want it to land on. This would make the seams look messy in 3D space. part of CURA-11974 --- include/InsetOrderOptimizer.h | 2 +- include/PathOrderOptimizer.h | 8 +++++++- include/path_ordering.h | 6 ++++++ src/InsetOrderOptimizer.cpp | 13 ++++++++----- 4 files changed, 22 insertions(+), 7 deletions(-) diff --git a/include/InsetOrderOptimizer.h b/include/InsetOrderOptimizer.h index bc76920154..624f19ab13 100644 --- a/include/InsetOrderOptimizer.h +++ b/include/InsetOrderOptimizer.h @@ -122,7 +122,7 @@ class InsetOrderOptimizer * however still deviate from this, for example when the seam-point placed here isn't suppored * by the layer below. */ - void insertSeamPoint(ExtrusionLine& closed_line); + std::optional insertSeamPoint(ExtrusionLine& closed_line); /*! * Determine if the paths should be reversed diff --git a/include/PathOrderOptimizer.h b/include/PathOrderOptimizer.h index cf54338a06..18db16ebe8 100644 --- a/include/PathOrderOptimizer.h +++ b/include/PathOrderOptimizer.h @@ -130,10 +130,11 @@ class PathOrderOptimizer * Add a new polygon to be optimized. * \param polygon The polygon to optimize. */ - void addPolygon(const Path& polygon) + void addPolygon(const Path& polygon, std::optional force_start_index = std::nullopt) { constexpr bool is_closed = true; paths_.emplace_back(polygon, is_closed); + paths_.back().force_start_index_ = force_start_index; } /*! @@ -695,6 +696,11 @@ class PathOrderOptimizer return vert; } + if (path.force_start_index_.has_value()) + { + return path.force_start_index_.value(); + } + // Precompute segments lengths because we are going to need them multiple times std::vector segments_sizes(path.converted_->size()); coord_t total_length = 0; diff --git a/include/path_ordering.h b/include/path_ordering.h index 5a5de96e17..0a7fb1af13 100644 --- a/include/path_ordering.h +++ b/include/path_ordering.h @@ -76,6 +76,12 @@ struct PathOrdering */ bool backwards_; + /*! + * Force the start point of the path to be at a specific location. + * Will only happen if not empty, and this point is actually on the path. + */ + std::optional force_start_index_; + /*! * Get vertex data from the custom path type. * diff --git a/src/InsetOrderOptimizer.cpp b/src/InsetOrderOptimizer.cpp index 4a2aabacc4..005e9c8377 100644 --- a/src/InsetOrderOptimizer.cpp +++ b/src/InsetOrderOptimizer.cpp @@ -112,15 +112,17 @@ bool InsetOrderOptimizer::addToLayer() group_outer_walls, disallowed_areas_for_seams_); + for (auto& line : walls_to_be_added) { if (line.is_closed_) { + std::optional force_start; if (! settings_.get("z_seam_on_vertex")) { - insertSeamPoint(line); + force_start = insertSeamPoint(line); } - order_optimizer.addPolygon(&line); + order_optimizer.addPolygon(&line, force_start); } else { @@ -170,7 +172,7 @@ bool InsetOrderOptimizer::addToLayer() return added_something; } -void InsetOrderOptimizer::insertSeamPoint(ExtrusionLine& closed_line) +std::optional InsetOrderOptimizer::insertSeamPoint(ExtrusionLine& closed_line) { assert(closed_line.is_closed_); assert(closed_line.size() >= 3); @@ -185,7 +187,7 @@ void InsetOrderOptimizer::insertSeamPoint(ExtrusionLine& closed_line) request_point = gcode_layer_.getLastPlannedPositionOrStartingPosition(); break; default: - return; + return std::nullopt; } // Find the 'closest' point on the polygon to the request_point. @@ -242,7 +244,7 @@ void InsetOrderOptimizer::insertSeamPoint(ExtrusionLine& closed_line) constexpr coord_t smallest_dist_sqd = 25; if (vSize2(closest_point - start_pt.p_) <= smallest_dist_sqd || vSize2(closest_point - end_pt.p_) <= smallest_dist_sqd) { - return; + return std::nullopt; } // NOTE: This could also be done on a single axis (skipping the implied sqrt), but figuring out which one and then using the right values became a bit messy/verbose. @@ -252,6 +254,7 @@ void InsetOrderOptimizer::insertSeamPoint(ExtrusionLine& closed_line) const coord_t w = ((end_pt.w_ * end_dist) / total_dist) + ((start_pt.w_ * start_dist) / total_dist); closed_line.junctions_.insert(closed_line.junctions_.begin() + closest_junction_idx + 1, ExtrusionJunction(closest_point, w, start_pt.perimeter_index_)); + return closest_junction_idx + 1; } InsetOrderOptimizer::value_type InsetOrderOptimizer::getRegionOrder(const std::vector& extrusion_lines, const bool outer_to_inner) From 5c24075c5561a5709bebd2cff506b88b8e97bf6e Mon Sep 17 00:00:00 2001 From: Remco Burema Date: Wed, 19 Jun 2024 17:02:45 +0200 Subject: [PATCH 10/14] 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); } } From 94b4d8a81f43e4a91a90885c95bd0e3d460375bc Mon Sep 17 00:00:00 2001 From: Remco Burema Date: Thu, 20 Jun 2024 15:39:39 +0200 Subject: [PATCH 11/14] Add documentation (non-vertex seams), rename variable. part of CURA-11974 --- include/InsetOrderOptimizer.h | 8 ++++++-- include/PathOrderOptimizer.h | 1 + include/sliceDataStorage.h | 3 +++ include/utils/linearAlg2D.h | 12 ++++++++++++ src/InsetOrderOptimizer.cpp | 23 ++++++++++++++++++----- src/utils/linearAlg2D.cpp | 1 - 6 files changed, 40 insertions(+), 8 deletions(-) diff --git a/include/InsetOrderOptimizer.h b/include/InsetOrderOptimizer.h index 624f19ab13..34910c8628 100644 --- a/include/InsetOrderOptimizer.h +++ b/include/InsetOrderOptimizer.h @@ -56,7 +56,7 @@ class InsetOrderOptimizer const size_t wall_x_extruder_nr, const ZSeamConfig& z_seam_config, const std::vector& paths, - const Point2LL& center_point, + const Point2LL& model_center_point, const Shape& disallowed_areas_for_seams = {}); /*! @@ -108,7 +108,7 @@ class InsetOrderOptimizer const ZSeamConfig& z_seam_config_; const std::vector& paths_; const LayerIndex layer_nr_; - const Point2LL center_point_; + const Point2LL model_center_point_; // Center of the model (= all meshes) axis-aligned bounding-box. Shape disallowed_areas_for_seams_; std::vector> inset_polys_; // vector of vectors holding the inset polygons @@ -121,6 +121,10 @@ class InsetOrderOptimizer * 'best' vertex on that polygon. Under certain circumstances, the seam-placing algorithm can * however still deviate from this, for example when the seam-point placed here isn't suppored * by the layer below. + * + * \param closed_line The polygon to insert the seam point in. (It's assumed to be closed at least.) + * + * \return The index of the inserted seam point, or std::nullopt if no seam point was inserted. */ std::optional insertSeamPoint(ExtrusionLine& closed_line); diff --git a/include/PathOrderOptimizer.h b/include/PathOrderOptimizer.h index 18db16ebe8..be8e61873b 100644 --- a/include/PathOrderOptimizer.h +++ b/include/PathOrderOptimizer.h @@ -698,6 +698,7 @@ class PathOrderOptimizer if (path.force_start_index_.has_value()) { + // Start index already known, since we forced it, return. return path.force_start_index_.value(); } diff --git a/include/sliceDataStorage.h b/include/sliceDataStorage.h index 6e126feee4..be95224cbf 100644 --- a/include/sliceDataStorage.h +++ b/include/sliceDataStorage.h @@ -425,6 +425,9 @@ class SliceDataStorage : public NoCopy const int extruder_nr = -1, const bool include_models = true) const; + /*! + * Get the axis-aligned bounding-box of the complete model (all meshes). + */ AABB3D getModelBoundingBox() const; /*! diff --git a/include/utils/linearAlg2D.h b/include/utils/linearAlg2D.h index 71cd4d3e80..58059055c3 100644 --- a/include/utils/linearAlg2D.h +++ b/include/utils/linearAlg2D.h @@ -67,6 +67,18 @@ class LinearAlg2D return -1; } + /*! + * A single-shot line-segment/line-segment intersection that returns the parameters and doesn't require a grid-calculation beforehand. + * + * \param p1 The start point of the first line segment. + * \param p2 The end point of the first line segment. + * \param p3 The start point of the second line segment. + * \param p4 The end point of the second line segment. + * \param t The parameter of the intersection on the first line segment (intersection = p1 + t * (p2 - p1)). + * \param u The parameter of the intersection on the second line segment (intersection = p3 + u * (p4 - p3)). + * + * \return Whether the two line segments intersect. + */ static bool lineSegmentLineSegmentIntersection(const Point2LL& p1, const Point2LL& p2, const Point2LL& p3, const Point2LL& p4, float* t, float* u); static bool lineLineIntersection(const Point2LL& a, const Point2LL& b, const Point2LL& c, const Point2LL& d, Point2LL& output); diff --git a/src/InsetOrderOptimizer.cpp b/src/InsetOrderOptimizer.cpp index 005e9c8377..edc7177907 100644 --- a/src/InsetOrderOptimizer.cpp +++ b/src/InsetOrderOptimizer.cpp @@ -51,7 +51,7 @@ InsetOrderOptimizer::InsetOrderOptimizer( const size_t wall_x_extruder_nr, const ZSeamConfig& z_seam_config, const std::vector& paths, - const Point2LL& center_point, + const Point2LL& model_center_point, const Shape& disallowed_areas_for_seams) : gcode_writer_(gcode_writer) , storage_(storage) @@ -72,7 +72,7 @@ InsetOrderOptimizer::InsetOrderOptimizer( , z_seam_config_(z_seam_config) , paths_(paths) , layer_nr_(gcode_layer.getLayerNr()) - , center_point_(center_point) + , model_center_point_(model_center_point) , disallowed_areas_for_seams_{ disallowed_areas_for_seams } { } @@ -112,7 +112,6 @@ bool InsetOrderOptimizer::addToLayer() group_outer_walls, disallowed_areas_for_seams_); - for (auto& line : walls_to_be_added) { if (line.is_closed_) @@ -120,6 +119,7 @@ bool InsetOrderOptimizer::addToLayer() std::optional force_start; if (! settings_.get("z_seam_on_vertex")) { + // If the user indicated that we may deviate from the vertices for the seam, we can insert a seam point, if needed. force_start = insertSeamPoint(line); } order_optimizer.addPolygon(&line, force_start); @@ -191,14 +191,18 @@ std::optional InsetOrderOptimizer::insertSeamPoint(ExtrusionLine& closed } // Find the 'closest' point on the polygon to the request_point. - // This isn't actually the closest, since that'd make for pertty messy seams on 'round' objects, but instead on a ray from the ray-origin to the request_point. Point2LL closest_point; size_t closest_junction_idx = 0; coord_t closest_distance_sqd = std::numeric_limits::max(); bool should_reclaculate_closest = false; if (z_seam_config_.type_ == EZSeamType::USER_SPECIFIED) { - const Point2LL ray_origin = center_point_; + // For user-defined seams you usually don't _actually_ want the _closest_ point, per-se, + // since you want the seam-line to be continuous in 3D space. + // To that end, take the center of the 3D model (not of the current polygon, as that would give the same problems) + // and project the point along the ray from the center to the request_point. + + const Point2LL ray_origin = model_center_point_; request_point = ray_origin + (request_point - ray_origin) * 10; for (const auto& [i, junction] : closed_line.junctions_ | ranges::views::enumerate) @@ -222,6 +226,10 @@ std::optional InsetOrderOptimizer::insertSeamPoint(ExtrusionLine& closed } if (closest_distance_sqd >= std::numeric_limits::max()) { + // If it the method isn't 'user-defined', or the attempt to do user-defined above failed + // (since we don't take the center of the polygon, but of the model, there's a chance there's no intersection), + // then just find the closest point on the polygon. + for (const auto& [i, junction] : closed_line.junctions_ | ranges::views::enumerate) { const auto& next_junction = closed_line.junctions_[(i + 1) % closed_line.junctions_.size()]; @@ -239,11 +247,16 @@ std::optional InsetOrderOptimizer::insertSeamPoint(ExtrusionLine& closed const auto& end_pt = closed_line.junctions_[(closest_junction_idx + 1) % closed_line.junctions_.size()]; if (should_reclaculate_closest) { + // In the second case (see above) the closest point hasn't actually been calculated yet, + // since in that case we'de need the start and end points. So do that here. closest_point = LinearAlg2D::getClosestOnLineSegment(request_point, start_pt.p_, end_pt.p_); } constexpr coord_t smallest_dist_sqd = 25; if (vSize2(closest_point - start_pt.p_) <= smallest_dist_sqd || vSize2(closest_point - end_pt.p_) <= smallest_dist_sqd) { + // Early out if the closest point is too close to the start or end point. + // NOTE: Maybe return the index here anyway, since this is the point the current caller would want to force the seam to. + // However, then the index returned would have a caveat that it _can_ point to an already exisiting point then. return std::nullopt; } diff --git a/src/utils/linearAlg2D.cpp b/src/utils/linearAlg2D.cpp index e6093e0af0..e3df0ccf34 100644 --- a/src/utils/linearAlg2D.cpp +++ b/src/utils/linearAlg2D.cpp @@ -272,7 +272,6 @@ Point3Matrix LinearAlg2D::rotateAround(const Point2LL& middle, double rotation) return Point3Matrix::translate(middle).compose(rotation_matrix_homogeneous).compose(Point3Matrix::translate(-middle)); } -// A single-shot line-segment/line-segment intersection that returns the parameters and doesn't require a grid-calculation beforehand. bool LinearAlg2D::lineSegmentLineSegmentIntersection(const Point2LL& p1, const Point2LL& p2, const Point2LL& p3, const Point2LL& p4, float* t, float* u) { const float x1mx2 = p1.X - p2.X; From 3560815e6e75d697b38647dce5f6cff660fc9ea0 Mon Sep 17 00:00:00 2001 From: rburema Date: Thu, 20 Jun 2024 13:40:18 +0000 Subject: [PATCH 12/14] Applied clang-format. --- include/InsetOrderOptimizer.h | 4 ++-- include/utils/linearAlg2D.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/InsetOrderOptimizer.h b/include/InsetOrderOptimizer.h index 34910c8628..3a28040376 100644 --- a/include/InsetOrderOptimizer.h +++ b/include/InsetOrderOptimizer.h @@ -121,9 +121,9 @@ class InsetOrderOptimizer * 'best' vertex on that polygon. Under certain circumstances, the seam-placing algorithm can * however still deviate from this, for example when the seam-point placed here isn't suppored * by the layer below. - * + * * \param closed_line The polygon to insert the seam point in. (It's assumed to be closed at least.) - * + * * \return The index of the inserted seam point, or std::nullopt if no seam point was inserted. */ std::optional insertSeamPoint(ExtrusionLine& closed_line); diff --git a/include/utils/linearAlg2D.h b/include/utils/linearAlg2D.h index 58059055c3..ac7ef9fbd2 100644 --- a/include/utils/linearAlg2D.h +++ b/include/utils/linearAlg2D.h @@ -69,14 +69,14 @@ class LinearAlg2D /*! * A single-shot line-segment/line-segment intersection that returns the parameters and doesn't require a grid-calculation beforehand. - * + * * \param p1 The start point of the first line segment. * \param p2 The end point of the first line segment. * \param p3 The start point of the second line segment. * \param p4 The end point of the second line segment. * \param t The parameter of the intersection on the first line segment (intersection = p1 + t * (p2 - p1)). * \param u The parameter of the intersection on the second line segment (intersection = p3 + u * (p4 - p3)). - * + * * \return Whether the two line segments intersect. */ static bool lineSegmentLineSegmentIntersection(const Point2LL& p1, const Point2LL& p2, const Point2LL& p3, const Point2LL& p4, float* t, float* u); From 677aa84198409fd43c9925011ad7a70ff23f9819 Mon Sep 17 00:00:00 2001 From: Remco Burema Date: Thu, 20 Jun 2024 17:57:36 +0200 Subject: [PATCH 13/14] Code review request: Merge line(Segment) intersection code. done as part of CURA-11974 --- include/utils/linearAlg2D.h | 3 ++- src/InsetOrderOptimizer.cpp | 2 +- src/utils/linearAlg2D.cpp | 29 ++++++++++------------------- 3 files changed, 13 insertions(+), 21 deletions(-) diff --git a/include/utils/linearAlg2D.h b/include/utils/linearAlg2D.h index ac7ef9fbd2..e92eeba0c4 100644 --- a/include/utils/linearAlg2D.h +++ b/include/utils/linearAlg2D.h @@ -79,7 +79,8 @@ class LinearAlg2D * * \return Whether the two line segments intersect. */ - static bool lineSegmentLineSegmentIntersection(const Point2LL& p1, const Point2LL& p2, const Point2LL& p3, const Point2LL& p4, float* t, float* u); + static bool segmentSegmentIntersection(const Point2LL& p1, const Point2LL& p2, const Point2LL& p3, const Point2LL& p4, float* t, float* u); + static bool lineLineIntersection(const Point2LL& p1, const Point2LL& p2, const Point2LL& p3, const Point2LL& p4, float* t, float* u); static bool lineLineIntersection(const Point2LL& a, const Point2LL& b, const Point2LL& c, const Point2LL& d, Point2LL& output); diff --git a/src/InsetOrderOptimizer.cpp b/src/InsetOrderOptimizer.cpp index edc7177907..b4cdeb6333 100644 --- a/src/InsetOrderOptimizer.cpp +++ b/src/InsetOrderOptimizer.cpp @@ -211,7 +211,7 @@ std::optional InsetOrderOptimizer::insertSeamPoint(ExtrusionLine& closed const auto& next_junction = closed_line.junctions_[(i + 1) % closed_line.junctions_.size()]; float t, u; - if (LinearAlg2D::lineSegmentLineSegmentIntersection(ray_origin, request_point, junction.p_, next_junction.p_, &t, &u)) + if (LinearAlg2D::segmentSegmentIntersection(ray_origin, request_point, junction.p_, next_junction.p_, &t, &u)) { const Point2LL intersection = ray_origin + (request_point - ray_origin) * t; const coord_t distance_sqd = vSize2(request_point - intersection); diff --git a/src/utils/linearAlg2D.cpp b/src/utils/linearAlg2D.cpp index e3df0ccf34..ca445713f3 100644 --- a/src/utils/linearAlg2D.cpp +++ b/src/utils/linearAlg2D.cpp @@ -272,7 +272,7 @@ Point3Matrix LinearAlg2D::rotateAround(const Point2LL& middle, double rotation) return Point3Matrix::translate(middle).compose(rotation_matrix_homogeneous).compose(Point3Matrix::translate(-middle)); } -bool LinearAlg2D::lineSegmentLineSegmentIntersection(const Point2LL& p1, const Point2LL& p2, const Point2LL& p3, const Point2LL& p4, float* t, float* u) +bool LinearAlg2D::lineLineIntersection(const Point2LL& p1, const Point2LL& p2, const Point2LL& p3, const Point2LL& p4, float* t, float* u) { const float x1mx2 = p1.X - p2.X; const float x1mx3 = p1.X - p3.X; @@ -293,31 +293,22 @@ bool LinearAlg2D::lineSegmentLineSegmentIntersection(const Point2LL& p1, const P // but this is easier & when the intersection _does_ happen and we want the normalized parameters returned anyway. t[0] /= div; u[0] /= div; - return t[0] >= 0.0f && u[0] >= 0.0f && t[0] <= 1.0f && u[0] <= 1.0f; + return true; +} + +bool LinearAlg2D::segmentSegmentIntersection(const Point2LL& p1, const Point2LL& p2, const Point2LL& p3, const Point2LL& p4, float* t, float* u) +{ + return lineLineIntersection(p1, p2, p3, p4, t, u) && t[0] >= 0.0f && u[0] >= 0.0f && t[0] <= 1.0f && u[0] <= 1.0f; } bool LinearAlg2D::lineLineIntersection(const Point2LL& a, const Point2LL& b, const Point2LL& c, const Point2LL& d, Point2LL& output) { - // Adapted from Apex: https://github.com/Ghostkeeper/Apex/blob/eb75f0d96e36c7193d1670112826842d176d5214/include/apex/line_segment.hpp#L91 - // Adjusted to work with lines instead of line segments. - const Point2LL l1_delta = b - a; - const Point2LL l2_delta = d - c; - const coord_t divisor = cross(l1_delta, l2_delta); // Pre-compute divisor needed for the intersection check. - if (divisor == 0) + float t, u; + if (! lineLineIntersection(a, b, c, d, &t, &u)) { - // The lines are parallel if the cross product of their directions is zero. return false; } - - // Create a parametric representation of each line. - // We'll equate the parametric equations to each other to find the intersection then. - // Parametric equation is L = P + Vt (where P and V are a starting point and directional vector). - // We'll map the starting point of one line onto the parameter system of the other line. - // Then using the divisor we can see whether and where they cross. - const Point2LL starts_delta = a - c; - const coord_t l1_parametric = cross(l2_delta, starts_delta); - Point2LL result = a + Point2LL(round_divide_signed(l1_parametric * l1_delta.X, divisor), round_divide_signed(l1_parametric * l1_delta.Y, divisor)); - + const Point2LL result = a + (b - a) * t; if (std::abs(result.X) > std::numeric_limits::max() || std::abs(result.Y) > std::numeric_limits::max()) { // Intersection is so far away that it could lead to integer overflows. From 98c040735b45a8613e917fb2cf7071b83a230b53 Mon Sep 17 00:00:00 2001 From: Erwan MATHIEU Date: Fri, 21 Jun 2024 07:54:58 +0200 Subject: [PATCH 14/14] Pass output values by reference instead of pointers CURA-11974 --- include/utils/linearAlg2D.h | 4 ++-- src/InsetOrderOptimizer.cpp | 2 +- src/utils/linearAlg2D.cpp | 16 ++++++++-------- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/include/utils/linearAlg2D.h b/include/utils/linearAlg2D.h index e92eeba0c4..56beab5608 100644 --- a/include/utils/linearAlg2D.h +++ b/include/utils/linearAlg2D.h @@ -79,8 +79,8 @@ class LinearAlg2D * * \return Whether the two line segments intersect. */ - static bool segmentSegmentIntersection(const Point2LL& p1, const Point2LL& p2, const Point2LL& p3, const Point2LL& p4, float* t, float* u); - static bool lineLineIntersection(const Point2LL& p1, const Point2LL& p2, const Point2LL& p3, const Point2LL& p4, float* t, float* u); + static bool segmentSegmentIntersection(const Point2LL& p1, const Point2LL& p2, const Point2LL& p3, const Point2LL& p4, float& t, float& u); + static bool lineLineIntersection(const Point2LL& p1, const Point2LL& p2, const Point2LL& p3, const Point2LL& p4, float& t, float& u); static bool lineLineIntersection(const Point2LL& a, const Point2LL& b, const Point2LL& c, const Point2LL& d, Point2LL& output); diff --git a/src/InsetOrderOptimizer.cpp b/src/InsetOrderOptimizer.cpp index b4cdeb6333..ad036f62b9 100644 --- a/src/InsetOrderOptimizer.cpp +++ b/src/InsetOrderOptimizer.cpp @@ -211,7 +211,7 @@ std::optional InsetOrderOptimizer::insertSeamPoint(ExtrusionLine& closed const auto& next_junction = closed_line.junctions_[(i + 1) % closed_line.junctions_.size()]; float t, u; - if (LinearAlg2D::segmentSegmentIntersection(ray_origin, request_point, junction.p_, next_junction.p_, &t, &u)) + if (LinearAlg2D::segmentSegmentIntersection(ray_origin, request_point, junction.p_, next_junction.p_, t, u)) { const Point2LL intersection = ray_origin + (request_point - ray_origin) * t; const coord_t distance_sqd = vSize2(request_point - intersection); diff --git a/src/utils/linearAlg2D.cpp b/src/utils/linearAlg2D.cpp index ca445713f3..3fccba9e00 100644 --- a/src/utils/linearAlg2D.cpp +++ b/src/utils/linearAlg2D.cpp @@ -272,7 +272,7 @@ Point3Matrix LinearAlg2D::rotateAround(const Point2LL& middle, double rotation) return Point3Matrix::translate(middle).compose(rotation_matrix_homogeneous).compose(Point3Matrix::translate(-middle)); } -bool LinearAlg2D::lineLineIntersection(const Point2LL& p1, const Point2LL& p2, const Point2LL& p3, const Point2LL& p4, float* t, float* u) +bool LinearAlg2D::lineLineIntersection(const Point2LL& p1, const Point2LL& p2, const Point2LL& p3, const Point2LL& p4, float& t, float& u) { const float x1mx2 = p1.X - p2.X; const float x1mx3 = p1.X - p3.X; @@ -281,8 +281,8 @@ bool LinearAlg2D::lineLineIntersection(const Point2LL& p1, const Point2LL& p2, c const float y1my3 = p1.Y - p3.Y; const float y3my4 = p3.Y - p4.Y; - t[0] = x1mx3 * y3my4 - y1my3 * x3mx4; - u[0] = x1mx3 * y1my2 - y1my3 * x1mx2; + t = x1mx3 * y3my4 - y1my3 * x3mx4; + u = x1mx3 * y1my2 - y1my3 * x1mx2; const float div = x1mx2 * y3my4 - y1my2 * x3mx4; if (div == 0.0f) { @@ -291,20 +291,20 @@ bool LinearAlg2D::lineLineIntersection(const Point2LL& p1, const Point2LL& p2, c // NOTE: In theory the comparison 0 <= par <= 1 can now done without division for each parameter (as an early-out), // but this is easier & when the intersection _does_ happen and we want the normalized parameters returned anyway. - t[0] /= div; - u[0] /= div; + t /= div; + u /= div; return true; } -bool LinearAlg2D::segmentSegmentIntersection(const Point2LL& p1, const Point2LL& p2, const Point2LL& p3, const Point2LL& p4, float* t, float* u) +bool LinearAlg2D::segmentSegmentIntersection(const Point2LL& p1, const Point2LL& p2, const Point2LL& p3, const Point2LL& p4, float& t, float& u) { - return lineLineIntersection(p1, p2, p3, p4, t, u) && t[0] >= 0.0f && u[0] >= 0.0f && t[0] <= 1.0f && u[0] <= 1.0f; + return lineLineIntersection(p1, p2, p3, p4, t, u) && t >= 0.0f && u >= 0.0f && t <= 1.0f && u <= 1.0f; } bool LinearAlg2D::lineLineIntersection(const Point2LL& a, const Point2LL& b, const Point2LL& c, const Point2LL& d, Point2LL& output) { float t, u; - if (! lineLineIntersection(a, b, c, d, &t, &u)) + if (! lineLineIntersection(a, b, c, d, t, u)) { return false; }