Wall gaps from unretract on inner wall when there is no inner wall

[richfelker]

The fix for #1458 (commit db248b2) seems to have caused gaps in the outer wall when printing with a single wall, due to unretracting on top of an inner wall that does not exist. The below image shows the effect (right) in a hollow cylinder printed with no infill and only one wall, versus the same created with actual hollow geometry (so that there is no "interior" to move the nozzle into before unretracting).

It was not actually this commit that created the bug; it just exposed it by activating the functionality. The bug is just the logic to travel inside the model before starting the outer wall, which should only be done if the wall line count is greater than one. However the logic added in that commit probably needs to be reviewed too, as it might be wrong without this travel move being present (it might move the unretract before a different/much longer travel move, which would be even more harmful).

[richfelker]

After reading the emitted gcode and experimenting with patching the source, I'm pretty sure I was wrong about the cause of this problem. It's not that the unretract at the beginning of the wall happens in the wrong place (somehow, it doesn't) but that the final retract before moving to the start of the next layer is happening at the wrong place. FffGcodeWriter::addMeshPartToGCode is performing a moveInsideCombBoundary that pulls the end of the outer wall inward due to tension with unretracted material in the nozzle. This is fine if there's an inner wall (or opposing outer wall, for thin parts) there since it will just act as a wipe, but very wrong if there's only a single wall.

[richfelker]

One thing I'm confused about is why one of these places is pulling the wall line count and width settings from mesh and the other is pulling it from extruder...

[ky438]

any update on this? I am seeing this also 😢

[richfelker]

Here's the (mildly sloppy from rebase after a conflicting patch prettied up formatting) draft patch I'm using that fixed it:

commit fa3cec26050f116eb2a55feda1d8eb67355745ee
Author: Rich Felker <dalias@aerifal.cx>
Date:   Sun Oct 2 00:59:20 2022 +0000

    only un/retract on inner wall if there is more than one wall

diff --git a/src/FffGcodeWriter.cpp b/src/FffGcodeWriter.cpp
index c125a2120..214ecad49 100644
--- a/src/FffGcodeWriter.cpp
+++ b/src/FffGcodeWriter.cpp
@@ -1513,7 +1513,7 @@ void FffGcodeWriter::addMeshPartToGCode(const SliceDataStorage& storage, const S
     added_something = added_something | processSkin(storage, gcode_layer, mesh, extruder_nr, mesh_config, part);
 
     // After a layer part, make sure the nozzle is inside the comb boundary, so we do not retract on the perimeter.
-    if (added_something && (! mesh_group_settings.get<bool>("magic_spiralize") || gcode_layer.getLayerNr() < static_cast<LayerIndex>(mesh.settings.get<size_t>("initial_bottom_layers"))))
+    if (added_something && (!mesh_group_settings.get<bool>("magic_spiralize") || gcode_layer.getLayerNr() < static_cast<LayerIndex>(mesh.settings.get<size_t>("initial_bottom_layers"))) && (mesh.settings.get<size_t>("wall_line_count") > 1))
     {
         coord_t innermost_wall_line_width = mesh.settings.get<coord_t>((mesh.settings.get<size_t>("wall_line_count") > 1) ? "wall_line_width_x" : "wall_line_width_0");
         if (gcode_layer.getLayerNr() == 0)
diff --git a/src/LayerPlan.cpp b/src/LayerPlan.cpp
index 85c2f8a52..240f6bada 100644
--- a/src/LayerPlan.cpp
+++ b/src/LayerPlan.cpp
@@ -469,7 +469,8 @@ GCodePath& LayerPlan::addTravel(const Point p, const bool force_retract)
             // 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;
+            path->unretract_before_last_travel_move = path->retract && unretract_before_last_travel_move
+                && (extruder->settings.get<size_t>("wall_line_count") > 1);
         }
     }
 
@@ -485,6 +486,7 @@ GCodePath& LayerPlan::addTravel(const Point p, const bool force_retract)
     // no combing? retract only when path is not shorter than minimum travel distance
     if (! combed && ! is_first_travel_of_layer && last_planned_position && ! shorterThen(*last_planned_position - p, retraction_config.retraction_min_travel_distance))
     {
+        if (extruder->settings.get<size_t>("wall_line_count") > 1)
         if (was_inside) // when the previous location was from printing something which is considered inside (not support or prime tower etc)
         { // then move inside the printed part, so that we don't ooze on the outer wall while retraction, but on the inside of the print.
             assert(extruder != nullptr);