From 68b7a9ec36d0a1eba6ac4a5be8165247a1d45003 Mon Sep 17 00:00:00 2001 From: Konstantinos Karmas Date: Wed, 28 Jul 2021 09:29:05 +0200 Subject: [PATCH 01/19] Unretract prior to the last travel move before an outer wall When moving to print an outer wall, the printer should not unretract while the nozzle is already on top of the outer wall. Unretracting there can cause dimensional accuracy issues and visual since the nozzle is sitting on top of the outer wall while unretracting and melting the material, which can leave a blip. Instead, the printer should unretract before the last travel move when going to print an outer wall. After the unretraction is complete, it can apply the last travel move and start printing the outer wall. CURA-8365 --- src/FffGcodeWriter.cpp | 4 ++-- src/InsetOrderOptimizer.cpp | 16 ++++++++-------- src/LayerPlan.cpp | 18 +++++++++++++----- src/LayerPlan.h | 6 +++--- src/LayerPlanBuffer.cpp | 3 ++- src/pathPlanning/GCodePath.cpp | 1 + src/pathPlanning/GCodePath.h | 1 + 7 files changed, 30 insertions(+), 19 deletions(-) diff --git a/src/FffGcodeWriter.cpp b/src/FffGcodeWriter.cpp index 30086cc783..13a16d5522 100644 --- a/src/FffGcodeWriter.cpp +++ b/src/FffGcodeWriter.cpp @@ -2064,12 +2064,12 @@ bool FffGcodeWriter::processInsets(const SliceDataStorage& storage, LayerPlan& g if (!compensate_overlap_0) { WallOverlapComputation* wall_overlap_computation(nullptr); - gcode_layer.addWalls(outer_wall, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlap_computation, z_seam_config, mesh.settings.get("wall_0_wipe_dist"), flow, retract_before_outer_wall); + gcode_layer.addWalls(outer_wall, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlap_computation, z_seam_config, mesh.settings.get("wall_0_wipe_dist"), flow, retract_before_outer_wall, /*unretract_before_last_travel_move = */true); } else { WallOverlapComputation wall_overlap_computation(outer_wall, mesh_config.inset0_config.getLineWidth()); - gcode_layer.addWalls(outer_wall, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, &wall_overlap_computation, z_seam_config, mesh.settings.get("wall_0_wipe_dist"), flow, retract_before_outer_wall); + gcode_layer.addWalls(outer_wall, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, &wall_overlap_computation, z_seam_config, mesh.settings.get("wall_0_wipe_dist"), flow, retract_before_outer_wall, /*unretract_before_last_travel_move = */true); } } } diff --git a/src/InsetOrderOptimizer.cpp b/src/InsetOrderOptimizer.cpp index 871711d2e7..ea36b9e1f7 100644 --- a/src/InsetOrderOptimizer.cpp +++ b/src/InsetOrderOptimizer.cpp @@ -283,7 +283,7 @@ void InsetOrderOptimizer::processHoleInsets() { if (extruder_nr == mesh.settings.get("wall_0_extruder_nr").extruder_nr) { - gcode_layer.addWalls(hole_outer_wall, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, z_seam_config, wall_0_wipe_dist, flow, retract_before_outer_wall); + gcode_layer.addWalls(hole_outer_wall, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, z_seam_config, wall_0_wipe_dist, flow, retract_before_outer_wall, /*unretract_before_last_travel_move = */true); } gcode_layer.addWalls(hole_inner_walls, mesh, mesh_config.insetX_config, mesh_config.bridge_insetX_config, wall_overlapper_x); } @@ -311,7 +311,7 @@ void InsetOrderOptimizer::processHoleInsets() gcode_layer.addWalls(hole_inner_walls, mesh, mesh_config.insetX_config, mesh_config.bridge_insetX_config, wall_overlapper_x); if (extruder_nr == mesh.settings.get("wall_0_extruder_nr").extruder_nr) { - gcode_layer.addWall(hole_outer_wall[0], outer_poly_start_idx, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, wall_0_wipe_dist, flow, retract_before_outer_wall); + gcode_layer.addWall(hole_outer_wall[0], outer_poly_start_idx, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, wall_0_wipe_dist, flow, retract_before_outer_wall, /*unretract_before_last_travel_move = */true); // move inside so an immediately following retract doesn't occur on the outer wall moveInside(); } @@ -330,11 +330,11 @@ void InsetOrderOptimizer::processHoleInsets() { // align z-seam of hole with z-seam of outer wall - makes a nicer job when printing tubes const unsigned point_idx = PolygonUtils::findNearestVert(start_point, hole_outer_wall.back()); - gcode_layer.addWall(hole_outer_wall.back(), point_idx, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, wall_0_wipe_dist, flow, retract_before_outer_wall); + gcode_layer.addWall(hole_outer_wall.back(), point_idx, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, wall_0_wipe_dist, flow, retract_before_outer_wall, /*unretract_before_last_travel_move = */true); } else { - gcode_layer.addWalls(hole_outer_wall, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, z_seam_config, wall_0_wipe_dist, flow, retract_before_outer_wall); + gcode_layer.addWalls(hole_outer_wall, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, z_seam_config, wall_0_wipe_dist, flow, retract_before_outer_wall, /*unretract_before_last_travel_move = */true); } // move inside so an immediately following retract doesn't occur on the outer wall moveInside(); @@ -428,7 +428,7 @@ void InsetOrderOptimizer::processOuterWallInsets(const bool include_outer, const constexpr bool always_retract = false; for (unsigned int wall_idx : orderOptimizer.polyOrder) { - gcode_layer.addWall(part_inner_walls[wall_idx], orderOptimizer.polyStart[wall_idx], mesh, mesh_config.insetX_config, mesh_config.bridge_insetX_config, wall_overlapper_x, wall_0_wipe_dist, flow_ratio, always_retract); + gcode_layer.addWall(part_inner_walls[wall_idx], orderOptimizer.polyStart[wall_idx], mesh, mesh_config.insetX_config, mesh_config.bridge_insetX_config, wall_overlapper_x, wall_0_wipe_dist, flow_ratio, always_retract, /*unretract_before_last_travel_move = */false); } }; @@ -436,7 +436,7 @@ void InsetOrderOptimizer::processOuterWallInsets(const bool include_outer, const { if (include_outer && extruder_nr == mesh.settings.get("wall_0_extruder_nr").extruder_nr) { - gcode_layer.addWall(*inset_polys[0][0], outer_poly_start_idx, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, wall_0_wipe_dist, flow, retract_before_outer_wall); + gcode_layer.addWall(*inset_polys[0][0], outer_poly_start_idx, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, wall_0_wipe_dist, flow, retract_before_outer_wall, /*unretract_before_last_travel_move = */false); } addInnerWalls(); } @@ -445,7 +445,7 @@ void InsetOrderOptimizer::processOuterWallInsets(const bool include_outer, const addInnerWalls(); if (include_outer && extruder_nr == mesh.settings.get("wall_0_extruder_nr").extruder_nr) { - gcode_layer.addWall(*inset_polys[0][0], outer_poly_start_idx, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, wall_0_wipe_dist, flow, retract_before_outer_wall); + gcode_layer.addWall(*inset_polys[0][0], outer_poly_start_idx, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, wall_0_wipe_dist, flow, retract_before_outer_wall, /*unretract_before_last_travel_move = */false); // move inside so an immediately following retract doesn't occur on the outer wall moveInside(); } @@ -460,7 +460,7 @@ void InsetOrderOptimizer::processOuterWallInsets(const bool include_outer, const gcode_layer.setIsInside(true); // going to print stuff inside print object Polygons part_outer_wall; part_outer_wall.add(*inset_polys[0][0]); - gcode_layer.addWalls(part_outer_wall, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, z_seam_config, wall_0_wipe_dist, flow, retract_before_outer_wall); + gcode_layer.addWalls(part_outer_wall, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, z_seam_config, wall_0_wipe_dist, flow, retract_before_outer_wall, /*unretract_before_last_travel_move = */true); // move inside so an immediately following retract doesn't occur on the outer wall moveInside(); added_something = true; diff --git a/src/LayerPlan.cpp b/src/LayerPlan.cpp index e1962f1536..d7d93d75e7 100644 --- a/src/LayerPlan.cpp +++ b/src/LayerPlan.cpp @@ -359,7 +359,7 @@ std::optional> LayerPlan::getFirstTravelDestinationState( return ret; } -GCodePath& LayerPlan::addTravel(const Point p, const bool force_retract) +GCodePath& LayerPlan::addTravel(const Point p, const bool force_retract, const bool unretract_before_last_travel_move) { const GCodePathConfig& travel_config = configs_storage.travel_config_per_extruder[getExtruder()]; const RetractionConfig& retraction_config = storage.retraction_config_per_extruder[getExtruder()]; @@ -492,6 +492,8 @@ GCodePath& LayerPlan::addTravel(const Point p, const bool force_retract) // must start new travel path as retraction can be enabled or not depending on path length, etc. forceNewPathStart(); + path->unretract_before_last_travel_move = unretract_before_last_travel_move; + GCodePath& ret = addTravel_simple(p, path); was_inside = is_inside; return ret; @@ -796,7 +798,7 @@ void LayerPlan::addWallLine(const Point& p0, const Point& p1, const SliceMeshSto } } -void LayerPlan::addWall(ConstPolygonRef wall, int start_idx, const SliceMeshStorage& mesh, const GCodePathConfig& non_bridge_config, const GCodePathConfig& bridge_config, WallOverlapComputation* wall_overlap_computation, coord_t wall_0_wipe_dist, float flow_ratio, bool always_retract) +void LayerPlan::addWall(ConstPolygonRef wall, int start_idx, const SliceMeshStorage& mesh, const GCodePathConfig& non_bridge_config, const GCodePathConfig& bridge_config, WallOverlapComputation* wall_overlap_computation, coord_t wall_0_wipe_dist, float flow_ratio, bool always_retract, bool unretract_before_last_travel_move) { // make sure wall start point is not above air! start_idx = locateFirstSupportedVertex(wall, start_idx); @@ -917,7 +919,7 @@ void LayerPlan::addWall(ConstPolygonRef wall, int start_idx, const SliceMeshStor { if (first_line || travel_required) { - addTravel(p0, (first_line) ? always_retract : wall_min_flow_retract); + addTravel(p0, (first_line) ? always_retract : wall_min_flow_retract, unretract_before_last_travel_move); first_line = false; travel_required = false; } @@ -997,7 +999,7 @@ void LayerPlan::addWall(ConstPolygonRef wall, int start_idx, const SliceMeshStor } } -void LayerPlan::addWalls(const Polygons& walls, const SliceMeshStorage& mesh, const GCodePathConfig& non_bridge_config, const GCodePathConfig& bridge_config, WallOverlapComputation* wall_overlap_computation, const ZSeamConfig& z_seam_config, coord_t wall_0_wipe_dist, float flow_ratio, bool always_retract) +void LayerPlan::addWalls(const Polygons& walls, const SliceMeshStorage& mesh, const GCodePathConfig& non_bridge_config, const GCodePathConfig& bridge_config, WallOverlapComputation* wall_overlap_computation, const ZSeamConfig& z_seam_config, coord_t wall_0_wipe_dist, float flow_ratio, bool always_retract, bool unretract_before_last_travel_move) { PathOrderOptimizer orderOptimizer(getLastPlannedPositionOrStartingPosition(), z_seam_config); for (unsigned int poly_idx = 0; poly_idx < walls.size(); poly_idx++) @@ -1007,7 +1009,7 @@ void LayerPlan::addWalls(const Polygons& walls, const SliceMeshStorage& mesh, co orderOptimizer.optimize(); for (unsigned int poly_idx : orderOptimizer.polyOrder) { - addWall(walls[poly_idx], orderOptimizer.polyStart[poly_idx], mesh, non_bridge_config, bridge_config, wall_overlap_computation, wall_0_wipe_dist, flow_ratio, always_retract); + addWall(walls[poly_idx], orderOptimizer.polyStart[poly_idx], mesh, non_bridge_config, bridge_config, wall_overlap_computation, wall_0_wipe_dist, flow_ratio, always_retract, unretract_before_last_travel_move); } } @@ -1648,6 +1650,12 @@ void LayerPlan::writeGCode(GCodeExport& gcode) { gcode.writeTravel(path.points[point_idx], speed); } + if (path.unretract_before_last_travel_move) + { + // We need to unretract before the last travel move of the path if the next path is an outer wall. + gcode.writeUnretractionAndPrime(); + } + gcode.writeTravel(path.points[path.points.size() - 1], speed); continue; } diff --git a/src/LayerPlan.h b/src/LayerPlan.h index 6794a2a17b..13a0143e13 100644 --- a/src/LayerPlan.h +++ b/src/LayerPlan.h @@ -477,7 +477,7 @@ class LayerPlan : public NoCopy * \param p The point to travel to. * \param force_comb_retract Whether to force a retraction to occur. */ - GCodePath& addTravel(const Point p, const bool force_retract = false); + GCodePath& addTravel(const Point p, const bool force_retract = false, const bool unretract_before_last_travel_move = false); /*! * Add a travel path to a certain point and retract if needed. @@ -578,7 +578,7 @@ class LayerPlan : public NoCopy * \param flow_ratio The ratio with which to multiply the extrusion amount * \param always_retract Whether to force a retraction when moving to the start of the wall (used for outer walls) */ - void addWall(ConstPolygonRef polygon, int start_idx, const SliceMeshStorage& mesh, const GCodePathConfig& non_bridge_config, const GCodePathConfig& bridge_config, WallOverlapComputation* wall_overlap_computation, coord_t wall_0_wipe_dist, float flow_ratio, bool always_retract); + void addWall(ConstPolygonRef polygon, int start_idx, const SliceMeshStorage& mesh, const GCodePathConfig& non_bridge_config, const GCodePathConfig& bridge_config, WallOverlapComputation* wall_overlap_computation, coord_t wall_0_wipe_dist, float flow_ratio, bool always_retract, bool unretract_before_last_travel_move); /*! * Add walls (polygons) to the gcode with optimized order. @@ -592,7 +592,7 @@ class LayerPlan : public NoCopy * \param flow_ratio The ratio with which to multiply the extrusion amount * \param always_retract Whether to force a retraction when moving to the start of a wall (used for outer walls) */ - void addWalls(const Polygons& walls, const SliceMeshStorage& mesh, const GCodePathConfig& non_bridge_config, const GCodePathConfig& bridge_config, WallOverlapComputation* wall_overlap_computation, const ZSeamConfig& z_seam_config = ZSeamConfig(), coord_t wall_0_wipe_dist = 0, float flow_ratio = 1.0, bool always_retract = false); + void addWalls(const Polygons& walls, const SliceMeshStorage& mesh, const GCodePathConfig& non_bridge_config, const GCodePathConfig& bridge_config, WallOverlapComputation* wall_overlap_computation, const ZSeamConfig& z_seam_config = ZSeamConfig(), coord_t wall_0_wipe_dist = 0, float flow_ratio = 1.0, bool always_retract = false, bool unretract_before_last_travel_move = false); /*! * Add lines to the gcode with optimized order. diff --git a/src/LayerPlanBuffer.cpp b/src/LayerPlanBuffer.cpp index 0a2db6c4f2..b6b59bdb83 100644 --- a/src/LayerPlanBuffer.cpp +++ b/src/LayerPlanBuffer.cpp @@ -98,8 +98,9 @@ void LayerPlanBuffer::addConnectingTravelMove(LayerPlan* prev_layer, const Layer prev_layer->setIsInside(new_layer_destination_state->second); const bool force_retract = extruder_settings.get("retract_at_layer_change") || (mesh_group_settings.get("travel_retract_before_outer_wall") && (mesh_group_settings.get("outer_inset_first") || mesh_group_settings.get("wall_line_count") == 1)); //Moving towards an outer wall. + const bool unretract_before_last_travel_move = force_retract; prev_layer->final_travel_z = newest_layer->z; - GCodePath &path = prev_layer->addTravel(first_location_new_layer, force_retract); + GCodePath &path = prev_layer->addTravel(first_location_new_layer, force_retract, unretract_before_last_travel_move); if (force_retract && !path.retract) { // addTravel() won't use retraction if the travel distance is less than retraction minimum travel setting diff --git a/src/pathPlanning/GCodePath.cpp b/src/pathPlanning/GCodePath.cpp index 8843f135b0..2292650869 100644 --- a/src/pathPlanning/GCodePath.cpp +++ b/src/pathPlanning/GCodePath.cpp @@ -13,6 +13,7 @@ space_fill_type(space_fill_type), flow(flow), speed_factor(speed_factor), retract(false), +unretract_before_last_travel_move(false), perform_z_hop(false), perform_prime(false), skip_agressive_merge_hint(false), diff --git a/src/pathPlanning/GCodePath.h b/src/pathPlanning/GCodePath.h index 8238ad941a..5c191d6115 100644 --- a/src/pathPlanning/GCodePath.h +++ b/src/pathPlanning/GCodePath.h @@ -34,6 +34,7 @@ class GCodePath Ratio flow; //!< A type-independent flow configuration (used for wall overlap compensation) Ratio speed_factor; //!< A speed factor that is multiplied with the travel speed. This factor can be used to change the travel speed. bool retract; //!< Whether the path is a move path preceded by a retraction move; whether the path is a retracted move path. + bool unretract_before_last_travel_move; //!< Whether the last move of the path should be preceded by an unretraction. Used to unretract in the last travel move before an outer wall bool perform_z_hop; //!< Whether to perform a z_hop in this path, which is assumed to be a travel path. bool perform_prime; //!< Whether this path is preceded by a prime (blob) bool skip_agressive_merge_hint; //!< Wheter this path needs to skip merging if any travel paths are in between the extrusions. From 406bfd42caeb6b0e9837c6dcec5d830ce145fed9 Mon Sep 17 00:00:00 2001 From: Konstantinos Karmas Date: Wed, 28 Jul 2021 12:59:12 +0200 Subject: [PATCH 02/19] Fix checking for the previous-to-last point in the travel path CURA-8365 --- src/LayerPlan.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/LayerPlan.cpp b/src/LayerPlan.cpp index d7d93d75e7..9e4955214b 100644 --- a/src/LayerPlan.cpp +++ b/src/LayerPlan.cpp @@ -1646,7 +1646,7 @@ void LayerPlan::writeGCode(GCodeExport& gcode) // Prevent the final travel(s) from resetting to the 'previous' layer height. gcode.setZ(final_travel_z); } - for(unsigned int point_idx = 0; point_idx < path.points.size(); point_idx++) + for(unsigned int point_idx = 0; point_idx < path.points.size() - 1; point_idx++) { gcode.writeTravel(path.points[point_idx], speed); } From 3fe2fdc7fd83f54400fddf2e3fa66d837bad715c Mon Sep 17 00:00:00 2001 From: Konstantinos Karmas Date: Wed, 28 Jul 2021 14:31:12 +0200 Subject: [PATCH 03/19] Unretract on layer change if necessary When the layer is changed and the next layer is about to start with an outer wall, then we need to ensure that an unretract will happen **before** the last travel move of the previous layer. This way, the unretract will not happen on top of the outer wall of the next layer, which could cause artifacts. CURA-8365 --- src/LayerPlanBuffer.cpp | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/LayerPlanBuffer.cpp b/src/LayerPlanBuffer.cpp index b6b59bdb83..a396195c9c 100644 --- a/src/LayerPlanBuffer.cpp +++ b/src/LayerPlanBuffer.cpp @@ -94,13 +94,13 @@ void LayerPlanBuffer::addConnectingTravelMove(LayerPlan* prev_layer, const Layer if (!prev_layer->last_planned_position || *prev_layer->last_planned_position != first_location_new_layer) { const Settings& mesh_group_settings = Application::getInstance().current_slice->scene.current_mesh_group->settings; - const Settings& extruder_settings = Application::getInstance().current_slice->scene.extruders[prev_layer->extruder_plans.back().extruder_nr].settings; + const Settings& prev_extruder_settings = Application::getInstance().current_slice->scene.extruders[prev_layer->extruder_plans.back().extruder_nr].settings; + const Settings& next_extruder_settings = Application::getInstance().current_slice->scene.extruders[newest_layer->extruder_plans.back().extruder_nr].settings; prev_layer->setIsInside(new_layer_destination_state->second); - const bool force_retract = extruder_settings.get("retract_at_layer_change") || - (mesh_group_settings.get("travel_retract_before_outer_wall") && (mesh_group_settings.get("outer_inset_first") || mesh_group_settings.get("wall_line_count") == 1)); //Moving towards an outer wall. - const bool unretract_before_last_travel_move = force_retract; + const bool force_retract = prev_extruder_settings.get("retract_at_layer_change") || + (mesh_group_settings.get("travel_retract_before_outer_wall") && (mesh_group_settings.get("outer_inset_first") || mesh_group_settings.get("wall_line_count") == 1)); //Moving towards an outer wall. prev_layer->final_travel_z = newest_layer->z; - GCodePath &path = prev_layer->addTravel(first_location_new_layer, force_retract, unretract_before_last_travel_move); + GCodePath &path = prev_layer->addTravel(first_location_new_layer, force_retract); if (force_retract && !path.retract) { // addTravel() won't use retraction if the travel distance is less than retraction minimum travel setting @@ -108,6 +108,16 @@ void LayerPlanBuffer::addConnectingTravelMove(LayerPlan* prev_layer, const Layer // we force the path to use retraction path.retract = true; } + if (path.retract && !path.unretract_before_last_travel_move && + next_extruder_settings.get("outer_inset_first") && + (next_extruder_settings.get("infill_line_distance") == 0 || !next_extruder_settings.get("infill_before_walls"))) + { + // If there is a retraction and the next layer's first path is an outer wall, then we need to make sure that + // we unretract before the last travel move of the previous layer. This way we make sure that the unretract + // does not happen while the nozzle is on top of the new layer's outer wall, which could cause artefacts and + // decrease dimensional accuracy. + path.unretract_before_last_travel_move = true; + } } } From fcc7cbf8701a1e05e7bee41cf5d40d7879f5b9dc Mon Sep 17 00:00:00 2001 From: Konstantinos Karmas Date: Thu, 29 Jul 2021 16:38:46 +0200 Subject: [PATCH 04/19] Add tests to check the unretraction before the last travel move CURA-8365 --- tests/LayerPlanTest.cpp | 39 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/tests/LayerPlanTest.cpp b/tests/LayerPlanTest.cpp index 8e8af0f833..09bd5e4ceb 100644 --- a/tests/LayerPlanTest.cpp +++ b/tests/LayerPlanTest.cpp @@ -398,6 +398,17 @@ class AddTravelTest : public LayerPlanTest, public testing::WithParamInterface Date: Thu, 29 Jul 2021 17:02:32 +0200 Subject: [PATCH 05/19] Request unretraction only if there was a prior retraction CURA-8365 --- src/LayerPlan.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/LayerPlan.cpp b/src/LayerPlan.cpp index 9e4955214b..f5e04ee0ac 100644 --- a/src/LayerPlan.cpp +++ b/src/LayerPlan.cpp @@ -492,7 +492,7 @@ GCodePath& LayerPlan::addTravel(const Point p, const bool force_retract, const b // must start new travel path as retraction can be enabled or not depending on path length, etc. forceNewPathStart(); - path->unretract_before_last_travel_move = unretract_before_last_travel_move; + path->unretract_before_last_travel_move = path->retract && unretract_before_last_travel_move; GCodePath& ret = addTravel_simple(p, path); was_inside = is_inside; From 21a05dafb0e739098619f139ac49c0dc508b8dae Mon Sep 17 00:00:00 2001 From: Konstantinos Karmas Date: Thu, 29 Jul 2021 17:03:08 +0200 Subject: [PATCH 06/19] Fix documentation As suggested in review comment https://github.com/Ultimaker/CuraEngine/pull/1474#discussion_r678376709. CURA-8365 --- src/LayerPlan.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/LayerPlan.h b/src/LayerPlan.h index 13a0143e13..0edcea090d 100644 --- a/src/LayerPlan.h +++ b/src/LayerPlan.h @@ -475,7 +475,8 @@ class LayerPlan : public NoCopy * no combing and no retraction. This travel move needs to be fixed * afterwards. * \param p The point to travel to. - * \param force_comb_retract Whether to force a retraction to occur. + * \param force_retract Whether to force a retraction to occur. + * \param unretract_before_last_travel_move Whether to unretract before the last travel move of the travel path. */ GCodePath& addTravel(const Point p, const bool force_retract = false, const bool unretract_before_last_travel_move = false); From dc23b02675da79f511edd236714df4e3a46b5126 Mon Sep 17 00:00:00 2001 From: Konstantinos Karmas Date: Thu, 29 Jul 2021 17:20:28 +0200 Subject: [PATCH 07/19] Enable retractions when testing for unretractions. The cases with no prior retractions are already covered by the parametrized test run and the `NoUnretractBeforeLastTravelMoveIfNoPriorRetraction` test case, so no need to test them again. CURA-8365 --- tests/LayerPlanTest.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/LayerPlanTest.cpp b/tests/LayerPlanTest.cpp index 09bd5e4ceb..a46a916c49 100644 --- a/tests/LayerPlanTest.cpp +++ b/tests/LayerPlanTest.cpp @@ -400,14 +400,21 @@ class AddTravelTest : public LayerPlanTest, public testing::WithParamInterfaceadd("retraction_enable", "true"); + settings->add("retraction_hop_enabled", "true"); + settings->add("retraction_combing_max_distance", "1"); + layer_plan.last_planned_position = Point(0, 0); + return layer_plan.addTravel(destination, true, unretract_before_last_travel_move); } }; From c8e0ba75c136393cf04d09cff95ed9e04fb4c26f Mon Sep 17 00:00:00 2001 From: Konstantinos Karmas Date: Tue, 3 Aug 2021 12:04:43 +0200 Subject: [PATCH 08/19] Make the unretract_before_last_travel move a constexpr Use a constexpr instead of the old style for readable variable names, as suggested by review https://github.com/Ultimaker/CuraEngine/pull/1474#pullrequestreview-720331200. CURA-8365 --- src/FffGcodeWriter.cpp | 7 +++++-- src/InsetOrderOptimizer.cpp | 26 +++++++++++++++++--------- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/FffGcodeWriter.cpp b/src/FffGcodeWriter.cpp index 13a16d5522..29f0007b88 100644 --- a/src/FffGcodeWriter.cpp +++ b/src/FffGcodeWriter.cpp @@ -2061,15 +2061,18 @@ bool FffGcodeWriter::processInsets(const SliceDataStorage& storage, LayerPlan& g gcode_layer.setIsInside(true); // going to print stuff inside print object ZSeamConfig z_seam_config(mesh.settings.get("z_seam_type"), mesh.getZSeamHint(), mesh.settings.get("z_seam_corner")); Polygons outer_wall = part.insets[0]; + + // When adding an outer wall we need to make sure that we unretract before the last travel move before that wall gets printed + constexpr bool unretract_before_last_travel_move = true; if (!compensate_overlap_0) { WallOverlapComputation* wall_overlap_computation(nullptr); - gcode_layer.addWalls(outer_wall, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlap_computation, z_seam_config, mesh.settings.get("wall_0_wipe_dist"), flow, retract_before_outer_wall, /*unretract_before_last_travel_move = */true); + gcode_layer.addWalls(outer_wall, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlap_computation, z_seam_config, mesh.settings.get("wall_0_wipe_dist"), flow, retract_before_outer_wall, unretract_before_last_travel_move); } else { WallOverlapComputation wall_overlap_computation(outer_wall, mesh_config.inset0_config.getLineWidth()); - gcode_layer.addWalls(outer_wall, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, &wall_overlap_computation, z_seam_config, mesh.settings.get("wall_0_wipe_dist"), flow, retract_before_outer_wall, /*unretract_before_last_travel_move = */true); + gcode_layer.addWalls(outer_wall, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, &wall_overlap_computation, z_seam_config, mesh.settings.get("wall_0_wipe_dist"), flow, retract_before_outer_wall, unretract_before_last_travel_move); } } } diff --git a/src/InsetOrderOptimizer.cpp b/src/InsetOrderOptimizer.cpp index ea36b9e1f7..af33e5877a 100644 --- a/src/InsetOrderOptimizer.cpp +++ b/src/InsetOrderOptimizer.cpp @@ -273,6 +273,10 @@ void InsetOrderOptimizer::processHoleInsets() } } + // When adding an outer wall we need to make sure that we unretract before the last travel move before that + // wall gets printed. This way we avoid blips caused by unretractions when the nozzle is sitting on the outer + // wall. + constexpr bool unretract_before_last_travel_move = true; if (hole_inner_walls.size() > 0 && extruder_nr == mesh.settings.get("wall_x_extruder_nr").extruder_nr) { // output the inset polys @@ -283,7 +287,7 @@ void InsetOrderOptimizer::processHoleInsets() { if (extruder_nr == mesh.settings.get("wall_0_extruder_nr").extruder_nr) { - gcode_layer.addWalls(hole_outer_wall, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, z_seam_config, wall_0_wipe_dist, flow, retract_before_outer_wall, /*unretract_before_last_travel_move = */true); + gcode_layer.addWalls(hole_outer_wall, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, z_seam_config, wall_0_wipe_dist, flow, retract_before_outer_wall, unretract_before_last_travel_move); } gcode_layer.addWalls(hole_inner_walls, mesh, mesh_config.insetX_config, mesh_config.bridge_insetX_config, wall_overlapper_x); } @@ -296,7 +300,7 @@ void InsetOrderOptimizer::processHoleInsets() const unsigned outer_poly_idx = order_optimizer.polyOrder[outer_poly_order_idx]; unsigned outer_poly_start_idx = gcode_layer.locateFirstSupportedVertex(hole_outer_wall[0], order_optimizer.polyStart[outer_poly_idx]); - // detect special case where where the z-seam is located on the sharpest corner and there is only 1 hole and + // detect special case where the z-seam is located on the sharpest corner and there is only 1 hole and // the gap between the walls is just a few line widths if (z_seam_config.type == EZSeamType::SHARPEST_CORNER && inset_polys[0].size() == 2 && PolygonUtils::polygonOutlinesAdjacent(*inset_polys[0][1], *inset_polys[0][0], max_gap * 4)) { @@ -311,7 +315,7 @@ void InsetOrderOptimizer::processHoleInsets() gcode_layer.addWalls(hole_inner_walls, mesh, mesh_config.insetX_config, mesh_config.bridge_insetX_config, wall_overlapper_x); if (extruder_nr == mesh.settings.get("wall_0_extruder_nr").extruder_nr) { - gcode_layer.addWall(hole_outer_wall[0], outer_poly_start_idx, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, wall_0_wipe_dist, flow, retract_before_outer_wall, /*unretract_before_last_travel_move = */true); + gcode_layer.addWall(hole_outer_wall[0], outer_poly_start_idx, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, wall_0_wipe_dist, flow, retract_before_outer_wall, unretract_before_last_travel_move); // move inside so an immediately following retract doesn't occur on the outer wall moveInside(); } @@ -330,11 +334,11 @@ void InsetOrderOptimizer::processHoleInsets() { // align z-seam of hole with z-seam of outer wall - makes a nicer job when printing tubes const unsigned point_idx = PolygonUtils::findNearestVert(start_point, hole_outer_wall.back()); - gcode_layer.addWall(hole_outer_wall.back(), point_idx, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, wall_0_wipe_dist, flow, retract_before_outer_wall, /*unretract_before_last_travel_move = */true); + gcode_layer.addWall(hole_outer_wall.back(), point_idx, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, wall_0_wipe_dist, flow, retract_before_outer_wall, unretract_before_last_travel_move); } else { - gcode_layer.addWalls(hole_outer_wall, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, z_seam_config, wall_0_wipe_dist, flow, retract_before_outer_wall, /*unretract_before_last_travel_move = */true); + gcode_layer.addWalls(hole_outer_wall, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, z_seam_config, wall_0_wipe_dist, flow, retract_before_outer_wall, unretract_before_last_travel_move); } // move inside so an immediately following retract doesn't occur on the outer wall moveInside(); @@ -410,6 +414,8 @@ void InsetOrderOptimizer::processOuterWallInsets(const bool include_outer, const order_optimizer.optimize(); const unsigned outer_poly_start_idx = gcode_layer.locateFirstSupportedVertex(*inset_polys[0][0], order_optimizer.polyStart[0]); const Point z_seam_location = (*inset_polys[0][0])[outer_poly_start_idx]; + // Since we are adding inner walls here, we don't have to unretract before the last travel move + constexpr bool unretract_before_last_travel_move = false; std::function addInnerWalls = [this, part_inner_walls, outer_inset_first, z_seam_location]() { @@ -428,7 +434,7 @@ void InsetOrderOptimizer::processOuterWallInsets(const bool include_outer, const constexpr bool always_retract = false; for (unsigned int wall_idx : orderOptimizer.polyOrder) { - gcode_layer.addWall(part_inner_walls[wall_idx], orderOptimizer.polyStart[wall_idx], mesh, mesh_config.insetX_config, mesh_config.bridge_insetX_config, wall_overlapper_x, wall_0_wipe_dist, flow_ratio, always_retract, /*unretract_before_last_travel_move = */false); + gcode_layer.addWall(part_inner_walls[wall_idx], orderOptimizer.polyStart[wall_idx], mesh, mesh_config.insetX_config, mesh_config.bridge_insetX_config, wall_overlapper_x, wall_0_wipe_dist, flow_ratio, always_retract, unretract_before_last_travel_move); } }; @@ -436,7 +442,7 @@ void InsetOrderOptimizer::processOuterWallInsets(const bool include_outer, const { if (include_outer && extruder_nr == mesh.settings.get("wall_0_extruder_nr").extruder_nr) { - gcode_layer.addWall(*inset_polys[0][0], outer_poly_start_idx, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, wall_0_wipe_dist, flow, retract_before_outer_wall, /*unretract_before_last_travel_move = */false); + gcode_layer.addWall(*inset_polys[0][0], outer_poly_start_idx, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, wall_0_wipe_dist, flow, retract_before_outer_wall, unretract_before_last_travel_move); } addInnerWalls(); } @@ -445,7 +451,7 @@ void InsetOrderOptimizer::processOuterWallInsets(const bool include_outer, const addInnerWalls(); if (include_outer && extruder_nr == mesh.settings.get("wall_0_extruder_nr").extruder_nr) { - gcode_layer.addWall(*inset_polys[0][0], outer_poly_start_idx, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, wall_0_wipe_dist, flow, retract_before_outer_wall, /*unretract_before_last_travel_move = */false); + gcode_layer.addWall(*inset_polys[0][0], outer_poly_start_idx, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, wall_0_wipe_dist, flow, retract_before_outer_wall, unretract_before_last_travel_move); // move inside so an immediately following retract doesn't occur on the outer wall moveInside(); } @@ -455,12 +461,14 @@ void InsetOrderOptimizer::processOuterWallInsets(const bool include_outer, const else if (include_outer && extruder_nr == mesh.settings.get("wall_0_extruder_nr").extruder_nr) { // just the outer wall, no inners + // When adding an outer wall we need to make sure that we unretract before the last travel move before that wall gets printed + constexpr bool unretract_before_last_travel_move = true; gcode_writer.setExtruder_addPrime(storage, gcode_layer, extruder_nr); gcode_layer.setIsInside(true); // going to print stuff inside print object Polygons part_outer_wall; part_outer_wall.add(*inset_polys[0][0]); - gcode_layer.addWalls(part_outer_wall, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, z_seam_config, wall_0_wipe_dist, flow, retract_before_outer_wall, /*unretract_before_last_travel_move = */true); + gcode_layer.addWalls(part_outer_wall, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, z_seam_config, wall_0_wipe_dist, flow, retract_before_outer_wall, unretract_before_last_travel_move); // move inside so an immediately following retract doesn't occur on the outer wall moveInside(); added_something = true; From d20bd013525033c7a9bb5954eedc36e3a51a9787 Mon Sep 17 00:00:00 2001 From: Konstantinos Karmas Date: Tue, 3 Aug 2021 12:05:03 +0200 Subject: [PATCH 09/19] Update documentation As suggested by review https://github.com/Ultimaker/CuraEngine/pull/1474#pullrequestreview-720331200. CURA-8365 --- src/LayerPlan.h | 9 ++++++++- tests/LayerPlanTest.cpp | 7 +++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/LayerPlan.h b/src/LayerPlan.h index 0edcea090d..5d87fd2eb5 100644 --- a/src/LayerPlan.h +++ b/src/LayerPlan.h @@ -476,7 +476,10 @@ class LayerPlan : public NoCopy * afterwards. * \param p The point to travel to. * \param force_retract Whether to force a retraction to occur. - * \param unretract_before_last_travel_move Whether to unretract before the last travel move of the travel path. + * \param unretract_before_last_travel_move Whether to unretract before the last travel move of the travel path, + * which comes before the wall to be printed. This should be true when we are adding outer walls to make sure + * that the unretraction will happen before the last travel move BEFORE going to that wall. This way, the nozzle + * doesn't sit still on top of the outer wall's path while it is unretracting, avoiding possible blips. */ GCodePath& addTravel(const Point p, const bool force_retract = false, const bool unretract_before_last_travel_move = false); @@ -578,6 +581,10 @@ class LayerPlan : public NoCopy * \param wall_0_wipe_dist The distance to travel along the wall after it has been laid down, in order to wipe the start and end of the wall together * \param flow_ratio The ratio with which to multiply the extrusion amount * \param always_retract Whether to force a retraction when moving to the start of the wall (used for outer walls) + * \param unretract_before_last_travel_move Whether to unretract before the last travel move of the travel path, + * which comes before the wall to be printed. This should be true when we are adding outer walls to make sure + * that the unretraction will happen before the last travel move BEFORE going to that wall. This way, the nozzle + * doesn't sit still on top of the outer wall's path while it is unretracting, avoiding possible blips. */ void addWall(ConstPolygonRef polygon, int start_idx, const SliceMeshStorage& mesh, const GCodePathConfig& non_bridge_config, const GCodePathConfig& bridge_config, WallOverlapComputation* wall_overlap_computation, coord_t wall_0_wipe_dist, float flow_ratio, bool always_retract, bool unretract_before_last_travel_move); diff --git a/tests/LayerPlanTest.cpp b/tests/LayerPlanTest.cpp index a46a916c49..cd69e26d60 100644 --- a/tests/LayerPlanTest.cpp +++ b/tests/LayerPlanTest.cpp @@ -401,12 +401,15 @@ class AddTravelTest : public LayerPlanTest, public testing::WithParamInterface Date: Tue, 3 Aug 2021 12:05:43 +0200 Subject: [PATCH 10/19] Add line breaks in boolean expressions To make it more readable, as suggested by review https://github.com/Ultimaker/CuraEngine/pull/1474#pullrequestreview-720331200. CURA-8365 --- src/LayerPlanBuffer.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/LayerPlanBuffer.cpp b/src/LayerPlanBuffer.cpp index a396195c9c..fa78b05c06 100644 --- a/src/LayerPlanBuffer.cpp +++ b/src/LayerPlanBuffer.cpp @@ -98,7 +98,9 @@ void LayerPlanBuffer::addConnectingTravelMove(LayerPlan* prev_layer, const Layer const Settings& next_extruder_settings = Application::getInstance().current_slice->scene.extruders[newest_layer->extruder_plans.back().extruder_nr].settings; prev_layer->setIsInside(new_layer_destination_state->second); const bool force_retract = prev_extruder_settings.get("retract_at_layer_change") || - (mesh_group_settings.get("travel_retract_before_outer_wall") && (mesh_group_settings.get("outer_inset_first") || mesh_group_settings.get("wall_line_count") == 1)); //Moving towards an outer wall. + (mesh_group_settings.get("travel_retract_before_outer_wall") && + (mesh_group_settings.get("outer_inset_first") || + mesh_group_settings.get("wall_line_count") == 1)); //Moving towards an outer wall. prev_layer->final_travel_z = newest_layer->z; GCodePath &path = prev_layer->addTravel(first_location_new_layer, force_retract); if (force_retract && !path.retract) @@ -110,7 +112,8 @@ void LayerPlanBuffer::addConnectingTravelMove(LayerPlan* prev_layer, const Layer } if (path.retract && !path.unretract_before_last_travel_move && next_extruder_settings.get("outer_inset_first") && - (next_extruder_settings.get("infill_line_distance") == 0 || !next_extruder_settings.get("infill_before_walls"))) + (next_extruder_settings.get("infill_line_distance") == 0 || + !next_extruder_settings.get("infill_before_walls"))) { // If there is a retraction and the next layer's first path is an outer wall, then we need to make sure that // we unretract before the last travel move of the previous layer. This way we make sure that the unretract From 804b820bd82d69d28170079b923666e57a3a50e0 Mon Sep 17 00:00:00 2001 From: Konstantinos Karmas Date: Tue, 3 Aug 2021 12:06:11 +0200 Subject: [PATCH 11/19] Use vector::back instead of the old-school size()-1 As suggested by review https://github.com/Ultimaker/CuraEngine/pull/1474#pullrequestreview-720331200. CURA-8365 --- src/LayerPlan.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/LayerPlan.cpp b/src/LayerPlan.cpp index f5e04ee0ac..f82bbc47dd 100644 --- a/src/LayerPlan.cpp +++ b/src/LayerPlan.cpp @@ -1655,7 +1655,7 @@ void LayerPlan::writeGCode(GCodeExport& gcode) // We need to unretract before the last travel move of the path if the next path is an outer wall. gcode.writeUnretractionAndPrime(); } - gcode.writeTravel(path.points[path.points.size() - 1], speed); + gcode.writeTravel(path.points.back(), speed); continue; } From 7d669a6cc1f55928edad7858541111434912f0a5 Mon Sep 17 00:00:00 2001 From: Konstantinos Karmas Date: Tue, 3 Aug 2021 12:09:31 +0200 Subject: [PATCH 12/19] Remove unused variable CURA-8365 --- src/FffGcodeWriter.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/FffGcodeWriter.cpp b/src/FffGcodeWriter.cpp index 29f0007b88..75e28a4e6a 100644 --- a/src/FffGcodeWriter.cpp +++ b/src/FffGcodeWriter.cpp @@ -1828,7 +1828,6 @@ bool FffGcodeWriter::partitionInfillBySkinAbove(Polygons& infill_below_skin, Pol } // need to take skin/infill overlap that was added in SkinInfillAreaComputation::generateInfill() into account - const coord_t infill_skin_overlap = mesh.settings.get((part.insets.size() > 1) ? "wall_line_width_x" : "wall_line_width_0") / 2; const Polygons infill_below_skin_overlap = infill_below_skin.offset(-tiny_infill_offset); return ! infill_below_skin_overlap.empty() && ! infill_not_below_skin.empty(); From 305ce372f87e9d02cb7fb56497e7774cabb720dc Mon Sep 17 00:00:00 2001 From: Konstantinos Karmas Date: Wed, 4 Aug 2021 16:12:07 +0200 Subject: [PATCH 13/19] Get the first of the extruders in the next layer CURA-8365 Co-authored-by: Ghostkeeper --- src/LayerPlanBuffer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/LayerPlanBuffer.cpp b/src/LayerPlanBuffer.cpp index fa78b05c06..b771a21adf 100644 --- a/src/LayerPlanBuffer.cpp +++ b/src/LayerPlanBuffer.cpp @@ -95,7 +95,7 @@ void LayerPlanBuffer::addConnectingTravelMove(LayerPlan* prev_layer, const Layer { const Settings& mesh_group_settings = Application::getInstance().current_slice->scene.current_mesh_group->settings; const Settings& prev_extruder_settings = Application::getInstance().current_slice->scene.extruders[prev_layer->extruder_plans.back().extruder_nr].settings; - const Settings& next_extruder_settings = Application::getInstance().current_slice->scene.extruders[newest_layer->extruder_plans.back().extruder_nr].settings; + const Settings& next_extruder_settings = Application::getInstance().current_slice->scene.extruders[newest_layer->extruder_plans.front().extruder_nr].settings; prev_layer->setIsInside(new_layer_destination_state->second); const bool force_retract = prev_extruder_settings.get("retract_at_layer_change") || (mesh_group_settings.get("travel_retract_before_outer_wall") && From a509447a2046bb37c8ec3ba9b8f3cd77b115ebd9 Mon Sep 17 00:00:00 2001 From: Jelle Spijker Date: Sat, 7 Aug 2021 10:58:16 +0200 Subject: [PATCH 14/19] Template type deduced operator+ and - added The old operator+ only allowed for int additions. the `Wpedantic` warning flag throws an error over this. Added a template type deduced operator+/- that performs a `static_cast` to a `double`. This should enable the use of all arithmetic types with AngleDegrees for the + and - operator. --- src/settings/types/Angle.h | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/settings/types/Angle.h b/src/settings/types/Angle.h index 2bb7a2fccf..8170a5e90b 100644 --- a/src/settings/types/Angle.h +++ b/src/settings/types/Angle.h @@ -53,9 +53,10 @@ class AngleDegrees { return std::fmod(std::fmod(value + other.value, 360) + 360, 360); } - AngleDegrees operator +(const int& other) const + template + AngleDegrees operator +(const T& other) const { - return operator+(AngleDegrees(other)); + return operator+(AngleDegrees(static_cast(other))); } AngleDegrees& operator +=(const AngleDegrees& other) { @@ -66,6 +67,11 @@ class AngleDegrees { return std::fmod(std::fmod(value - other.value, 360) + 360, 360); } + template + AngleDegrees operator -(const T& other) const + { + return operator-(AngleDegrees(static_cast(other))); + } AngleDegrees& operator -=(const AngleDegrees& other) { value = std::fmod(std::fmod(value - other.value, 360) + 360, 360); From 3c8ba55639a1b93d457054bcd128873b396f7cbc Mon Sep 17 00:00:00 2001 From: Remco Burema Date: Tue, 10 Aug 2021 14:48:14 +0200 Subject: [PATCH 15/19] Consistency: Both Angle classes are class now. --- src/settings/types/Angle.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/settings/types/Angle.h b/src/settings/types/Angle.h index 8170a5e90b..ba79710ed4 100644 --- a/src/settings/types/Angle.h +++ b/src/settings/types/Angle.h @@ -92,8 +92,9 @@ class AngleDegrees * This is a facade. It behaves like a double, but this is using clock * arithmetic which guarantees that the value is always between 0 and 2 * pi. */ -struct AngleRadians +class AngleRadians { +public: /* * \brief Default constructor setting the angle to 0. */ From ae5d418af259cc46166a53e770191a6a37c83684 Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Tue, 10 Aug 2021 17:08:34 +0200 Subject: [PATCH 16/19] Always add as starting point if nothing connects to it yet If a loose line has no lines connecting to it yet, it needs to be added as a starting point. Otherwise it will not get printed at all. This is regardless of whether it then splits into multiple segments, whether it has only 1 connected line as part of a sequence, or whether it's a single loose line. Contributes to issue CURA-7928. --- src/PathOrderMonotonic.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/PathOrderMonotonic.h b/src/PathOrderMonotonic.h index ab16dae22b..9544548915 100644 --- a/src/PathOrderMonotonic.h +++ b/src/PathOrderMonotonic.h @@ -161,14 +161,14 @@ class PathOrderMonotonic : public PathOrder } else //Not a string of polylines, but simply adjacent line segments. { + if(connected_lines.find(*polyline_it) == connected_lines.end()) //Nothing connects to this line yet. + { + starting_lines.insert(*polyline_it); //This is a starting point then. + } const std::vector overlapping_lines = getOverlappingLines(polyline_it, perpendicular, polylines); if(overlapping_lines.size() == 1) //If we're not a string of polylines, but adjacent to only one other polyline, create a sequence of polylines. { connections[*polyline_it] = overlapping_lines[0]; - if(connected_lines.find(*polyline_it) == connected_lines.end()) //Nothing connects to this line yet. - { - starting_lines.insert(*polyline_it); //This is a starting point then. - } if(connected_lines.find(overlapping_lines[0]) != connected_lines.end()) //This line was already connected to. { starting_lines.insert(overlapping_lines[0]); //Multiple lines connect to it, so we must be able to start there. From 4334864eec4d09c189da49434da04ace30a9429d Mon Sep 17 00:00:00 2001 From: Ghostkeeper Date: Tue, 10 Aug 2021 18:30:53 +0200 Subject: [PATCH 17/19] Fix rotational direction of the monotonic vector We were generating the monotonic vector in the simple mathematical way: Cosine of the angle is the X coordinate; sine of the angle is the Y coordinate. This creates a vector that rotates counter-clockwise as the angle increases. However the infill patterns take an angle that was used as starter for the monotonic vector. The infill pattern generator mirrors this angle though, rotating the infill pattern clockwise rather than counter-clockwise as the angle increases. To make the monotonic vector correct, we also need to mirror it. This means we have to add a minus sign to either X or Y. Secondarily, it also turned out that the lines and zigzag infill patterns generate lines perpendicular to this vector rather than parallel. So we need to remove the +90 to make our monotonic vector perpendicular to the individual lines of the lines pattern. Contributes to issue CURA-7928. --- src/FffGcodeWriter.cpp | 2 +- src/PathOrderMonotonic.h | 3 ++- src/TopSurface.cpp | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/FffGcodeWriter.cpp b/src/FffGcodeWriter.cpp index 4a66e32020..45f914066b 100644 --- a/src/FffGcodeWriter.cpp +++ b/src/FffGcodeWriter.cpp @@ -2513,7 +2513,7 @@ void FffGcodeWriter::processSkinPrintFeature(const SliceDataStorage& storage, La if(monotonic) { - const AngleRadians monotonic_direction = AngleRadians(skin_angle + 90); + const AngleRadians monotonic_direction = AngleRadians(skin_angle); constexpr Ratio flow = 1.0_r; const coord_t max_adjacent_distance = config.getLineWidth() * 1.1; //Lines are considered adjacent if they are 1 line width apart, with 10% extra play. The monotonic order is enforced if they are adjacent. if(pattern == EFillMethod::GRID || pattern == EFillMethod::LINES || pattern == EFillMethod::TRIANGLES || pattern == EFillMethod::CUBIC || pattern == EFillMethod::TETRAHEDRAL || pattern == EFillMethod::QUARTER_CUBIC || pattern == EFillMethod::CUBICSUBDIV) diff --git a/src/PathOrderMonotonic.h b/src/PathOrderMonotonic.h index 9544548915..1895b81ad4 100644 --- a/src/PathOrderMonotonic.h +++ b/src/PathOrderMonotonic.h @@ -44,7 +44,8 @@ class PathOrderMonotonic : public PathOrder using PathOrder::coincident_point_distance; PathOrderMonotonic(const AngleRadians monotonic_direction, const coord_t max_adjacent_distance, const Point start_point) - : monotonic_vector(std::cos(monotonic_direction) * monotonic_vector_resolution, std::sin(monotonic_direction) * monotonic_vector_resolution) + //The monotonic vector needs to rotate clockwise instead of counter-clockwise, the same as how the infill patterns are generated. + : monotonic_vector(-std::cos(monotonic_direction) * monotonic_vector_resolution, std::sin(monotonic_direction) * monotonic_vector_resolution) , max_adjacent_distance(max_adjacent_distance) { this->start_point = start_point; diff --git a/src/TopSurface.cpp b/src/TopSurface.cpp index fed6bac479..dfdbcf437b 100644 --- a/src/TopSurface.cpp +++ b/src/TopSurface.cpp @@ -129,7 +129,7 @@ bool TopSurface::ironing(const SliceMeshStorage& mesh, const GCodePathConfig& li else { const coord_t max_adjacent_distance = line_spacing * 1.1; //Lines are considered adjacent - meaning they need to be printed in monotonic order - if spaced 1 line apart, with 10% extra play. - layer.addLinesMonotonic(ironing_lines, line_config, SpaceFillType::PolyLines, AngleRadians(direction + 90.0), max_adjacent_distance); + layer.addLinesMonotonic(ironing_lines, line_config, SpaceFillType::PolyLines, AngleRadians(direction), max_adjacent_distance); } added = true; } From db248b242d28127fbce5880d751cba4712a88628 Mon Sep 17 00:00:00 2001 From: Konstantinos Karmas Date: Thu, 12 Aug 2021 16:53:51 +0200 Subject: [PATCH 18/19] Move the decision to unretract when combing happens When we move to the next object in a layer and combing is enabled, it will decide to add extra travel moves if moving from object A to the outer wall of object B. This is done to ensure that the nozzle won't end up on top of the outer wall, but rather it will first move to the first inner wall and THEN move to the outer wall. In such cases, the unretraction should happen while the nozzle is still on top of the inner wall, so that the last travel move towards the outer wall already contains the unretraction. This ensures that there will be no blips on the outer wall, which would otherwise would've been caused while the nozzle is unretracting on top of it. CURA-8365 --- src/FffGcodeWriter.cpp | 6 ++---- src/InsetOrderOptimizer.cpp | 24 ++++++++---------------- src/LayerPlan.cpp | 22 +++++++++++++--------- src/LayerPlan.h | 16 ++++------------ src/LayerPlanBuffer.cpp | 20 +++----------------- src/pathPlanning/Comb.cpp | 18 ++++++++++++++---- src/pathPlanning/Comb.h | 6 +++++- tests/LayerPlanTest.cpp | 35 +---------------------------------- 8 files changed, 50 insertions(+), 97 deletions(-) diff --git a/src/FffGcodeWriter.cpp b/src/FffGcodeWriter.cpp index 75e28a4e6a..5a625b43ba 100644 --- a/src/FffGcodeWriter.cpp +++ b/src/FffGcodeWriter.cpp @@ -2061,17 +2061,15 @@ bool FffGcodeWriter::processInsets(const SliceDataStorage& storage, LayerPlan& g ZSeamConfig z_seam_config(mesh.settings.get("z_seam_type"), mesh.getZSeamHint(), mesh.settings.get("z_seam_corner")); Polygons outer_wall = part.insets[0]; - // When adding an outer wall we need to make sure that we unretract before the last travel move before that wall gets printed - constexpr bool unretract_before_last_travel_move = true; if (!compensate_overlap_0) { WallOverlapComputation* wall_overlap_computation(nullptr); - gcode_layer.addWalls(outer_wall, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlap_computation, z_seam_config, mesh.settings.get("wall_0_wipe_dist"), flow, retract_before_outer_wall, unretract_before_last_travel_move); + gcode_layer.addWalls(outer_wall, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlap_computation, z_seam_config, mesh.settings.get("wall_0_wipe_dist"), flow, retract_before_outer_wall); } else { WallOverlapComputation wall_overlap_computation(outer_wall, mesh_config.inset0_config.getLineWidth()); - gcode_layer.addWalls(outer_wall, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, &wall_overlap_computation, z_seam_config, mesh.settings.get("wall_0_wipe_dist"), flow, retract_before_outer_wall, unretract_before_last_travel_move); + gcode_layer.addWalls(outer_wall, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, &wall_overlap_computation, z_seam_config, mesh.settings.get("wall_0_wipe_dist"), flow, retract_before_outer_wall); } } } diff --git a/src/InsetOrderOptimizer.cpp b/src/InsetOrderOptimizer.cpp index af33e5877a..568bbf9b91 100644 --- a/src/InsetOrderOptimizer.cpp +++ b/src/InsetOrderOptimizer.cpp @@ -273,10 +273,6 @@ void InsetOrderOptimizer::processHoleInsets() } } - // When adding an outer wall we need to make sure that we unretract before the last travel move before that - // wall gets printed. This way we avoid blips caused by unretractions when the nozzle is sitting on the outer - // wall. - constexpr bool unretract_before_last_travel_move = true; if (hole_inner_walls.size() > 0 && extruder_nr == mesh.settings.get("wall_x_extruder_nr").extruder_nr) { // output the inset polys @@ -287,7 +283,7 @@ void InsetOrderOptimizer::processHoleInsets() { if (extruder_nr == mesh.settings.get("wall_0_extruder_nr").extruder_nr) { - gcode_layer.addWalls(hole_outer_wall, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, z_seam_config, wall_0_wipe_dist, flow, retract_before_outer_wall, unretract_before_last_travel_move); + gcode_layer.addWalls(hole_outer_wall, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, z_seam_config, wall_0_wipe_dist, flow, retract_before_outer_wall); } gcode_layer.addWalls(hole_inner_walls, mesh, mesh_config.insetX_config, mesh_config.bridge_insetX_config, wall_overlapper_x); } @@ -315,7 +311,7 @@ void InsetOrderOptimizer::processHoleInsets() gcode_layer.addWalls(hole_inner_walls, mesh, mesh_config.insetX_config, mesh_config.bridge_insetX_config, wall_overlapper_x); if (extruder_nr == mesh.settings.get("wall_0_extruder_nr").extruder_nr) { - gcode_layer.addWall(hole_outer_wall[0], outer_poly_start_idx, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, wall_0_wipe_dist, flow, retract_before_outer_wall, unretract_before_last_travel_move); + gcode_layer.addWall(hole_outer_wall[0], outer_poly_start_idx, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, wall_0_wipe_dist, flow, retract_before_outer_wall); // move inside so an immediately following retract doesn't occur on the outer wall moveInside(); } @@ -334,11 +330,11 @@ void InsetOrderOptimizer::processHoleInsets() { // align z-seam of hole with z-seam of outer wall - makes a nicer job when printing tubes const unsigned point_idx = PolygonUtils::findNearestVert(start_point, hole_outer_wall.back()); - gcode_layer.addWall(hole_outer_wall.back(), point_idx, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, wall_0_wipe_dist, flow, retract_before_outer_wall, unretract_before_last_travel_move); + gcode_layer.addWall(hole_outer_wall.back(), point_idx, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, wall_0_wipe_dist, flow, retract_before_outer_wall); } else { - gcode_layer.addWalls(hole_outer_wall, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, z_seam_config, wall_0_wipe_dist, flow, retract_before_outer_wall, unretract_before_last_travel_move); + gcode_layer.addWalls(hole_outer_wall, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, z_seam_config, wall_0_wipe_dist, flow, retract_before_outer_wall); } // move inside so an immediately following retract doesn't occur on the outer wall moveInside(); @@ -414,8 +410,6 @@ void InsetOrderOptimizer::processOuterWallInsets(const bool include_outer, const order_optimizer.optimize(); const unsigned outer_poly_start_idx = gcode_layer.locateFirstSupportedVertex(*inset_polys[0][0], order_optimizer.polyStart[0]); const Point z_seam_location = (*inset_polys[0][0])[outer_poly_start_idx]; - // Since we are adding inner walls here, we don't have to unretract before the last travel move - constexpr bool unretract_before_last_travel_move = false; std::function addInnerWalls = [this, part_inner_walls, outer_inset_first, z_seam_location]() { @@ -434,7 +428,7 @@ void InsetOrderOptimizer::processOuterWallInsets(const bool include_outer, const constexpr bool always_retract = false; for (unsigned int wall_idx : orderOptimizer.polyOrder) { - gcode_layer.addWall(part_inner_walls[wall_idx], orderOptimizer.polyStart[wall_idx], mesh, mesh_config.insetX_config, mesh_config.bridge_insetX_config, wall_overlapper_x, wall_0_wipe_dist, flow_ratio, always_retract, unretract_before_last_travel_move); + gcode_layer.addWall(part_inner_walls[wall_idx], orderOptimizer.polyStart[wall_idx], mesh, mesh_config.insetX_config, mesh_config.bridge_insetX_config, wall_overlapper_x, wall_0_wipe_dist, flow_ratio, always_retract); } }; @@ -442,7 +436,7 @@ void InsetOrderOptimizer::processOuterWallInsets(const bool include_outer, const { if (include_outer && extruder_nr == mesh.settings.get("wall_0_extruder_nr").extruder_nr) { - gcode_layer.addWall(*inset_polys[0][0], outer_poly_start_idx, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, wall_0_wipe_dist, flow, retract_before_outer_wall, unretract_before_last_travel_move); + gcode_layer.addWall(*inset_polys[0][0], outer_poly_start_idx, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, wall_0_wipe_dist, flow, retract_before_outer_wall); } addInnerWalls(); } @@ -451,7 +445,7 @@ void InsetOrderOptimizer::processOuterWallInsets(const bool include_outer, const addInnerWalls(); if (include_outer && extruder_nr == mesh.settings.get("wall_0_extruder_nr").extruder_nr) { - gcode_layer.addWall(*inset_polys[0][0], outer_poly_start_idx, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, wall_0_wipe_dist, flow, retract_before_outer_wall, unretract_before_last_travel_move); + gcode_layer.addWall(*inset_polys[0][0], outer_poly_start_idx, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, wall_0_wipe_dist, flow, retract_before_outer_wall); // move inside so an immediately following retract doesn't occur on the outer wall moveInside(); } @@ -461,14 +455,12 @@ void InsetOrderOptimizer::processOuterWallInsets(const bool include_outer, const else if (include_outer && extruder_nr == mesh.settings.get("wall_0_extruder_nr").extruder_nr) { // just the outer wall, no inners - // When adding an outer wall we need to make sure that we unretract before the last travel move before that wall gets printed - constexpr bool unretract_before_last_travel_move = true; gcode_writer.setExtruder_addPrime(storage, gcode_layer, extruder_nr); gcode_layer.setIsInside(true); // going to print stuff inside print object Polygons part_outer_wall; part_outer_wall.add(*inset_polys[0][0]); - gcode_layer.addWalls(part_outer_wall, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, z_seam_config, wall_0_wipe_dist, flow, retract_before_outer_wall, unretract_before_last_travel_move); + gcode_layer.addWalls(part_outer_wall, mesh, mesh_config.inset0_config, mesh_config.bridge_inset0_config, wall_overlapper_0, z_seam_config, wall_0_wipe_dist, flow, retract_before_outer_wall); // move inside so an immediately following retract doesn't occur on the outer wall moveInside(); added_something = true; diff --git a/src/LayerPlan.cpp b/src/LayerPlan.cpp index f82bbc47dd..f4dcbfaef7 100644 --- a/src/LayerPlan.cpp +++ b/src/LayerPlan.cpp @@ -359,7 +359,7 @@ std::optional> LayerPlan::getFirstTravelDestinationState( return ret; } -GCodePath& LayerPlan::addTravel(const Point p, const bool force_retract, const bool unretract_before_last_travel_move) +GCodePath& LayerPlan::addTravel(const Point p, const bool force_retract) { const GCodePathConfig& travel_config = configs_storage.travel_config_per_extruder[getExtruder()]; const RetractionConfig& retraction_config = storage.retraction_config_per_extruder[getExtruder()]; @@ -405,7 +405,8 @@ GCodePath& LayerPlan::addTravel(const Point p, const bool force_retract, const b // Multiply by 2 because if two lines start and end points places very close then will be applied combing with retractions. (Ex: for brim) const coord_t max_distance_ignored = extruder->settings.get("machine_nozzle_tip_outer_diameter") / 2 * 2; - combed = comb->calc(*extruder, *last_planned_position, p, combPaths, was_inside, is_inside, max_distance_ignored); + bool unretract_before_last_travel_move = false; // Decided when calculating the combing + combed = comb->calc(*extruder, *last_planned_position, p, combPaths, was_inside, is_inside, max_distance_ignored, unretract_before_last_travel_move); //Here is the magic happening if (combed) { bool retract = path->retract || (combPaths.size() > 1 && retraction_enable); @@ -442,7 +443,7 @@ GCodePath& LayerPlan::addTravel(const Point p, const bool force_retract, const b Point last_point((last_planned_position) ? *last_planned_position : Point(0, 0)); for (CombPath& combPath : combPaths) { // add all comb paths (don't do anything special for paths which are moving through air) - if (combPath.size() == 0) + if (combPath.empty()) { continue; } @@ -460,6 +461,11 @@ GCodePath& LayerPlan::addTravel(const Point p, const bool force_retract, const b path->retract = retract || (retract_threshold > 0 && distance > retract_threshold && retraction_enable); // don't perform a z-hop } + // Whether to unretract before the last travel move of the travel path, which comes before the wall to be printed. + // This should be true when traveling towards an outer wall to make sure that the unretraction will happen before the + // last travel move BEFORE going to that wall. This way, the nozzle doesn't sit still on top of the outer wall's + // path while it is unretracting, avoiding possible blips. + path->unretract_before_last_travel_move = path->retract && unretract_before_last_travel_move; } } @@ -492,8 +498,6 @@ GCodePath& LayerPlan::addTravel(const Point p, const bool force_retract, const b // must start new travel path as retraction can be enabled or not depending on path length, etc. forceNewPathStart(); - path->unretract_before_last_travel_move = path->retract && unretract_before_last_travel_move; - GCodePath& ret = addTravel_simple(p, path); was_inside = is_inside; return ret; @@ -798,7 +802,7 @@ void LayerPlan::addWallLine(const Point& p0, const Point& p1, const SliceMeshSto } } -void LayerPlan::addWall(ConstPolygonRef wall, int start_idx, const SliceMeshStorage& mesh, const GCodePathConfig& non_bridge_config, const GCodePathConfig& bridge_config, WallOverlapComputation* wall_overlap_computation, coord_t wall_0_wipe_dist, float flow_ratio, bool always_retract, bool unretract_before_last_travel_move) +void LayerPlan::addWall(ConstPolygonRef wall, int start_idx, const SliceMeshStorage& mesh, const GCodePathConfig& non_bridge_config, const GCodePathConfig& bridge_config, WallOverlapComputation* wall_overlap_computation, coord_t wall_0_wipe_dist, float flow_ratio, bool always_retract) { // make sure wall start point is not above air! start_idx = locateFirstSupportedVertex(wall, start_idx); @@ -919,7 +923,7 @@ void LayerPlan::addWall(ConstPolygonRef wall, int start_idx, const SliceMeshStor { if (first_line || travel_required) { - addTravel(p0, (first_line) ? always_retract : wall_min_flow_retract, unretract_before_last_travel_move); + addTravel(p0, (first_line) ? always_retract : wall_min_flow_retract); first_line = false; travel_required = false; } @@ -999,7 +1003,7 @@ void LayerPlan::addWall(ConstPolygonRef wall, int start_idx, const SliceMeshStor } } -void LayerPlan::addWalls(const Polygons& walls, const SliceMeshStorage& mesh, const GCodePathConfig& non_bridge_config, const GCodePathConfig& bridge_config, WallOverlapComputation* wall_overlap_computation, const ZSeamConfig& z_seam_config, coord_t wall_0_wipe_dist, float flow_ratio, bool always_retract, bool unretract_before_last_travel_move) +void LayerPlan::addWalls(const Polygons& walls, const SliceMeshStorage& mesh, const GCodePathConfig& non_bridge_config, const GCodePathConfig& bridge_config, WallOverlapComputation* wall_overlap_computation, const ZSeamConfig& z_seam_config, coord_t wall_0_wipe_dist, float flow_ratio, bool always_retract) { PathOrderOptimizer orderOptimizer(getLastPlannedPositionOrStartingPosition(), z_seam_config); for (unsigned int poly_idx = 0; poly_idx < walls.size(); poly_idx++) @@ -1009,7 +1013,7 @@ void LayerPlan::addWalls(const Polygons& walls, const SliceMeshStorage& mesh, co orderOptimizer.optimize(); for (unsigned int poly_idx : orderOptimizer.polyOrder) { - addWall(walls[poly_idx], orderOptimizer.polyStart[poly_idx], mesh, non_bridge_config, bridge_config, wall_overlap_computation, wall_0_wipe_dist, flow_ratio, always_retract, unretract_before_last_travel_move); + addWall(walls[poly_idx], orderOptimizer.polyStart[poly_idx], mesh, non_bridge_config, bridge_config, wall_overlap_computation, wall_0_wipe_dist, flow_ratio, always_retract); } } diff --git a/src/LayerPlan.h b/src/LayerPlan.h index 5d87fd2eb5..16cbafc526 100644 --- a/src/LayerPlan.h +++ b/src/LayerPlan.h @@ -476,12 +476,8 @@ class LayerPlan : public NoCopy * afterwards. * \param p The point to travel to. * \param force_retract Whether to force a retraction to occur. - * \param unretract_before_last_travel_move Whether to unretract before the last travel move of the travel path, - * which comes before the wall to be printed. This should be true when we are adding outer walls to make sure - * that the unretraction will happen before the last travel move BEFORE going to that wall. This way, the nozzle - * doesn't sit still on top of the outer wall's path while it is unretracting, avoiding possible blips. */ - GCodePath& addTravel(const Point p, const bool force_retract = false, const bool unretract_before_last_travel_move = false); + GCodePath& addTravel(const Point p, const bool force_retract = false); /*! * Add a travel path to a certain point and retract if needed. @@ -581,12 +577,8 @@ class LayerPlan : public NoCopy * \param wall_0_wipe_dist The distance to travel along the wall after it has been laid down, in order to wipe the start and end of the wall together * \param flow_ratio The ratio with which to multiply the extrusion amount * \param always_retract Whether to force a retraction when moving to the start of the wall (used for outer walls) - * \param unretract_before_last_travel_move Whether to unretract before the last travel move of the travel path, - * which comes before the wall to be printed. This should be true when we are adding outer walls to make sure - * that the unretraction will happen before the last travel move BEFORE going to that wall. This way, the nozzle - * doesn't sit still on top of the outer wall's path while it is unretracting, avoiding possible blips. */ - void addWall(ConstPolygonRef polygon, int start_idx, const SliceMeshStorage& mesh, const GCodePathConfig& non_bridge_config, const GCodePathConfig& bridge_config, WallOverlapComputation* wall_overlap_computation, coord_t wall_0_wipe_dist, float flow_ratio, bool always_retract, bool unretract_before_last_travel_move); + void addWall(ConstPolygonRef polygon, int start_idx, const SliceMeshStorage& mesh, const GCodePathConfig& non_bridge_config, const GCodePathConfig& bridge_config, WallOverlapComputation* wall_overlap_computation, coord_t wall_0_wipe_dist, float flow_ratio, bool always_retract); /*! * Add walls (polygons) to the gcode with optimized order. @@ -600,7 +592,7 @@ class LayerPlan : public NoCopy * \param flow_ratio The ratio with which to multiply the extrusion amount * \param always_retract Whether to force a retraction when moving to the start of a wall (used for outer walls) */ - void addWalls(const Polygons& walls, const SliceMeshStorage& mesh, const GCodePathConfig& non_bridge_config, const GCodePathConfig& bridge_config, WallOverlapComputation* wall_overlap_computation, const ZSeamConfig& z_seam_config = ZSeamConfig(), coord_t wall_0_wipe_dist = 0, float flow_ratio = 1.0, bool always_retract = false, bool unretract_before_last_travel_move = false); + void addWalls(const Polygons& walls, const SliceMeshStorage& mesh, const GCodePathConfig& non_bridge_config, const GCodePathConfig& bridge_config, WallOverlapComputation* wall_overlap_computation, const ZSeamConfig& z_seam_config = ZSeamConfig(), coord_t wall_0_wipe_dist = 0, float flow_ratio = 1.0, bool always_retract = false); /*! * Add lines to the gcode with optimized order. @@ -655,7 +647,7 @@ class LayerPlan : public NoCopy * * \param extruder_plan_idx The index of the current extruder plan * \param path_idx The index of the current retracted path - * \return Whether the path should be an extgruder switch retracted path + * \return Whether the path should be an extruder switch retracted path */ bool makeRetractSwitchRetract(unsigned int extruder_plan_idx, unsigned int path_idx); diff --git a/src/LayerPlanBuffer.cpp b/src/LayerPlanBuffer.cpp index b771a21adf..0591ce40fd 100644 --- a/src/LayerPlanBuffer.cpp +++ b/src/LayerPlanBuffer.cpp @@ -94,13 +94,10 @@ void LayerPlanBuffer::addConnectingTravelMove(LayerPlan* prev_layer, const Layer if (!prev_layer->last_planned_position || *prev_layer->last_planned_position != first_location_new_layer) { const Settings& mesh_group_settings = Application::getInstance().current_slice->scene.current_mesh_group->settings; - const Settings& prev_extruder_settings = Application::getInstance().current_slice->scene.extruders[prev_layer->extruder_plans.back().extruder_nr].settings; - const Settings& next_extruder_settings = Application::getInstance().current_slice->scene.extruders[newest_layer->extruder_plans.front().extruder_nr].settings; + const Settings& extruder_settings = Application::getInstance().current_slice->scene.extruders[prev_layer->extruder_plans.back().extruder_nr].settings; prev_layer->setIsInside(new_layer_destination_state->second); - const bool force_retract = prev_extruder_settings.get("retract_at_layer_change") || - (mesh_group_settings.get("travel_retract_before_outer_wall") && - (mesh_group_settings.get("outer_inset_first") || - mesh_group_settings.get("wall_line_count") == 1)); //Moving towards an outer wall. + const bool force_retract = extruder_settings.get("retract_at_layer_change") || + (mesh_group_settings.get("travel_retract_before_outer_wall") && (mesh_group_settings.get("outer_inset_first") || mesh_group_settings.get("wall_line_count") == 1)); //Moving towards an outer wall. prev_layer->final_travel_z = newest_layer->z; GCodePath &path = prev_layer->addTravel(first_location_new_layer, force_retract); if (force_retract && !path.retract) @@ -110,17 +107,6 @@ void LayerPlanBuffer::addConnectingTravelMove(LayerPlan* prev_layer, const Layer // we force the path to use retraction path.retract = true; } - if (path.retract && !path.unretract_before_last_travel_move && - next_extruder_settings.get("outer_inset_first") && - (next_extruder_settings.get("infill_line_distance") == 0 || - !next_extruder_settings.get("infill_before_walls"))) - { - // If there is a retraction and the next layer's first path is an outer wall, then we need to make sure that - // we unretract before the last travel move of the previous layer. This way we make sure that the unretract - // does not happen while the nozzle is on top of the new layer's outer wall, which could cause artefacts and - // decrease dimensional accuracy. - path.unretract_before_last_travel_move = true; - } } } diff --git a/src/pathPlanning/Comb.cpp b/src/pathPlanning/Comb.cpp index 24caf5f948..12259c0aad 100644 --- a/src/pathPlanning/Comb.cpp +++ b/src/pathPlanning/Comb.cpp @@ -78,13 +78,13 @@ Comb::~Comb() } } -bool Comb::calc(const ExtruderTrain& train, Point startPoint, Point endPoint, CombPaths& combPaths, bool _startInside, bool _endInside, coord_t max_comb_distance_ignored) +bool Comb::calc(const ExtruderTrain& train, Point startPoint, Point endPoint, CombPaths& combPaths, bool _startInside, bool _endInside, coord_t max_comb_distance_ignored, bool &unretract_before_last_travel_move) { if (shorterThen(endPoint - startPoint, max_comb_distance_ignored)) { return true; } - + const Point travel_end_point_before_combing = endPoint; //Move start and end point inside the optimal comb boundary unsigned int start_inside_poly = NO_INDEX; const bool startInside = moveInside(boundary_inside_optimal, _startInside, inside_loc_to_line_optimal, startPoint, start_inside_poly); @@ -106,7 +106,11 @@ bool Comb::calc(const ExtruderTrain& train, Point startPoint, Point endPoint, Co { PolygonsPart part = partsView_inside_optimal.assemblePart(start_part_idx); combPaths.emplace_back(); - return LinePolygonsCrossings::comb(part, *inside_loc_to_line_optimal, startPoint, endPoint, combPaths.back(), -offset_dist_to_get_from_on_the_polygon_to_outside, max_comb_distance_ignored, fail_on_unavoidable_obstacles); + const bool combing_succeeded = LinePolygonsCrossings::comb(part, *inside_loc_to_line_optimal, startPoint, endPoint, combPaths.back(), -offset_dist_to_get_from_on_the_polygon_to_outside, max_comb_distance_ignored, fail_on_unavoidable_obstacles); + // If the endpoint of the travel path changes with combing, then it means that we are moving to an outer wall + // and we should unretract before the last travel move when travelling to that outer wall + unretract_before_last_travel_move = combing_succeeded && endPoint != travel_end_point_before_combing; + return combing_succeeded; } //Move start and end point inside the minimum comb boundary @@ -132,6 +136,9 @@ bool Comb::calc(const ExtruderTrain& train, Point startPoint, Point endPoint, Co comb_result = LinePolygonsCrossings::comb(part, *inside_loc_to_line_minimum, startPoint, endPoint, result_path, -offset_dist_to_get_from_on_the_polygon_to_outside, max_comb_distance_ignored, fail_on_unavoidable_obstacles); Comb::moveCombPathInside(boundary_inside_minimum, boundary_inside_optimal, result_path, combPaths.back()); // add altered result_path to combPaths.back() + // If the endpoint of the travel path changes with combing, then it means that we are moving to an outer wall + // and we should unretract before the last travel move when travelling to that outer wall + unretract_before_last_travel_move = comb_result && endPoint != travel_end_point_before_combing; return comb_result; } @@ -198,7 +205,7 @@ bool Comb::calc(const ExtruderTrain& train, Point startPoint, Point endPoint, Co } } - // throught air from boundary to boundary + // through air from boundary to boundary if (travel_avoid_other_parts && !skip_avoid_other_parts_path) { combPaths.emplace_back(); @@ -251,6 +258,9 @@ bool Comb::calc(const ExtruderTrain& train, Point startPoint, Point endPoint, Co combPaths.emplace_back(); bool combing_succeeded = LinePolygonsCrossings::comb(end_crossing.dest_part, *inside_loc_to_line_optimal, end_crossing.in_or_mid, endPoint, combPaths.back(), -offset_dist_to_get_from_on_the_polygon_to_outside, max_comb_distance_ignored, fail_on_unavoidable_obstacles); + // If the endpoint of the travel path changes with combing, then it means that we are moving to an outer wall + // and we should unretract before the last travel move when travelling to that outer wall + unretract_before_last_travel_move = combing_succeeded && endPoint != travel_end_point_before_combing; if (!combing_succeeded) { // Couldn't comb between end point and computed crossing to the end part! Happens for very thin parts when the offset_to_get_off_boundary moves points to outside the polygon return false; diff --git a/src/pathPlanning/Comb.h b/src/pathPlanning/Comb.h index dc363e082f..90debb8f57 100644 --- a/src/pathPlanning/Comb.h +++ b/src/pathPlanning/Comb.h @@ -195,9 +195,13 @@ class Comb * \p startPoint (?) and \p endPoint. * \param startInside Whether we want to start inside the comb boundary. * \param endInside Whether we want to end up inside the comb boundary. + * \param unretract_before_last_travel_move Whether we should unretract before the last travel move when travelling + * because of combing. If the endpoint of a travel path changes with combing, then it means that an outer wall is + * involved, which means that we should then unretract before the last travel move to that wall to avoid any blips + * being introduced due to the unretraction. * \return Whether combing has succeeded; otherwise a retraction is needed. */ - bool calc(const ExtruderTrain& train, Point startPoint, Point endPoint, CombPaths& combPaths, bool startInside, bool endInside, coord_t max_comb_distance_ignored); + bool calc(const ExtruderTrain& train, Point startPoint, Point endPoint, CombPaths& combPaths, bool startInside, bool endInside, coord_t max_comb_distance_ignored, bool &unretract_before_last_travel_move); }; }//namespace cura diff --git a/tests/LayerPlanTest.cpp b/tests/LayerPlanTest.cpp index cd69e26d60..50b283ef2e 100644 --- a/tests/LayerPlanTest.cpp +++ b/tests/LayerPlanTest.cpp @@ -146,7 +146,7 @@ class LayerPlanTest : public testing::Test settings->add("raft_surface_speed", "53"); settings->add("raft_surface_thickness", "0.103"); settings->add("retraction_amount", "8"); - settings->add("retraction_combing", "off"); + settings->add("retraction_combing", "All"); settings->add("retraction_count_max", "30"); settings->add("retraction_enable", "false"); settings->add("retraction_extra_prime_amount", "1"); @@ -398,27 +398,6 @@ class AddTravelTest : public LayerPlanTest, public testing::WithParamInterfaceadd("retraction_enable", "true"); - settings->add("retraction_hop_enabled", "true"); - settings->add("retraction_combing_max_distance", "1"); - layer_plan.last_planned_position = Point(0, 0); - return layer_plan.addTravel(destination, true, unretract_before_last_travel_move); - } }; INSTANTIATE_TEST_CASE_P(AllCombinations, AddTravelTest, testing::Combine( @@ -571,16 +550,4 @@ TEST_P(AddTravelTest, NoUnretractBeforeLastTravelMoveIfNoPriorRetraction) } } -/*! - * Test to verify that when adding a travel, if it is requested to unretract before the last travel move, - * then this should be reflected in the path. - */ -TEST_F(AddTravelTest, UnretractBeforeLastTravelMove) -{ - const GCodePath path_with_unretraction = run(true); - EXPECT_TRUE(path_with_unretraction.unretract_before_last_travel_move) << "If the travel path is requested to unretract before the last travel move, then this should be reflected in the path."; - const GCodePath path_without_unretraction = run(false); - EXPECT_FALSE(path_without_unretraction.unretract_before_last_travel_move) << "If the travel path is not requested to unretract before the last travel move, then this should be reflected in the path."; -} - } From c3cb930201e8986e8dc929ef0993c2b67fac9339 Mon Sep 17 00:00:00 2001 From: Konstantinos Karmas Date: Thu, 12 Aug 2021 17:00:15 +0200 Subject: [PATCH 19/19] Restore retraction_combing to off when testing CURA-8365 --- tests/LayerPlanTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/LayerPlanTest.cpp b/tests/LayerPlanTest.cpp index 50b283ef2e..fb69fab9fa 100644 --- a/tests/LayerPlanTest.cpp +++ b/tests/LayerPlanTest.cpp @@ -146,7 +146,7 @@ class LayerPlanTest : public testing::Test settings->add("raft_surface_speed", "53"); settings->add("raft_surface_thickness", "0.103"); settings->add("retraction_amount", "8"); - settings->add("retraction_combing", "All"); + settings->add("retraction_combing", "off"); settings->add("retraction_count_max", "30"); settings->add("retraction_enable", "false"); settings->add("retraction_extra_prime_amount", "1");