From de70adca9cd70f7243eeb5c8f186f88b481453a8 Mon Sep 17 00:00:00 2001 From: bubnikv Date: Thu, 9 Jan 2020 10:00:38 +0100 Subject: [PATCH] Optimization of G-code export: Don't make copies of ExtrusionEntities when sorting them into Extruders / Islands / Regions. --- src/libslic3r/ExtrusionEntityCollection.cpp | 32 ++++++++------------- src/libslic3r/ExtrusionEntityCollection.hpp | 4 ++- src/libslic3r/GCode.cpp | 27 ++++++++--------- src/libslic3r/GCode.hpp | 7 +++-- 4 files changed, 34 insertions(+), 36 deletions(-) diff --git a/src/libslic3r/ExtrusionEntityCollection.cpp b/src/libslic3r/ExtrusionEntityCollection.cpp index e1a9709d1..7e0e1fc9c 100644 --- a/src/libslic3r/ExtrusionEntityCollection.cpp +++ b/src/libslic3r/ExtrusionEntityCollection.cpp @@ -74,31 +74,23 @@ void ExtrusionEntityCollection::remove(size_t i) this->entities.erase(this->entities.begin() + i); } -ExtrusionEntityCollection ExtrusionEntityCollection::chained_path_from(const Point &start_near, ExtrusionRole role) const +ExtrusionEntityCollection ExtrusionEntityCollection::chained_path_from(const ExtrusionEntitiesPtr& extrusion_entities, const Point &start_near, ExtrusionRole role) { ExtrusionEntityCollection out; - if (this->no_sort) { - out = *this; - } else { - if (role == erMixed) - out = *this; - else { - for (const ExtrusionEntity *ee : this->entities) { - if (role != erMixed) { - // The caller wants only paths with a specific extrusion role. - auto role2 = ee->role(); - if (role != role2) { - // This extrusion entity does not match the role asked. - assert(role2 != erMixed); - continue; - } - } - out.entities.emplace_back(ee->clone()); + for (const ExtrusionEntity *ee : extrusion_entities) { + if (role != erMixed) { + // The caller wants only paths with a specific extrusion role. + auto role2 = ee->role(); + if (role != role2) { + // This extrusion entity does not match the role asked. + assert(role2 != erMixed); + continue; } } - chain_and_reorder_extrusion_entities(out.entities, &start_near); + out.entities.emplace_back(ee->clone()); } - return out; + chain_and_reorder_extrusion_entities(out.entities, &start_near); + return out; } void ExtrusionEntityCollection::polygons_covered_by_width(Polygons &out, const float scaled_epsilon) const diff --git a/src/libslic3r/ExtrusionEntityCollection.hpp b/src/libslic3r/ExtrusionEntityCollection.hpp index 3084e5741..e529bea02 100644 --- a/src/libslic3r/ExtrusionEntityCollection.hpp +++ b/src/libslic3r/ExtrusionEntityCollection.hpp @@ -65,7 +65,9 @@ public: } void replace(size_t i, const ExtrusionEntity &entity); void remove(size_t i); - ExtrusionEntityCollection chained_path_from(const Point &start_near, ExtrusionRole role = erMixed) const; + static ExtrusionEntityCollection chained_path_from(const ExtrusionEntitiesPtr &extrusion_entities, const Point &start_near, ExtrusionRole role = erMixed); + ExtrusionEntityCollection chained_path_from(const Point &start_near, ExtrusionRole role = erMixed) const + { return (this->no_sort || role == erMixed) ? *this : chained_path_from(this->entities, start_near, role); } void reverse(); const Point& first_point() const { return this->entities.front()->first_point(); } const Point& last_point() const { return this->entities.back()->last_point(); } diff --git a/src/libslic3r/GCode.cpp b/src/libslic3r/GCode.cpp index 3b2d2ca74..f6396e8f7 100644 --- a/src/libslic3r/GCode.cpp +++ b/src/libslic3r/GCode.cpp @@ -2797,7 +2797,7 @@ std::string GCode::extrude_perimeters(const Print &print, const std::vectorconfig()); - for (ExtrusionEntity *ee : region.perimeters.entities) + for (const ExtrusionEntity *ee : region.perimeters) gcode += this->extrude_entity(*ee, "perimeter", -1., &lower_layer_edge_grid); } return gcode; @@ -2809,7 +2809,9 @@ std::string GCode::extrude_infill(const Print &print, const std::vectorconfig()); - for (ExtrusionEntity *fill : region.infills.chained_path_from(m_last_pos).entities) { +// for (ExtrusionEntity *fill : ExtrusionEntityCollection::chained_path_from(region.infills, m_last_pos).entities) { + // Don't sort the infills, they contain gap fill, which shall be extruded after normal fills. + for (const ExtrusionEntity *fill : region.infills) { auto *eec = dynamic_cast(fill); if (eec) { for (ExtrusionEntity *ee : eec->chained_path_from(m_last_pos).entities) @@ -3300,8 +3302,8 @@ const std::vector& GCode::ObjectByExtru // Now we are going to iterate through perimeters and infills and pick ones that are supposed to be printed // References are used so that we don't have to repeat the same code for (int iter = 0; iter < 2; ++iter) { - const ExtrusionEntitiesPtr& entities = (iter ? reg.infills.entities : reg.perimeters.entities); - ExtrusionEntityCollection& target_eec = (iter ? by_region_per_copy_cache.back().infills : by_region_per_copy_cache.back().perimeters); + const ExtrusionEntitiesPtr& entities = (iter ? reg.infills : reg.perimeters); + ExtrusionEntitiesPtr& target_eec = (iter ? by_region_per_copy_cache.back().infills : by_region_per_copy_cache.back().perimeters); const std::vector& overrides = (iter ? reg.infills_overrides : reg.perimeters_overrides); // Now the most important thing - which extrusion should we print. @@ -3312,8 +3314,7 @@ const std::vector& GCode::ObjectByExtru const WipingExtrusions::ExtruderPerCopy *this_override = overrides[i]; // This copy (aka object instance) should be printed with this extruder, which overrides the default one. if (this_override != nullptr && (*this_override)[copy] == extruder) - // Clone the ExtrusionEntity. This is quite expensive. - target_eec.append((*entities[i])); + target_eec.emplace_back(entities[i]); } } else { // Apply normal extrusions (non-overrides) for this region. @@ -3322,12 +3323,10 @@ const std::vector& GCode::ObjectByExtru const WipingExtrusions::ExtruderPerCopy *this_override = overrides[i]; // This copy (aka object instance) should be printed with this extruder, which shall be equal to the default one. if (this_override == nullptr || (*this_override)[copy] == -extruder-1) - // Clone the ExtrusionEntity. This is quite expensive. - target_eec.append((*entities[i])); + target_eec.emplace_back(entities[i]); } for (; i < overrides.size(); ++ i) - // Clone the ExtrusionEntity. This is quite expensive. - target_eec.append(*entities[i]); + target_eec.emplace_back(entities[i]); } } } @@ -3339,7 +3338,7 @@ const std::vector& GCode::ObjectByExtru void GCode::ObjectByExtruder::Island::Region::append(const Type type, const ExtrusionEntityCollection* eec, const WipingExtrusions::ExtruderPerCopy* copies_extruder) { // We are going to manipulate either perimeters or infills, exactly in the same way. Let's create pointers to the proper structure to not repeat ourselves: - ExtrusionEntityCollection* perimeters_or_infills; + ExtrusionEntitiesPtr* perimeters_or_infills; std::vector* perimeters_or_infills_overrides; switch (type) { @@ -3356,8 +3355,10 @@ void GCode::ObjectByExtruder::Island::Region::append(const Type type, const Extr } // First we append the entities, there are eec->entities.size() of them: - size_t old_size = perimeters_or_infills->entities.size(); - perimeters_or_infills->append(eec->entities); + size_t old_size = perimeters_or_infills->size(); + perimeters_or_infills->reserve(perimeters_or_infills->size() + eec->entities.size()); + for (auto* ee : eec->entities) + perimeters_or_infills->emplace_back(ee); if (copies_extruder != nullptr) { perimeters_or_infills_overrides->reserve(old_size + eec->entities.size()); diff --git a/src/libslic3r/GCode.hpp b/src/libslic3r/GCode.hpp index 934dcaece..e8bc88a29 100644 --- a/src/libslic3r/GCode.hpp +++ b/src/libslic3r/GCode.hpp @@ -252,8 +252,11 @@ protected: struct Island { struct Region { - ExtrusionEntityCollection perimeters; - ExtrusionEntityCollection infills; + // Non-owned references to LayerRegion::perimeters::entities + // std::vector would be better here, but there is no way in C++ to convert from std::vector std::vector without copying. + ExtrusionEntitiesPtr perimeters; + // Non-owned references to LayerRegion::fills::entities + ExtrusionEntitiesPtr infills; std::vector infills_overrides; std::vector perimeters_overrides;