From dabb5f38232c8dd12d0e08a3ba4ae53c91e9390f Mon Sep 17 00:00:00 2001 From: Joseph Lenox Date: Sun, 6 Jan 2019 16:58:08 -0600 Subject: [PATCH] Fix skirt/Brim (#4669) * Start implementing skirt/brim tests. * Move skirt_height_z (representing the highest skirt height in actual Z coordinates) to Print and prime it with make_brim so it can be used in PrintGCode. Updated test to correctly verify skirt is generated for one and multiple objects. * Fallout from the config refactor; use correct accessors. * Fix bug where brim was not generated (inverted logic) * Finish porting tests, left one intentional failure as support material is not generated yet (segfaults). --- src/test/libslic3r/test_skirt_brim.cpp | 190 +++++++++++++++++++------ src/test/test_data.hpp | 2 + xs/src/libslic3r/Print.cpp | 12 +- xs/src/libslic3r/Print.hpp | 1 + xs/src/libslic3r/PrintGCode.cpp | 7 +- 5 files changed, 162 insertions(+), 50 deletions(-) diff --git a/src/test/libslic3r/test_skirt_brim.cpp b/src/test/libslic3r/test_skirt_brim.cpp index 6af9aeda0..c526cecd0 100644 --- a/src/test/libslic3r/test_skirt_brim.cpp +++ b/src/test/libslic3r/test_skirt_brim.cpp @@ -1,68 +1,118 @@ #include #include "test_data.hpp" // get access to init_print, etc #include "GCodeReader.hpp" +#include "Config.hpp" +#include "Geometry.hpp" +#include using namespace Slic3r::Test; using namespace Slic3r; -SCENARIO("Original Slic3r Skirt/Brim tests", "[!mayfail]") { - GIVEN("Configuration with a skirt height of 2") { - auto config {Config::new_from_defaults()}; - config->set("skirts", 1); - config->set("skirt_height", 2); - config->set("perimeters", 0); - config->set("support_material_speed", 99); +/// Helper method to find the tool used for the brim (always the first extrusion) +int get_brim_tool(std::stringstream& gcode, Slic3r::GCodeReader& parser) { + int brim_tool = -1; + std::regex tool_regex("^T(\\d+)"); + std::smatch m; + int tool = -1; - // avoid altering speeds unexpectedly - config->set("cooling", 0); - config->set("first_layer_speed", "100%"); - - WHEN("multiple objects are printed") { - auto gcode {std::stringstream("")}; - Slic3r::Model model; - auto print {Slic3r::Test::init_print({TestMesh::cube_20x20x20, TestMesh::cube_20x20x20}, model, config)}; - std::map layers_with_skirt; - Slic3r::Test::gcode(gcode, print); - auto parser {Slic3r::GCodeReader()}; - parser.parse_stream(gcode, [&layers_with_skirt, &config] (Slic3r::GCodeReader& self, const Slic3r::GCodeReader::GCodeLine& line) - { - if (self.Z > 0) { - if (line.extruding() && line.new_F() == config->getFloat("support_material_speed") * 60.0) { - layers_with_skirt[self.Z] = 1; - } + parser.parse_stream(gcode, [&tool, &brim_tool, &m, tool_regex] (Slic3r::GCodeReader& self, const Slic3r::GCodeReader::GCodeLine& line) + { + // if the command is a T command, set the the current tool + if (std::regex_match(line.cmd, m, tool_regex)) { + tool = std::stoi(m[1].str()); + } else if (line.cmd == "G1" && line.extruding() && line.dist_XY() > 0 && brim_tool < 0) { + brim_tool = tool; } - }); + }); - THEN("skirt_height is honored") { - REQUIRE(layers_with_skirt.size() == (size_t)config->getInt("skirt_height")); - } - } + return brim_tool; +} + + +TEST_CASE("Skirt height is honored") { + auto config {Config::new_from_defaults()}; + config->set("skirts", 1); + config->set("skirt_height", 5); + config->set("perimeters", 0); + config->set("support_material_speed", 99); + + // avoid altering speeds unexpectedly + config->set("cooling", false); + config->set("first_layer_speed", "100%"); + auto support_speed = config->get("support_material_speed") * MM_PER_MIN; + + std::map layers_with_skirt; + auto gcode {std::stringstream("")}; + gcode.clear(); + auto parser {Slic3r::GCodeReader()}; + Slic3r::Model model; + + SECTION("printing a single object") { + auto print {Slic3r::Test::init_print({TestMesh::cube_20x20x20}, model, config)}; + Slic3r::Test::gcode(gcode, print); } + + SECTION("printing multiple objects") { + auto print {Slic3r::Test::init_print({TestMesh::cube_20x20x20, TestMesh::cube_20x20x20}, model, config)}; + Slic3r::Test::gcode(gcode, print); + } + parser.parse_stream(gcode, [&layers_with_skirt, &support_speed] (Slic3r::GCodeReader& self, const Slic3r::GCodeReader::GCodeLine& line) + { + if (line.extruding() && self.F == Approx(support_speed)) { + layers_with_skirt[self.Z] = 1; + } + }); + + REQUIRE(layers_with_skirt.size() == (size_t)config->getInt("skirt_height")); +} + +SCENARIO("Original Slic3r Skirt/Brim tests", "[!mayfail]") { + auto parser {Slic3r::GCodeReader()}; + Slic3r::Model model; + auto gcode {std::stringstream("")}; + gcode.clear(); GIVEN("A default configuration") { auto config {Config::new_from_defaults()}; config->set("support_material_speed", 99); + config->set("first_layer_height", 0.3); + config->set("gcode_comments", true); // avoid altering speeds unexpectedly - config->set("cooling", 0); + config->set("cooling", false); config->set("first_layer_speed", "100%"); // remove noise from top/solid layers config->set("top_solid_layers", 0); - config->set("bottom_solid_layers", 0); + config->set("bottom_solid_layers", 1); WHEN("Brim width is set to 5") { config->set("perimeters", 0); config->set("skirts", 0); config->set("brim_width", 5); THEN("Brim is generated") { - REQUIRE(false); + auto print {Slic3r::Test::init_print({TestMesh::cube_20x20x20}, model, config)}; + Slic3r::Test::gcode(gcode, print); + bool brim_generated = false; + auto support_speed = config->get("support_material_speed") * MM_PER_MIN; + parser.parse_stream(gcode, [&brim_generated, support_speed] (Slic3r::GCodeReader& self, const Slic3r::GCodeReader::GCodeLine& line) + { + if (self.Z == Approx(0.3) || line.new_Z() == Approx(0.3)) { + if (line.extruding() && self.F == Approx(support_speed)) { + brim_generated = true; + } + } + }); + REQUIRE(brim_generated); } } WHEN("Skirt area is smaller than the brim") { config->set("skirts", 1); config->set("brim_width", 10); - THEN("GCode generates successfully.") { - REQUIRE(false); + auto print {Slic3r::Test::init_print({TestMesh::cube_20x20x20}, model, config)}; + THEN("Gcode generates") { + Slic3r::Test::gcode(gcode, print); + auto exported {gcode.str()}; + REQUIRE(exported.size() > 0); } } @@ -70,19 +120,37 @@ SCENARIO("Original Slic3r Skirt/Brim tests", "[!mayfail]") { config->set("skirts", 2); config->set("skirt_height", 0); - THEN("GCode generates successfully.") { - REQUIRE(false); + auto print {Slic3r::Test::init_print({TestMesh::cube_20x20x20}, model, config)}; + THEN("Gcode generates") { + Slic3r::Test::gcode(gcode, print); + auto exported {gcode.str()}; + REQUIRE(exported.size() > 0); } } WHEN("Perimeter extruder = 2 and support extruders = 3") { + config->set("skirts", 0); + config->set("brim_width", 5); + config->set("perimeter_extruder", 2); + config->set("support_material_extruder", 3); THEN("Brim is printed with the extruder used for the perimeters of first object") { - REQUIRE(false); + auto print {Slic3r::Test::init_print({TestMesh::cube_20x20x20}, model, config)}; + Slic3r::Test::gcode(gcode, print); + int tool = get_brim_tool(gcode, parser); + REQUIRE(tool == config->getInt("perimeter_extruder") - 1); } } WHEN("Perimeter extruder = 2, support extruders = 3, raft is enabled") { + config->set("skirts", 0); + config->set("brim_width", 5); + config->set("perimeter_extruder", 2); + config->set("support_material_extruder", 3); + config->set("raft_layers", 1); THEN("brim is printed with same extruder as skirt") { - REQUIRE(false); + auto print {Slic3r::Test::init_print({TestMesh::cube_20x20x20}, model, config)}; + Slic3r::Test::gcode(gcode, print); + int tool = get_brim_tool(gcode, parser); + REQUIRE(tool == config->getInt("support_material_extruder") - 1); } } @@ -94,17 +162,55 @@ SCENARIO("Original Slic3r Skirt/Brim tests", "[!mayfail]") { config->set("support_material_speed", 99); config->set("perimeter_extruder", 1); config->set("support_material_extruder", 2); - config->set("cooling", 0); // to prevent speeds to be altered + config->set("infill_extruder", 3); // ensure that a tool command gets emitted. + config->set("cooling", false); // to prevent speeds to be altered config->set("first_layer_speed", "100%"); // to prevent speeds to be altered Slic3r::Model model; auto print {Slic3r::Test::init_print({TestMesh::overhang}, model, config)}; print->process(); - - config->set("support_material", true); // to prevent speeds to be altered + + // config->set("support_material", true); // to prevent speeds to be altered THEN("skirt length is large enough to contain object with support") { - REQUIRE(false); + CHECK(config->getBool("support_material")); // test is not valid if support material is off + double skirt_length = 0.0; + Points extrusion_points; + int tool = -1; + std::regex tool_regex("^T(\\d+)"); + std::smatch m; + + auto print {Slic3r::Test::init_print({TestMesh::cube_20x20x20}, model, config)}; + Slic3r::Test::gcode(gcode, print); + + auto support_speed = config->get("support_material_speed") * MM_PER_MIN; + parser.parse_stream(gcode, [tool_regex, &m, config, &extrusion_points, &tool, &skirt_length, support_speed] (Slic3r::GCodeReader& self, const Slic3r::GCodeReader::GCodeLine& line) + { + std::cerr << line.cmd << "\n"; + if (std::regex_match(line.cmd, m, tool_regex)) { + tool = std::stoi(m[1].str()); + } else if (self.Z == Approx(config->get("first_layer_height").value)) { + // on first layer + if (line.extruding() && line.dist_XY() > 0) { + auto speed = ( self.F > 0 ? self.F : line.new_F()); + std::cerr << "Tool " << tool << "\n"; + if (speed == Approx(support_speed) && tool == config->getInt("perimeter_extruder") - 1) { + // Skirt uses first material extruder, support material speed. + skirt_length += line.dist_XY(); + } else { + extrusion_points.push_back(Slic3r::Point::new_scale(line.new_X(), line.new_Y())); + } + } + } + + if (self.Z == Approx(0.3) || line.new_Z() == Approx(0.3)) { + if (line.extruding() && self.F == Approx(support_speed)) { + } + } + }); + auto convex_hull = Slic3r::Geometry::convex_hull(extrusion_points); + auto hull_perimeter = unscale(convex_hull.split_at_first_point().length()); + REQUIRE(skirt_length > hull_perimeter); } } WHEN("Large minimum skirt length is used.") { diff --git a/src/test/test_data.hpp b/src/test/test_data.hpp index 3738f2495..8de0b7e5c 100644 --- a/src/test/test_data.hpp +++ b/src/test/test_data.hpp @@ -11,6 +11,8 @@ namespace Slic3r { namespace Test { +constexpr double MM_PER_MIN = 60.0; + /// Enumeration of test meshes enum class TestMesh { A, diff --git a/xs/src/libslic3r/Print.cpp b/xs/src/libslic3r/Print.cpp index d768cc5eb..239d4a32d 100644 --- a/xs/src/libslic3r/Print.cpp +++ b/xs/src/libslic3r/Print.cpp @@ -172,7 +172,7 @@ Print::make_skirt() // include the thickest object first. It is just guaranteed that a skirt is // prepended to the first 'n' layers (with 'n' = skirt_height). // $skirt_height_z in this case is the highest possible skirt height for safety. - double skirt_height_z {-1.0}; + this->skirt_height_z = -1.0; for (const auto* object : this->objects) { const size_t skirt_height { this->has_infinite_skirt() @@ -180,7 +180,7 @@ Print::make_skirt() : std::min(size_t(this->config.skirt_height()), object->layer_count()) }; const Layer* highest_layer { object->get_layer(skirt_height - 1) }; - skirt_height_z = std::max(skirt_height_z, highest_layer->print_z); + this->skirt_height_z = std::max(skirt_height_z, highest_layer->print_z); } // collect points from all layers contained in skirt height @@ -188,16 +188,16 @@ Print::make_skirt() for (auto* object : this->objects) { Points object_points; - // get object layers up to skirt_height_z + // get object layers up to this->skirt_height_z for (const auto* layer : object->layers) { - if (layer->print_z > skirt_height_z) break; + if (layer->print_z > this->skirt_height_z) break; for (const ExPolygon ex : layer->slices) append_to(object_points, static_cast(ex)); } - // get support layers up to skirt_height_z + // get support layers up to this->skirt_height_z for (const auto* layer : object->support_layers) { - if (layer->print_z > skirt_height_z) break; + if (layer->print_z > this->skirt_height_z) break; for (auto* ee : layer->support_fills) append_to(object_points, ee->as_polyline().points); for (auto* ee : layer->support_interface_fills) diff --git a/xs/src/libslic3r/Print.hpp b/xs/src/libslic3r/Print.hpp index 250ed6e96..494e1fe7e 100644 --- a/xs/src/libslic3r/Print.hpp +++ b/xs/src/libslic3r/Print.hpp @@ -243,6 +243,7 @@ class Print // ordered collections of extrusion paths to build skirt loops and brim ExtrusionEntityCollection skirt, brim; + double skirt_height_z {-1.0}; Print(); ~Print(); diff --git a/xs/src/libslic3r/PrintGCode.cpp b/xs/src/libslic3r/PrintGCode.cpp index 195c2274a..967204b21 100644 --- a/xs/src/libslic3r/PrintGCode.cpp +++ b/xs/src/libslic3r/PrintGCode.cpp @@ -491,8 +491,11 @@ PrintGCode::process_layer(size_t idx, const Layer* layer, const Points& copies) // extrude skirt along raft layers and normal obj layers // (not along interlaced support material layers) + if (layer->id() < static_cast(obj.config.raft_layers) - || ((_print.has_infinite_skirt() || _skirt_done.size() == 0 || (_skirt_done.rbegin())->first < _print.config.skirt_height) + || ((_print.has_infinite_skirt() + || _skirt_done.size() == 0 + || (_skirt_done.rbegin())->first < scale_(_print.skirt_height_z)) && _skirt_done.count(scale_(layer->print_z)) == 0 && typeid(layer) != typeid(SupportLayer*)) ) { @@ -547,7 +550,7 @@ PrintGCode::process_layer(size_t idx, const Layer* layer, const Points& copies) } // extrude brim - if (this->_brim_done) { + if (!this->_brim_done) { gcode += _gcodegen.set_extruder(_print.brim_extruder() - 1); _gcodegen.set_origin(Pointf(0,0)); _gcodegen.avoid_crossing_perimeters.use_external_mp = true;