From c1baacf8a084f1b7754175462b8ab892e9a4fb01 Mon Sep 17 00:00:00 2001 From: Ian Bassi Date: Tue, 6 Oct 2026 14:09:36 -0300 Subject: [PATCH] fix: thin-walled holes classic walls flip the direction (#16051) Co-authored-by: Ioannis Giannakas <59056762+igiannakas@users.noreply.github.com> --- src/libslic3r/PerimeterGenerator.cpp | 33 ++++--- tests/fff_print/test_perimeters.cpp | 138 +++++++++++++++++++++++++++ 2 files changed, 157 insertions(+), 14 deletions(-) diff --git a/src/libslic3r/PerimeterGenerator.cpp b/src/libslic3r/PerimeterGenerator.cpp index e2b5090213..41db0ed9db 100644 --- a/src/libslic3r/PerimeterGenerator.cpp +++ b/src/libslic3r/PerimeterGenerator.cpp @@ -61,6 +61,8 @@ public: bool is_smaller_width_perimeter; // Depth in the hierarchy. External perimeter has depth = 0. An external perimeter could be both a contour and a hole. unsigned short depth; + // ORCA: an external hole perimeter touching the next outer wall all around, with nothing between them. + bool is_thin_wall_hole = false; // Children contour, may be both CCW and CW oriented (outer contours or holes). std::vector children; @@ -118,7 +120,7 @@ static bool detect_steep_overhang(const PrintRegionConfig *config, } static ExtrusionEntityCollection traverse_loops(const PerimeterGenerator &perimeter_generator, const PerimeterGeneratorLoops &loops, ThickPolylines &thin_walls, - bool &steep_overhang_contour, bool &steep_overhang_hole, bool reverse_thin_wall_hole) + bool &steep_overhang_contour, bool &steep_overhang_hole) { // loops is an arrayref of ::Loop objects // turn each one into an ExtrusionLoop object @@ -285,24 +287,17 @@ static ExtrusionEntityCollection traverse_loops(const PerimeterGenerator &perime } else { const PerimeterGeneratorLoop &loop = loops[idx.first]; assert(thin_walls.empty()); - const bool reverse_children_thin_wall_hole = loops.size() == 1 && loop.is_contour && loop.children.size() == 1 && - (!loop.children.front().is_contour) && loop.children.front().children.empty(); - ExtrusionEntityCollection children = traverse_loops(perimeter_generator, loop.children, thin_walls, steep_overhang_contour, - steep_overhang_hole, reverse_children_thin_wall_hole); + ExtrusionEntityCollection children = traverse_loops(perimeter_generator, loop.children, thin_walls, steep_overhang_contour, steep_overhang_hole); out.entities.reserve(out.entities.size() + children.entities.size() + 1); ExtrusionLoop *eloop = static_cast(coll.entities[idx.first]); coll.entities[idx.first] = nullptr; - if ((perimeter_generator.config->wall_direction == WallDirection::CounterClockwise) == (loop.is_contour || reverse_thin_wall_hole)) + // Orca: holes run against the contour, except thin wall holes, which run with it. + if ((perimeter_generator.config->wall_direction == WallDirection::CounterClockwise) == (loop.is_contour || loop.is_thin_wall_hole)) eloop->make_counter_clockwise(); else eloop->make_clockwise(); - // Orca: Reverse print order for thin wall holes. - if (reverse_thin_wall_hole) { - std::reverse(out.entities.begin(), out.entities.end()); - } - eloop->inset_idx = loop.depth; if (loop.is_contour) { out.append(std::move(children.entities)); @@ -1499,6 +1494,8 @@ void PerimeterGenerator::process_classic() coord_t min_spacing = coord_t(perimeter_spacing * (1 - INSET_OVERLAP_TOLERANCE)); coord_t ext_min_spacing = coord_t(ext_perimeter_spacing * (1 - INSET_OVERLAP_TOLERANCE)); bool has_gap_fill = this->config->gap_infill_speed.get_at(get_extruder_index(*print_config, this->config->outer_wall_filament_id - 1)) > 0; + // ORCA: Use the smaller width as the lower bound to avoid overestimating safe overlap + const double gap_fill_min_width = 0.2 * std::min(perimeter_width, ext_perimeter_width) * (1 - INSET_OVERLAP_TOLERANCE); // BBS: this flow is for smaller external perimeter for small area coord_t ext_min_spacing_smaller = coord_t(ext_perimeter_spacing * (1 - SMALLER_EXT_INSET_OVERLAP_TOLERANCE)); @@ -1693,6 +1690,15 @@ void PerimeterGenerator::process_classic() last = std::move(offsets); + // ORCA: a thin wall hole has no room for gap fill or an inner wall anywhere beside its outer wall. + if (i == 0 && ! holes[0].empty()) { + const float room = float(0.5 * (ext_perimeter_spacing2 + gap_fill_min_width)); + const Polygons reach = to_polygons(offset2_ex(last, -room, room + float(SCALED_EPSILON))); + for (PerimeterGeneratorLoop &hole : holes[0]) + hole.is_thin_wall_hole = intersection_pl(Polylines{ hole.polygon.split_at_first_point() }, + ClipperUtils::clip_clipper_polygons_with_subject_bbox(reach, get_extents(hole.polygon).inflated(SCALED_EPSILON))).empty(); + } + //BBS: refer to superslicer //store surface for top infill if only_one_wall_top if (i == 0 && i!=loop_number && only_one_wall_top && !surface.is_bridge() && this->upper_slices != NULL) { @@ -1819,7 +1825,7 @@ void PerimeterGenerator::process_classic() steep_overhang_contour = true; steep_overhang_hole = true; } - ExtrusionEntityCollection entities = traverse_loops(*this, contours.front(), thin_walls, steep_overhang_contour, steep_overhang_hole, false); + ExtrusionEntityCollection entities = traverse_loops(*this, contours.front(), thin_walls, steep_overhang_contour, steep_overhang_hole); // All walls are counter-clockwise initially, so we don't need to reorient it if that's what we want if (config->overhang_reverse) { reorient_perimeters(entities, steep_overhang_contour, steep_overhang_hole, @@ -1946,8 +1952,7 @@ void PerimeterGenerator::process_classic() // fill gaps if (! gaps.empty()) { // collapse - // ORCA: Use the smaller width as the lower bound to avoid overestimating safe overlap - double min = 0.2 * std::min(perimeter_width, ext_perimeter_width) * (1 - INSET_OVERLAP_TOLERANCE); + double min = gap_fill_min_width; double max = 2. * perimeter_spacing; ExPolygons gaps_ex = diff_ex( //FIXME offset2 would be enough and cheaper. diff --git a/tests/fff_print/test_perimeters.cpp b/tests/fff_print/test_perimeters.cpp index 1cbd9e9351..070912c307 100644 --- a/tests/fff_print/test_perimeters.cpp +++ b/tests/fff_print/test_perimeters.cpp @@ -856,3 +856,141 @@ TEST_CASE("Fuzzy skin leaves the walls over an empty layer smooth", "[Perimeters CHECK(floating_first_layer >= 0.); CHECK(floating_first_layer < 0.001); } + +namespace { + +// A 20x20x5mm square tube whose walls are `wall` mm thick, except the far one at `far_wall` mm. +Print &square_tube(Print &print, Model &model, const DynamicPrintConfig &config, double wall, double far_wall) +{ + ModelObject *object = model.add_object(); + object->name = "square_tube.stl"; + object->add_volume(make_cube(20., 20., 5.), ModelVolumeType::MODEL_PART, false); + TriangleMesh cavity = make_cube(20. - 2. * wall, 20. - wall - far_wall, 5.); + cavity.translate(float(wall), float(wall), 0.f); + object->add_volume(std::move(cavity), ModelVolumeType::NEGATIVE_VOLUME, false); + object->add_instance(); + object->ensure_on_bed(); + + print.auto_assign_extruders(object); + print.apply(model, config); + print.validate(); + print.set_status_silent(); + print.process(); + return print; +} + +// Every setting the wall thicknesses below are measured against. With these widths the two outer walls of +// a 0.8mm wall touch, a 1mm wall leaves a gap between them for gap fill, and an inner wall needs about 1.5mm. +// Both one wall options are on, so the first and the last layer have a single wall whatever wall_loops asks. +DynamicPrintConfig hole_direction_config(int wall_loops, const char *wall_direction) +{ + DynamicPrintConfig config = DynamicPrintConfig::full_print_config(); + config.set_deserialize_strict({ + { "wall_generator", "classic" }, + { "wall_direction", wall_direction }, + { "wall_loops", wall_loops }, + { "layer_height", 0.2 }, + { "initial_layer_print_height", 0.2 }, + { "outer_wall_line_width", 0.42 }, + { "inner_wall_line_width", 0.45 }, + { "detect_thin_wall", false }, + { "filter_out_gap_fill", 0 }, + { "top_shell_layers", 3 }, + { "bottom_shell_layers", 3 }, + { "only_one_wall_top", true }, + { "only_one_wall_first_layer", true }, + { "overhang_reverse", false }, + { "sparse_infill_density", "15%" }, + }); + return config; +} + +// The outer walls of a layer, contours and holes apart, and how many inner walls and gap fills it has. +struct WallDirections { + std::vector contours_ccw; + std::vector holes_ccw; + int inner_walls = 0; + size_t gap_fills = 0; +}; + +std::vector wall_directions(const Print &print) +{ + std::vector out; + for (const Layer *layer : print.objects().front()->layers()) { + WallDirections &walls = out.emplace_back(); + for (const LayerRegion *region : layer->regions()) { + walls.gap_fills += region->thin_fills.flatten().entities.size(); + for (const ExtrusionEntity *entity : region->perimeters.flatten().entities) { + if (! entity->is_loop()) + continue; + const ExtrusionLoop *loop = static_cast(entity); + if (loop->inset_idx > 0) + ++ walls.inner_walls; + else + (loop->loop_role() == elrHole ? walls.holes_ccw : walls.contours_ccw).push_back(loop->polygon().is_counter_clockwise()); + } + } + } + return out; +} + +} // namespace + +// Holes run against the wall direction, so the inside of a hole keeps its direction on the layers where the +// hole opens into the contour. That holds on every layer, the single wall ones included, as soon as anything +// fits beside the outer wall of the hole: infill, an inner wall, or only gap fill. The 1mm tube with a 3mm far +// side is the shape that used to flip, where the far side has an inner wall on most layers and gap fill runs +// around the rest. +TEST_CASE("Holes run against the wall direction", "[Perimeters]") +{ + const auto [wall, far_wall] = GENERATE(std::make_pair(7., 7.), std::make_pair(1., 1.), std::make_pair(1., 3.)); + const int wall_loops = GENERATE(1, 2); + const char *wall_direction = GENERATE("ccw", "cw"); + CAPTURE(wall, far_wall, wall_loops, wall_direction); + + Print print; + Model model; + square_tube(print, model, hole_direction_config(wall_loops, wall_direction), wall, far_wall); + const std::vector layers = wall_directions(print); + // 5mm at 0.2mm layers. + REQUIRE(layers.size() == 25); + // Without gap fill between the outer walls the thin tubes would test the case below instead. + if (wall < 2.) + REQUIRE(layers[layers.size() / 2].gap_fills > 0); + + const bool ccw = std::string(wall_direction) == "ccw"; + for (size_t i = 0; i < layers.size(); ++ i) { + CAPTURE(i); + REQUIRE(layers[i].contours_ccw.size() == 1); + REQUIRE(layers[i].holes_ccw.size() == 1); + CHECK(layers[i].contours_ccw.front() == ccw); + CHECK(layers[i].holes_ccw.front() == ! ccw); + } +} + +// A hole whose outer wall touches the contour's all around, with nothing between them, runs with the contour +// on every layer, so the two walls of a thin tube are laid side by side in the same direction. +TEST_CASE("A hole whose outer wall touches the contour's runs with it", "[Perimeters]") +{ + const int wall_loops = GENERATE(1, 2); + const char *wall_direction = GENERATE("ccw", "cw"); + CAPTURE(wall_loops, wall_direction); + + Print print; + Model model; + square_tube(print, model, hole_direction_config(wall_loops, wall_direction), 0.8, 0.8); + const std::vector layers = wall_directions(print); + REQUIRE(layers.size() == 25); + // Nothing fits between the two outer walls. + REQUIRE(layers[layers.size() / 2].gap_fills == 0); + REQUIRE(layers[layers.size() / 2].inner_walls == 0); + + const bool ccw = std::string(wall_direction) == "ccw"; + for (size_t i = 0; i < layers.size(); ++ i) { + CAPTURE(i); + REQUIRE(layers[i].contours_ccw.size() == 1); + REQUIRE(layers[i].holes_ccw.size() == 1); + CHECK(layers[i].contours_ccw.front() == ccw); + CHECK(layers[i].holes_ccw.front() == ccw); + } +}