From 08279ec5d8b4ae01b6c09fca8b8e02b146d0de7d Mon Sep 17 00:00:00 2001 From: Alessandro Ranellucci Date: Wed, 21 May 2014 15:03:31 +0200 Subject: [PATCH] Bugfix: thin walls forming a closed loop had overlapping segments at their endpoints. #1948 #1875 --- t/thin.t | 6 ++++-- xs/src/ClipperUtils.cpp | 42 +++++++++++++++++++++-------------------- xs/src/ExPolygon.cpp | 2 ++ xs/src/Geometry.cpp | 15 +++++++++++++++ 4 files changed, 43 insertions(+), 22 deletions(-) diff --git a/t/thin.t b/t/thin.t index eb31b6e29..e02e1b33d 100644 --- a/t/thin.t +++ b/t/thin.t @@ -1,4 +1,4 @@ -use Test::More tests => 11; +use Test::More tests => 13; use strict; use warnings; @@ -62,7 +62,9 @@ if (0) { ); my $expolygon = Slic3r::ExPolygon->new($square, $hole_in_square); 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'; + is scalar(@$res), 1, 'medial axis of a square shape is a single path'; + isa_ok $res->[0], 'Slic3r::Polyline', 'medial axis result is a polyline'; + ok $res->[0]->first_point->coincides_with($res->[0]->last_point), 'polyline forms a closed loop'; ok $res->[0]->length > $hole_in_square->length && $res->[0]->length < $square->length, 'medial axis loop has reasonable length'; } diff --git a/xs/src/ClipperUtils.cpp b/xs/src/ClipperUtils.cpp index 3f2e48d50..d49aba529 100644 --- a/xs/src/ClipperUtils.cpp +++ b/xs/src/ClipperUtils.cpp @@ -339,36 +339,25 @@ void _clipper(ClipperLib::ClipType clipType, const Slic3r::Polygons &subject, void _clipper(ClipperLib::ClipType clipType, const Slic3r::Polylines &subject, const Slic3r::Polygons &clip, Slic3r::Polylines &retval, bool safety_offset_) { + + /* Clipper will remove a polyline segment if first point coincides with last one. + Until that bug is not fixed upstream, we move one of those points slightly. */ + Slic3r::Polylines polylines = subject; // temp copy to avoid dropping the const qualifier + for (Slic3r::Polylines::iterator polyline = polylines.begin(); polyline != polylines.end(); ++polyline) + polyline->points.front().translate(1, 0); + // perform operation ClipperLib::PolyTree polytree; - _clipper_do(clipType, subject, clip, polytree, ClipperLib::pftNonZero, safety_offset_); + _clipper_do(clipType, polylines, clip, polytree, ClipperLib::pftNonZero, safety_offset_); // convert into Polylines ClipperLib::Paths output; ClipperLib::PolyTreeToPaths(polytree, output); ClipperPaths_to_Slic3rMultiPoints(output, retval); -} - -void _clipper(ClipperLib::ClipType clipType, const Slic3r::Polygons &subject, - const Slic3r::Polygons &clip, Slic3r::Polylines &retval, bool safety_offset_) -{ - // transform input polygons into polylines - Slic3r::Polylines polylines; - polylines.reserve(subject.size()); - for (Slic3r::Polygons::const_iterator polygon = subject.begin(); polygon != subject.end(); ++polygon) - polylines.push_back(*polygon); // implicit call to split_at_first_point() - - /* Clipper will remove a polyline segment if first point coincides with last one. - Until that bug is not fixed upstream, we move one of those points slightly. */ - for (Slic3r::Polylines::iterator polyline = polylines.begin(); polyline != polylines.end(); ++polyline) - polyline->points.front().translate(1, 0); - - // perform clipping - _clipper(clipType, polylines, clip, retval, safety_offset_); // compensate for the above hack for (Slic3r::Polylines::iterator polyline = retval.begin(); polyline != retval.end(); ++polyline) { - for (Slic3r::Polylines::iterator subj_polyline = polylines.begin(); subj_polyline != polylines.end(); ++subj_polyline) { + for (Slic3r::Polylines::const_iterator subj_polyline = polylines.begin(); subj_polyline != polylines.end(); ++subj_polyline) { // if first point of clipped line coincides with first point of subject line, compensate for hack if (polyline->points.front().coincides_with(subj_polyline->points.front())) { polyline->points.front().translate(-1, 0); @@ -381,6 +370,19 @@ void _clipper(ClipperLib::ClipType clipType, const Slic3r::Polygons &subject, } } } +} + +void _clipper(ClipperLib::ClipType clipType, const Slic3r::Polygons &subject, + const Slic3r::Polygons &clip, Slic3r::Polylines &retval, bool safety_offset_) +{ + // transform input polygons into polylines + Slic3r::Polylines polylines; + polylines.reserve(subject.size()); + for (Slic3r::Polygons::const_iterator polygon = subject.begin(); polygon != subject.end(); ++polygon) + polylines.push_back(*polygon); // implicit call to split_at_first_point() + + // perform clipping + _clipper(clipType, polylines, clip, retval, safety_offset_); /* If the split_at_first_point() call above happens to split the polygon inside the clipping area we would get two consecutive polylines instead of a single one, so we go through them in order diff --git a/xs/src/ExPolygon.cpp b/xs/src/ExPolygon.cpp index e2ee364a8..78a658bd6 100644 --- a/xs/src/ExPolygon.cpp +++ b/xs/src/ExPolygon.cpp @@ -155,7 +155,9 @@ ExPolygon::medial_axis(double max_width, double min_width, Polylines* polylines) ma.build(polylines); // extend initial and final segments of each polyline (they will be clipped) + // unless they represent closed loops for (Polylines::iterator polyline = polylines->begin(); polyline != polylines->end(); ++polyline) { + if (polyline->points.front().coincides_with(polyline->points.back())) continue; polyline->extend_start(max_width); polyline->extend_end(max_width); } diff --git a/xs/src/Geometry.cpp b/xs/src/Geometry.cpp index c0d9e66cd..0295d54de 100644 --- a/xs/src/Geometry.cpp +++ b/xs/src/Geometry.cpp @@ -123,6 +123,21 @@ MedialAxis::build(Polylines* polylines) construct_voronoi(this->lines.begin(), this->lines.end(), &this->vd); + /* + // DEBUG: dump all Voronoi edges + { + for (VD::const_edge_iterator edge = this->vd.edges().begin(); edge != this->vd.edges().end(); ++edge) { + if (edge->is_infinite()) continue; + + Polyline polyline; + polyline.points.push_back(Point( edge->vertex0()->x(), edge->vertex0()->y() )); + polyline.points.push_back(Point( edge->vertex1()->x(), edge->vertex1()->y() )); + polylines->push_back(polyline); + } + return; + } + */ + // collect valid edges (i.e. prune those not belonging to MAT) // note: this keeps twins, so it contains twice the number of the valid edges this->edges.clear();