From 48f1fab49ffe2ea6f387f21c04e2c38c7071810c Mon Sep 17 00:00:00 2001 From: Alessandro Ranellucci Date: Tue, 1 Dec 2015 20:40:00 +0100 Subject: [PATCH] Bugfix: an error in porting caused bad perimeter ordering. Includes regression test and more unit tests for PerimeterGenerator --- t/perimeters.t | 44 ++++++++++++++++++++++++- xs/src/libslic3r/PerimeterGenerator.cpp | 41 ++++++++++++----------- 2 files changed, 64 insertions(+), 21 deletions(-) diff --git a/t/perimeters.t b/t/perimeters.t index 134c23536..47b844887 100644 --- a/t/perimeters.t +++ b/t/perimeters.t @@ -1,4 +1,4 @@ -use Test::More tests => 33; +use Test::More tests => 59; use strict; use warnings; @@ -63,10 +63,23 @@ use Slic3r::Test; $expected{total}, 'expected number of loops'; is scalar(grep $_->role == EXTR_ROLE_EXTERNAL_PERIMETER, map @$_, @loops), $expected{external}, 'expected number of external loops'; + is_deeply [ map { ($_->role == EXTR_ROLE_EXTERNAL_PERIMETER) || 0 } map @$_, @loops ], + $expected{ext_order}, 'expected external order'; is scalar(grep $_->role == EXTRL_ROLE_CONTOUR_INTERNAL_PERIMETER, @loops), $expected{cinternal}, 'expected number of internal contour loops'; is scalar(grep $_->polygon->is_counter_clockwise, @loops), $expected{ccw}, 'expected number of ccw loops'; + is_deeply [ map $_->polygon->is_counter_clockwise, @loops ], + $expected{ccw_order}, 'expected ccw/cw order'; + + if ($expected{nesting}) { + foreach my $nesting (@{ $expected{nesting} }) { + for my $i (1..$#$nesting) { + ok $loops[$nesting->[$i-1]]->polygon->contains_point($loops[$nesting->[$i]]->first_point), + 'expected nesting order'; + } + } + } }; $config->set('perimeters', 3); @@ -78,8 +91,11 @@ use Slic3r::Test; ], total => 3, external => 1, + ext_order => [0,0,1], cinternal => 1, ccw => 3, + ccw_order => [1,1,1], + nesting => [ [2,1,0] ], ); $test->( [ @@ -90,8 +106,11 @@ use Slic3r::Test; ], total => 6, external => 2, + ext_order => [0,0,1,0,0,1], cinternal => 1, ccw => 3, + ccw_order => [0,0,0,1,1,1], + nesting => [ [5,4,3,0,1,2] ], ); $test->( [ @@ -107,8 +126,31 @@ use Slic3r::Test; ], total => 4*3, external => 4, + ext_order => [0,0,1,0,0,1,0,0,1,0,0,1], cinternal => 2, ccw => 2*3, + ccw_order => [0,0,0,1,1,1,0,0,0,1,1,1], + ); + + $config->set('perimeters', 2); + $test->( + [ + Slic3r::ExPolygon->new( + Slic3r::Polygon->new_scale([0,0], [50,0], [50,50], [0,50]), + Slic3r::Polygon->new_scale([7.5,7.5], [7.5,12.5], [12.5,12.5], [12.5,7.5]), + Slic3r::Polygon->new_scale([7.5,17.5], [7.5,22.5], [12.5,22.5], [12.5,17.5]), + Slic3r::Polygon->new_scale([7.5,27.5], [7.5,32.5], [12.5,32.5], [12.5,27.5]), + Slic3r::Polygon->new_scale([7.5,37.5], [7.5,42.5], [12.5,42.5], [12.5,37.5]), + Slic3r::Polygon->new_scale([17.5,7.5], [17.5,12.5], [22.5,12.5], [22.5,7.5]), + ), + ], + total => 12, + external => 6, + ext_order => [0,1,0,1,0,1,0,1,0,1,0,1], + cinternal => 1, + ccw => 2, + ccw_order => [0,0,0,0,0,0,0,0,0,0,1,1], + nesting => [ [0,1],[2,3],[4,5],[6,7],[8,9] ], ); } diff --git a/xs/src/libslic3r/PerimeterGenerator.cpp b/xs/src/libslic3r/PerimeterGenerator.cpp index 32662f59c..969a85259 100644 --- a/xs/src/libslic3r/PerimeterGenerator.cpp +++ b/xs/src/libslic3r/PerimeterGenerator.cpp @@ -185,7 +185,7 @@ PerimeterGenerator::process() } } } - + // if no hole contains this hole, find the contour loop that contains it for (signed short t = loop_number; t >= 0; --t) { for (signed short j = 0; j < contours[t].size(); ++j) { @@ -237,7 +237,7 @@ PerimeterGenerator::process() if (this->config->external_perimeters_first || (this->layer_id == 0 && this->print_config->brim_width.value > 0)) entities.reverse(); - + // append perimeters for this slice as a collection if (!entities.empty()) this->loops->append(entities); @@ -400,34 +400,35 @@ PerimeterGenerator::_traverse_loops(const PerimeterGeneratorLoops &loops, } // append thin walls to the nearest-neighbor search (only for first iteration) - if (!thin_walls.empty()) { - for (Polylines::const_iterator polyline = thin_walls.begin(); polyline != thin_walls.end(); ++polyline) { - ExtrusionPath path(erExternalPerimeter); - path.polyline = *polyline; - path.mm3_per_mm = this->_mm3_per_mm; - path.width = this->perimeter_flow.width; - path.height = this->layer_height; - coll.append(path); - } - - thin_walls.clear(); + for (Polylines::const_iterator polyline = thin_walls.begin(); polyline != thin_walls.end(); ++polyline) { + ExtrusionPath path(erExternalPerimeter); + path.polyline = *polyline; + path.mm3_per_mm = this->_mm3_per_mm; + path.width = this->perimeter_flow.width; + path.height = this->layer_height; + coll.append(path); } + thin_walls.clear(); - // sort entities + // sort entities into a new collection using a nearest-neighbor search, + // preserving the original indices which are useful for detecting thin walls ExtrusionEntityCollection sorted_coll; coll.chained_path(&sorted_coll, false, &sorted_coll.orig_indices); - // traverse children + // traverse children and build the final collection ExtrusionEntityCollection entities; - for (unsigned short i = 0; i < sorted_coll.orig_indices.size(); ++i) { - size_t idx = sorted_coll.orig_indices[i]; - if (idx >= loops.size()) { + for (std::vector::const_iterator idx = sorted_coll.orig_indices.begin(); + idx != sorted_coll.orig_indices.end(); + ++idx) { + + if (*idx >= loops.size()) { // this is a thin wall // let's get it from the sorted collection as it might have been reversed + size_t i = idx - sorted_coll.orig_indices.begin(); entities.append(*sorted_coll.entities[i]); } else { - const PerimeterGeneratorLoop &loop = loops[i]; - ExtrusionLoop eloop = *dynamic_cast(coll.entities[idx]); + const PerimeterGeneratorLoop &loop = loops[*idx]; + ExtrusionLoop eloop = *dynamic_cast(coll.entities[*idx]); ExtrusionEntityCollection children = this->_traverse_loops(loop.children, thin_walls); if (loop.is_contour) {