From f654b980a90fcd16ba4e7c8b9d073a958ba44036 Mon Sep 17 00:00:00 2001 From: supermerill Date: Mon, 1 Jun 2020 19:13:33 +0200 Subject: [PATCH] fix #242 --- src/libslic3r/PerimeterGenerator.cpp | 46 +++++++++++++++-------- src/libslic3r/PerimeterGenerator.hpp | 2 +- tests/superslicerlibslic3r/test_print.cpp | 40 +++++++++++++++++++- 3 files changed, 70 insertions(+), 18 deletions(-) diff --git a/src/libslic3r/PerimeterGenerator.cpp b/src/libslic3r/PerimeterGenerator.cpp index 2b8287e76..f5d932f94 100644 --- a/src/libslic3r/PerimeterGenerator.cpp +++ b/src/libslic3r/PerimeterGenerator.cpp @@ -107,7 +107,6 @@ void PerimeterGenerator::process() Surfaces all_surfaces = this->slices->surfaces; //store surface for bridge infill to avoid unsupported perimeters (but the first one, this one is always good) - if (this->config->no_perimeter_unsupported_algo != npuaNone && this->lower_slices != NULL && !this->lower_slices->empty()) { @@ -683,8 +682,10 @@ void PerimeterGenerator::process() // TODO: add test for perimeter order if (this->config->external_perimeters_first || (this->layer->id() == 0 && this->object_config->brim_width.value > 0)) { - if (this->config->external_perimeters_nothole.value) { - if (this->config->external_perimeters_hole.value) { + if (this->config->external_perimeters_nothole.value || + (this->layer->id() == 0 && this->object_config->brim_width.value > 0)) { + if (this->config->external_perimeters_hole.value || + (this->layer->id() == 0 && this->object_config->brim_width.value > 0)) { entities.reverse(); } else { //reverse only not-hole perimeters @@ -926,31 +927,44 @@ ExtrusionEntityCollection PerimeterGenerator::_traverse_loops( coll.append(tw.entities); thin_walls.clear(); } - + // traverse children and build the final collection Point zero_point(0, 0); //result is [idx, needReverse] ? std::vector> chain = chain_extrusion_entities(coll.entities, &zero_point); - ExtrusionEntityCollection entities; - for (const std::pair &idx : chain) { + ExtrusionEntityCollection coll_out; + + //little check: if you have external holes with only one extrusion and internal things, please draw the internal first, just in case it can help print the hole better. + std::vector> better_chain; + for (const std::pair& idx : chain) { + if (!loops[idx.first].is_external() || (!loops[idx.first].is_contour && !loops[idx.first].children.empty())) + better_chain.push_back(idx); + } + for (const std::pair& idx : chain) { + if (loops[idx.first].is_external() && !(!loops[idx.first].is_contour && !loops[idx.first].children.empty())) + better_chain.push_back(idx); + } + + //move from coll to coll_out and gettign children of each in the same time. (deep first) + for (const std::pair &idx : better_chain) { if (idx.first >= loops.size()) { // this is a thin wall // let's get it from the sorted collection as it might have been reversed - entities.entities.reserve(entities.entities.size() + 1); - entities.entities.emplace_back(coll.entities[idx.first]); + coll_out.entities.reserve(coll_out.entities.size() + 1); + coll_out.entities.emplace_back(coll.entities[idx.first]); coll.entities[idx.first] = nullptr; if (idx.second) - entities.entities.back()->reverse(); + coll_out.entities.back()->reverse(); //if thin extrusion is a loop, make it ccw like a normal contour. - if (ExtrusionLoop* loop = dynamic_cast(entities.entities.back())) { + if (ExtrusionLoop* loop = dynamic_cast(coll_out.entities.back())) { loop->make_counter_clockwise(); } } else { const PerimeterGeneratorLoop &loop = loops[idx.first]; assert(thin_walls.empty()); ExtrusionEntityCollection children = this->_traverse_loops(loop.children, thin_walls); - entities.entities.reserve(entities.entities.size() + children.entities.size() + 1); + coll_out.entities.reserve(coll_out.entities.size() + children.entities.size() + 1); ExtrusionLoop *eloop = static_cast(coll.entities[idx.first]); coll.entities[idx.first] = nullptr; if (loop.is_contour) { @@ -959,16 +973,16 @@ ExtrusionEntityCollection PerimeterGenerator::_traverse_loops( eloop->make_clockwise(); else eloop->make_counter_clockwise(); - entities.append(std::move(children.entities)); - entities.append(*eloop); + coll_out.append(std::move(children.entities)); + coll_out.append(*eloop); } else { eloop->make_clockwise(); - entities.append(*eloop); - entities.append(std::move(children.entities)); + coll_out.append(*eloop); + coll_out.append(std::move(children.entities)); } } } - return entities; + return coll_out; } PerimeterIntersectionPoint diff --git a/src/libslic3r/PerimeterGenerator.hpp b/src/libslic3r/PerimeterGenerator.hpp index 8d66520b8..853f99176 100644 --- a/src/libslic3r/PerimeterGenerator.hpp +++ b/src/libslic3r/PerimeterGenerator.hpp @@ -43,7 +43,7 @@ public: polygon(polygon), is_contour(is_contour), depth(depth), is_steep_overhang(is_steep_overhang) {} // External perimeter. It may be CCW or CW oriented (outer contour or hole contour). bool is_external() const { return this->depth == 0; } - // An island, which may have holes, but it does not have another internal island. + // it's the last loop of the contour (not hol), so the first to be printed (if all goes well) bool is_internal_contour() const; }; diff --git a/tests/superslicerlibslic3r/test_print.cpp b/tests/superslicerlibslic3r/test_print.cpp index 6a9d85b33..d442ea640 100644 --- a/tests/superslicerlibslic3r/test_print.cpp +++ b/tests/superslicerlibslic3r/test_print.cpp @@ -1,5 +1,5 @@ -#define CATCH_CONFIG_DISABLE +//#define CATCH_CONFIG_DISABLE //#include #include @@ -176,3 +176,41 @@ SCENARIO("Print: Brim generation") { } } } + +SCENARIO("Print: perimeter generation") { + GIVEN("cube with hole, just enough space for two loops at a point") { + DynamicPrintConfig& config = Slic3r::DynamicPrintConfig::full_print_config(); + Slic3r::Model model{}; + config.set_key_value("first_layer_extrusion_width", new ConfigOptionFloatOrPercent(0.42, false)); + config.set_deserialize("nozzle_diameter", "0.4"); + config.set_deserialize("layer_height", "0.2"); + config.set_deserialize("first_layer_height", "0.2"); + config.set_key_value("only_one_perimeter_top", new ConfigOptionBool(false)); + Print print{}; + auto facets = std::vector{ + Vec3i32(1,4,3),Vec3i32(4,1,2),Vec3i32(16,12,14),Vec3i32(16,10,12),Vec3i32(10,4,6),Vec3i32(4,10,16),Vec3i32(8,14,12),Vec3i32(8,2,14), + Vec3i32(6,2,8),Vec3i32(2,6,4),Vec3i32(14,15,16),Vec3i32(15,14,13),Vec3i32(15,4,16),Vec3i32(4,15,3),Vec3i32(13,11,15),Vec3i32(13,7,11), + Vec3i32(7,1,5),Vec3i32(1,7,13),Vec3i32(9,15,11),Vec3i32(9,3,15),Vec3i32(5,3,9),Vec3i32(3,5,1),Vec3i32(1,14,2),Vec3i32(14,1,13), + Vec3i32(9,12,10),Vec3i32(12,9,11),Vec3i32(6,9,10),Vec3i32(9,6,5),Vec3i32(8,5,6),Vec3i32(5,8,7),Vec3i32(7,12,11),Vec3i32(12,7,8) + }; + for (Vec3i32& vec : facets) + vec -= Vec3i32(1, 1, 1); + TriangleMesh tm = TriangleMesh{ std::vector{Vec3d(-5,-5,-0.1),Vec3d(-5,-5,0.1),Vec3d(-5,5,-0.1),Vec3d(-5,5,0.1), + Vec3d(-1.328430,0,-0.1),Vec3d(-1.328430,0,0.1),Vec3d(1.5,-2.828430,-0.1),Vec3d(1.5,-2.828430,0.1), + Vec3d(1.5,2.828430,-0.1),Vec3d(1.5,2.828430,0.1),Vec3d(4.328430,0,-0.1),Vec3d(4.328430,0,0.1), + Vec3d(5,-5,-0.1),Vec3d(5,-5,0.1),Vec3d(5,5,-0.1),Vec3d(5,5,0.1)}, + facets }; + Slic3r::Test::init_print(print, {tm}, model, &config); + print.process(); + THEN("hole perimeter should not be printed first") { + ExtrusionEntity* loop = print.objects()[0]->layers()[0]->regions()[0]->perimeters.entities[0]; + REQUIRE(loop->is_collection()); + loop = ((ExtrusionEntityCollection*)loop)->entities.front(); + REQUIRE(loop->is_loop()); + // what we don't want in first is something that is (((ExtrusionLoop*)loop)->loop_role() & ExtrusionLoopRole::elrHole) != 0 && loop->role() == elrExternalPerimeter + REQUIRE( (((ExtrusionLoop*)loop)->loop_role() & ExtrusionLoopRole::elrHole) == 0); + + } + } +} +