Browse Source

Fix more regressions in test suite regarding the workaround for Clipper bug

Alessandro Ranellucci 10 years ago
parent
commit
5e6ff952df
2 changed files with 9 additions and 8 deletions
  1. 1 6
      t/clipper.t
  2. 8 2
      xs/src/ClipperUtils.cpp

+ 1 - 6
t/clipper.t

@@ -82,12 +82,7 @@ use Slic3r::Geometry::Clipper qw(intersection_ex union_ex diff_ex diff_pl);
     my $res = diff_pl([$square_pl], []);
     is scalar(@$res), 1, 'no-op diff_pl returns the right number of polylines';
     isa_ok $res->[0], 'Slic3r::Polyline', 'no-op diff_pl result';
-    
-    ### NOTE: this test will fail when a bug in Clipper is fixed that is currently
-    ### causing loss of one segment when input polyline has coinciding endpoints.
-    ### When the bug is fixed in Clipper, this test should be reverted from isnt() to is()
-    ### and workarounds in Slic3r::Polygon::clip_as_polyline() should be removed.
-    isnt scalar(@{$res->[0]}), scalar(@$square_pl), 'no-op diff_pl returns the unmodified input polyline';
+    is scalar(@{$res->[0]}), scalar(@$square_pl), 'no-op diff_pl returns the unmodified input polyline';
 }
 
 __END__

+ 8 - 2
xs/src/ClipperUtils.cpp

@@ -343,8 +343,13 @@ void _clipper(ClipperLib::ClipType clipType, const Slic3r::Polylines &subject,
     /* 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);
+    std::vector<bool> translated(subject.size(), false); // keep track of polylines we applied the hack to
+    for (Slic3r::Polylines::iterator polyline = polylines.begin(); polyline != polylines.end(); ++polyline) {
+        if (polyline->points.front().coincides_with(polyline->points.back())) {
+            polyline->points.front().translate(1, 0);
+            translated[ polyline - polylines.begin() ] = true;
+        }
+    }
     
     // perform operation
     ClipperLib::PolyTree polytree;
@@ -358,6 +363,7 @@ void _clipper(ClipperLib::ClipType clipType, const Slic3r::Polylines &subject,
     // compensate for the above hack
     for (Slic3r::Polylines::iterator polyline = retval.begin(); polyline != retval.end(); ++polyline) {
         for (Slic3r::Polylines::const_iterator subj_polyline = polylines.begin(); subj_polyline != polylines.end(); ++subj_polyline) {
+            if (!translated[ subj_polyline - polylines.begin() ]) continue;
             // 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);