diff --git a/lib/Slic3r/Layer/Region.pm b/lib/Slic3r/Layer/Region.pm index 83fd13b679..81473b688b 100644 --- a/lib/Slic3r/Layer/Region.pm +++ b/lib/Slic3r/Layer/Region.pm @@ -223,11 +223,13 @@ sub make_perimeters { $self->perimeters->append(@loops); # process thin walls by collapsing slices to single passes - # the following offset2 ensures nothing in @thin_walls is narrower than $pwidth/10 - @thin_walls = @{offset2_ex([ map @$_, @thin_walls ], -$pwidth/10, +$pwidth/10)}; + # the following offset2 ensures almost nothing in @thin_walls is narrower than $min_width + # (actually, something larger than that still may exist due to mitering or other causes) + my $min_width = $pwidth / 4; + @thin_walls = @{offset2_ex([ map @$_, @thin_walls ], -$min_width/2, +$min_width/2)}; if (@thin_walls) { # the maximum thickness of our thin wall area is equal to the minimum thickness of a single loop - my @p = map @{$_->medial_axis($pwidth + $pspacing)}, @thin_walls; + my @p = map @{$_->medial_axis($pwidth + $pspacing, $min_width)}, @thin_walls; if (0) { require "Slic3r/SVG.pm"; @@ -241,9 +243,7 @@ sub make_perimeters { } my @paths = (); - my $min_thin_wall_length = 2*$pwidth; for my $p (@p) { - next if $p->length < $min_thin_wall_length; my %params = ( role => EXTR_ROLE_EXTERNAL_PERIMETER, mm3_per_mm => $mm3_per_mm, diff --git a/t/thin.t b/t/thin.t index d11bcc41af..390c74fa7d 100644 --- a/t/thin.t +++ b/t/thin.t @@ -1,4 +1,4 @@ -use Test::More tests => 9; +use Test::More tests => 12; use strict; use warnings; @@ -9,9 +9,9 @@ BEGIN { use Slic3r; use List::Util qw(first); -use Slic3r::Geometry qw(epsilon scale unscale); +use Slic3r::Geometry qw(epsilon scale unscale scaled_epsilon Y); use Slic3r::Test; -goto TTT; + { my $config = Slic3r::Config->new_from_defaults; $config->set('layer_height', 0.2); @@ -59,20 +59,20 @@ goto TTT; [160, 140], ); my $expolygon = Slic3r::ExPolygon->new($square, $hole_in_square); - my $res = $expolygon->medial_axis(scale 10); + my $res = $expolygon->medial_axis(scale 1, scale 0.5); is scalar(@$res), 1, 'medial axis of a square shape is a single closed loop'; ok $res->[0]->length > $hole_in_square->length && $res->[0]->length < $square->length, 'medial axis loop has reasonable length'; } -TTT: { +{ my $expolygon = Slic3r::ExPolygon->new(Slic3r::Polygon->new_scale( [100, 100], [120, 100], [120, 200], [100, 200], )); - my $res = $expolygon->medial_axis(scale 10); + my $res = $expolygon->medial_axis(scale 1, scale 0.5); is scalar(@$res), 1, 'medial axis of a narrow rectangle is a single line'; ok unscale($res->[0]->length) >= (200-100 - (120-100)) - epsilon, 'medial axis has reasonable length'; @@ -83,13 +83,11 @@ TTT: { [105, 200], # extra point in the short side [100, 200], )); - my $res2 = $expolygon->medial_axis(scale 10); - use Slic3r::SVG; - Slic3r::SVG::output( - "thin.svg", - expolygons => [$expolygon], - polylines => $res2, - ); + my $res2 = $expolygon->medial_axis(scale 1, scale 0.5); + is scalar(@$res), 1, 'medial axis of a narrow rectangle with an extra vertex is still a single line'; + ok unscale($res->[0]->length) >= (200-100 - (120-100)) - epsilon, 'medial axis has still a reasonable length'; + ok !(grep { abs($_ - scale 150) < scaled_epsilon } map $_->[Y], map @$_, @$res2), "extra vertices don't influence medial axis"; + } { @@ -99,7 +97,7 @@ TTT: { [112, 200], [108, 200], )); - my $res = $expolygon->medial_axis(scale 10); + my $res = $expolygon->medial_axis(scale 1, scale 0.5); is scalar(@$res), 1, 'medial axis of a narrow trapezoid is a single line'; ok unscale($res->[0]->length) >= (200-100 - (120-100)) - epsilon, 'medial axis has reasonable length'; } @@ -113,19 +111,10 @@ TTT: { [200, 200], [100, 200], )); - my $res = $expolygon->medial_axis(scale 20); + my $res = $expolygon->medial_axis(scale 1, scale 0.5); is scalar(@$res), 1, 'medial axis of a L shape is a single polyline'; my $len = unscale($res->[0]->length) + 20; # 20 is the thickness of the expolygon, which is subtracted from the ends ok $len > 80*2 && $len < 100*2, 'medial axis has reasonable length'; - - if (0) { - require "Slic3r/SVG.pm"; - Slic3r::SVG::output( - "thin.svg", - expolygons => [$expolygon], - polylines => $res, - ); - } } __END__ diff --git a/xs/src/ExPolygon.cpp b/xs/src/ExPolygon.cpp index ec3c5d4929..4eefd0b162 100644 --- a/xs/src/ExPolygon.cpp +++ b/xs/src/ExPolygon.cpp @@ -135,19 +135,15 @@ ExPolygon::simplify(double tolerance, ExPolygons &expolygons) const } void -ExPolygon::medial_axis(double width, Polylines* polylines) const +ExPolygon::medial_axis(double max_width, double min_width, Polylines* polylines) const { // init helper object - Slic3r::Geometry::MedialAxis ma(width); + Slic3r::Geometry::MedialAxis ma(max_width, min_width); // populate list of segments for the Voronoi diagram - ExPolygons expp; - this->simplify(scale_(0.01), expp); - for (ExPolygons::const_iterator expolygon = expp.begin(); expolygon != expp.end(); ++expolygon) { - expolygon->contour.lines(&ma.lines); - for (Polygons::const_iterator hole = expolygon->holes.begin(); hole != expolygon->holes.end(); ++hole) - hole->lines(&ma.lines); - } + this->contour.lines(&ma.lines); + for (Polygons::const_iterator hole = this->holes.begin(); hole != this->holes.end(); ++hole) + hole->lines(&ma.lines); // compute the Voronoi diagram ma.build(polylines); diff --git a/xs/src/ExPolygon.hpp b/xs/src/ExPolygon.hpp index e7db3e6277..3b81752041 100644 --- a/xs/src/ExPolygon.hpp +++ b/xs/src/ExPolygon.hpp @@ -26,7 +26,7 @@ class ExPolygon Polygons simplify_p(double tolerance) const; ExPolygons simplify(double tolerance) const; void simplify(double tolerance, ExPolygons &expolygons) const; - void medial_axis(double width, Polylines* polylines) const; + void medial_axis(double max_width, double min_width, Polylines* polylines) const; #ifdef SLIC3RXS void from_SV(SV* poly_sv); diff --git a/xs/src/Geometry.cpp b/xs/src/Geometry.cpp index 1d3fc6740b..bb673813be 100644 --- a/xs/src/Geometry.cpp +++ b/xs/src/Geometry.cpp @@ -119,68 +119,63 @@ MedialAxis::build(Polylines* polylines) // note: this keeps twins, so it contains twice the number of the valid edges this->edges.clear(); for (VD::const_edge_iterator edge = this->vd.edges().begin(); edge != this->vd.edges().end(); ++edge) { - if (this->is_valid_edge(*edge)) this->edges.insert(&*edge); + // if we only process segments representing closed loops, none if the + // infinite edges (if any) would be part of our MAT anyway + if (edge->is_secondary() || edge->is_infinite()) continue; + this->edges.insert(&*edge); } // count valid segments for each vertex - std::map< const VD::vertex_type*,std::list > vertex_edges; - std::list entry_nodes; + std::map< const VD::vertex_type*,std::set > vertex_edges; + std::set entry_nodes; for (VD::const_vertex_iterator vertex = this->vd.vertices().begin(); vertex != this->vd.vertices().end(); ++vertex) { // get a reference to the list of valid edges originating from this vertex - std::list& edges = vertex_edges[&*vertex]; + std::set& edges = vertex_edges[&*vertex]; // get one random edge originating from this vertex const VD::edge_type* edge = vertex->incident_edge(); do { if (this->edges.count(edge) > 0) // only count valid edges - edges.push_back(edge); + edges.insert(edge); edge = edge->rot_next(); // next edge originating from this vertex } while (edge != vertex->incident_edge()); // if there's only one edge starting at this vertex then it's a leaf - if (edges.size() == 1) entry_nodes.push_back(&*vertex); + size_t edge_count = edges.size(); + if (edge_count == 1) { + entry_nodes.insert(&*vertex); + } } - // iterate through the leafs to prune short branches - for (std::list::const_iterator vertex = entry_nodes.begin(); vertex != entry_nodes.end(); ++vertex) { - const VD::vertex_type* v = *vertex; + // prune recursively + while (!entry_nodes.empty()) { + // get a random entry node + const VD::vertex_type* v = *entry_nodes.begin(); + + // get edge starting from v + assert(!vertex_edges[v].empty()); + const VD::edge_type* edge = *vertex_edges[v].begin(); - // start a polyline from this vertex - Polyline polyline; - polyline.points.push_back(Point(v->x(), v->y())); - - // keep track of visited edges to prevent infinite loops - std::set visited_edges; - - do { - // get edge starting from v - const VD::edge_type* edge = vertex_edges[v].front(); + if (!this->is_valid_edge(*edge)) { + // if edge is not valid, erase it from edge list + (void)this->edges.erase(edge); + (void)this->edges.erase(edge->twin()); - // if we picked the edge going backwards (thus the twin of the previous edge) - if (visited_edges.count(edge->twin()) > 0) { - edge = vertex_edges[v].back(); - } + // decrement edge counters for the affected nodes + const VD::vertex_type* v1 = edge->vertex1(); + (void)vertex_edges[v].erase(edge); + (void)vertex_edges[v1].erase(edge->twin()); - // avoid getting twice on the same edge - if (visited_edges.count(edge) > 0) break; - visited_edges.insert(edge); - - // get ending vertex for this edge and append it to the polyline - v = edge->vertex1(); - polyline.points.push_back(Point( v->x(), v->y() )); - - // if two edges start at this vertex (one forward one backwards) then - // it's not branching and we can go on - } while (vertex_edges[v].size() == 2); - - // if this branch is too short, invalidate all of its edges so that - // they will be ignored when building actual polylines in the loop below - if (polyline.length() < this->width) { - for (std::set::const_iterator edge = visited_edges.begin(); edge != visited_edges.end(); ++edge) { - (void)this->edges.erase(*edge); - (void)this->edges.erase((*edge)->twin()); + // also, check whether the end vertex is a new leaf + if (vertex_edges[v1].size() == 1) { + entry_nodes.insert(v1); + } else if (vertex_edges[v1].empty()) { + entry_nodes.erase(v1); } } + + // remove node from the set to prevent it from being visited again + entry_nodes.erase(v); } // iterate through the valid edges to build polylines @@ -204,8 +199,9 @@ MedialAxis::build(Polylines* polylines) this->process_edge_neighbors(*edge.twin(), &pp); polyline.points.insert(polyline.points.begin(), pp.rbegin(), pp.rend()); - // append polyline to result - polylines->push_back(polyline); + // append polyline to result if it's not too small + if (polyline.length() > this->max_width) + polylines->push_back(polyline); } } @@ -236,10 +232,6 @@ MedialAxis::process_edge_neighbors(const VD::edge_type& edge, Points* points) bool MedialAxis::is_valid_edge(const VD::edge_type& edge) const { - // if we only process segments representing closed loops, none if the - // infinite edges (if any) would be part of our MAT anyway - if (edge.is_secondary() || edge.is_infinite()) return false; - /* If the cells sharing this edge have a common vertex, we're not interested in this edge. Why? Because it means that the edge lies on the bisector of two contiguous input lines and it was included in the Voronoi graph because @@ -264,7 +256,7 @@ MedialAxis::is_valid_edge(const VD::edge_type& edge) const // so we allow some tolerance (say, 30°) if (fabs(angle) < PI - PI/3) return false; - /* + // each vertex is equidistant to both cell segments // but such distance might differ between the two vertices; // in this case it means the shape is getting narrow (like a corner) @@ -274,19 +266,28 @@ MedialAxis::is_valid_edge(const VD::edge_type& edge) const Point v1( edge.vertex1()->x(), edge.vertex1()->y() ); double dist0 = v0.distance_to(segment1); double dist1 = v1.distance_to(segment1); + + /* double diff = fabs(dist1 - dist0); double dist_between_segments1 = segment1.a.distance_to(segment2); double dist_between_segments2 = segment1.b.distance_to(segment2); - printf("w = %f, dist0 = %f, dist1 = %f, diff = %f, seglength = %f, edgelen = %f, s2s = %f / %f\n", - unscale(this->width), - unscale(dist0), unscale(dist1), unscale(diff), unscale(segment1.length()), + printf("w = %f/%f, dist0 = %f, dist1 = %f, diff = %f, seg1len = %f, seg2len = %f, edgelen = %f, s2s = %f / %f\n", + unscale(this->max_width), unscale(this->min_width), + unscale(dist0), unscale(dist1), unscale(diff), + unscale(segment1.length()), unscale(segment2.length()), unscale(this->edge_to_line(edge).length()), unscale(dist_between_segments1), unscale(dist_between_segments2) ); - if (dist0 < SCALED_EPSILON && dist1 < SCALED_EPSILON) { - printf(" => too thin, skipping\n"); - //return false; + */ + + // if this segment is the centerline for a very thin area, we might want to skip it + // in case the area is too thin + if (dist0 < this->min_width/2 || dist1 < this->min_width/2) { + //printf(" => too thin, skipping\n"); + return false; } + + /* // if distance between this edge and the thin area boundary is greater // than half the max width, then it's not a true medial axis segment if (dist1 > this->width*2) { @@ -295,9 +296,10 @@ MedialAxis::is_valid_edge(const VD::edge_type& edge) const } */ + return true; } - return true; + return false; } Line diff --git a/xs/src/Geometry.hpp b/xs/src/Geometry.hpp index 22bd97cfbc..9c758c6168 100644 --- a/xs/src/Geometry.hpp +++ b/xs/src/Geometry.hpp @@ -20,8 +20,9 @@ class MedialAxis { public: Points points; Lines lines; - double width; - MedialAxis(double _width) : width(_width) {}; + double max_width; + double min_width; + MedialAxis(double _max_width, double _min_width) : max_width(_max_width), min_width(_min_width) {}; void build(Polylines* polylines); private: diff --git a/xs/xsp/ExPolygon.xsp b/xs/xsp/ExPolygon.xsp index caadb9e56f..6c2ad1c462 100644 --- a/xs/xsp/ExPolygon.xsp +++ b/xs/xsp/ExPolygon.xsp @@ -25,8 +25,8 @@ bool contains_point(Point* point); ExPolygons simplify(double tolerance); Polygons simplify_p(double tolerance); - Polylines medial_axis(double width) - %code{% THIS->medial_axis(width, &RETVAL); %}; + Polylines medial_axis(double max_width, double min_width) + %code{% THIS->medial_axis(max_width, min_width, &RETVAL); %}; %{ ExPolygon*