Skip to content

Commit

Permalink
Improve ExtrusionLine::simplify, eliminating many very-short extrusio…
Browse files Browse the repository at this point in the history 
…n segments which led to blemishes in thin-wall models sliced with Arachne - Take 2 (#3750)

* Improve toolpath simplification to avoid blemishes when using Arachne on thin walls.

* Avoid reducing closed perimeters below 3 points.

Downstream code contains assumptions that closed ExtrusionLines are not
degenerate. Open ExtrusionLines already are not reduced below 2 points by
ExtrusionLine::simplify().

---------

Co-authored-by: SoftFever <[email protected]>
  • Loading branch information
@sethml @SoftFever
sethml and SoftFever authored Jan 30, 2024
1 parent e56a0de commit 4110ecc
Showing 1 changed file with 40 additions and 23 deletions.
63 changes: 40 additions & 23 deletions src/libslic3r/Arachne/utils/ExtrusionLine.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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,
Expand All @@ -76,16 +78,26 @@ 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];

// Don't simplify closed polygons below 3 junctions.
if (this->is_closed && new_junctions.size() + (junctions.size() - point_idx) <= 3) {
new_junctions.push_back(current);
continue;
}

// 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
Expand Down Expand Up @@ -133,12 +145,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<int64_t>().squaredNorm() > smallest_line_segment_squared // The intersection point is way too far from the 'previous'
|| (intersection_point - next.p).cast<int64_t>().squaredNorm() > smallest_line_segment_squared) // and 'next' points, so it shouldn't replace 'current'
|| (intersection_point - current.p).cast<int64_t>().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...
Expand Down Expand Up @@ -174,15 +192,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<int64_t>().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;
Expand Down Expand Up @@ -267,4 +284,4 @@ void extrusion_paths_append(ExtrusionPaths &dst, const Arachne::ExtrusionLine &e
ThickPolyline thick_polyline = Arachne::to_thick_polyline(extrusion);
Slic3r::append(dst, thick_polyline_to_multi_path(thick_polyline, role, flow, scaled<float>(0.05), float(SCALED_EPSILON)).paths);
}
} // namespace Slic3r
} // namespace Slic3r

0 comments on commit 4110ecc

Please sign in to comment.