From fc0feed553252a23a75655446c87a7ebbd0c3a63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Hejl?= Date: Fri, 6 Dec 2024 16:04:06 +0100 Subject: [PATCH] SPE-2597: Fix clipping logic for clipping arcs with negative radius. When we are clipping the arc with a negative radius (we are taking the longer angle here), we have to check if we still need to take the longer angle after clipping. Otherwise, we must flip the radius sign to take the shorter angle. --- src/libslic3r/Geometry/ArcWelder.cpp | 15 ++++++--- tests/libslic3r/test_arc_welder.cpp | 47 ++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/src/libslic3r/Geometry/ArcWelder.cpp b/src/libslic3r/Geometry/ArcWelder.cpp index 840cf5397b..4f047c9c5c 100644 --- a/src/libslic3r/Geometry/ArcWelder.cpp +++ b/src/libslic3r/Geometry/ArcWelder.cpp @@ -739,10 +739,17 @@ double clip_end(Path &path, double distance) // Rotate the segment end point in reverse towards the start point. if (last.ccw()) angle *= -1.; - path.push_back({ - last.point.rotated(angle * (distance / len), - arc_center(path.back().point.cast(), last.point.cast(), double(last.radius), last.ccw()).cast()), - last.radius, last.orientation }); + + const double rotate_by_angle = angle * (distance / len); + + // When we are clipping the arc with a negative radius (we are taking the longer angle here), + // we have to check if we still need to take the longer angle after clipping. + // Otherwise, we must flip the radius sign to take the shorter angle. + const bool flip_radius_sign = last.radius < 0 && std::abs(angle) > M_PI && std::abs(angle - rotate_by_angle) <= M_PI; + + path.push_back({last.point.rotated(rotate_by_angle, arc_center(path.back().point.cast(), last.point.cast(), double(last.radius), last.ccw()).cast()), + (flip_radius_sign ? -last.radius : last.radius), last.orientation}); + // Length to go is zero. return 0; } diff --git a/tests/libslic3r/test_arc_welder.cpp b/tests/libslic3r/test_arc_welder.cpp index 6ee486bd1a..35eda98a77 100644 --- a/tests/libslic3r/test_arc_welder.cpp +++ b/tests/libslic3r/test_arc_welder.cpp @@ -4,6 +4,7 @@ #include #include +#include #include #include #include @@ -470,6 +471,52 @@ TEST_CASE("ExtrusionMultiPath simplification", "[ArcWelderMultiPathSimplify][!ma REQUIRE(min_segment_length >= resolution); } +TEST_CASE("SmoothPath clipping test", "[ArcWelder]") { + using namespace Slic3r::Geometry; + + const Polyline polyline = { + Point(9237362, -279099), Point(9239309, -204770), Point(9232158, 477899), Point(9153712, 1292530), + Point(9014384, 2036579), Point(8842322, 2697128), Point(8569131, 3468590), Point(8287136, 4090253), + Point(8050736, 4537759), Point(7786167, 4978071), Point(7502123, 5396751), Point(7085512, 5937730), + Point(6536631, 6536722), Point(5937701, 7085536), Point(5336389, 7545178), Point(4766354, 7921046), + Point(4287299, 8181151), Point(3798566, 8424823), Point(3161891, 8687141), Point(2477384, 8903260), + Point(1985727, 9025657), Point(1488659, 9120891), Point(811611, 9208824), Point(229795, 9234222), + Point(-477899, 9232158), Point(-1292541, 9153710), Point(-1963942, 9030487), Point(-2483966, 8901437), + Point(-2967612, 8752145), Point(-3606656, 8511944), Point(-4098726, 8277235), Point(-4583048, 8025111), + Point(-5164553, 7667365), Point(-5602853, 7343037), Point(-6030084, 7003203), Point(-6532687, 6541035), + Point(-7085558, 5937673), Point(-7502041, 5396860), Point(-7802209, 4952884), Point(-8061668, 4518435), + Point(-8375899, 3912214), Point(-8689042, 3156205), Point(-8915304, 2433948), Point(-9073554, 1769674), + Point(-9194504, 960323), Point(-9238723, 227049), Point(-9237360, -279112), Point(-9194498, -960380), + Point(-9073524, -1769810), Point(-8895452, -2505523), Point(-8689032, -3156238), Point(-8375859, -3912298), + Point(-8025112, -4583044), Point(-7667378, -5164532), Point(-7180536, -5822455), Point(-6729193, -6334406), + Point(-6350620, -6713810), Point(-5973693, -7051366), Point(-5438560, -7475505), Point(-4756170, -7927163), + Point(-4110103, -8277232), Point(-3651006, -8489813), Point(-3015355, -8738921), Point(-2492584, -8893770), + Point(-1963947, -9030483), Point(-1286636, -9154696), Point(-590411, -9222659), Point(14602, -9244383), + Point(974789, -9192915), Point(1634833, -9095889), Point(2193590, -8977466), Point(2851102, -8793883), + Point(3612042, -8509372), Point(4098709, -8277242), Point(4583076, -8025095), Point(5164577, -7667349), + Point(5822437, -7180551), Point(6388368, -6677987), Point(6866030, -6190211), Point(7236430, -5740880), + Point(7660739, -5174380), Point(8088357, -4476558), Point(8394013, -3866175), Point(8593000, -3400880), + Point(8768650, -2918284), Point(8915319, -2433894), Point(9073549, -1769711), Point(9194508, -960282), + Point(9237362, -279099) + }; + + const ExtrusionAttributes extrusion_attributes(ExtrusionRole::Perimeter, ExtrusionFlow{1.0, 1.0, 1.0}); + const GCode::SmoothPath smooth_path = {GCode::SmoothPathElement{extrusion_attributes, ArcWelder::fit_path(polyline.points, 32000., 0.05)}}; + const double smooth_path_length = GCode::length(smooth_path); + + const size_t clip_segment_cnt = 20; + for (size_t segment_idx = 1; segment_idx <= clip_segment_cnt; ++segment_idx) { + const double clip_length = static_cast(segment_idx) * (smooth_path_length / (clip_segment_cnt + 1)); + GCode::SmoothPath smooth_path_clipped = smooth_path; + + clip_end(smooth_path_clipped, smooth_path_length - clip_length, scaled(GCode::ExtrusionOrder::min_gcode_segment_length)); + + const double smooth_path_clipped_length = GCode::length(smooth_path_clipped); + const double relative_diff = std::abs(1. - (clip_length / smooth_path_clipped_length)); + REQUIRE(relative_diff <= 0.000001); + } +} + #if 0 // For quantization //#include