diff --git a/src/libslic3r/GCode.cpp b/src/libslic3r/GCode.cpp index dedd024f2b..4c02cae7b4 100644 --- a/src/libslic3r/GCode.cpp +++ b/src/libslic3r/GCode.cpp @@ -7310,12 +7310,12 @@ static std::unique_ptr calculate_layer_edge_grid(const Layer& la return out; } -std::string GCode::extrude_loop(const ExtrusionLoop& loop_ref, - const std::string& description, - double speed, - const ExtrusionEntitiesPtr& region_perimeters, - const Point* start_point, - const WipeInwardSupport* wipe_support) +std::string GCode::extrude_loop(const ExtrusionLoop& loop_ref, + const std::string& description, + double speed, + const std::vector& region_perimeters, + const Point* start_point, + const WipeInwardSupport* wipe_support) { // get a copy; don't modify the orientation of the original loop object otherwise // next copies (if any) would not detect the correct orientation @@ -7659,11 +7659,11 @@ std::string GCode::extrude_multi_path(const ExtrusionMultiPath& multipath, const return gcode; } -std::string GCode::extrude_entity(const ExtrusionEntity& entity, - const std::string& description, - double speed, - const ExtrusionEntitiesPtr& region_perimeters, - const WipeInwardSupport* wipe_support) +std::string GCode::extrude_entity(const ExtrusionEntity& entity, + const std::string& description, + double speed, + const std::vector& region_perimeters, + const WipeInwardSupport* wipe_support) { if (const ExtrusionPath* path = dynamic_cast(&entity)) return this->extrude_path(*path, description, speed); @@ -7754,23 +7754,32 @@ std::string GCode::extrude_perimeters(const Print &print, const std::vector &by_region, bool ironing) { - std::string gcode; - ExtrusionEntitiesPtr extrusions; - const char* extrusion_name = ironing ? "ironing" : "infill"; + std::string gcode; + std::vector extrusions; + std::vector> reversed; + const char* extrusion_name = ironing ? "ironing" : "infill"; for (const ObjectByExtruder::Island::Region ®ion : by_region) if (! region.infills.empty()) { extrusions.clear(); extrusions.reserve(region.infills.size()); - for (ExtrusionEntity *ee : region.infills) + for (const ExtrusionEntity *ee : region.infills) if ((ee->role() == erIroning) == ironing) extrusions.emplace_back(ee); if (! extrusions.empty()) { m_config.apply(print.get_print_region(®ion - &by_region.front()).config()); - chain_and_reorder_extrusion_entities(extrusions, m_last_pos.to_point()); + reversed.clear(); + chain_and_reorder_extrusion_entities(extrusions, m_last_pos.to_point(), reversed); + // The reversed copies are in chain order. + auto next_reversed = reversed.begin(); for (const ExtrusionEntity *fill : extrusions) { + ExtrusionEntity *own_copy = next_reversed != reversed.end() && next_reversed->get() == fill ? (next_reversed++)->get() : nullptr; auto *eec = dynamic_cast(fill); if (eec) { - for (ExtrusionEntity *ee : eec->chained_path_from(m_last_pos.to_point()).entities) + // A reversed copy is owned here and can be moved from. + ExtrusionEntityCollection chained = own_copy ? std::move(static_cast(*own_copy)) : ExtrusionEntityCollection(*eec); + if (!chained.no_sort) + chain_and_reorder_extrusion_entities(chained.entities, m_last_pos.to_point()); + for (ExtrusionEntity *ee : chained.entities) gcode += this->extrude_entity(*ee, extrusion_name); } else gcode += this->extrude_entity(*fill, extrusion_name); @@ -7808,9 +7817,9 @@ std::string GCode::extrude_support(const ExtrusionEntityCollection &support_fill std::string gcode; if (!support_fills.entities.empty()) { - ExtrusionEntitiesPtr extrusions; + std::vector extrusions; extrusions.reserve(support_fills.entities.size()); - for (ExtrusionEntity* ee : support_fills.entities) { + for (const ExtrusionEntity* ee : support_fills.entities) { const auto role = ee->role(); if ((role == support_extrusion_role) || (support_extrusion_role == erMixed && role != erIroning)) { extrusions.emplace_back(ee); @@ -7819,9 +7828,10 @@ std::string GCode::extrude_support(const ExtrusionEntityCollection &support_fill if (extrusions.empty()) return gcode; + std::vector> reversed; //ORCA: Respect no_sort to preserve support base outline->fill order. if (!support_fills.no_sort) - chain_and_reorder_extrusion_entities(extrusions, m_last_pos.to_point()); + chain_and_reorder_extrusion_entities(extrusions, m_last_pos.to_point(), reversed); for (const ExtrusionEntity *ee : extrusions) { ExtrusionRole role = ee->role(); @@ -10054,8 +10064,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 : reg.perimeters); - ExtrusionEntitiesPtr& target_eec = (iter ? by_region_per_copy_cache.back().infills : by_region_per_copy_cache.back().perimeters); + const std::vector& entities = (iter ? reg.infills : reg.perimeters); + std::vector& 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. @@ -10090,7 +10100,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: - ExtrusionEntitiesPtr* perimeters_or_infills; + std::vector* perimeters_or_infills; std::vector* perimeters_or_infills_overrides; switch (type) { @@ -10114,7 +10124,7 @@ void GCode::ObjectByExtruder::Island::Region::append(const Type type, const Extr for (auto* ee : eec->entities) perimeters_or_infills->emplace_back(ee); } else - perimeters_or_infills->emplace_back(const_cast(eec)); + perimeters_or_infills->emplace_back(eec); if (copies_extruder != nullptr) { // Don't reallocate overrides if not needed. diff --git a/src/libslic3r/GCode.hpp b/src/libslic3r/GCode.hpp index 6516a3f6fd..29a8ae5a34 100644 --- a/src/libslic3r/GCode.hpp +++ b/src/libslic3r/GCode.hpp @@ -450,19 +450,19 @@ private: double &y_acceleration_limit_res, double &accumulated_mass_res); // Orca: pass the complete collection of region perimeters to the extrude loop to check whether the wipe before external loop // should be executed - std::string extrude_entity(const ExtrusionEntity& entity, - const std::string& description = "", - double speed = -1., - const ExtrusionEntitiesPtr& region_perimeters = ExtrusionEntitiesPtr(), - const WipeInwardSupport* wipe_support = nullptr); + std::string extrude_entity(const ExtrusionEntity& entity, + const std::string& description = "", + double speed = -1., + const std::vector& region_perimeters = {}, + const WipeInwardSupport* wipe_support = nullptr); // Orca: pass the complete collection of region perimeters to the extrude loop to check whether the wipe before external loop // should be executed - std::string extrude_loop(const ExtrusionLoop& loop, - const std::string& description, - double speed = -1., - const ExtrusionEntitiesPtr& region_perimeters = ExtrusionEntitiesPtr(), - const Point* start_point = nullptr, - const WipeInwardSupport* wipe_support = nullptr); + std::string extrude_loop(const ExtrusionLoop& loop, + const std::string& description, + double speed = -1., + const std::vector& region_perimeters = {}, + const Point* start_point = nullptr, + const WipeInwardSupport* wipe_support = nullptr); std::string extrude_multi_path(const ExtrusionMultiPath& multipath, const std::string& description = "", double speed = -1.); std::string extrude_path(const ExtrusionPath& path, const std::string& description = "", double speed = -1.); @@ -493,10 +493,9 @@ private: { struct Region { // 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; + std::vector perimeters; // Non-owned references to LayerRegion::fills::entities - ExtrusionEntitiesPtr infills; + std::vector infills; std::vector infills_overrides; std::vector perimeters_overrides; diff --git a/src/libslic3r/ShortestPath.cpp b/src/libslic3r/ShortestPath.cpp index eb3fc66927..08f973e20b 100644 --- a/src/libslic3r/ShortestPath.cpp +++ b/src/libslic3r/ShortestPath.cpp @@ -1024,13 +1024,14 @@ std::vector> chain_segments_greedy2(SegmentEndPointFunc return chain_segments_greedy_constrained_reversals2_(end_point_func, could_reverse_func, num_segments, start_near); } -std::vector> chain_extrusion_entities(std::vector &entities, const Point *start_near) +template +static std::vector> chain_extrusion_entities_impl(const std::vector &entities, const Point *start_near) { auto segment_end_point = [&entities](size_t idx, bool first_point) -> Point { return first_point ? entities[idx]->first_point() : entities[idx]->last_point(); }; auto could_reverse = [&entities](size_t idx) { const ExtrusionEntity *ee = entities[idx]; return ee->is_loop() || ee->can_reverse(); }; std::vector> out = chain_segments_greedy_constrained_reversals(segment_end_point, could_reverse, entities.size(), start_near); for (std::pair &segment : out) { - ExtrusionEntity *ee = entities[segment.first]; + const ExtrusionEntity *ee = entities[segment.first]; if (ee->is_loop()) // Ignore reversals for loops, as the start point equals the end point. segment.second = false; @@ -1040,6 +1041,20 @@ std::vector> chain_extrusion_entities(std::vector> chain_extrusion_entities(std::vector &entities, const Point *start_near) +{ + return chain_extrusion_entities_impl(entities, start_near); +} + +// Orca: Reordering queries first_point() / last_point(); drop entities that cannot provide valid endpoints. +template +static void remove_entities_without_endpoints(std::vector &entities) +{ + entities.erase(std::remove_if(entities.begin(), entities.end(), + [](const ExtrusionEntity *entity) { return !extrusion_entity_has_endpoints(entity); }), + entities.end()); +} + void reorder_extrusion_entities(std::vector &entities, const std::vector> &chain) { assert(entities.size() == chain.size()); @@ -1061,14 +1076,27 @@ void chain_and_reorder_extrusion_entities(std::vector &entitie void chain_and_reorder_extrusion_entities(std::vector &entities, const Point *start_near) { - // Orca: Reordering queries first_point() / last_point(); drop entities that cannot provide valid endpoints. - entities.erase(std::remove_if(entities.begin(), entities.end(), [](ExtrusionEntity *entity) { - return !extrusion_entity_has_endpoints(entity); - }), - entities.end()); + remove_entities_without_endpoints(entities); reorder_extrusion_entities(entities, chain_extrusion_entities(entities, start_near)); } +void chain_and_reorder_extrusion_entities(std::vector &entities, const Point &start_near, + std::vector> &reversed_clones) +{ + remove_entities_without_endpoints(entities); + std::vector out; + out.reserve(entities.size()); + for (const auto &[idx, reverse] : chain_extrusion_entities_impl(entities, &start_near)) { + if (reverse) { + ExtrusionEntity *clone = reversed_clones.emplace_back(entities[idx]->clone()).get(); + clone->reverse(); + out.emplace_back(clone); + } else + out.emplace_back(entities[idx]); + } + entities.swap(out); +} + std::vector> chain_extrusion_paths(std::vector &extrusion_paths, const Point *start_near) { auto segment_end_point = [&extrusion_paths](size_t idx, bool first_point) -> Point { return first_point ? extrusion_paths[idx].first_point() : extrusion_paths[idx].last_point(); }; diff --git a/src/libslic3r/ShortestPath.hpp b/src/libslic3r/ShortestPath.hpp index 5de7d8ef4a..c7a61d99ee 100644 --- a/src/libslic3r/ShortestPath.hpp +++ b/src/libslic3r/ShortestPath.hpp @@ -5,6 +5,7 @@ #include "ExtrusionEntity.hpp" #include "Point.hpp" +#include #include #include @@ -24,6 +25,9 @@ std::vector> chain_extrusion_entities(std::vector &entities, const std::vector> &chain); void chain_and_reorder_extrusion_entities(std::vector &entities, const Point &start_near); void chain_and_reorder_extrusion_entities(std::vector &entities, const Point *start_near = nullptr); +// Each entity the chain reverses is replaced by a reversed clone that reversed_clones owns, so the originals stay unchanged. +void chain_and_reorder_extrusion_entities(std::vector &entities, const Point &start_near, + std::vector> &reversed_clones); std::vector> chain_extrusion_paths(std::vector &extrusion_paths, const Point *start_near = nullptr); void reorder_extrusion_paths(std::vector &extrusion_paths, std::vector> &chain); diff --git a/tests/fff_print/test_helpers.cpp b/tests/fff_print/test_helpers.cpp index 64493d5f74..89f9f9fdd1 100644 --- a/tests/fff_print/test_helpers.cpp +++ b/tests/fff_print/test_helpers.cpp @@ -225,7 +225,7 @@ DynamicPrintConfig multifilament_config(unsigned int filaments, std::initializer } void init_print(std::vector &&meshes, Slic3r::Print &print, Slic3r::Model &model, const DynamicPrintConfig &config_in, - const std::vector> *per_object_overrides, bool arrange) + const std::vector> *per_object_overrides, bool arrange, size_t instances) { DynamicPrintConfig config = DynamicPrintConfig::full_print_config(); config.apply(config_in); @@ -236,7 +236,8 @@ void init_print(std::vector &&meshes, Slic3r::Print &print, Slic3r ModelObject *object = model.add_object(); object->name += "object.stl"; object->add_volume(std::move(t)); - object->add_instance(); + for (size_t i = 0; i < instances; ++i) + object->add_instance(); if (per_object_overrides && object_idx < per_object_overrides->size() && !(*per_object_overrides)[object_idx].empty()) { DynamicPrintConfig oc; diff --git a/tests/fff_print/test_helpers.hpp b/tests/fff_print/test_helpers.hpp index cb3069ed02..cbdc90c3d9 100644 --- a/tests/fff_print/test_helpers.hpp +++ b/tests/fff_print/test_helpers.hpp @@ -72,9 +72,11 @@ Slic3r::Model model(const std::string& model_name, TriangleMesh&& _mesh); DynamicPrintConfig multifilament_config(unsigned int filaments, std::initializer_list extra = {}); -// Apply `meshes` and config to `print`/`model`; optional per-object overrides, auto-arranged unless `arrange` is false. +// Apply `meshes` and config to `print`/`model`, each object with `instances` copies; optional per-object overrides, +// auto-arranged unless `arrange` is false. void init_print(std::vector &&meshes, Slic3r::Print &print, Slic3r::Model &model, const DynamicPrintConfig &config_in, - const std::vector> *per_object_overrides = nullptr, bool arrange = true); + const std::vector> *per_object_overrides = nullptr, bool arrange = true, + size_t instances = 1); void init_print(std::initializer_list meshes, Slic3r::Print &print, Slic3r::Model &model, const Slic3r::DynamicPrintConfig &config_in = Slic3r::DynamicPrintConfig::full_print_config()); void init_print(std::initializer_list meshes, Slic3r::Print &print, Slic3r::Model &model, const Slic3r::DynamicPrintConfig &config_in = Slic3r::DynamicPrintConfig::full_print_config()); void init_print(std::initializer_list meshes, Slic3r::Print &print, Slic3r::Model &model, std::initializer_list config_items); diff --git a/tests/fff_print/test_print.cpp b/tests/fff_print/test_print.cpp index a28669b8d4..4176256021 100644 --- a/tests/fff_print/test_print.cpp +++ b/tests/fff_print/test_print.cpp @@ -561,6 +561,40 @@ TEST_CASE("export_gcode writes G-code without a result pointer", "[Print][export REQUIRE_FALSE(gcode.empty()); } +TEST_CASE("Exporting a sliced print again gives the same G-code", "[Print][export_gcode][Regression]") +{ + const int instances = GENERATE(1, 3); + CAPTURE(instances); + DynamicPrintConfig config = DynamicPrintConfig::full_print_config(); + TestMesh mesh = TestMesh::ipadstand; + SECTION("infill reversed by chaining") { config.set_deserialize_strict({{"sparse_infill_pattern", "gyroid"}}); } + SECTION("support reversed by chaining") { + mesh = TestMesh::overhang; + config.set_deserialize_strict({{"enable_support", true}, {"support_interface_pattern", "concentric"}}); + } + Print print; + Model model; + Slic3r::Test::init_print({Slic3r::Test::mesh(mesh)}, print, model, config, nullptr, true, instances); + + const auto export_without_timestamp = [&print]() { + std::string gcode = Slic3r::Test::gcode(print); + const size_t line = gcode.find("; generated by "); + REQUIRE(line != std::string::npos); + gcode.erase(line, gcode.find('\n', line) - line); + return gcode; + }; + const std::string first = export_without_timestamp(); + const std::string second = export_without_timestamp(); + + // Shows the first differing line on failure. + const size_t diff = std::mismatch(first.begin(), first.end(), second.begin(), second.end()).first - first.begin(); + const size_t line_start = diff == 0 ? 0 : first.rfind('\n', diff - 1) + 1; + INFO("first export: " << first.substr(line_start, first.find('\n', diff) - line_start)); + INFO("second export: " << second.substr(line_start, second.find('\n', diff) - line_start)); + CHECK(diff == first.size()); + CHECK(first.size() == second.size()); +} + TEST_CASE("Sequential printing follows model order", "[Print]") { // Two objects of different heights, taller one added first. Orca prints