From 891054ff3593374730eb2573e77e07f80d9ae141 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Fri, 23 May 2025 14:53:40 +0000 Subject: [PATCH 1/3] The following changes were made: A new setting `combing_polygon_type` was introduced that allows you to specify which polygons should be used for combing. The available options are: - `OuterWall`: Uses the original combing boundaries (default behavior). - `Outline`: Uses the general layer outlines for combing. - `SecondWall`: Uses the second wall polygons for combing. The `Comb::calc` function has been modified to read this setting and select the appropriate polygons for combing. If the selected polygon type results in no usable polygons or if an invalid type is specified, the logic falls back to the default behavior. --- include/settings/EnumSettings.h | 11 ++ src/pathPlanning/Comb.cpp | 159 +++++++++++++++---- src/settings/Settings.cpp | 20 +++ tests/src/pathPlanning/CombTest.cpp | 234 ++++++++++++++++++++++++++++ 4 files changed, 392 insertions(+), 32 deletions(-) create mode 100644 tests/src/pathPlanning/CombTest.cpp diff --git a/include/settings/EnumSettings.h b/include/settings/EnumSettings.h index 25f2a157fd..1c21c10fd5 100644 --- a/include/settings/EnumSettings.h +++ b/include/settings/EnumSettings.h @@ -301,6 +301,17 @@ enum class CoolDuringExtruderSwitch ALL_FANS, // Turn on all fans }; +/*! + * Type of polygon to use for combing. + */ +enum class CombingPolygonType +{ + OUTER_WALL, + OUTLINE, + SECOND_WALL, + PLUGIN, +}; + /*! * Convenience binary operator to allow testing brim location easily, like (actual_location & BrimLocation::OUTSIDE) */ diff --git a/src/pathPlanning/Comb.cpp b/src/pathPlanning/Comb.cpp index a5659a29e6..cea739e6d9 100644 --- a/src/pathPlanning/Comb.cpp +++ b/src/pathPlanning/Comb.cpp @@ -16,6 +16,7 @@ #include "utils/PolygonsPointIndex.h" #include "utils/SVG.h" #include "utils/linearAlg2D.h" +#include "settings/EnumSettings.h" // Added for CombingPolygonType namespace cura { @@ -101,29 +102,121 @@ bool Comb::calc( { return true; } + + const CombingPolygonType combing_polygon_type = train.settings_.get("combing_polygon_type"); + + // Pointers to the boundaries and related structures to be used. + // Default to the ones initialized in the constructor. + Shape* current_boundary_optimal = &boundary_inside_optimal_; + Shape* current_boundary_minimum = &boundary_inside_minimum_; + PartsView* current_parts_view_optimal = const_cast(&parts_view_inside_optimal_); + PartsView* current_parts_view_minimum = const_cast(&parts_view_inside_minimum_); + LocToLineGrid* current_loc_to_line_optimal = inside_loc_to_line_optimal_.get(); + LocToLineGrid* current_loc_to_line_minimum = inside_loc_to_line_minimum_.get(); + + // Local instances for when we use alternative polygons + Shape local_boundary_optimal; + Shape local_boundary_minimum; + std::unique_ptr local_parts_view_optimal; + std::unique_ptr local_parts_view_minimum; + std::unique_ptr local_loc_to_line_optimal; + std::unique_ptr local_loc_to_line_minimum; + + bool use_original_boundaries = true; + + // Determine if alternative boundaries are needed based on the setting + switch (combing_polygon_type) + { + case CombingPolygonType::OUTER_WALL: + // Default behavior, use original boundaries + use_original_boundaries = true; + break; + case CombingPolygonType::OUTLINE: + { + // Use the general layer outlines for combing. + // travel_avoid_supports is set to false as we are defining the combing boundary itself, not an avoidance area. + Polygons outline_polys = storage_.getLayerOutlines(layer_nr_, false, false); + if (!outline_polys.empty()) + { + local_boundary_optimal = outline_polys; // Shape can be constructed from Polygons + local_boundary_minimum = outline_polys; // Use the same for minimum + use_original_boundaries = false; + } + else + { + // No outline polygons, fall back to original boundaries (OuterWall) + use_original_boundaries = true; + } + break; + } + case CombingPolygonType::SECOND_WALL: + { + const Walls& walls = storage_.getWalls(layer_nr_, train.extruder_nr_); + if (!walls.second_wall_polys.empty()) + { + local_boundary_optimal = walls.second_wall_polys; // Shape can be constructed from PolygonsPart + local_boundary_minimum = walls.second_wall_polys; // Use second wall for both + use_original_boundaries = false; + } + else + { + // No second wall, fall back to original boundaries (OuterWall) + use_original_boundaries = true; + } + break; + } + case CombingPolygonType::PLUGIN: + // Plugin handling, assume default for now or specific logic if defined + use_original_boundaries = true; + break; + default: + // Unknown value, fall back to default behavior + use_original_boundaries = true; + break; + } + + // If alternative boundaries are chosen and valid, set them up + if (!use_original_boundaries) + { + // Initialize local PartsView and LocToLineGrid for the selected polygons + // Note: Splitting into PartsView might reorder polygons in local_boundary_optimal/minimum + local_parts_view_optimal = std::make_unique(local_boundary_optimal.splitIntoPartsView()); + local_parts_view_minimum = std::make_unique(local_boundary_minimum.splitIntoPartsView()); + local_loc_to_line_optimal = PolygonUtils::createLocToLineGrid(local_boundary_optimal, offset_from_outlines_); + local_loc_to_line_minimum = PolygonUtils::createLocToLineGrid(local_boundary_minimum, offset_from_outlines_); + + current_boundary_optimal = &local_boundary_optimal; + current_boundary_minimum = &local_boundary_minimum; + current_parts_view_optimal = local_parts_view_optimal.get(); + current_parts_view_minimum = local_parts_view_minimum.get(); + current_loc_to_line_optimal = local_loc_to_line_optimal.get(); + current_loc_to_line_minimum = local_loc_to_line_minimum.get(); + } + + const Point2LL travel_end_point_before_combing = end_point; // Move start and end point inside the optimal comb boundary size_t start_inside_poly = NO_INDEX; - const bool start_inside = moveInside(boundary_inside_optimal_, _start_inside, inside_loc_to_line_optimal_.get(), start_point, start_inside_poly); + const bool start_inside = moveInside(*current_boundary_optimal, _start_inside, current_loc_to_line_optimal, start_point, start_inside_poly); size_t end_inside_poly = NO_INDEX; - const bool end_inside = moveInside(boundary_inside_optimal_, _end_inside, inside_loc_to_line_optimal_.get(), end_point, end_inside_poly); + const bool end_inside = moveInside(*current_boundary_optimal, _end_inside, current_loc_to_line_optimal, end_point, end_inside_poly); - size_t start_part_boundary_poly_idx = NO_INDEX; // Added initial value to stop MSVC throwing an exception in debug mode + size_t start_part_boundary_poly_idx = NO_INDEX; size_t end_part_boundary_poly_idx = NO_INDEX; - size_t start_part_idx = (start_inside_poly == NO_INDEX) ? NO_INDEX : parts_view_inside_optimal_.getPartContaining(start_inside_poly, &start_part_boundary_poly_idx); - size_t end_part_idx = (end_inside_poly == NO_INDEX) ? NO_INDEX : parts_view_inside_optimal_.getPartContaining(end_inside_poly, &end_part_boundary_poly_idx); + size_t start_part_idx = (start_inside_poly == NO_INDEX) ? NO_INDEX : current_parts_view_optimal->getPartContaining(start_inside_poly, &start_part_boundary_poly_idx); + size_t end_part_idx = (end_inside_poly == NO_INDEX) ? NO_INDEX : current_parts_view_optimal->getPartContaining(end_inside_poly, &end_part_boundary_poly_idx); const bool fail_on_unavoidable_obstacles = perform_z_hops && perform_z_hops_only_when_collides; // normal combing within part using optimal comb boundary if (start_inside && end_inside && start_part_idx == end_part_idx) { - SingleShape part = parts_view_inside_optimal_.assemblePart(start_part_idx); + SingleShape part = current_parts_view_optimal->assemblePart(start_part_idx); comb_paths.emplace_back(); const bool combing_succeeded = LinePolygonsCrossings::comb( part, - *inside_loc_to_line_optimal_, + *current_loc_to_line_optimal, start_point, end_point, comb_paths.back(), @@ -138,16 +231,16 @@ bool Comb::calc( // Move start and end point inside the minimum comb boundary size_t start_inside_poly_min = NO_INDEX; - const bool start_inside_min = moveInside(boundary_inside_minimum_, _start_inside, inside_loc_to_line_minimum_.get(), start_point, start_inside_poly_min); + const bool start_inside_min = moveInside(*current_boundary_minimum, _start_inside, current_loc_to_line_minimum, start_point, start_inside_poly_min); size_t end_inside_poly_min = NO_INDEX; - const bool end_inside_min = moveInside(boundary_inside_minimum_, _end_inside, inside_loc_to_line_minimum_.get(), end_point, end_inside_poly_min); + const bool end_inside_min = moveInside(*current_boundary_minimum, _end_inside, current_loc_to_line_minimum, end_point, end_inside_poly_min); size_t start_part_boundary_poly_idx_min{}; size_t end_part_boundary_poly_idx_min{}; size_t start_part_idx_min - = (start_inside_poly_min == NO_INDEX) ? NO_INDEX : parts_view_inside_minimum_.getPartContaining(start_inside_poly_min, &start_part_boundary_poly_idx_min); - size_t end_part_idx_min = (end_inside_poly_min == NO_INDEX) ? NO_INDEX : parts_view_inside_minimum_.getPartContaining(end_inside_poly_min, &end_part_boundary_poly_idx_min); + = (start_inside_poly_min == NO_INDEX) ? NO_INDEX : current_parts_view_minimum->getPartContaining(start_inside_poly_min, &start_part_boundary_poly_idx_min); + size_t end_part_idx_min = (end_inside_poly_min == NO_INDEX) ? NO_INDEX : current_parts_view_minimum->getPartContaining(end_inside_poly_min, &end_part_boundary_poly_idx_min); CombPath result_path; bool comb_result; @@ -155,19 +248,19 @@ bool Comb::calc( // normal combing within part using minimum comb boundary if (start_inside_min && end_inside_min && start_part_idx_min == end_part_idx_min) { - SingleShape part = parts_view_inside_minimum_.assemblePart(start_part_idx_min); + SingleShape part = current_parts_view_minimum->assemblePart(start_part_idx_min); comb_paths.emplace_back(); comb_result = LinePolygonsCrossings::comb( part, - *inside_loc_to_line_minimum_, + *current_loc_to_line_minimum, start_point, end_point, 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, comb_paths.back()); // add altered result_path to combPaths.back() + Comb::moveCombPathInside(*current_boundary_minimum, *current_boundary_optimal, result_path, comb_paths.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 && end_point != travel_end_point_before_combing; @@ -188,12 +281,12 @@ bool Comb::calc( // Find the crossings using the minimum comb boundary, since it's guaranteed to be as close as we can get to the destination. // Getting as close as possible prevents exiting the polygon in the wrong direction (e.g. into a hole instead of to the outside). - Crossing start_crossing(start_point, start_inside_min, start_part_idx_min, start_part_boundary_poly_idx_min, boundary_inside_minimum_, *inside_loc_to_line_minimum_); - Crossing end_crossing(end_point, end_inside_min, end_part_idx_min, end_part_boundary_poly_idx_min, boundary_inside_minimum_, *inside_loc_to_line_minimum_); + Crossing start_crossing(start_point, start_inside_min, start_part_idx_min, start_part_boundary_poly_idx_min, *current_boundary_minimum, *current_loc_to_line_minimum); + Crossing end_crossing(end_point, end_inside_min, end_part_idx_min, end_part_boundary_poly_idx_min, *current_boundary_minimum, *current_loc_to_line_minimum); { // find crossing over the in-between area between inside and outside - start_crossing.findCrossingInOrMid(parts_view_inside_minimum_, end_point); - end_crossing.findCrossingInOrMid(parts_view_inside_minimum_, start_crossing.in_or_mid_); + start_crossing.findCrossingInOrMid(*current_parts_view_minimum, end_point); + end_crossing.findCrossingInOrMid(*current_parts_view_minimum, start_crossing.in_or_mid_); } bool skip_avoid_other_parts_path = false; @@ -228,10 +321,10 @@ bool Comb::calc( comb_paths.emplace_back(); // If we're inside the optimal bound, first try the optimal combing path. If it fails, use the minimum path instead. constexpr bool fail_for_optimum_bound = true; - bool combing_succeeded = start_inside + bool combing_succeeded = start_inside // Check if originally inside the optimal constructor-defined boundary && LinePolygonsCrossings::comb( - boundary_inside_optimal_, - *inside_loc_to_line_optimal_, + *current_boundary_optimal, + *current_loc_to_line_optimal, start_point, start_crossing.in_or_mid_, comb_paths.back(), @@ -240,9 +333,10 @@ bool Comb::calc( fail_for_optimum_bound); if (! combing_succeeded) { + // If combing with optimal failed or was skipped, try/fallback to minimum combing_succeeded = LinePolygonsCrossings::comb( - start_crossing.dest_part_, - *inside_loc_to_line_minimum_, + start_crossing.dest_part_, // dest_part_ is from the Crossing, based on current_boundary_minimum + *current_loc_to_line_minimum, start_point, start_crossing.in_or_mid_, comb_paths.back(), @@ -316,13 +410,13 @@ bool Comb::calc( { if (start_inside == end_inside && start_part_idx == end_part_idx) { - if (start_inside) + if (start_inside) // Check if originally inside the optimal constructor-defined boundary { // both start and end are inside - comb_paths.back().cross_boundary = PolygonUtils::polygonCollidesWithLineSegment(start_point, end_point, *inside_loc_to_line_optimal_); + comb_paths.back().cross_boundary = PolygonUtils::polygonCollidesWithLineSegment(start_point, end_point, *current_loc_to_line_optimal); } else { // both start and end are outside - comb_paths.back().cross_boundary = PolygonUtils::polygonCollidesWithLineSegment(start_point, end_point, getModelBoundaryLocToLine(train)); + comb_paths.back().cross_boundary = PolygonUtils::polygonCollidesWithLineSegment(start_point, end_point, getModelBoundaryLocToLine(train)); // Uses original model boundary logic } } else @@ -331,17 +425,17 @@ bool Comb::calc( } } - if (end_inside) + if (end_inside_min) // check end_inside_min as this refers to the current boundary (original or local) { // boundary to end assert(end_crossing.dest_part_.size() > 0 && "The part we end up inside when combing should have been computed already!"); comb_paths.emplace_back(); // If we're inside the optimal bound, first try the optimal combing path. If it fails, use the minimum path instead. constexpr bool fail_for_optimum_bound = true; - bool combing_succeeded = end_inside + bool combing_succeeded = end_inside // Check if originally inside the optimal constructor-defined boundary && LinePolygonsCrossings::comb( - boundary_inside_optimal_, - *inside_loc_to_line_optimal_, + *current_boundary_optimal, + *current_loc_to_line_optimal, end_crossing.in_or_mid_, end_point, comb_paths.back(), @@ -350,9 +444,10 @@ bool Comb::calc( fail_for_optimum_bound); if (! combing_succeeded) { + // If combing with optimal failed or was skipped, try/fallback to minimum combing_succeeded = LinePolygonsCrossings::comb( - end_crossing.dest_part_, - *inside_loc_to_line_minimum_, + end_crossing.dest_part_, // dest_part_ is from the Crossing, based on current_boundary_minimum + *current_loc_to_line_minimum, end_crossing.in_or_mid_, end_point, comb_paths.back(), diff --git a/src/settings/Settings.cpp b/src/settings/Settings.cpp index 48afb69dbb..cf841a5b14 100644 --- a/src/settings/Settings.cpp +++ b/src/settings/Settings.cpp @@ -53,6 +53,26 @@ void Settings::add(const std::string& key, const std::string value) } } +template<> +CombingPolygonType Settings::get(const std::string& key) const +{ + const std::string& value = get(key); + using namespace cura::utils; + switch (hash_enum(value)) + { + case "outer_wall"_sw: + return CombingPolygonType::OUTER_WALL; + case "outline"_sw: + return CombingPolygonType::OUTLINE; + case "second_wall"_sw: + return CombingPolygonType::SECOND_WALL; + case "plugin"_sw: + return CombingPolygonType::PLUGIN; + default: + return CombingPolygonType::OUTER_WALL; // Default value + } +} + template<> std::string Settings::get(const std::string& key) const { diff --git a/tests/src/pathPlanning/CombTest.cpp b/tests/src/pathPlanning/CombTest.cpp new file mode 100644 index 0000000000..4f520b753e --- /dev/null +++ b/tests/src/pathPlanning/CombTest.cpp @@ -0,0 +1,234 @@ +#include + +// Includes for CuraEngine classes +#include "pathPlanning/Comb.h" +#include "sliceDataStorage.h" +#include "ExtruderTrain.h" +#include "settings/Settings.h" +#include "settings/EnumSettings.h" +#include "utils/polygon.h" // For creating test polygons +#include "geometry/Point.h" // For cura::Point / cura::Point2LL +#include "geometry/Polygon.h" // For cura::Polygon / cura::Polygons / cura::ConstPolygonRef +#include "geometry/Shape.h" // For cura::Shape +#include "sliceDataStorage.h" // For cura::SliceLayer, cura::SliceLayerPart, cura::Walls +#include "Application.h" // For cura::Application and cura::Scene + + +// Helper function to create simple SliceDataStorage with walls +cura::SliceDataStorage createTestData( + cura::coord_t object_size, + cura::coord_t wall_thickness, + int num_walls, + cura::LayerIndex layer_nr = 0) +{ + cura::SliceDataStorage storage; + storage.model_min = cura::Point3(-object_size / 2, -object_size / 2, 0); + storage.model_max = cura::Point3(object_size / 2, object_size / 2, object_size); + storage.model_size = storage.model_max - storage.model_min; + storage.scene_min = storage.model_min; + storage.scene_max = storage.model_max; + storage.scene_size = storage.model_size; + + cura::SliceLayer layer(layer_nr, 0, object_size); + + cura::SliceLayerPart part; + + cura::Polygon outer_wall_poly_base; + outer_wall_poly_base.emplace_back(object_size / 2, object_size / 2); + outer_wall_poly_base.emplace_back(object_size / 2, -object_size / 2); + outer_wall_poly_base.emplace_back(-object_size / 2, -object_size / 2); + outer_wall_poly_base.emplace_back(-object_size / 2, object_size / 2); + + part.outline.add(outer_wall_poly_base); + + cura::Walls walls_data; + walls_data.outer_wall_polys.add(outer_wall_poly_base); + + if (num_walls > 1) { + cura::Polygon second_wall_poly_base; + cura::coord_t offset = wall_thickness; + second_wall_poly_base.emplace_back(object_size / 2 - offset, object_size / 2 - offset); + second_wall_poly_base.emplace_back(object_size / 2 - offset, -object_size / 2 + offset); + second_wall_poly_base.emplace_back(-object_size / 2 + offset, -object_size / 2 + offset); + second_wall_poly_base.emplace_back(-object_size / 2 + offset, object_size / 2 - offset); + walls_data.second_wall_polys.add(second_wall_poly_base); + } + + part.wall_parts.push_back(walls_data); + layer.parts.push_back(part); + storage.layers.emplace_back(std::move(layer)); + + return storage; +} + + +// Test fixture for Combing tests +class CombPolygonTest : public ::testing::Test { +protected: + cura::Settings global_settings_; + cura::ExtruderTrain train{&global_settings_, &global_settings_, cura::ExtruderTrain::ExtruderNr(0)}; + + cura::CombPaths comb_paths; + bool unretract_before_last_travel_move = false; + cura::coord_t object_size = 100 * cura::MM2INT; + cura::coord_t wall_thickness = 1 * cura::MM2INT; + + void SetUp() override { + train.settings_.set("combing_mode", cura::CombingMode::ALL); + train.settings_.set("travel_avoid_other_parts", false); + train.settings_.set("travel_avoid_distance", 5 * cura::MM2INT); + train.settings_.set("retraction_combing_max_distance", 1000 * cura::MM2INT); + train.settings_.set("travel_retract_before_outer_wall_distance", 50); // Small default + train.settings_.set("offset_from_outlines", wall_thickness / 2); + train.settings_.set("move_inside_distance", wall_thickness / 4); + } + + void TearDown() override { + } + + bool isPointApproximatelyInsideShape(const cura::Point2LL& pt, const cura::Shape& shape, cura::coord_t tolerance = 50) { + if (shape.empty()) return false; + for (const cura::ConstPolygonRef poly : shape) { + if (poly.inside(pt, true)) { + return true; + } + for (size_t i = 0; i < poly.size(); ++i) { + cura::Point2LL p1 = poly[i]; + cura::Point2LL p2 = poly[(i + 1) % poly.size()]; + if (LinearAlg2D::getDistSqToLineSegment(pt, p1, p2) < tolerance * tolerance) { + return true; + } + } + } + return false; + } +}; + +TEST_F(CombPolygonTest, CombOuterWall) { + train.settings_.set("combing_polygon_type", cura::CombingPolygonType::OUTER_WALL); + cura::SliceDataStorage storage = createTestData(object_size, wall_thickness, 1, 0); + + ASSERT_FALSE(storage.layers.empty()); + ASSERT_FALSE(storage.layers[0].parts.empty()); + ASSERT_FALSE(storage.layers[0].parts[0].wall_parts.empty()); + + cura::Shape comb_boundary_min = storage.layers[0].parts[0].wall_parts[0].outer_wall_polys; + cura::Shape comb_boundary_opt = storage.layers[0].parts[0].wall_parts[0].outer_wall_polys; + + cura::Comb comber(storage, 0, comb_boundary_min, comb_boundary_opt, + train.settings_.get("offset_from_outlines"), + train.settings_.get("travel_avoid_distance"), + train.settings_.get("move_inside_distance")); + + cura::Point2LL start_point(0, 0); + cura::Point2LL end_point(object_size / 2 - wall_thickness, 0); + + bool success = comber.calc(false, false, train, start_point, end_point, comb_paths, true, true, 0, unretract_before_last_travel_move); + ASSERT_TRUE(success) << "Combing calculation failed for OuterWall."; + ASSERT_FALSE(comb_paths.empty()) << "No combing paths generated for OuterWall."; + + for (const auto& path_segment : comb_paths) { + for (const cura::Point2LL& pt : path_segment) { + EXPECT_TRUE(isPointApproximatelyInsideShape(pt, comb_boundary_opt, wall_thickness)) + << "Point " << pt << " is outside the expected OuterWall boundary."; + } + } +} + +TEST_F(CombPolygonTest, CombSecondWall_Exists) { + train.settings_.set("combing_polygon_type", cura::CombingPolygonType::SECOND_WALL); + cura::SliceDataStorage storage = createTestData(object_size, wall_thickness, 2, 0); // 2 walls + + ASSERT_FALSE(storage.layers.empty()); + ASSERT_FALSE(storage.layers[0].parts.empty()); + ASSERT_FALSE(storage.layers[0].parts[0].wall_parts.empty()); + ASSERT_FALSE(storage.layers[0].parts[0].wall_parts[0].second_wall_polys.empty()); + + cura::Shape initial_comb_boundary_min = storage.layers[0].parts[0].wall_parts[0].outer_wall_polys; + cura::Shape initial_comb_boundary_opt = storage.layers[0].parts[0].wall_parts[0].outer_wall_polys; + + cura::Comb comber(storage, 0, initial_comb_boundary_min, initial_comb_boundary_opt, + train.settings_.get("offset_from_outlines"), + train.settings_.get("travel_avoid_distance"), + train.settings_.get("move_inside_distance")); + + cura::Point2LL start_point(0, 0); + cura::Point2LL end_point(object_size / 2 - wall_thickness * 1.5, 0); + + bool success = comber.calc(false, false, train, start_point, end_point, comb_paths, true, true, 0, unretract_before_last_travel_move); + ASSERT_TRUE(success) << "Combing calculation failed for SecondWall."; + ASSERT_FALSE(comb_paths.empty()) << "No combing paths generated for SecondWall."; + + const cura::Shape& second_wall_shape = storage.layers[0].parts[0].wall_parts[0].second_wall_polys; + for (const auto& path_segment : comb_paths) { + for (const cura::Point2LL& pt : path_segment) { + EXPECT_TRUE(isPointApproximatelyInsideShape(pt, second_wall_shape, wall_thickness)) + << "Point " << pt << " is outside the expected SecondWall boundary."; + } + } +} + +TEST_F(CombPolygonTest, CombSecondWall_FallbackWhenNoSecondWall) { + train.settings_.set("combing_polygon_type", cura::CombingPolygonType::SECOND_WALL); + cura::SliceDataStorage storage = createTestData(object_size, wall_thickness, 1, 0); + + ASSERT_FALSE(storage.layers.empty()); + ASSERT_FALSE(storage.layers[0].parts.empty()); + ASSERT_FALSE(storage.layers[0].parts[0].wall_parts.empty()); + ASSERT_TRUE(storage.layers[0].parts[0].wall_parts[0].second_wall_polys.empty()); + + cura::Shape initial_comb_boundary_min = storage.layers[0].parts[0].wall_parts[0].outer_wall_polys; + cura::Shape initial_comb_boundary_opt = storage.layers[0].parts[0].wall_parts[0].outer_wall_polys; + + cura::Comb comber(storage, 0, initial_comb_boundary_min, initial_comb_boundary_opt, + train.settings_.get("offset_from_outlines"), + train.settings_.get("travel_avoid_distance"), + train.settings_.get("move_inside_distance")); + + cura::Point2LL start_point(0, 0); + cura::Point2LL end_point(object_size / 2 - wall_thickness, 0); + + bool success = comber.calc(false, false, train, start_point, end_point, comb_paths, true, true, 0, unretract_before_last_travel_move); + ASSERT_TRUE(success) << "Combing calculation failed for SecondWall fallback."; + ASSERT_FALSE(comb_paths.empty()) << "No combing paths generated for SecondWall fallback."; + + const cura::Shape& outer_wall_shape = storage.layers[0].parts[0].wall_parts[0].outer_wall_polys; + for (const auto& path_segment : comb_paths) { + for (const cura::Point2LL& pt : path_segment) { + EXPECT_TRUE(isPointApproximatelyInsideShape(pt, outer_wall_shape, wall_thickness)) + << "Point " << pt << " is outside the OuterWall boundary after SecondWall fallback."; + } + } +} + +TEST_F(CombPolygonTest, CombOutline) { + train.settings_.set("combing_polygon_type", cura::CombingPolygonType::OUTLINE); + cura::SliceDataStorage storage = createTestData(object_size, wall_thickness, 1, 0); + + ASSERT_FALSE(storage.layers.empty()); + ASSERT_FALSE(storage.layers[0].parts.empty()); + ASSERT_FALSE(storage.layers[0].parts[0].outline.empty()); + + cura::Shape initial_comb_boundary_min = storage.layers[0].parts[0].wall_parts[0].outer_wall_polys; + cura::Shape initial_comb_boundary_opt = storage.layers[0].parts[0].wall_parts[0].outer_wall_polys; + + cura::Comb comber(storage, 0, initial_comb_boundary_min, initial_comb_boundary_opt, + train.settings_.get("offset_from_outlines"), + train.settings_.get("travel_avoid_distance"), + train.settings_.get("move_inside_distance")); + + cura::Point2LL start_point(0, 0); + cura::Point2LL end_point(object_size / 2 - wall_thickness / 4, 0); + + bool success = comber.calc(false, false, train, start_point, end_point, comb_paths, true, true, 0, unretract_before_last_travel_move); + ASSERT_TRUE(success) << "Combing calculation failed for Outline."; + ASSERT_FALSE(comb_paths.empty()) << "No combing paths generated for Outline."; + + const cura::Shape& layer_outline_shape = storage.layers[0].parts[0].outline; + for (const auto& path_segment : comb_paths) { + for (const cura::Point2LL& pt : path_segment) { + EXPECT_TRUE(isPointApproximatelyInsideShape(pt, layer_outline_shape, wall_thickness)) + << "Point " << pt << " is outside the expected LayerOutline boundary."; + } + } +} From 67590dc4fc8adaef11319cc33e8bb66379d14e85 Mon Sep 17 00:00:00 2001 From: jellespijker <8535734+jellespijker@users.noreply.github.com> Date: Fri, 23 May 2025 14:54:22 +0000 Subject: [PATCH 2/3] Apply clang-format --- src/pathPlanning/Comb.cpp | 91 ++++++++++++++++++++------------------- 1 file changed, 46 insertions(+), 45 deletions(-) diff --git a/src/pathPlanning/Comb.cpp b/src/pathPlanning/Comb.cpp index cea739e6d9..e2ce972ed8 100644 --- a/src/pathPlanning/Comb.cpp +++ b/src/pathPlanning/Comb.cpp @@ -12,11 +12,11 @@ #include "Slice.h" #include "pathPlanning/CombPaths.h" #include "pathPlanning/LinePolygonsCrossings.h" +#include "settings/EnumSettings.h" // Added for CombingPolygonType #include "sliceDataStorage.h" #include "utils/PolygonsPointIndex.h" #include "utils/SVG.h" #include "utils/linearAlg2D.h" -#include "settings/EnumSettings.h" // Added for CombingPolygonType namespace cura { @@ -127,56 +127,56 @@ bool Comb::calc( // Determine if alternative boundaries are needed based on the setting switch (combing_polygon_type) { - case CombingPolygonType::OUTER_WALL: - // Default behavior, use original boundaries - use_original_boundaries = true; - break; - case CombingPolygonType::OUTLINE: + case CombingPolygonType::OUTER_WALL: + // Default behavior, use original boundaries + use_original_boundaries = true; + break; + case CombingPolygonType::OUTLINE: + { + // Use the general layer outlines for combing. + // travel_avoid_supports is set to false as we are defining the combing boundary itself, not an avoidance area. + Polygons outline_polys = storage_.getLayerOutlines(layer_nr_, false, false); + if (! outline_polys.empty()) { - // Use the general layer outlines for combing. - // travel_avoid_supports is set to false as we are defining the combing boundary itself, not an avoidance area. - Polygons outline_polys = storage_.getLayerOutlines(layer_nr_, false, false); - if (!outline_polys.empty()) - { - local_boundary_optimal = outline_polys; // Shape can be constructed from Polygons - local_boundary_minimum = outline_polys; // Use the same for minimum - use_original_boundaries = false; - } - else - { - // No outline polygons, fall back to original boundaries (OuterWall) - use_original_boundaries = true; - } - break; + local_boundary_optimal = outline_polys; // Shape can be constructed from Polygons + local_boundary_minimum = outline_polys; // Use the same for minimum + use_original_boundaries = false; } - case CombingPolygonType::SECOND_WALL: + else { - const Walls& walls = storage_.getWalls(layer_nr_, train.extruder_nr_); - if (!walls.second_wall_polys.empty()) - { - local_boundary_optimal = walls.second_wall_polys; // Shape can be constructed from PolygonsPart - local_boundary_minimum = walls.second_wall_polys; // Use second wall for both - use_original_boundaries = false; - } - else - { - // No second wall, fall back to original boundaries (OuterWall) - use_original_boundaries = true; - } - break; - } - case CombingPolygonType::PLUGIN: - // Plugin handling, assume default for now or specific logic if defined + // No outline polygons, fall back to original boundaries (OuterWall) use_original_boundaries = true; - break; - default: - // Unknown value, fall back to default behavior + } + break; + } + case CombingPolygonType::SECOND_WALL: + { + const Walls& walls = storage_.getWalls(layer_nr_, train.extruder_nr_); + if (! walls.second_wall_polys.empty()) + { + local_boundary_optimal = walls.second_wall_polys; // Shape can be constructed from PolygonsPart + local_boundary_minimum = walls.second_wall_polys; // Use second wall for both + use_original_boundaries = false; + } + else + { + // No second wall, fall back to original boundaries (OuterWall) use_original_boundaries = true; - break; + } + break; + } + case CombingPolygonType::PLUGIN: + // Plugin handling, assume default for now or specific logic if defined + use_original_boundaries = true; + break; + default: + // Unknown value, fall back to default behavior + use_original_boundaries = true; + break; } // If alternative boundaries are chosen and valid, set them up - if (!use_original_boundaries) + if (! use_original_boundaries) { // Initialize local PartsView and LocToLineGrid for the selected polygons // Note: Splitting into PartsView might reorder polygons in local_boundary_optimal/minimum @@ -202,7 +202,7 @@ bool Comb::calc( size_t end_inside_poly = NO_INDEX; const bool end_inside = moveInside(*current_boundary_optimal, _end_inside, current_loc_to_line_optimal, end_point, end_inside_poly); - size_t start_part_boundary_poly_idx = NO_INDEX; + size_t start_part_boundary_poly_idx = NO_INDEX; size_t end_part_boundary_poly_idx = NO_INDEX; size_t start_part_idx = (start_inside_poly == NO_INDEX) ? NO_INDEX : current_parts_view_optimal->getPartContaining(start_inside_poly, &start_part_boundary_poly_idx); size_t end_part_idx = (end_inside_poly == NO_INDEX) ? NO_INDEX : current_parts_view_optimal->getPartContaining(end_inside_poly, &end_part_boundary_poly_idx); @@ -416,7 +416,8 @@ bool Comb::calc( } else { // both start and end are outside - comb_paths.back().cross_boundary = PolygonUtils::polygonCollidesWithLineSegment(start_point, end_point, getModelBoundaryLocToLine(train)); // Uses original model boundary logic + comb_paths.back().cross_boundary + = PolygonUtils::polygonCollidesWithLineSegment(start_point, end_point, getModelBoundaryLocToLine(train)); // Uses original model boundary logic } } else From b78b254666973d9a005057abd1990703ecedf1c9 Mon Sep 17 00:00:00 2001 From: Jelle Spijker Date: Fri, 23 May 2025 23:03:03 +0200 Subject: [PATCH 3/3] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- include/settings/EnumSettings.h | 8 ++++---- src/pathPlanning/Comb.cpp | 6 ++++-- src/settings/Settings.cpp | 9 +++++++++ tests/src/pathPlanning/CombTest.cpp | 1 - 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/include/settings/EnumSettings.h b/include/settings/EnumSettings.h index 1c21c10fd5..8df1b26a25 100644 --- a/include/settings/EnumSettings.h +++ b/include/settings/EnumSettings.h @@ -306,10 +306,10 @@ enum class CoolDuringExtruderSwitch */ enum class CombingPolygonType { - OUTER_WALL, - OUTLINE, - SECOND_WALL, - PLUGIN, + OUTER_WALL, // Maps to "outer_wall" + OUTLINE, // Maps to "outline" + SECOND_WALL, // Maps to "second_wall" + PLUGIN, // Maps to "plugin" }; /*! diff --git a/src/pathPlanning/Comb.cpp b/src/pathPlanning/Comb.cpp index e2ce972ed8..4019e2c28c 100644 --- a/src/pathPlanning/Comb.cpp +++ b/src/pathPlanning/Comb.cpp @@ -109,8 +109,10 @@ bool Comb::calc( // Default to the ones initialized in the constructor. Shape* current_boundary_optimal = &boundary_inside_optimal_; Shape* current_boundary_minimum = &boundary_inside_minimum_; - PartsView* current_parts_view_optimal = const_cast(&parts_view_inside_optimal_); - PartsView* current_parts_view_minimum = const_cast(&parts_view_inside_minimum_); + PartsView local_parts_view_inside_optimal = parts_view_inside_optimal_; + PartsView local_parts_view_inside_minimum = parts_view_inside_minimum_; + PartsView* current_parts_view_optimal = &local_parts_view_inside_optimal; + PartsView* current_parts_view_minimum = &local_parts_view_inside_minimum; LocToLineGrid* current_loc_to_line_optimal = inside_loc_to_line_optimal_.get(); LocToLineGrid* current_loc_to_line_minimum = inside_loc_to_line_minimum_.get(); diff --git a/src/settings/Settings.cpp b/src/settings/Settings.cpp index cf841a5b14..2bc18c8f0f 100644 --- a/src/settings/Settings.cpp +++ b/src/settings/Settings.cpp @@ -53,6 +53,15 @@ void Settings::add(const std::string& key, const std::string value) } } +// Specialization of the `get` method for `CombingPolygonType`. +// This function maps string values to `CombingPolygonType` enum values. +// The supported string values and their corresponding enum values are: +// - "outer_wall" -> CombingPolygonType::OUTER_WALL +// - "outline" -> CombingPolygonType::OUTLINE +// - "second_wall" -> CombingPolygonType::SECOND_WALL +// - "plugin" -> CombingPolygonType::PLUGIN +// If the string value does not match any of the above, the default value +// `CombingPolygonType::OUTER_WALL` is returned. template<> CombingPolygonType Settings::get(const std::string& key) const { diff --git a/tests/src/pathPlanning/CombTest.cpp b/tests/src/pathPlanning/CombTest.cpp index 4f520b753e..94f0c277fb 100644 --- a/tests/src/pathPlanning/CombTest.cpp +++ b/tests/src/pathPlanning/CombTest.cpp @@ -10,7 +10,6 @@ #include "geometry/Point.h" // For cura::Point / cura::Point2LL #include "geometry/Polygon.h" // For cura::Polygon / cura::Polygons / cura::ConstPolygonRef #include "geometry/Shape.h" // For cura::Shape -#include "sliceDataStorage.h" // For cura::SliceLayer, cura::SliceLayerPart, cura::Walls #include "Application.h" // For cura::Application and cura::Scene