Refactoring of ExtrusionEntity / ExtrusionEntityCollection:

Iterator loops replaced with C++11 loops.
Fixed clone() methods to return an ExtrusionEntity*.
PerimeterGenerator now uses move semantics on ExtrusionEntity a little
bit more.

src/libslic3r/ExtrusionEntity.cpp

@@ -12,44 +12,35 @@
 
 namespace Slic3r {
     
-void
+void ExtrusionPath::intersect_expolygons(const ExPolygonCollection &collection, ExtrusionEntityCollection* retval) const
-ExtrusionPath::intersect_expolygons(const ExPolygonCollection &collection, ExtrusionEntityCollection* retval) const
 {
     this->_inflate_collection(intersection_pl(this->polyline, collection), retval);
 }
 
-void
+void ExtrusionPath::subtract_expolygons(const ExPolygonCollection &collection, ExtrusionEntityCollection* retval) const
-ExtrusionPath::subtract_expolygons(const ExPolygonCollection &collection, ExtrusionEntityCollection* retval) const
 {
     this->_inflate_collection(diff_pl(this->polyline, collection), retval);
 }
 
-void
+void ExtrusionPath::clip_end(double distance)
-ExtrusionPath::clip_end(double distance)
 {
     this->polyline.clip_end(distance);
 }
 
-void
+void ExtrusionPath::simplify(double tolerance)
-ExtrusionPath::simplify(double tolerance)
 {
     this->polyline.simplify(tolerance);
 }
 
-double
+double ExtrusionPath::length() const
-ExtrusionPath::length() const
 {
     return this->polyline.length();
 }
 
-void
+void ExtrusionPath::_inflate_collection(const Polylines &polylines, ExtrusionEntityCollection* collection) const
-ExtrusionPath::_inflate_collection(const Polylines &polylines, ExtrusionEntityCollection* collection) const
 {
-    for (Polylines::const_iterator it = polylines.begin(); it != polylines.end(); ++it) {
+    for (const Polyline &polyline : polylines)
-        ExtrusionPath* path = this->clone();
+        collection->entities.emplace_back(new ExtrusionPath(polyline, *this));
-        path->polyline = *it;
-        collection->entities.push_back(path);
-    }
 }
 
 void ExtrusionPath::polygons_covered_by_width(Polygons &out, const float scaled_epsilon) const
@@ -67,36 +58,36 @@ void ExtrusionPath::polygons_covered_by_spacing(Polygons &out, const float scale
 
 void ExtrusionMultiPath::reverse()
 {
-    for (ExtrusionPaths::iterator path = this->paths.begin(); path != this->paths.end(); ++path)
+    for (ExtrusionPath &path : this->paths)
-        path->reverse();
+        path.reverse();
     std::reverse(this->paths.begin(), this->paths.end());
 }
 
 double ExtrusionMultiPath::length() const
 {
     double len = 0;
-    for (ExtrusionPaths::const_iterator path = this->paths.begin(); path != this->paths.end(); ++path)
+    for (const ExtrusionPath &path : this->paths)
-        len += path->polyline.length();
+        len += path.polyline.length();
     return len;
 }
 
 void ExtrusionMultiPath::polygons_covered_by_width(Polygons &out, const float scaled_epsilon) const
 {
-    for (ExtrusionPaths::const_iterator path = this->paths.begin(); path != this->paths.end(); ++path)
+    for (const ExtrusionPath &path : this->paths)
-        path->polygons_covered_by_width(out, scaled_epsilon);
+        path.polygons_covered_by_width(out, scaled_epsilon);
 }
 
 void ExtrusionMultiPath::polygons_covered_by_spacing(Polygons &out, const float scaled_epsilon) const
 {
-    for (ExtrusionPaths::const_iterator path = this->paths.begin(); path != this->paths.end(); ++path)
+    for (const ExtrusionPath &path : this->paths)
-        path->polygons_covered_by_spacing(out, scaled_epsilon);
+        path.polygons_covered_by_spacing(out, scaled_epsilon);
 }
 
 double ExtrusionMultiPath::min_mm3_per_mm() const
 {
     double min_mm3_per_mm = std::numeric_limits<double>::max();
-    for (ExtrusionPaths::const_iterator path = this->paths.begin(); path != this->paths.end(); ++path)
+    for (const ExtrusionPath &path : this->paths)
-        min_mm3_per_mm = std::min(min_mm3_per_mm, path->mm3_per_mm);
+        min_mm3_per_mm = std::min(min_mm3_per_mm, path.mm3_per_mm);
     return min_mm3_per_mm;
 }
 
@@ -121,52 +112,46 @@ Polyline ExtrusionMultiPath::as_polyline() const
     return out;
 }
 
-bool
+bool ExtrusionLoop::make_clockwise()
-ExtrusionLoop::make_clockwise()
 {
     bool was_ccw = this->polygon().is_counter_clockwise();
     if (was_ccw) this->reverse();
     return was_ccw;
 }
 
-bool
+bool ExtrusionLoop::make_counter_clockwise()
-ExtrusionLoop::make_counter_clockwise()
 {
     bool was_cw = this->polygon().is_clockwise();
     if (was_cw) this->reverse();
     return was_cw;
 }
 
-void
+void ExtrusionLoop::reverse()
-ExtrusionLoop::reverse()
 {
-    for (ExtrusionPaths::iterator path = this->paths.begin(); path != this->paths.end(); ++path)
+    for (ExtrusionPath &path : this->paths)
-        path->reverse();
+        path.reverse();
     std::reverse(this->paths.begin(), this->paths.end());
 }
 
-Polygon
+Polygon ExtrusionLoop::polygon() const
-ExtrusionLoop::polygon() const
 {
     Polygon polygon;
-    for (ExtrusionPaths::const_iterator path = this->paths.begin(); path != this->paths.end(); ++path) {
+    for (const ExtrusionPath &path : this->paths) {
         // for each polyline, append all points except the last one (because it coincides with the first one of the next polyline)
-        polygon.points.insert(polygon.points.end(), path->polyline.points.begin(), path->polyline.points.end()-1);
+        polygon.points.insert(polygon.points.end(), path.polyline.points.begin(), path.polyline.points.end()-1);
     }
     return polygon;
 }
 
-double
+double ExtrusionLoop::length() const
-ExtrusionLoop::length() const
 {
     double len = 0;
-    for (ExtrusionPaths::const_iterator path = this->paths.begin(); path != this->paths.end(); ++path)
+    for (const ExtrusionPath &path : this->paths)
-        len += path->polyline.length();
+        len += path.polyline.length();
     return len;
 }
 
-bool
+bool ExtrusionLoop::split_at_vertex(const Point &point)
-ExtrusionLoop::split_at_vertex(const Point &point)
 {
     for (ExtrusionPaths::iterator path = this->paths.begin(); path != this->paths.end(); ++path) {
         int idx = path->polyline.find_point(point);
@@ -220,18 +205,18 @@ void ExtrusionLoop::split_at(const Point &point, bool prefer_non_overhang)
         Point  p_non_overhang;
         size_t path_idx_non_overhang = 0;
         double min_non_overhang = std::numeric_limits<double>::max();
-        for (ExtrusionPaths::const_iterator path = this->paths.begin(); path != this->paths.end(); ++path) {
+        for (const ExtrusionPath &path : this->paths) {
-            Point p_tmp = point.projection_onto(path->polyline);
+            Point p_tmp = point.projection_onto(path.polyline);
             double dist = (p_tmp - point).cast<double>().norm();
             if (dist < min) {
                 p = p_tmp;
                 min = dist;
-                path_idx = path - this->paths.begin();
+                path_idx = &path - &this->paths.front();
             } 
-            if (prefer_non_overhang && ! is_bridge(path->role()) && dist < min_non_overhang) {
+            if (prefer_non_overhang && ! is_bridge(path.role()) && dist < min_non_overhang) {
                 p_non_overhang = p_tmp;
                 min_non_overhang = dist;
-                path_idx_non_overhang = path - this->paths.begin();
+                path_idx_non_overhang = &path - &this->paths.front();
             }
         }
         if (prefer_non_overhang && min_non_overhang != std::numeric_limits<double>::max()) {
@@ -267,8 +252,7 @@ void ExtrusionLoop::split_at(const Point &point, bool prefer_non_overhang)
     this->split_at_vertex(p);
 }
 
-void
+void ExtrusionLoop::clip_end(double distance, ExtrusionPaths* paths) const
-ExtrusionLoop::clip_end(double distance, ExtrusionPaths* paths) const
 {
     *paths = this->paths;
     
@@ -285,15 +269,14 @@ ExtrusionLoop::clip_end(double distance, ExtrusionPaths* paths) const
     }
 }
 
-bool
+bool ExtrusionLoop::has_overhang_point(const Point &point) const
-ExtrusionLoop::has_overhang_point(const Point &point) const
 {
-    for (ExtrusionPaths::const_iterator path = this->paths.begin(); path != this->paths.end(); ++path) {
+    for (const ExtrusionPath &path : this->paths) {
-        int pos = path->polyline.find_point(point);
+        int pos = path.polyline.find_point(point);
         if (pos != -1) {
             // point belongs to this path
             // we consider it overhang only if it's not an endpoint
-            return (is_bridge(path->role()) && pos > 0 && pos != (int)(path->polyline.points.size())-1);
+            return (is_bridge(path.role()) && pos > 0 && pos != (int)(path.polyline.points.size())-1);
         }
     }
     return false;
@@ -301,22 +284,21 @@ ExtrusionLoop::has_overhang_point(const Point &point) const
 
 void ExtrusionLoop::polygons_covered_by_width(Polygons &out, const float scaled_epsilon) const
 {
-    for (ExtrusionPaths::const_iterator path = this->paths.begin(); path != this->paths.end(); ++path)
+    for (const ExtrusionPath &path : this->paths)
-        path->polygons_covered_by_width(out, scaled_epsilon);
+        path.polygons_covered_by_width(out, scaled_epsilon);
 }
 
 void ExtrusionLoop::polygons_covered_by_spacing(Polygons &out, const float scaled_epsilon) const
 {
-    for (ExtrusionPaths::const_iterator path = this->paths.begin(); path != this->paths.end(); ++path)
+    for (const ExtrusionPath &path : this->paths)
-        path->polygons_covered_by_spacing(out, scaled_epsilon);
+        path.polygons_covered_by_spacing(out, scaled_epsilon);
 }
 
-double
+double ExtrusionLoop::min_mm3_per_mm() const
-ExtrusionLoop::min_mm3_per_mm() const
 {
     double min_mm3_per_mm = std::numeric_limits<double>::max();
-    for (ExtrusionPaths::const_iterator path = this->paths.begin(); path != this->paths.end(); ++path)
+    for (const ExtrusionPath &path : this->paths)
-        min_mm3_per_mm = std::min(min_mm3_per_mm, path->mm3_per_mm);
+        min_mm3_per_mm = std::min(min_mm3_per_mm, path.mm3_per_mm);
     return min_mm3_per_mm;
 }
 
@@ -344,15 +326,4 @@ std::string ExtrusionEntity::role_to_string(ExtrusionRole role)
     return "";
 }
 
-//std::string ExtrusionLoop::role_to_string(ExtrusionLoopRole role)
-//{
-//    switch (role) {
-//        case elrDefault                 : return "elrDefault";
-//        case elrContourInternalPerimeter: return "elrContourInternalPerimeter";
-//        case elrSkirt                   : return "elrSkirt";
-//        default                         : assert(false);
-//    }
-//};
-
-
 }

src/libslic3r/ExtrusionEntity.hpp

@@ -75,6 +75,8 @@ public:
     virtual bool is_loop() const { return false; }
     virtual bool can_reverse() const { return true; }
     virtual ExtrusionEntity* clone() const = 0;
+    // Create a new object, initialize it with this object using the move semantics.
+    virtual ExtrusionEntity* clone_move() = 0;
     virtual ~ExtrusionEntity() {}
     virtual void reverse() = 0;
     virtual Point first_point() const = 0;
@@ -123,13 +125,17 @@ public:
     ExtrusionPath(ExtrusionRole role) : mm3_per_mm(-1), width(-1), height(-1), feedrate(0.0f), extruder_id(0), cp_color_id(0), m_role(role) {}
     ExtrusionPath(ExtrusionRole role, double mm3_per_mm, float width, float height) : mm3_per_mm(mm3_per_mm), width(width), height(height), feedrate(0.0f), extruder_id(0), cp_color_id(0), m_role(role) {}
     ExtrusionPath(const ExtrusionPath &rhs) : polyline(rhs.polyline), mm3_per_mm(rhs.mm3_per_mm), width(rhs.width), height(rhs.height), feedrate(rhs.feedrate), extruder_id(rhs.extruder_id), cp_color_id(rhs.cp_color_id), m_role(rhs.m_role) {}
-    ExtrusionPath(ExtrusionPath &&rhs) : polyline(std::move(rhs.polyline)), mm3_per_mm(rhs.mm3_per_mm), width(rhs.width), height(rhs.height), feedrate(rhs.feedrate), extruder_id(rhs.extruder_id), cp_color_id(rhs.cp_color_id), m_role(rhs.m_role) {}
+	ExtrusionPath(const Polyline &polyline, const ExtrusionPath &rhs) : polyline(polyline), mm3_per_mm(rhs.mm3_per_mm), width(rhs.width), height(rhs.height), feedrate(rhs.feedrate), extruder_id(rhs.extruder_id), cp_color_id(rhs.cp_color_id), m_role(rhs.m_role) {}
+	ExtrusionPath(ExtrusionPath &&rhs) : polyline(std::move(rhs.polyline)), mm3_per_mm(rhs.mm3_per_mm), width(rhs.width), height(rhs.height), feedrate(rhs.feedrate), extruder_id(rhs.extruder_id), cp_color_id(rhs.cp_color_id), m_role(rhs.m_role) {}
+	ExtrusionPath(Polyline &&polyline, const ExtrusionPath &rhs) : polyline(std::move(polyline)), mm3_per_mm(rhs.mm3_per_mm), width(rhs.width), height(rhs.height), feedrate(rhs.feedrate), extruder_id(rhs.extruder_id), cp_color_id(rhs.cp_color_id), m_role(rhs.m_role) {}
 //    ExtrusionPath(ExtrusionRole role, const Flow &flow) : m_role(role), mm3_per_mm(flow.mm3_per_mm()), width(flow.width), height(flow.height), feedrate(0.0f), extruder_id(0) {};
 
     ExtrusionPath& operator=(const ExtrusionPath &rhs) { m_role = rhs.m_role; this->mm3_per_mm = rhs.mm3_per_mm; this->width = rhs.width; this->height = rhs.height; this->feedrate = rhs.feedrate; this->extruder_id = rhs.extruder_id; this->cp_color_id = rhs.cp_color_id; this->polyline = rhs.polyline; return *this; }
     ExtrusionPath& operator=(ExtrusionPath &&rhs) { m_role = rhs.m_role; this->mm3_per_mm = rhs.mm3_per_mm; this->width = rhs.width; this->height = rhs.height; this->feedrate = rhs.feedrate; this->extruder_id = rhs.extruder_id; this->cp_color_id = rhs.cp_color_id; this->polyline = std::move(rhs.polyline); return *this; }
 
-    ExtrusionPath* clone() const override { return new ExtrusionPath (*this); }
+	ExtrusionEntity* clone() const override { return new ExtrusionPath(*this); }
+    // Create a new object, initialize it with this object using the move semantics.
+	ExtrusionEntity* clone_move() override { return new ExtrusionPath(std::move(*this)); }
     void reverse() override { this->polyline.reverse(); }
     Point first_point() const override { return this->polyline.points.front(); }
     Point last_point() const override { return this->polyline.points.back(); }
@@ -188,7 +194,9 @@ public:
 
     bool is_loop() const override { return false; }
     bool can_reverse() const override { return true; }
-    ExtrusionMultiPath* clone() const override { return new ExtrusionMultiPath(*this); }
+	ExtrusionEntity* clone() const override { return new ExtrusionMultiPath(*this); }
+    // Create a new object, initialize it with this object using the move semantics.
+	ExtrusionEntity* clone_move() override { return new ExtrusionMultiPath(std::move(*this)); }
     void reverse() override;
     Point first_point() const override { return this->paths.front().polyline.points.front(); }
     Point last_point() const override { return this->paths.back().polyline.points.back(); }
@@ -227,7 +235,9 @@ public:
         { this->paths.emplace_back(std::move(path)); }
     bool is_loop() const override{ return true; }
     bool can_reverse() const override { return false; }
-    ExtrusionLoop* clone() const override{ return new ExtrusionLoop (*this); }
+	ExtrusionEntity* clone() const override{ return new ExtrusionLoop (*this); }
+    // Create a new object, initialize it with this object using the move semantics.
+	ExtrusionEntity* clone_move() override { return new ExtrusionLoop(std::move(*this)); }
     bool make_clockwise();
     bool make_counter_clockwise();
     void reverse() override;

src/libslic3r/ExtrusionEntityCollection.cpp

@@ -21,8 +21,7 @@ ExtrusionEntityCollection& ExtrusionEntityCollection::operator= (const Extrusion
     return *this;
 }
 
-void
+void ExtrusionEntityCollection::swap(ExtrusionEntityCollection &c)
-ExtrusionEntityCollection::swap(ExtrusionEntityCollection &c)
 {
     std::swap(this->entities, c.entities);
     std::swap(this->orig_indices, c.orig_indices);
@@ -39,15 +38,14 @@ void ExtrusionEntityCollection::clear()
 ExtrusionEntityCollection::operator ExtrusionPaths() const
 {
     ExtrusionPaths paths;
-    for (ExtrusionEntitiesPtr::const_iterator it = this->entities.begin(); it != this->entities.end(); ++it) {
+    for (const ExtrusionEntity *ptr : this->entities) {
-        if (const ExtrusionPath* path = dynamic_cast<const ExtrusionPath*>(*it))
+        if (const ExtrusionPath *path = dynamic_cast<const ExtrusionPath*>(ptr))
             paths.push_back(*path);
     }
     return paths;
 }
 
-ExtrusionEntityCollection*
+ExtrusionEntity* ExtrusionEntityCollection::clone() const
-ExtrusionEntityCollection::clone() const
 {
     ExtrusionEntityCollection* coll = new ExtrusionEntityCollection(*this);
     for (size_t i = 0; i < coll->entities.size(); ++i)
@@ -55,41 +53,36 @@ ExtrusionEntityCollection::clone() const
     return coll;
 }
 
-void
+void ExtrusionEntityCollection::reverse()
-ExtrusionEntityCollection::reverse()
 {
-    for (ExtrusionEntitiesPtr::iterator it = this->entities.begin(); it != this->entities.end(); ++it) {
+    for (ExtrusionEntity *ptr : this->entities)
         // Don't reverse it if it's a loop, as it doesn't change anything in terms of elements ordering
         // and caller might rely on winding order
-        if (!(*it)->is_loop()) (*it)->reverse();
+        if (! ptr->is_loop())
-    }
+        	ptr->reverse();
     std::reverse(this->entities.begin(), this->entities.end());
 }
 
-void
+void ExtrusionEntityCollection::replace(size_t i, const ExtrusionEntity &entity)
-ExtrusionEntityCollection::replace(size_t i, const ExtrusionEntity &entity)
 {
     delete this->entities[i];
     this->entities[i] = entity.clone();
 }
 
-void
+void ExtrusionEntityCollection::remove(size_t i)
-ExtrusionEntityCollection::remove(size_t i)
 {
     delete this->entities[i];
     this->entities.erase(this->entities.begin() + i);
 }
 
-ExtrusionEntityCollection
+ExtrusionEntityCollection ExtrusionEntityCollection::chained_path(bool no_reverse, ExtrusionRole role) const
-ExtrusionEntityCollection::chained_path(bool no_reverse, ExtrusionRole role) const
 {
     ExtrusionEntityCollection coll;
     this->chained_path(&coll, no_reverse, role);
     return coll;
 }
 
-void
+void ExtrusionEntityCollection::chained_path(ExtrusionEntityCollection* retval, bool no_reverse, ExtrusionRole role, std::vector<size_t>* orig_indices) const
-ExtrusionEntityCollection::chained_path(ExtrusionEntityCollection* retval, bool no_reverse, ExtrusionRole role, std::vector<size_t>* orig_indices) const
 {
     if (this->entities.empty()) return;
     this->chained_path_from(this->entities.front()->first_point(), retval, no_reverse, role, orig_indices);
@@ -108,6 +101,7 @@ void ExtrusionEntityCollection::chained_path_from(Point start_near, ExtrusionEnt
         *retval = *this;
         return;
     }
+
     retval->entities.reserve(this->entities.size());
     retval->orig_indices.reserve(this->entities.size());
     
@@ -115,10 +109,10 @@ void ExtrusionEntityCollection::chained_path_from(Point start_near, ExtrusionEnt
     std::map<ExtrusionEntity*,size_t> indices_map;
     
     ExtrusionEntitiesPtr my_paths;
-    for (ExtrusionEntitiesPtr::const_iterator it = this->entities.begin(); it != this->entities.end(); ++it) {
+    for (ExtrusionEntity * const &entity_src : this->entities) {
         if (role != erMixed) {
             // The caller wants only paths with a specific extrusion role.
-            auto role2 = (*it)->role();
+            auto role2 = entity_src->role();
             if (role != role2) {
                 // This extrusion entity does not match the role asked.
                 assert(role2 != erMixed);
@@ -126,32 +120,30 @@ void ExtrusionEntityCollection::chained_path_from(Point start_near, ExtrusionEnt
             }
         }
 
-        ExtrusionEntity* entity = (*it)->clone();
+        ExtrusionEntity *entity = entity_src->clone();
         my_paths.push_back(entity);
-        if (orig_indices != NULL) indices_map[entity] = it - this->entities.begin();
+        if (orig_indices != nullptr)
+        	indices_map[entity] = &entity_src - &this->entities.front();
     }
     
     Points endpoints;
-    for (ExtrusionEntitiesPtr::iterator it = my_paths.begin(); it != my_paths.end(); ++it) {
+    for (const ExtrusionEntity *entity : my_paths) {
-        endpoints.push_back((*it)->first_point());
+        endpoints.push_back(entity->first_point());
-        if (no_reverse || !(*it)->can_reverse()) {
+        endpoints.push_back((no_reverse || ! entity->can_reverse()) ?
-            endpoints.push_back((*it)->first_point());
+        	entity->first_point() : entity->last_point());
-        } else {
-            endpoints.push_back((*it)->last_point());
-        }
     }
     
-    while (!my_paths.empty()) {
+    while (! my_paths.empty()) {
         // find nearest point
         int start_index = start_near.nearest_point_index(endpoints);
         int path_index = start_index/2;
         ExtrusionEntity* entity = my_paths.at(path_index);
         // never reverse loops, since it's pointless for chained path and callers might depend on orientation
-        if (start_index % 2 && !no_reverse && entity->can_reverse()) {
+        if (start_index % 2 && !no_reverse && entity->can_reverse())
             entity->reverse();
-        }
         retval->entities.push_back(my_paths.at(path_index));
-        if (orig_indices != NULL) orig_indices->push_back(indices_map[entity]);
+        if (orig_indices != nullptr)
+        	orig_indices->push_back(indices_map[entity]);
         my_paths.erase(my_paths.begin() + path_index);
         endpoints.erase(endpoints.begin() + 2*path_index, endpoints.begin() + 2*path_index + 2);
         start_near = retval->entities.back()->last_point();
@@ -160,60 +152,50 @@ void ExtrusionEntityCollection::chained_path_from(Point start_near, ExtrusionEnt
 
 void ExtrusionEntityCollection::polygons_covered_by_width(Polygons &out, const float scaled_epsilon) const
 {
-    for (ExtrusionEntitiesPtr::const_iterator it = this->entities.begin(); it != this->entities.end(); ++it)
+    for (const ExtrusionEntity *entity : this->entities)
-        (*it)->polygons_covered_by_width(out, scaled_epsilon);
+        entity->polygons_covered_by_width(out, scaled_epsilon);
 }
 
 void ExtrusionEntityCollection::polygons_covered_by_spacing(Polygons &out, const float scaled_epsilon) const
 {
-    for (ExtrusionEntitiesPtr::const_iterator it = this->entities.begin(); it != this->entities.end(); ++it)
+    for (const ExtrusionEntity *entity : this->entities)
-        (*it)->polygons_covered_by_spacing(out, scaled_epsilon);
+        entity->polygons_covered_by_spacing(out, scaled_epsilon);
 }
 
-/* Recursively count paths and loops contained in this collection */
+// Recursively count paths and loops contained in this collection.
-size_t
+size_t ExtrusionEntityCollection::items_count() const
-ExtrusionEntityCollection::items_count() const
 {
     size_t count = 0;
-    for (ExtrusionEntitiesPtr::const_iterator it = this->entities.begin(); it != this->entities.end(); ++it) {
+    for (const ExtrusionEntity *entity : this->entities)
-        if ((*it)->is_collection()) {
+        if (entity->is_collection())
-            ExtrusionEntityCollection* collection = dynamic_cast<ExtrusionEntityCollection*>(*it);
+            count += static_cast<const ExtrusionEntityCollection*>(entity)->items_count();
-            count += collection->items_count();
+        else
-        } else {
+            ++ count;
-            ++count;
-        }
-    }
     return count;
 }
 
-/* Returns a single vector of pointers to all non-collection items contained in this one */
+// Returns a single vector of pointers to all non-collection items contained in this one.
-void
+void ExtrusionEntityCollection::flatten(ExtrusionEntityCollection* retval) const
-ExtrusionEntityCollection::flatten(ExtrusionEntityCollection* retval) const
 {
-    for (ExtrusionEntitiesPtr::const_iterator it = this->entities.begin(); it != this->entities.end(); ++it) {
+    for (const ExtrusionEntity *entity : this->entities)
-        if ((*it)->is_collection()) {
+        if (entity->is_collection())
-            ExtrusionEntityCollection* collection = dynamic_cast<ExtrusionEntityCollection*>(*it);
+            retval->append(static_cast<const ExtrusionEntityCollection*>(entity)->flatten().entities);
-            retval->append(collection->flatten().entities);
+        else
-        } else {
+            retval->append(*entity);
-            retval->append(**it);
-        }
-    }
 }
 
-ExtrusionEntityCollection
+ExtrusionEntityCollection ExtrusionEntityCollection::flatten() const
-ExtrusionEntityCollection::flatten() const
 {
     ExtrusionEntityCollection coll;
     this->flatten(&coll);
     return coll;
 }
 
-double
+double ExtrusionEntityCollection::min_mm3_per_mm() const
-ExtrusionEntityCollection::min_mm3_per_mm() const
 {
     double min_mm3_per_mm = std::numeric_limits<double>::max();
-    for (ExtrusionEntitiesPtr::const_iterator it = this->entities.begin(); it != this->entities.end(); ++it)
+    for (const ExtrusionEntity *entity : this->entities)
-        min_mm3_per_mm = std::min(min_mm3_per_mm, (*it)->min_mm3_per_mm());
+    	min_mm3_per_mm = std::min(min_mm3_per_mm, entity->min_mm3_per_mm());
     return min_mm3_per_mm;
 }
 

src/libslic3r/ExtrusionEntityCollection.hpp

@@ -9,7 +9,10 @@ namespace Slic3r {
 class ExtrusionEntityCollection : public ExtrusionEntity
 {
 public:
-    ExtrusionEntityCollection* clone() const;
+    ExtrusionEntity* clone() const override;
+    // Create a new object, initialize it with this object using the move semantics.
+	ExtrusionEntity* clone_move() override { return new ExtrusionEntityCollection(std::move(*this)); }
+
     ExtrusionEntitiesPtr entities;     // we own these entities
     std::vector<size_t> orig_indices;  // handy for XS
     bool no_sort;
@@ -36,11 +39,12 @@ public:
     bool empty() const { return this->entities.empty(); };
     void clear();
     void swap (ExtrusionEntityCollection &c);
-    void append(const ExtrusionEntity &entity) { this->entities.push_back(entity.clone()); }
+    void append(const ExtrusionEntity &entity) { this->entities.emplace_back(entity.clone()); }
-    void append(const ExtrusionEntitiesPtr &entities) { 
+    void append(ExtrusionEntity &&entity) { this->entities.emplace_back(entity.clone_move()); }
+    void append(const ExtrusionEntitiesPtr &entities) {
         this->entities.reserve(this->entities.size() + entities.size());
-        for (ExtrusionEntitiesPtr::const_iterator ptr = entities.begin(); ptr != entities.end(); ++ptr)
+        for (const ExtrusionEntity *ptr : entities)
-            this->entities.push_back((*ptr)->clone());
+            this->entities.emplace_back(ptr->clone());
     }
     void append(ExtrusionEntitiesPtr &&src) {
         if (entities.empty())

src/libslic3r/PerimeterGenerator.cpp

@@ -189,42 +189,36 @@ static ExtrusionEntityCollection traverse_loops(const PerimeterGenerator &perime
         coll.append(ExtrusionLoop(paths, loop_role));
     }
     
-    // append thin walls to the nearest-neighbor search (only for first iteration)
+    // Append thin walls to the nearest-neighbor search (only for first iteration)
-    if (!thin_walls.empty()) {
+    if (! thin_walls.empty()) {
         ExtrusionEntityCollection tw = variable_width(thin_walls, erExternalPerimeter, perimeter_generator.ext_perimeter_flow);
         coll.append(tw.entities);
         thin_walls.clear();
     }
     
-    // sort entities into a new collection using a nearest-neighbor search,
+    // Sort entities into a new collection using a nearest-neighbor search,
-    // preserving the original indices which are useful for detecting thin walls
+    // preserving the original indices which are useful for detecting thin walls.
     ExtrusionEntityCollection sorted_coll;
     coll.chained_path(&sorted_coll, false, erMixed, &sorted_coll.orig_indices);
     
     // traverse children and build the final collection
     ExtrusionEntityCollection entities;
-    for (std::vector<size_t>::const_iterator idx = sorted_coll.orig_indices.begin();
+    for (const size_t &idx : sorted_coll.orig_indices) {
-        idx != sorted_coll.orig_indices.end();
+        if (idx >= loops.size()) {
-        ++idx) {
+            // This is a thin wall. Let's get it from the sorted collection as it might have been reversed.
-        
+            entities.append(std::move(*sorted_coll.entities[&idx - &sorted_coll.orig_indices.front()]));
-        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[*idx];
+            const PerimeterGeneratorLoop &loop = loops[idx];
-            ExtrusionLoop eloop = *dynamic_cast<ExtrusionLoop*>(coll.entities[*idx]);
+            ExtrusionLoop eloop = *dynamic_cast<ExtrusionLoop*>(coll.entities[idx]);
-            
             ExtrusionEntityCollection children = traverse_loops(perimeter_generator, loop.children, thin_walls);
             if (loop.is_contour) {
                 eloop.make_counter_clockwise();
-                entities.append(children.entities);
+                entities.append(std::move(children.entities));
-                entities.append(eloop);
+                entities.append(std::move(eloop));
             } else {
                 eloop.make_clockwise();
-                entities.append(eloop);
+                entities.append(std::move(eloop));
-                entities.append(children.entities);
+                entities.append(std::move(children.entities));
             }
         }
     }