From 040acc7e32bb979302c713d864ac84868ba35802 Mon Sep 17 00:00:00 2001 From: Vovodroid Date: Sat, 23 Dec 2023 14:08:17 +0200 Subject: [PATCH] Improve ExtrusionLine::simplify https://github.com/prusa3d/PrusaSlicer/pull/11811 --- src/libslic3r/Arachne/utils/ExtrusionLine.cpp | 57 +++++++++++-------- 1 file changed, 34 insertions(+), 23 deletions(-) diff --git a/src/libslic3r/Arachne/utils/ExtrusionLine.cpp b/src/libslic3r/Arachne/utils/ExtrusionLine.cpp index 8631d9e17bb..1839afc16df 100644 --- a/src/libslic3r/Arachne/utils/ExtrusionLine.cpp +++ b/src/libslic3r/Arachne/utils/ExtrusionLine.cpp @@ -44,8 +44,6 @@ void ExtrusionLine::simplify(const int64_t smallest_line_segment_squared, const if (junctions.size() <= min_path_size) return; - // TODO: allow for the first point to be removed in case of simplifying closed Extrusionlines. - /* ExtrusionLines are treated as (open) polylines, so in case an ExtrusionLine is actually a closed polygon, its * starting and ending points will be equal (or almost equal). Therefore, the simplification of the ExtrusionLine * should not touch the first and last points. As a result, start simplifying from point at index 1. @@ -54,12 +52,16 @@ void ExtrusionLine::simplify(const int64_t smallest_line_segment_squared, const // Starting junction should always exist in the simplified path new_junctions.emplace_back(junctions.front()); - /* Initially, previous_previous is always the same as previous because, for open ExtrusionLines the last junction - * cannot be taken into consideration when checking the points at index 1. For closed ExtrusionLines, the first and - * last junctions are anyway the same. - * */ - ExtrusionJunction previous_previous = junctions.front(); ExtrusionJunction previous = junctions.front(); + /* For open ExtrusionLines the last junction cannot be taken into consideration when checking the points at index 1. + * For closed ExtrusionLines, the first and last junctions are the same, so use the prior to last juction. + * */ + ExtrusionJunction previous_previous = this->is_closed ? junctions[junctions.size() - 2] : junctions.front(); + + /* TODO: When deleting, combining, or modifying junctions, it would + * probably be good to set the new junction's width to a weighted average + * of the junctions it is derived from. + */ /* When removing a vertex, we check the height of the triangle of the area being removed from the original polygon by the simplification. However, @@ -76,16 +78,20 @@ void ExtrusionLine::simplify(const int64_t smallest_line_segment_squared, const From this area we compute the height of the representative triangle using the standard formula for a triangle area: A = .5*b*h */ - const ExtrusionJunction& initial = junctions.at(1); + const ExtrusionJunction& initial = junctions[1]; int64_t accumulated_area_removed = int64_t(previous.p.x()) * int64_t(initial.p.y()) - int64_t(previous.p.y()) * int64_t(initial.p.x()); // Twice the Shoelace formula for area of polygon per line segment. - for (size_t point_idx = 1; point_idx < junctions.size() - 1; point_idx++) + // For a closed polygon we process the last point, which is the same as the first point. + for (size_t point_idx = 1; point_idx < junctions.size() - (this->is_closed ? 0 : 1); point_idx++) { - const ExtrusionJunction& current = junctions[point_idx]; + // For the last point of a closed polygon, use the first point of the new polygon in case we modified it. + const bool is_last = point_idx + 1 == junctions.size(); + const ExtrusionJunction& current = is_last ? new_junctions[0] : junctions[point_idx]; // Spill over in case of overflow, unless the [next] vertex will then be equal to [previous]. - const bool spill_over = point_idx + 1 == junctions.size() && new_junctions.size() > 1; - ExtrusionJunction& next = spill_over ? new_junctions[0] : junctions[point_idx + 1]; + const bool spill_over = this->is_closed && point_idx + 2 >= junctions.size() && + point_idx + 2 - junctions.size() < new_junctions.size(); + ExtrusionJunction& next = spill_over ? new_junctions[point_idx + 2 - junctions.size()] : junctions[point_idx + 1]; const int64_t removed_area_next = int64_t(current.p.x()) * int64_t(next.p.y()) - int64_t(current.p.y()) * int64_t(next.p.x()); // Twice the Shoelace formula for area of polygon per line segment. const int64_t negative_area_closing = int64_t(next.p.x()) * int64_t(previous.p.y()) - int64_t(next.p.y()) * int64_t(previous.p.x()); // Area between the origin and the short-cutting segment @@ -133,12 +139,18 @@ void ExtrusionLine::simplify(const int64_t smallest_line_segment_squared, const // We should instead move this point to a location where both edges are kept and then remove the previous point that we wanted to keep. // By taking the intersection of these two lines, we get a point that preserves the direction (so it makes the corner a bit more pointy). // We just need to be sure that the intersection point does not introduce an artifact itself. + // o < prev_prev + // | + // o < prev + // \ < short segment + // intersection > + o-------------------o < next + // ^ current Point intersection_point; bool has_intersection = Line(previous_previous.p, previous.p).intersection_infinite(Line(current.p, next.p), &intersection_point); if (!has_intersection || Line::distance_to_infinite_squared(intersection_point, previous.p, current.p) > double(allowed_error_distance_squared) || (intersection_point - previous.p).cast().squaredNorm() > smallest_line_segment_squared // The intersection point is way too far from the 'previous' - || (intersection_point - next.p).cast().squaredNorm() > smallest_line_segment_squared) // and 'next' points, so it shouldn't replace 'current' + || (intersection_point - current.p).cast().squaredNorm() > smallest_line_segment_squared) // and 'current' points, so it shouldn't replace 'current' { // We can't find a better spot for it, but the size of the line is more than 5 micron. // So the only thing we can do here is leave it in... @@ -174,15 +186,14 @@ void ExtrusionLine::simplify(const int64_t smallest_line_segment_squared, const new_junctions.push_back(current); } - // Ending junction (vertex) should always exist in the simplified path - new_junctions.emplace_back(junctions.back()); - - /* In case this is a closed polygon (instead of a poly-line-segments), the invariant that the first and last points are the same should be enforced. - * Since one of them didn't move, and the other can't have been moved further than the constraints, if originally equal, they can simply be equated. - */ - if ((junctions.front().p - junctions.back().p).cast().squaredNorm() == 0) - { - new_junctions.back().p = junctions.front().p; + if (this->is_closed) { + /* The first and last points should be the same for a closed polygon. + * We processed the last point above, so copy it into the first point. + */ + new_junctions.front().p = new_junctions.back().p; + } else { + // Ending junction (vertex) should always exist in the simplified path + new_junctions.emplace_back(junctions.back()); } junctions = new_junctions; @@ -267,4 +278,4 @@ void extrusion_paths_append(ExtrusionPaths &dst, const Arachne::ExtrusionLine &e ThickPolyline thick_polyline = Arachne::to_thick_polyline(extrusion); Slic3r::append(dst, PerimeterGenerator::thick_polyline_to_multi_path(thick_polyline, role, flow, scaled(0.05), float(SCALED_EPSILON)).paths); } -} // namespace Slic3r \ No newline at end of file +} // namespace Slic3r