Skip to content

irregular: fix crash in shape approximation when item_item_minimum_spacing meets holed items #462

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
HamzaYslmn wants to merge 1 commit into from
Closed

irregular: fix crash in shape approximation when item_item_minimum_spacing meets holed items #462

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions extern/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,16 @@ FetchContent_Declare(
FetchContent_MakeAvailable(optimizationtools)

# Fetch fontanf/shape.
# PATCH: approximate_by_line_segments seeded its union with the arc-bearing shape,
# leaking CircularArcs through compute_union -> is_polygon() throw on inflated holed
# shapes (the native item_item_minimum_spacing crash). Patch is idempotent: apply,
# or verify already-applied via --reverse --check.
set(SHAPE_BUILD_TEST OFF)
FetchContent_Declare(
shape
GIT_REPOSITORY https://github.com/fontanf/shape.git
GIT_TAG 1809a9c7a1f8734fc8c7330ffe00cc3270ba7d44
PATCH_COMMAND git apply --ignore-whitespace "${CMAKE_CURRENT_SOURCE_DIR}/shape-approximation-297.patch" || git apply --reverse --check --ignore-whitespace "${CMAKE_CURRENT_SOURCE_DIR}/shape-approximation-297.patch"
#SOURCE_DIR "${PROJECT_SOURCE_DIR}/../shape/"
EXCLUDE_FROM_ALL)
FetchContent_MakeAvailable(shape)
Expand Down
74 changes: 74 additions & 0 deletions extern/shape-approximation-297.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
diff --git a/src/approximation.cpp b/src/approximation.cpp
index 99a797d..6bb6f4f 100644
--- a/src/approximation.cpp
+++ b/src/approximation.cpp
@@ -359,7 +359,9 @@ ShapeWithHoles shape::approximate_shape_by_line_segments(
return shape_new;
}

- std::vector<ShapeWithHoles> union_input = {{shape}};
+ // Seed with the FLATTENED shape — seeding with `shape` leaks CircularArc
+ // elements through compute_union into the result (packingsolver #297).
+ std::vector<ShapeWithHoles> union_input = {shape_new};

for (const ShapeElement& element: shape.elements) {
switch (element.type) {
@@ -384,7 +386,11 @@ ShapeWithHoles shape::approximate_shape_by_line_segments(

//Writer().add_shapes_with_holes(union_input).write_json("union_input.json");
std::vector<ShapeWithHoles> union_output = compute_union(union_input);
- return union_output.front();
+ const ShapeWithHoles* best = &union_output.front();
+ for (const ShapeWithHoles& component: union_output)
+ if (component.shape.compute_area() > best->shape.compute_area())
+ best = &component;
+ return *best;
}

Shape shape::approximate_path_by_line_segments(
@@ -501,7 +507,10 @@ ShapeWithHoles shape::approximate_by_line_segments(
return shape_new;
}

- std::vector<ShapeWithHoles> union_input = {shape};
+ // Union the FLATTENED shape with the arc extras. Seeding with `shape` leaks
+ // CircularArc elements through compute_union into the result, whose holes then
+ // fail is_polygon() — crashes on inflated holed shapes (packingsolver #297).
+ std::vector<ShapeWithHoles> union_input = {shape_new};

for (const ShapeElement& element: shape.shape.elements) {
switch (element.type) {
@@ -519,7 +528,6 @@ ShapeWithHoles shape::approximate_by_line_segments(
}

for (const Shape& hole: shape.holes) {
- Shape hole_new;
for (const ShapeElement& element: hole.elements) {
switch (element.type) {
case ShapeElementType::LineSegment: {
@@ -534,7 +542,6 @@ ShapeWithHoles shape::approximate_by_line_segments(
}
}
}
- shape_new.holes.push_back(hole_new);
}

std::vector<ShapeWithHoles> union_output = compute_union(union_input);
@@ -545,10 +552,15 @@ ShapeWithHoles shape::approximate_by_line_segments(
.add_shapes_with_holes(union_output, "Union output")
.write_json("union_input.json");
#endif
- if (!union_output.front().is_polygon()) {
+ // Extras always touch the shape, but be defensive: keep the largest component.
+ const ShapeWithHoles* best = &union_output.front();
+ for (const ShapeWithHoles& component: union_output)
+ if (component.shape.compute_area() > best->shape.compute_area())
+ best = &component;
+ if (!best->is_polygon()) {
throw std::logic_error(FUNC_SIGNATURE);
}
- return union_output.front();
+ return *best;
}

void shape::approximate_shape_by_line_segments_export_inputs(
Loading