From 9b4fe076a652864d54cd277ca06f40ce5cf94cec Mon Sep 17 00:00:00 2001 From: Alessandro Ranellucci Date: Tue, 28 Mar 2017 15:58:01 +0200 Subject: [PATCH] Workaround for detect_surfaces_type() not being idempotent and causing artifacts after multiple runs. #3764 --- lib/Slic3r/Print/Object.pm | 30 +++++++++++++++++++++++------- xs/src/libslic3r/Layer.cpp | 12 +++++++----- xs/src/libslic3r/Print.hpp | 4 ++-- xs/src/libslic3r/PrintObject.cpp | 18 +++++++++++++++++- xs/xsp/Print.xsp | 3 ++- 5 files changed, 51 insertions(+), 16 deletions(-) diff --git a/lib/Slic3r/Print/Object.pm b/lib/Slic3r/Print/Object.pm index 937eb2e92..3e9f38d22 100644 --- a/lib/Slic3r/Print/Object.pm +++ b/lib/Slic3r/Print/Object.pm @@ -125,28 +125,44 @@ sub slice { sub make_perimeters { my ($self) = @_; + return if $self->step_done(STEP_PERIMETERS); + + # Temporary workaround for detect_surfaces_type() not being idempotent (see #3764). + # We can remove this when idempotence is restored. This make_perimeters() method + # will just call merge_slices() to undo the typed slices and invalidate posDetectSurfaces. + if ($self->typed_slices) { + $self->invalidate_step(STEP_SLICE); + } + # prerequisites $self->slice; $self->_make_perimeters; } +# This will assign a type (top/bottom/internal) to $layerm->slices +# and transform $layerm->fill_surfaces from expolygon +# to typed top/bottom/internal surfaces; +sub detect_surfaces_type { + my ($self) = @_; + + # prerequisites + $self->slice; + + $self->_detect_surfaces_type; +} + sub prepare_infill { my ($self) = @_; # prerequisites - $self->make_perimeters; + $self->make_perimeters; # do we need them? TODO: check + $self->detect_surfaces_type; return if $self->step_done(STEP_PREPARE_INFILL); $self->set_step_started(STEP_PREPARE_INFILL); $self->print->status_cb->(30, "Preparing infill"); - # this will assign a type (top/bottom/internal) to $layerm->slices - # and transform $layerm->fill_surfaces from expolygon - # to typed top/bottom/internal surfaces; - $self->detect_surfaces_type; - $self->set_typed_slices(1); - # decide what surfaces are to be filled $_->prepare_fill_surfaces for map @{$_->regions}, @{$self->layers}; diff --git a/xs/src/libslic3r/Layer.cpp b/xs/src/libslic3r/Layer.cpp index 722b539e5..4837af9c0 100644 --- a/xs/src/libslic3r/Layer.cpp +++ b/xs/src/libslic3r/Layer.cpp @@ -197,11 +197,11 @@ Layer::make_perimeters() // merge the surfaces assigned to each group SurfaceCollection new_slices; - for (std::map::const_iterator it = slices.begin(); it != slices.end(); ++it) { - ExPolygons expp = union_ex(it->second, true); - for (ExPolygons::iterator ex = expp.begin(); ex != expp.end(); ++ex) { - Surface s = it->second.front(); // clone type and extra_perimeters - s.expolygon = *ex; + for (const auto &it : slices) { + ExPolygons expp = union_ex(it.second, true); + for (ExPolygon &ex : expp) { + Surface s = it.second.front(); // clone type and extra_perimeters + s.expolygon = ex; new_slices.surfaces.push_back(s); } } @@ -274,6 +274,8 @@ Layer::detect_surfaces_type() Layer* const &lower_layer = this->lower_layer; // collapse very narrow parts (using the safety offset in the diff is not enough) + // TODO: this offset2 makes this method not idempotent (see #3764), so we should + // move it to where we generate fill_surfaces instead and leave slices unaltered const float offs = layerm.flow(frExternalPerimeter).scaled_width() / 10.f; const Polygons layerm_slices_surfaces = layerm.slices; diff --git a/xs/src/libslic3r/Print.hpp b/xs/src/libslic3r/Print.hpp index a76adde6a..18fc760ef 100644 --- a/xs/src/libslic3r/Print.hpp +++ b/xs/src/libslic3r/Print.hpp @@ -26,8 +26,8 @@ enum PrintStep { psSkirt, psBrim, }; enum PrintObjectStep { - posSlice, posPerimeters, posPrepareInfill, - posInfill, posSupportMaterial, + posSlice, posPerimeters, posDetectSurfaces, + posPrepareInfill, posInfill, posSupportMaterial, }; // To be instantiated over PrintStep or PrintObjectStep enums. diff --git a/xs/src/libslic3r/PrintObject.cpp b/xs/src/libslic3r/PrintObject.cpp index 61e0cc7cd..0e752849f 100644 --- a/xs/src/libslic3r/PrintObject.cpp +++ b/xs/src/libslic3r/PrintObject.cpp @@ -311,6 +311,8 @@ PrintObject::invalidate_step(PrintObjectStep step) this->invalidate_step(posPrepareInfill); this->_print->invalidate_step(psSkirt); this->_print->invalidate_step(psBrim); + } else if (step == posDetectSurfaces) { + this->invalidate_step(posPrepareInfill); } else if (step == posPrepareInfill) { this->invalidate_step(posInfill); } else if (step == posInfill) { @@ -318,6 +320,7 @@ PrintObject::invalidate_step(PrintObjectStep step) this->_print->invalidate_step(psBrim); } else if (step == posSlice) { this->invalidate_step(posPerimeters); + this->invalidate_step(posDetectSurfaces); this->invalidate_step(posSupportMaterial); } else if (step == posSupportMaterial) { this->_print->invalidate_step(psSkirt); @@ -351,11 +354,20 @@ PrintObject::has_support_material() const void PrintObject::detect_surfaces_type() { + // prerequisites + // this->slice(); + + if (this->state.is_done(posDetectSurfaces)) return; + this->state.set_started(posDetectSurfaces); + parallelize( std::queue(std::deque(this->layers.begin(), this->layers.end())), // cast LayerPtrs to std::queue boost::bind(&Slic3r::Layer::detect_surfaces_type, _1), this->_print->config.threads.value ); + + this->typed_slices = true; + this->state.set_done(posDetectSurfaces); } void @@ -835,11 +847,15 @@ PrintObject::_make_perimeters() this->state.set_started(posPerimeters); // merge slices if they were split into types + // This is not currently taking place because since merge_slices + detect_surfaces_type + // are not truly idempotent we are invalidating posSlice here (see the Perl part of + // this method). if (this->typed_slices) { + // merge_slices() undoes detect_surfaces_type() FOREACH_LAYER(this, layer_it) (*layer_it)->merge_slices(); this->typed_slices = false; - this->state.invalidate(posPrepareInfill); + this->state.invalidate(posDetectSurfaces); } // compare each layer to the one below, and mark those slices needing diff --git a/xs/xsp/Print.xsp b/xs/xsp/Print.xsp index c35da378f..e9a4c1a6d 100644 --- a/xs/xsp/Print.xsp +++ b/xs/xsp/Print.xsp @@ -14,6 +14,7 @@ _constant() ALIAS: STEP_SLICE = posSlice STEP_PERIMETERS = posPerimeters + STEP_DETECT_SURFACES = posDetectSurfaces STEP_PREPARE_INFILL = posPrepareInfill STEP_INFILL = posInfill STEP_SUPPORTMATERIAL = posSupportMaterial @@ -123,7 +124,7 @@ _constant() void set_step_started(PrintObjectStep step) %code%{ THIS->state.set_started(step); %}; - void detect_surfaces_type(); + %name{_detect_surfaces_type} void detect_surfaces_type(); void process_external_surfaces(); void bridge_over_infill(); void _slice();