diff --git a/docs/skill/references/docs/api/cut_rsolid.md b/docs/skill/references/docs/api/cut_rsolid.md index d315f164..0a863726 100644 --- a/docs/skill/references/docs/api/cut_rsolid.md +++ b/docs/skill/references/docs/api/cut_rsolid.md @@ -3,7 +3,7 @@ ## API Definition ```python -def cut_rsolid(*solids, skip_non_intersecting=True, tracking_policy='full', on_split='pieces') +def cut_rsolid(*solids, skip_non_intersecting=True, tracking_policy='full', on_split='pieces', clean=True) ``` *Source: operators/boolean.py* @@ -33,6 +33,10 @@ sequences, and returns a single `Solid`. - **Description**: FULL computes topology history and lineage. GRAPH keeps the replayable operation node without computing a TopoDelta. +### clean + +- **Description**: When True (the default) the faces and edges the cut leaves split are unified, as union does: a cylinder wall cut along its seam stays one face, so a later fillet or chamfer of its edges works. False keeps the raw kernel result. + ### on_split - **Description**: What a cut that splits the body (a slot through a bar) returns. diff --git a/docs/skill/references/docs/api/cut_rsolid_list.md b/docs/skill/references/docs/api/cut_rsolid_list.md index 930d17f1..e3e54020 100644 --- a/docs/skill/references/docs/api/cut_rsolid_list.md +++ b/docs/skill/references/docs/api/cut_rsolid_list.md @@ -3,7 +3,7 @@ ## API Definition ```python -def cut_rsolid_list(*solids, skip_non_intersecting=True, tracking_policy='full') +def cut_rsolid_list(*solids, skip_non_intersecting=True, tracking_policy='full', clean=True) ``` *Source: operators/boolean.py* @@ -39,6 +39,10 @@ a through slot. Use ``cut_rsolid`` when the result must be one solid. - **Description**: FULL computes topology history and lineage. GRAPH keeps the replayable operation node without computing a TopoDelta. +### clean + +- **Description**: When True (the default) the faces and edges the cut leaves split are unified, as union does: a cylinder wall cut along its seam stays one face, so a later fillet or chamfer of its edges works. False keeps the raw kernel result. + ## Returns list[Solid]: Every piece of the cut, in kernel order; empty when the tools diff --git a/docs/skill/references/docs/api/intersect_rsolid.md b/docs/skill/references/docs/api/intersect_rsolid.md index 023b0b3e..26ce58b7 100644 --- a/docs/skill/references/docs/api/intersect_rsolid.md +++ b/docs/skill/references/docs/api/intersect_rsolid.md @@ -3,7 +3,7 @@ ## API Definition ```python -def intersect_rsolid(*solids) +def intersect_rsolid(*solids, clean=True) ``` *Source: operators/boolean.py* @@ -26,6 +26,10 @@ error instead of returning an empty list. - **Description**: One or more Solid objects or sequences of Solid. Nested sequences are flattened before processing. +### clean + +- **Description**: When True (the default) the faces and edges the intersection leaves split are unified, as union does: a cylinder wall cut along its seam stays one face, so a later fillet or chamfer of its edges works. False keeps the raw kernel result. + ## Returns Solid: The overlap region as a single solid. diff --git a/docs/skill/references/docs/core/operation_graph_json_spec.md b/docs/skill/references/docs/core/operation_graph_json_spec.md index b0936824..f703e88f 100644 --- a/docs/skill/references/docs/core/operation_graph_json_spec.md +++ b/docs/skill/references/docs/core/operation_graph_json_spec.md @@ -1567,12 +1567,14 @@ Notes: | --- | --- | --- | | `tool_count` | `int` | number of subtractive tool solids | | `skip_non_intersecting` | `bool` | whether non-intersecting tools are skipped | +| `clean` | `bool` | whether the faces and edges the cut leaves split were unified | Notes: - Public `cut_rsolid(...)` defaults this flag to `true` for interactive convenience. - Graph replay defaults missing `skip_non_intersecting` to `false` so malformed or drifting graphs fail diagnostically instead of silently skipping tools. - Multi-tool cut preserves the full topology delta chain across all performed tool cuts. +- `clean` has no replay default: a graph without it is rejected like one without `tool_count`. #### `make_intersect_rsolid` @@ -1583,6 +1585,7 @@ Notes: | Key | Type | Meaning | | --- | --- | --- | | `input_count` | `int` | number of intersected solids | +| `clean` | `bool` | whether the faces and edges the intersection leaves split were unified | Notes: diff --git a/src/cpp/bindings/python/bind_ops_features.cpp b/src/cpp/bindings/python/bind_ops_features.cpp index 383961f8..2a502ba3 100644 --- a/src/cpp/bindings/python/bind_ops_features.cpp +++ b/src/cpp/bindings/python/bind_ops_features.cpp @@ -683,9 +683,10 @@ void bind_booleans(py::module_& m) { m.def( "cut_rsolid", [](const py::args& solids, bool skip_non_intersecting, const Policy& tracking_policy, - const Choice& on_split) -> AnyOf { + const Choice& on_split, bool clean) -> AnyOf { const std::vector operands = flattened(solids, "cut_rsolid"); - const CutOptions options{skip_non_intersecting, policy_text(tracking_policy)}; + CutOptions options{skip_non_intersecting, policy_text(tracking_policy)}; + options.clean = clean; if (on_split.value == "error") return {py::cast(unlocked(false, [&] { return cut_rsolid(operands, options); }))}; if (on_split.value != "pieces") @@ -696,27 +697,29 @@ void bind_booleans(py::module_& m) { return {solids_to_py(outcome.pieces)}; // the cut split the body: every piece, ordered by centroid }, py::kw_only(), py::arg("skip_non_intersecting") = true, py::arg("tracking_policy") = "full", - py::arg("on_split") = "pieces", + py::arg("on_split") = "pieces", py::arg("clean") = true, "Compute the boolean difference of solids: the first is the base, the rest are subtracted in order."); m.def( "cut_rsolid_list", - [](const py::args& solids, bool skip_non_intersecting, const Policy& tracking_policy) { + [](const py::args& solids, bool skip_non_intersecting, const Policy& tracking_policy, bool clean) { const std::vector operands = flattened(solids, "cut_rsolid_list"); - const CutOptions options{skip_non_intersecting, policy_text(tracking_policy)}; + CutOptions options{skip_non_intersecting, policy_text(tracking_policy)}; + options.clean = clean; return solids_to_py(unlocked(false, [&] { return cut_rsolid_list(operands, options); })); }, py::kw_only(), py::arg("skip_non_intersecting") = true, py::arg("tracking_policy") = "full", + py::arg("clean") = true, "Compute the boolean difference of solids and keep every piece: a cut that splits the body returns each part, " "and one that consumes the body returns an empty list."); m.def( "intersect_rsolid", - [](const py::args& solids) { + [](const py::args& solids, bool clean) { const std::vector operands = flattened(solids, "intersect_rsolid"); - return unlocked(false, [&] { return intersect_rsolid(operands); }); + return unlocked(false, [&] { return intersect_rsolid(operands, clean); }); }, - "Compute the boolean intersection of solids."); + py::kw_only(), py::arg("clean") = true, "Compute the boolean intersection of solids."); m.def( "boolean_rsolidlist", diff --git a/src/cpp/include/scad/kernel/booleans.hpp b/src/cpp/include/scad/kernel/booleans.hpp index 38b8f620..a5f96f02 100644 --- a/src/cpp/include/scad/kernel/booleans.hpp +++ b/src/cpp/include/scad/kernel/booleans.hpp @@ -22,6 +22,10 @@ struct BooleanOptions { bool fill_history = false; // keep BRepTools_History + section edges std::optional fuzzy; // fuzzy tolerance (SetFuzzyValue) bool glue = false; // BOPAlgo_GlueShift instead of GlueOff + // Unify the same-domain faces and edges of the result (what a cut leaves behind: a cylinder wall split along its + // seam, an edge split in two). The history, when kept, and the section edges follow the clean-up. A clean-up that + // fails, or that turns a valid result into an invalid one, is dropped: the raw result is the answer. + bool clean = false; }; struct BooleanResult { @@ -55,10 +59,10 @@ FuseHistoryResult fuse_shapes_with_history(const std::vector& shap TopoDS_Shape fuse_shapes(const std::vector& shapes, bool glue = false, std::optional tol = std::nullopt, bool clean = true); -// body - every tool. `body` itself when there are no tools. -TopoDS_Shape cut_shapes(const TopoDS_Shape& body, const std::vector& tools); +// body - every tool. `body` itself when there are no tools. With `clean`, the result is unified (see BooleanOptions). +TopoDS_Shape cut_shapes(const TopoDS_Shape& body, const std::vector& tools, bool clean = false); -// Intersection of all shapes, folded left to right. -TopoDS_Shape common_shapes(const std::vector& shapes); +// Intersection of all shapes, folded left to right. With `clean`, each step's result is unified. +TopoDS_Shape common_shapes(const std::vector& shapes, bool clean = false); } // namespace scad::kernel diff --git a/src/cpp/include/scad/ops/features/booleans.hpp b/src/cpp/include/scad/ops/features/booleans.hpp index 86e74811..c9c3d4a4 100644 --- a/src/cpp/include/scad/ops/features/booleans.hpp +++ b/src/cpp/include/scad/ops/features/booleans.hpp @@ -27,6 +27,9 @@ struct CutOptions { // cut_rsolid_list: order the pieces by centroid (x, then y, then z) instead of kernel order, so piece i is the same // piece on every OCCT version. Recorded as params.piece_order = "centroid" only when set. bool order_pieces = false; + // Unify the faces and edges the cut leaves split (a cylinder wall cut along its seam), as union does: OCCT fails to + // fillet such a split face, or returns an invalid solid. Recorded as params.clean. + bool clean = true; }; // What a cut that may split leaves: one Solid, or (`solid` empty) every piece, ordered by centroid. @@ -47,8 +50,8 @@ std::vector cut_rsolid_list(const std::vector& solids, const CutOp // cut_rsolid_list with order_pieces (recorded as make_cut_rsolid_list). An empty result is still an error. CutOutcome cut_rsolid_keep_split(const std::vector& solids, const CutOptions& options = {}); -// The overlap of all `solids`. Error "intersect_rsolid". -Solid intersect_rsolid(const std::vector& solids); +// The overlap of all `solids`; `clean` as for CutOptions. Error "intersect_rsolid". +Solid intersect_rsolid(const std::vector& solids, bool clean = true); // The error a non-Solid argument of union_rsolid / cut_rsolid / intersect_rsolid raises // (a binding that flattens Python arguments throws it for an item that is not a solid). diff --git a/src/cpp/include/scad/ops/features/diagnostics.hpp b/src/cpp/include/scad/ops/features/diagnostics.hpp index 2d966b7d..74568e17 100644 --- a/src/cpp/include/scad/ops/features/diagnostics.hpp +++ b/src/cpp/include/scad/ops/features/diagnostics.hpp @@ -21,7 +21,8 @@ namespace scad { // A classified fillet / chamfer failure. struct BlendDiagnosis { // "tangent_adjacent_faces" | "sliver_edges_in_selection" | "size_exceeds_face" | - // "partial_smooth_chain_on_periodic_face" | "blends_tangency_critical" | "vertex_blend_conflict" + // "partial_smooth_chain_on_periodic_face" | "blends_tangency_critical" | "vertex_blend_conflict" | + // "faces_split_on_one_surface" std::string failure_kind; std::string what_happened; std::vector possible_causes; @@ -54,6 +55,16 @@ class BlendVolumeViolation : public ValueError { void assert_blend_volume_monotonic(const Solid& source, const Solid& result, std::string_view operation_kind, std::optional size_value, const std::vector& edges); +// Exit check of a blend, next to the volume check: the result must be a valid solid (BRepCheck_Analyzer) whenever +// the input was. OCCT reports `IsDone` for fillets it got wrong (a face split along a seam, issue #88) and hands back +// an invalid solid with a wrong volume, which only a later boolean or an export finds. Throws BlendValidityViolation. +class BlendValidityViolation : public BlendVolumeViolation { + public: + using BlendVolumeViolation::BlendVolumeViolation; +}; +void assert_blend_valid(const Solid& source, const Solid& result, std::string_view operation_kind, + std::optional size_value, const std::vector& edges); + // A classified boolean failure. struct BooleanDiagnosis { std::string failure_kind; // "disjoint" | "non_manifold_contact" diff --git a/src/cpp/include/scad/semantic/tracking.hpp b/src/cpp/include/scad/semantic/tracking.hpp index dfde8f60..77ab47df 100644 --- a/src/cpp/include/scad/semantic/tracking.hpp +++ b/src/cpp/include/scad/semantic/tracking.hpp @@ -179,8 +179,9 @@ ModificationHistory modification_history(const BRepTools_History& history, const // tracked_cut. Face history of body ("body") and tool ("tool"), plus the // section edges. `solid` of the result is the *first* solid of the kernel's // result, empty when the cut left none; membership is judged against the whole -// kernel result. ValueError when the algorithm did not complete. -TrackedBooleanResult tracked_cut(const Solid& body, const Solid& tool); +// kernel result. ValueError when the algorithm did not complete. With `clean` the result is unified (same-domain +// faces and edges merged, see kernel::BooleanOptions::clean) and the history follows the clean-up. +TrackedBooleanResult tracked_cut(const Solid& body, const Solid& tool, bool clean = false); // tracked_union: a two-solid fuse with `track_union_history` on top. ValueError // unless the fuse yields exactly one solid. Glue is opt-in (see kernel::fuse_shapes_with_history). @@ -195,8 +196,8 @@ TrackedBooleanResult track_union_history(const std::vector& solids, const const BRepTools_History& history, const std::vector& section_edges = {}); -// tracked_intersect. -TrackedBooleanResult tracked_intersect(const Solid& body, const Solid& tool); +// tracked_intersect. `clean` as for tracked_cut. +TrackedBooleanResult tracked_intersect(const Solid& body, const Solid& tool, bool clean = false); namespace tracking { // How many topology-history queries the tracked operations have made in this process. A graph-policy diff --git a/src/cpp/src/kernel/booleans.cpp b/src/cpp/src/kernel/booleans.cpp index 46aff8eb..e62b6359 100644 --- a/src/cpp/src/kernel/booleans.cpp +++ b/src/cpp/src/kernel/booleans.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include @@ -53,6 +54,62 @@ std::vector tail_of(const std::vector& shapes) { return std::vector(shapes.begin() + 1, shapes.end()); } +bool is_valid_shape(const TopoDS_Shape& shape) { + return !shape.IsNull() && BRepCheck_Analyzer(shape).IsValid() == Standard_True; +} + +// Follow each section edge through a clean-up: it may have been merged into a longer edge or dropped with a removed +// face. Only the edges that are in `cleaned` survive, each once. +std::vector follow_section_edges(const opencascade::handle& clean_history, + const TopoDS_Shape& cleaned, + const std::vector& section_edges) { + SubShapeIndex result(cleaned); + TopTools_IndexedMapOfShape seen; + std::vector survivors; + for (const TopoDS_Shape& edge : section_edges) { + std::vector candidates; + for (const TopoDS_Shape& s : clean_history->Modified(edge)) + candidates.push_back(s); + for (const TopoDS_Shape& s : clean_history->Generated(edge)) + candidates.push_back(s); + if (candidates.empty() && !clean_history->IsRemoved(edge)) + candidates.push_back(edge); + for (const TopoDS_Shape& candidate : candidates) { + if (!result.contains(candidate)) + continue; + const int before = seen.Extent(); + seen.Add(candidate); + if (seen.Extent() > before) + survivors.push_back(candidate); + } + } + return survivors; +} + +// BooleanOptions::clean. The unified shape replaces the result only when it is as good: the same number of solids, and +// valid whenever the raw result was (FreeCAD and build123d fall back to the raw result the same way; unifying every +// boolean result has been seen to invalidate some). +void unify_result(BooleanResult& result) { + try { + ShapeUpgrade_UnifySameDomain unifier(result.shape, /*UnifyEdges=*/true, /*UnifyFaces=*/true, + /*ConcatBSplines=*/true); + unifier.Build(); + const TopoDS_Shape cleaned = unifier.Shape(); + if (cleaned.IsNull() || solids_of(cleaned).size() != solids_of(result.shape).size()) + return; + if (!is_valid_shape(cleaned) && is_valid_shape(result.shape)) + return; + if (!result.history.IsNull()) { + const opencascade::handle clean_history = unifier.History(); + result.history->Merge(clean_history); + result.section_edges = follow_section_edges(clean_history, cleaned, result.section_edges); + } + result.shape = cleaned; + } catch (const Standard_Failure&) { + // the raw result stands + } +} + } // namespace BooleanResult run_boolean(BooleanOp op, const TopoDS_Shape& argument, const std::vector& tools, @@ -86,6 +143,8 @@ BooleanResult run_boolean(BooleanOp op, const TopoDS_Shape& argument, const std: out.section_edges.clear(); } } + if (options.clean) + unify_result(out); return out; }); } @@ -138,29 +197,7 @@ FuseHistoryResult fuse_shapes_with_history(const std::vector& shap const opencascade::handle clean_history = unifier.History(); out.history->Merge(clean_history); - // Follow each section edge through the clean-up: it may have been merged - // into a longer edge or dropped with a removed face. - SubShapeIndex result(out.shape); - TopTools_IndexedMapOfShape seen; - std::vector survivors; - for (const TopoDS_Shape& edge : out.section_edges) { - std::vector candidates; - for (const TopoDS_Shape& s : clean_history->Modified(edge)) - candidates.push_back(s); - for (const TopoDS_Shape& s : clean_history->Generated(edge)) - candidates.push_back(s); - if (candidates.empty() && !clean_history->IsRemoved(edge)) - candidates.push_back(edge); - for (const TopoDS_Shape& candidate : candidates) { - if (!result.contains(candidate)) - continue; - const int before = seen.Extent(); - seen.Add(candidate); - if (seen.Extent() > before) - survivors.push_back(candidate); - } - } - out.section_edges = std::move(survivors); + out.section_edges = follow_section_edges(clean_history, out.shape, out.section_edges); return out; }); } @@ -182,21 +219,25 @@ TopoDS_Shape fuse_shapes(const std::vector& shapes, bool glue, std return clean ? clean_shape(fused.shape) : fused.shape; } -TopoDS_Shape cut_shapes(const TopoDS_Shape& body, const std::vector& tools) { +TopoDS_Shape cut_shapes(const TopoDS_Shape& body, const std::vector& tools, bool clean) { if (tools.empty()) return body; - const BooleanResult cut = run_boolean(BooleanOp::Cut, body, tools); + BooleanOptions options; + options.clean = clean; + const BooleanResult cut = run_boolean(BooleanOp::Cut, body, tools, options); if (!cut.done) throw ValueError("OCP cut failed"); return cut.shape; } -TopoDS_Shape common_shapes(const std::vector& shapes) { +TopoDS_Shape common_shapes(const std::vector& shapes, bool clean) { if (shapes.empty()) throw ValueError("common_shapes requires at least one shape"); TopoDS_Shape result = shapes.front(); for (std::size_t i = 1; i < shapes.size(); ++i) { - const BooleanResult common = run_boolean(BooleanOp::Common, result, {shapes[i]}); + BooleanOptions options; + options.clean = clean; + const BooleanResult common = run_boolean(BooleanOp::Common, result, {shapes[i]}, options); if (!common.done) throw ValueError("OCP common failed"); result = common.shape; diff --git a/src/cpp/src/ops/features/blend_diagnosis.cpp b/src/cpp/src/ops/features/blend_diagnosis.cpp index 6db55152..85355abb 100644 --- a/src/cpp/src/ops/features/blend_diagnosis.cpp +++ b/src/cpp/src/ops/features/blend_diagnosis.cpp @@ -1,5 +1,13 @@ // Why a fillet or chamfer failed: the probes of operators/_diagnostics.py, in its order. #include +#include +#include +#include +#include +#include +#include +#include +#include #include #include @@ -153,6 +161,80 @@ std::optional classify_tangent_adjacency(const Case& c) { return std::nullopt; } +// A boolean that was not cleaned leaves faces that lie on one surface split (a cylinder wall cut along its seam is two +// faces). The blend of an edge along such a face fails, or goes wrong, and the room probe below then blames the +// radius: the "face room" it measures is only the half the seam cut off. Unify the same-domain faces, carry the +// selected edges over with the history, and blend again: when that works, the split faces are the cause (issue #88). +std::optional classify_split_faces(const Case& c) { + ShapeUpgrade_UnifySameDomain unifier(c.solid.wrapped(), /*UnifyEdges=*/true, /*UnifyFaces=*/true, + /*ConcatBSplines=*/true); + unifier.Build(); + const TopoDS_Shape unified = unifier.Shape(); + if (unified.IsNull()) + return std::nullopt; + const auto faces_in = [](const TopoDS_Shape& shape) { + TopTools_IndexedMapOfShape faces; + TopExp::MapShapes(shape, TopAbs_FACE, faces); + return faces.Extent(); + }; + const int split = faces_in(c.solid.wrapped()) - faces_in(unified); + if (split <= 0) + return std::nullopt; + + const opencascade::handle history = unifier.History(); + TopTools_MapOfShape carried; + std::vector edges; + for (const Edge& edge : c.edges) { + std::vector now; + for (const TopoDS_Shape& s : history->Modified(edge.wrapped())) + now.push_back(s); + if (now.empty() && !history->IsRemoved(edge.wrapped())) + now.push_back(edge.wrapped()); + for (const TopoDS_Shape& s : now) { + if (s.ShapeType() == TopAbs_EDGE && carried.Add(s)) + edges.push_back(TopoDS::Edge(s)); + } + } + if (edges.empty()) + return std::nullopt; + + const bool chamfer = c.words.size == "distance"; + const char* kind = chamfer ? "chamfer" : "fillet"; + const bool works = occt_guard([&] { + if (chamfer) { + BRepFilletAPI_MakeChamfer blend(unified); + for (const TopoDS_Edge& edge : edges) + blend.Add(c.size_value, edge); + blend.Build(); + return blend.IsDone() && BRepCheck_Analyzer(blend.Shape()).IsValid() == Standard_True; + } + BRepFilletAPI_MakeFillet blend(unified); + for (const TopoDS_Edge& edge : edges) + blend.Add(c.size_value, edge); + blend.Build(); + return blend.IsDone() && BRepCheck_Analyzer(blend.Shape()).IsValid() == Standard_True; + }); + if (!works) + return std::nullopt; + + BlendDiagnosis out; + out.failure_kind = "faces_split_on_one_surface"; + out.what_happened = std::to_string(split) + + " face(s) of the solid are split on a surface they share with a neighbour " + + "(typically a boolean made without clean, or a cylinder wall cut along its seam); the same " + + kind + " " + c.words.size + " " + g4(c.size_value) + + " mm succeeds once those faces are unified, so the " + c.words.size + " is not the limit"; + out.possible_causes = {"A cut or intersection ran with clean=False and left a face split along its seam.", + "The solid came from a source that does not unify faces (an imported or hand-built shape)."}; + out.measurements = {{"split_faces", static_cast(split), "faces"}, {c.words.size, c.size_value, "mm"}}; + out.repair = { + "Make the cut / intersect with clean=True (the default), or pass the solid through union_rsolid(solid, " + "clean=True) before the " + + std::string(kind) + ".", + std::string("Keep the ") + c.words.size + " as it is: it is not too large."}; + return out; +} + std::optional classify_blend_room(const Case& c) { for (const Edge& edge : c.edges) { const auto* faces = c.faces_of(edge); @@ -336,6 +418,8 @@ std::optional diagnose_blend_failure(const Solid& solid, const s return found; if (auto found = classify_tangent_adjacency(c)) return found; + if (auto found = classify_split_faces(c)) + return found; if (auto found = classify_blend_room(c)) return found; if (auto found = classify_partial_smooth_chain(c)) @@ -426,4 +510,20 @@ void assert_blend_volume_monotonic(const Solid& source, const Solid& result, std } } +void assert_blend_valid(const Solid& source, const Solid& result, std::string_view operation_kind, + std::optional size_value, const std::vector& edges) { + // The cheap side first: a valid result is the normal case; the input is checked only to tell "OCCT broke it" from + // "it came in broken" (an imported solid that was already invalid is not the blend's fault). + const bool result_valid = occt_guard([&] { return BRepCheck_Analyzer(result.wrapped()).IsValid() == Standard_True; }); + if (result_valid) + return; + const bool source_valid = occt_guard([&] { return BRepCheck_Analyzer(source.wrapped()).IsValid() == Standard_True; }); + if (!source_valid) + return; + throw BlendValidityViolation(std::string(operation_kind) + + " result is not a valid solid (BRepCheck_Analyzer) although the input was: the kernel " + "returned broken geometry (size=" + + size_text(size_value) + ", selected_edges=" + std::to_string(edges.size()) + ")"); +} + } // namespace scad diff --git a/src/cpp/src/ops/features/blends.cpp b/src/cpp/src/ops/features/blends.cpp index ed3b4877..276bae4c 100644 --- a/src/cpp/src/ops/features/blends.cpp +++ b/src/cpp/src/ops/features/blends.cpp @@ -52,7 +52,11 @@ struct BlendAttempt { spec.possible_causes = diagnosis->possible_causes; const std::string details = - violation != nullptr ? "BlendVolumeViolation: " + std::string(violation->what()) : technical_details_of(error); + violation != nullptr + ? std::string(dynamic_cast(&error) != nullptr ? "BlendValidityViolation: " + : "BlendVolumeViolation: ") + + violation->what() + : technical_details_of(error); std::vector evidence; if (diagnosis && !diagnosis->evidence_edges.empty()) { // The solid with the implicated edges in orange; the same failure at the same size is drawn once per process @@ -85,6 +89,7 @@ Solid finish_blend(const Solid& solid, TrackedResult tracked, const BlendAttempt double check_size, FeatureRecord& rec, const BlendTags& tags, std::string_view patch_role, const std::function& between = {}) { assert_blend_volume_monotonic(solid, Solid(tracked.shape), kind, check_size, attempt.edges); + assert_blend_valid(solid, Solid(tracked.shape), kind, check_size, attempt.edges); inherit_metadata(tracked.shape, {&solid}); rec.param("edge_count", Value(static_cast(attempt.edges.size()))); diff --git a/src/cpp/src/ops/features/booleans.cpp b/src/cpp/src/ops/features/booleans.cpp index 2359a761..b04bd7a5 100644 --- a/src/cpp/src/ops/features/booleans.cpp +++ b/src/cpp/src/ops/features/booleans.cpp @@ -323,12 +323,12 @@ CutRun run_cuts(const std::vector& solids, const CutOptions& options, Tra std::vector cut_pieces; if (policy == TrackingPolicy::Full) { - TrackedBooleanResult tracked = tracked_cut(piece, candidate); + TrackedBooleanResult tracked = tracked_cut(piece, candidate, options.clean); cut_pieces = std::move(tracked.solids); run.deltas.push_back(std::move(tracked.delta)); merge_entries(run.entries, std::move(tracked.delta_entries)); } else { - for (const TopoDS_Solid& part : kernel::solids_of(kernel::cut_shapes(s1, {s2}))) + for (const TopoDS_Solid& part : kernel::solids_of(kernel::cut_shapes(s1, {s2}, options.clean))) cut_pieces.emplace_back(part); } run.intermediates.push_back(piece); @@ -350,6 +350,7 @@ void record_cut(const std::vector& solids, const CutOptions& options, Tra FeatureRecord& rec) { rec.param("tool_count", Value(static_cast(solids.size() - 1))) .param("skip_non_intersecting", Value(options.skip_non_intersecting)) + .param("clean", Value(options.clean)) .param("tracking_policy", Value(std::string(to_string(policy)))); add_operands(rec, solids); } @@ -485,7 +486,7 @@ std::vector cut_rsolid_list(const std::vector& solids, const CutOp // --------------------------------------------------------------------------- // intersect // --------------------------------------------------------------------------- -Solid intersect_rsolid(const std::vector& solids) { +Solid intersect_rsolid(const std::vector& solids, bool clean) { bool empty_result = false; try { return occt_guard([&]() -> Solid { @@ -503,7 +504,7 @@ Solid intersect_rsolid(const std::vector& solids) { const Solid& candidate = solids[i]; if (result_solid.wrapped().IsNull() || candidate.wrapped().IsNull()) throw ValueError("The input solid is invalid; the intersection cannot be computed."); - TrackedBooleanResult tracked = tracked_intersect(result_solid, candidate); + TrackedBooleanResult tracked = tracked_intersect(result_solid, candidate, clean); if (!tracked.solid) { empty_result = true; throw ValueError("The intersection is empty or OCC returned no valid solid"); @@ -522,7 +523,7 @@ Solid intersect_rsolid(const std::vector& solids) { result_solid.apply_tag("solid.boolean.intersect", false); FeatureRecord rec(kIntersectOp); - rec.param("input_count", Value(static_cast(solids.size()))); + rec.param("input_count", Value(static_cast(solids.size()))).param("clean", Value(clean)); add_operands(rec, solids); FinalizeSpec spec; spec.record = std::move(rec).take(); diff --git a/src/cpp/src/ops/features/executors.cpp b/src/cpp/src/ops/features/executors.cpp index d3dcd53d..555fe075 100644 --- a/src/cpp/src/ops/features/executors.cpp +++ b/src/cpp/src/ops/features/executors.cpp @@ -154,17 +154,19 @@ void register_booleans(reactive::ExecutorRegistry& registry) { return one(union_rsolid(rest(in, 0, in.size()), options)); }); registry.add("make_cut_rsolid", [](Params p, Inputs in, const ScalarValues&) { - return one( - cut_rsolid(rest(in, 0, in.size()), {flag(p, "skip_non_intersecting"), text(p, "tracking_policy")})); + CutOptions options{flag(p, "skip_non_intersecting"), text(p, "tracking_policy")}; + options.clean = flag(p, "clean"); + return one(cut_rsolid(rest(in, 0, in.size()), options)); }); registry.add("make_cut_rsolid_list", [](Params p, Inputs in, const ScalarValues&) { CutOptions options{flag(p, "skip_non_intersecting"), text(p, "tracking_policy")}; + options.clean = flag(p, "clean"); const Value* order = p.find("piece_order"); // absent on records made before ordered pieces options.order_pieces = order != nullptr && order->is_string() && order->as_string() == "centroid"; return many(cut_rsolid_list(rest(in, 0, in.size()), options)); }); - registry.add("make_intersect_rsolid", [](Params, Inputs in, const ScalarValues&) { - return one(intersect_rsolid(rest(in, 0, in.size()))); + registry.add("make_intersect_rsolid", [](Params p, Inputs in, const ScalarValues&) { + return one(intersect_rsolid(rest(in, 0, in.size()), flag(p, "clean"))); }); registry.add("make_boolean_rsolidlist", [](Params p, Inputs in, const ScalarValues&) { const auto left = static_cast(param(p, "left_count").as_int()); diff --git a/src/cpp/src/semantic/tracking_booleans.cpp b/src/cpp/src/semantic/tracking_booleans.cpp index fa51049c..2e326b1d 100644 --- a/src/cpp/src/semantic/tracking_booleans.cpp +++ b/src/cpp/src/semantic/tracking_booleans.cpp @@ -54,10 +54,12 @@ std::string input_role(std::size_t index) { // body op tool, with history. `Solid(result)` of the Python SDK is the first // solid of the kernel's answer (empty when there is none); the history filter // is the whole answer. -TrackedBooleanResult track_boolean(kernel::BooleanOp op, const Solid& body, const Solid& tool, const char* failure) { +TrackedBooleanResult track_boolean(kernel::BooleanOp op, const Solid& body, const Solid& tool, const char* failure, + bool clean) { return occt_guard([&] { kernel::BooleanOptions options; options.fill_history = true; + options.clean = clean; const kernel::BooleanResult run = kernel::run_boolean(op, body.wrapped(), {tool.wrapped()}, options); if (!run.done) throw ValueError(failure); @@ -87,12 +89,13 @@ TrackedBooleanResult track_boolean(kernel::BooleanOp op, const Solid& body, cons } // namespace -TrackedBooleanResult tracked_cut(const Solid& body, const Solid& tool) { - return track_boolean(kernel::BooleanOp::Cut, body, tool, "Boolean cut failed: OCC build did not complete"); +TrackedBooleanResult tracked_cut(const Solid& body, const Solid& tool, bool clean) { + return track_boolean(kernel::BooleanOp::Cut, body, tool, "Boolean cut failed: OCC build did not complete", clean); } -TrackedBooleanResult tracked_intersect(const Solid& body, const Solid& tool) { - return track_boolean(kernel::BooleanOp::Common, body, tool, "Boolean intersect failed: OCC build did not complete"); +TrackedBooleanResult tracked_intersect(const Solid& body, const Solid& tool, bool clean) { + return track_boolean(kernel::BooleanOp::Common, body, tool, "Boolean intersect failed: OCC build did not complete", + clean); } TrackedBooleanResult tracked_union(const Solid& body, const Solid& tool, bool glue, double tol) { diff --git a/src/cpp/tests/cpp/std/test_chain_bearing.cpp b/src/cpp/tests/cpp/std/test_chain_bearing.cpp index cfa728b4..8ed582a9 100644 --- a/src/cpp/tests/cpp/std/test_chain_bearing.cpp +++ b/src/cpp/tests/cpp/std/test_chain_bearing.cpp @@ -11,7 +11,7 @@ using doctest::Approx; TEST_CASE("sprocket: ISO 606 12B candidate") { const Solid sprocket = make_roller_chain_sprocket_rsolid(18, 19.05, 12.07, 11.1, 10.2); CHECK(sprocket.volume() == Approx(98201.67369173263).epsilon(1e-6)); - CHECK(sprocket.faces().size() == 54); + CHECK(sprocket.faces().size() == 39); // 54 before cut unified the faces a cylinder seam split (issue #88) const Value meta = sprocket.get_metadata("std.chain.roller_sprocket"); CHECK(meta.find("n_teeth")->as_int() == 18); CHECK(number(meta, "pitch_radius") == Approx(19.05 / (2.0 * std::sin(kPi / 18)))); diff --git a/src/cpp/tests/golden/std_sprocket.json b/src/cpp/tests/golden/std_sprocket.json index e703e28a..784c0a06 100644 --- a/src/cpp/tests/golden/std_sprocket.json +++ b/src/cpp/tests/golden/std_sprocket.json @@ -20,8 +20,8 @@ 59.38481593313661, 11.1 ], - "edges": 156, - "faces": 54, + "edges": 111, + "faces": 39, "tags": [ "solid.boolean.cut" ], diff --git a/src/cpp/tests/golden/std_sprocket_unbored.json b/src/cpp/tests/golden/std_sprocket_unbored.json index f305a0e0..d3b67db0 100644 --- a/src/cpp/tests/golden/std_sprocket_unbored.json +++ b/src/cpp/tests/golden/std_sprocket_unbored.json @@ -20,8 +20,8 @@ 27.35284634516466, 5.0 ], - "edges": 99, - "faces": 35, + "edges": 72, + "faces": 26, "tags": [ "solid.boolean.cut" ], diff --git a/src/cpp/tests/python/test_ops_features_booleans.py b/src/cpp/tests/python/test_ops_features_booleans.py index 8a5face3..03d27bb9 100644 --- a/src/cpp/tests/python/test_ops_features_booleans.py +++ b/src/cpp/tests/python/test_ops_features_booleans.py @@ -119,5 +119,5 @@ def info(face): def test_signatures_match_python(): assert parameter_names(scad.union_rsolid) == ["solids", "clean", "glue", "tol", "tracking_policy"] # *solids # on_split: a cut that splits the body returns its pieces by default (notebook values; a Part holds one solid), "error" refuses - assert parameter_names(scad.cut_rsolid) == ["solids", "skip_non_intersecting", "tracking_policy", "on_split"] + assert parameter_names(scad.cut_rsolid) == ["solids", "skip_non_intersecting", "tracking_policy", "on_split", "clean"] assert parameter_names(scad.boolean_rsolidlist) == ["left", "right", "operation"] diff --git a/src/simplecadapi/_glue/docstrings.json b/src/simplecadapi/_glue/docstrings.json index 8024dc76..8cbf5358 100644 --- a/src/simplecadapi/_glue/docstrings.json +++ b/src/simplecadapi/_glue/docstrings.json @@ -145,7 +145,7 @@ "constrain_tangent_rsketch": "Constrain supported curves to be tangent on an explicit branch.\n\n``at_a``/``at_b`` select ``\"start\"`` or ``\"end\"`` for arc and\nB-spline endpoint tangency. Circle-circle tangency accepts\n``mode=\"external\"`` or ``mode=\"internal\"``.", "constrain_vertical_rsketch": "Constrain a sketch line to be vertical.", "convert_value": "Convert a finite numeric value between dimensionally compatible units.", - "cut_rsolid": "Compute the boolean difference of solids.\n\nArgs:\n solids: One or more Solid objects or sequences of Solid. Nested sequences are\n flattened before processing; the first solid is the base, the rest are\n subtracted in order.\n skip_non_intersecting: When True, tools with no meaningful intersection are\n ignored for interactive convenience. Graph replay records this flag and\n should use False for strict diagnostic workflows.\n tracking_policy: FULL computes topology history and lineage. GRAPH keeps\n the replayable operation node without computing a TopoDelta.\n on_split: What a cut that splits the body (a slot through a bar) returns.\n ``\"pieces\"`` (default): every piece, as ``list[Solid]`` ordered by centroid\n (x, then y, then z), recorded like ``cut_rsolid_list``. ``\"error\"``: raise,\n for code that needs exactly one Solid.\n\nReturns:\n Solid: The cut result solid, when the result is one piece.\n list[Solid]: The pieces (a ``scad.Pieces``), when the cut split the body\n (``on_split=\"pieces\"``). Operations that take one shape run on every piece\n (``cut_rsolid(pieces, tool)``, ``translate_shape(pieces, ...)``) and return\n ``Pieces``; a selector's cardinality is checked over all pieces. They are notebook values: a Part holds exactly one solid, so make one Part of each piece\n (an Assembly of them is the several-piece result), not ``make_part_rpart(body=pieces)``.\n\nRaises:\n SimpleCADError: The cut produced no solid, or (``on_split=\"error\"``) split\n the body. Use ``cut_rsolid_list`` to always get a list.\n\nUsage:\n Accepts a base solid followed by one or more tool solids, including nested\n sequences, and returns a single `Solid`.", + "cut_rsolid": "Compute the boolean difference of solids.\n\nArgs:\n solids: One or more Solid objects or sequences of Solid. Nested sequences are\n flattened before processing; the first solid is the base, the rest are\n subtracted in order.\n skip_non_intersecting: When True, tools with no meaningful intersection are\n ignored for interactive convenience. Graph replay records this flag and\n should use False for strict diagnostic workflows.\n tracking_policy: FULL computes topology history and lineage. GRAPH keeps\n the replayable operation node without computing a TopoDelta.\n clean: When True (the default) the faces and edges the cut leaves split are unified, as union does: a cylinder wall cut along its seam stays one face, so a later fillet or chamfer of its edges works. False keeps the raw kernel result.\n on_split: What a cut that splits the body (a slot through a bar) returns.\n ``\"pieces\"`` (default): every piece, as ``list[Solid]`` ordered by centroid\n (x, then y, then z), recorded like ``cut_rsolid_list``. ``\"error\"``: raise,\n for code that needs exactly one Solid.\n\nReturns:\n Solid: The cut result solid, when the result is one piece.\n list[Solid]: The pieces (a ``scad.Pieces``), when the cut split the body\n (``on_split=\"pieces\"``). Operations that take one shape run on every piece\n (``cut_rsolid(pieces, tool)``, ``translate_shape(pieces, ...)``) and return\n ``Pieces``; a selector's cardinality is checked over all pieces. They are notebook values: a Part holds exactly one solid, so make one Part of each piece\n (an Assembly of them is the several-piece result), not ``make_part_rpart(body=pieces)``.\n\nRaises:\n SimpleCADError: The cut produced no solid, or (``on_split=\"error\"``) split\n the body. Use ``cut_rsolid_list`` to always get a list.\n\nUsage:\n Accepts a base solid followed by one or more tool solids, including nested\n sequences, and returns a single `Solid`.", "diagnose_blend_failure": "Classify a fillet/chamfer failure; never raises.\n\nProbe order (cheapest explanation wins):\n\n1. Tangent adjacency — the two faces meeting at a selected edge are\n already tangent (dihedral ~180°), so the blend surface degenerates to\n zero width regardless of size.\n2. Room — blend size exceeds the distance from the edge to the opposite\n boundary of an adjacent face.\n3. Interaction — per-edge diagnostic rerun (``retry_single``, capped at\n 64 edges) isolates edges that fail alone; when every edge passes\n alone but the joint blend fails, opposite blends on a shared face\n meeting tangentially (2×size >= face room) is the explanation.", "diagnose_boolean_failure": "Classify a boolean failure from its operands; never raises.\n\nModel: operands within ``tol`` of each other form one connected cluster\n(transitively). More than one cluster means the union is genuinely\nseparated — report the nearest inter-cluster pair. A single cluster that\nstill failed to fuse contains a touch-without-overlap pair — report it as\nnon-manifold contact. Returns None when no pair explains the failure.", "diff_sketch_rsketchdiff": "Report what an edit did: entity and constraint ids created, removed or changed between two sketches.\n\nReturns a dict with ``created``, ``removed``, ``changed`` (entity ids, points included) and\n``created_constraints``, ``removed_constraints``, ``changed_constraints``. Not recorded in the graph.", @@ -180,7 +180,7 @@ "inspect_sketch_rsketchprofiles": "List the profiles a sketch offers: the indices ``make_face_from_sketch_rface`` and ``make_wire_from_sketch_rwire`` take.\n\nReturns ``{\"profiles\": [...], \"unusable\": [...]}``. Each profile is ``{index, id, kind, closed, entity_ids, point_ids, reversed}``: ``kind`` is ``circle``, ``edge_loop`` or ``edge_chain``; ``entity_ids`` are in walking order and ``reversed`` says which edges are walked from end to start. ``unusable`` words why an edge set is no profile (a branch point carrying more than two edges, for one). A free circle is a profile; a loop is a connected set of edges whose points each carry exactly two. Not recorded in the graph; pass ``strict=False`` for a sketch that does not solve cleanly.", "inspect_sketch_rsketchresult": "Inspect sketch constraints by running the solver without recording graph nodes.", "internal_tag_binding": "Create a source-marked binding for transitional internal annotations.", - "intersect_rsolid": "Compute the boolean intersection of solids.\n\nArgs:\n solids: One or more Solid objects or sequences of Solid. Nested sequences are\n flattened before processing.\n\nReturns:\n Solid: The overlap region as a single solid.\n\nUsage:\n Accepts one or more solids, including nested sequences, and returns a single\n `Solid`. If the inputs do not overlap meaningfully, the API raises a clear\n error instead of returning an empty list.", + "intersect_rsolid": "Compute the boolean intersection of solids.\n\nArgs:\n solids: One or more Solid objects or sequences of Solid. Nested sequences are\n flattened before processing.\n clean: When True (the default) the faces and edges the intersection leaves split are unified, as union does: a cylinder wall cut along its seam stays one face, so a later fillet or chamfer of its edges works. False keeps the raw kernel result.\n\nReturns:\n Solid: The overlap region as a single solid.\n\nUsage:\n Accepts one or more solids, including nested sequences, and returns a single\n `Solid`. If the inputs do not overlap meaningfully, the API raises a clear\n error instead of returning an empty list.", "is_normalized_tag": "Check whether a tag matches the normalized dot-token format.", "isolated_recording": "Run a nested definition build as if it were called at top level.\n\nThe caller's session is hidden (no active session, recording not\nsuspended) and the world coordinate system is current, so the nested\nbuild records only into its own session and does not depend on where it\nwas called from. Everything is restored on exit.", "legacy_tag_binding": "Import one flat compatibility token without inventing assignment intent.", @@ -347,7 +347,7 @@ "validate_placement_ticks": "Validate a persisted tick frame with exact integer arithmetic.", "value": "Build a value key extractor for ordering and projection in QL.\n\nArgs:\n path: Property or metadata path to resolve.\n default: Fallback value when the path is missing.\n\nReturns:\n Serializable key function for `Query.order_by(...)`.", "var": "Create a physical or legacy scalar variable.\n\n``tolerance=0.1`` declares a symmetric ``+/-0.1`` tolerance. Use a\n``(lower_deviation, upper_deviation)`` pair for an asymmetric tolerance.\n``tolerance_unit`` defaults to ``unit`` when a nominal unit is declared.\nValues are converted to canonical CAD units only when evaluated, so the\ndeclaration and serialized expression node preserve the user's units.\n\nVariables without ``unit`` retain legacy unitless behavior. A unit-aware\nexpression cannot mix declared-unit variables with legacy variables.", - "cut_rsolid_list": "Compute the boolean difference of solids and keep every piece.\n\nA cut can split the body (a slot through a bar leaves two parts) or consume it\n(a tool that encloses the whole body leaves nothing). ``cut_rsolid`` refuses\nsuch a result because it returns exactly one ``Solid``; this operator returns\nthem all. A tool is cut from every piece it meets, so several tools may split\na body into several parts.\n\nArgs:\n solids: One or more Solid objects or sequences of Solid. Nested sequences are\n flattened before processing; the first solid is the base, the rest are\n subtracted in order.\n skip_non_intersecting: When True, a tool that meets none of the current\n pieces is ignored; when False it raises.\n tracking_policy: FULL computes topology history and lineage. GRAPH keeps\n the replayable operation node without computing a TopoDelta.\n\nReturns:\n list[Solid]: Every piece of the cut, in kernel order; empty when the tools\n consume the base.\n\nRaises:\n SimpleCADError: An input is not a valid Solid, or the kernel could not\n compute the cut.\n\nUsage:\n ``pieces = cut_rsolid_list(bar, slot)`` returns both halves of a bar cut by\n a through slot. Use ``cut_rsolid`` when the result must be one solid.\n", + "cut_rsolid_list": "Compute the boolean difference of solids and keep every piece.\n\nA cut can split the body (a slot through a bar leaves two parts) or consume it\n(a tool that encloses the whole body leaves nothing). ``cut_rsolid`` refuses\nsuch a result because it returns exactly one ``Solid``; this operator returns\nthem all. A tool is cut from every piece it meets, so several tools may split\na body into several parts.\n\nArgs:\n solids: One or more Solid objects or sequences of Solid. Nested sequences are\n flattened before processing; the first solid is the base, the rest are\n subtracted in order.\n skip_non_intersecting: When True, a tool that meets none of the current\n pieces is ignored; when False it raises.\n tracking_policy: FULL computes topology history and lineage. GRAPH keeps\n the replayable operation node without computing a TopoDelta.\n clean: When True (the default) the faces and edges the cut leaves split are unified, as union does: a cylinder wall cut along its seam stays one face, so a later fillet or chamfer of its edges works. False keeps the raw kernel result.\n\nReturns:\n list[Solid]: Every piece of the cut, in kernel order; empty when the tools\n consume the base.\n\nRaises:\n SimpleCADError: An input is not a valid Solid, or the kernel could not\n compute the cut.\n\nUsage:\n ``pieces = cut_rsolid_list(bar, slot)`` returns both halves of a bar cut by\n a through slot. Use ``cut_rsolid`` when the result must be one solid.\n", "Body": "The solid of a Part, with its display name, colour and opacity.\n\n``Body(body_id, solid, name=None, color=None, opacity=None)``: ``body_id`` is an\nidentifier (``\"body\"`` for the body of a Part), ``solid`` the body's Solid, ``name`` an optional\ndisplay name, ``color`` an optional display colour (RGB floats in [0, 1]) and\n``opacity`` an optional display opacity in [0, 1]. A one-body Part has one Body with\nthe id ``\"body\"``. The colour overrides the Part's material colour for display only;\nset it on an existing Part with ``set_body_appearance_rpart``.", "measure_part_mass_rmassreport": "Measure the volume, mass and centre of mass of a Part.\n\nArgs:\n part: The Part to measure.\n density: Density to use instead of the material's; ``None`` uses the\n Part's material, and without a material density every mass is ``None``.\n density_unit: Unit of ``density``: ``kg/m^3`` (default), ``g/cm^3``,\n ``kg/mm^3`` or ``g/mm^3``.\n\nReturns:\n MassReport: ``bodies`` (the Part's one ``BodyMass``: ``body_id``, ``volume_mm3``,\n ``mass_kg``, ``centroid_mm``) and the totals of the part (``volume_mm3``,\n ``mass_kg``, ``centroid_mm``).\n\nRaises:\n SimpleCADError: The value is not a Part, or the density or its unit is invalid.\n\nUsage:\n ``scad.measure_part_mass_rmassreport(bracket).mass_kg`` is the mass of the part.\n The part's material gives the density unless ``density`` is passed.", "inspect_curve_rcurvedata": "Read the NURBS data of an edge's curve: what the curve edits work on.\n\nReturns ``{type, analytic, editable, conversion, degree, poles, weights, knots, multiplicities, periodic, rational, closed, domain, reversed, parameter_range}``. ``poles`` are points in the current coordinate system, indices are 0-based (the ``moves`` of ``move_curve_poles_redge``); ``parameter_range`` is the edge's own range (where ``insert_curve_knot_redge`` may insert). For a line, circle or other analytic curve ``editable`` is false, ``conversion`` is ``\"convert_to_bspline_redge\"`` and the data is what that conversion would produce. Not recorded in the graph.", diff --git a/src/simplecadapi/recording/serializer.py b/src/simplecadapi/recording/serializer.py index 7ba05f78..4898733f 100644 --- a/src/simplecadapi/recording/serializer.py +++ b/src/simplecadapi/recording/serializer.py @@ -4290,7 +4290,7 @@ def _vector(value: Any) -> Any: if op_name == "make_cut_rsolid": ctx.require_params( - node.node_id, op_name, params, ("tool_count",) + node.node_id, op_name, params, ("tool_count", "clean") ) if len(node.inputs) < 2: if ctx.strict: @@ -4320,13 +4320,14 @@ def _vector(value: Any) -> Any: ) ), tracking_policy=str(params.get("tracking_policy", "full")), + clean=bool(params.get("clean")), ) _store_outputs(node, result) continue if op_name == "make_cut_rsolid_list": ctx.require_params( - node.node_id, op_name, params, ("tool_count", "input_refs") + node.node_id, op_name, params, ("tool_count", "input_refs", "clean") ) inputs = _ordered_input_shapes(ctx, graph, outputs, node, params) if len(inputs) < 2: @@ -4340,6 +4341,7 @@ def _vector(value: Any) -> Any: _param(ctx, node.node_id, op_name, params, "skip_non_intersecting", False) ), "tracking_policy": str(params.get("tracking_policy", "full")), + "clean": bool(params.get("clean")), } if params.get("piece_order") == "centroid": # a split recorded by cut_rsolid(on_split="pieces"): the pieces in centroid order @@ -4405,14 +4407,16 @@ def _vector(value: Any) -> Any: if op_name == "make_intersect_rsolid": ctx.require_params( - node.node_id, op_name, params, ("input_count",) + node.node_id, op_name, params, ("input_count", "clean") ) all_solids = [ cast(Solid, shape) for shape in _all_input_outputs(ctx, outputs, node) ] if len(all_solids) >= 2: - result = ops.intersect_rsolid(all_solids[0], all_solids[1:]) + result = ops.intersect_rsolid( + all_solids[0], all_solids[1:], clean=bool(params.get("clean")) + ) _store_outputs(node, result) elif ctx.strict: ctx.fail( diff --git a/test/test_boolean_clean.py b/test/test_boolean_clean.py new file mode 100644 index 00000000..0c5844a0 --- /dev/null +++ b/test/test_boolean_clean.py @@ -0,0 +1,128 @@ +"""cut / intersect unify what they leave split, and a blend never hands back an invalid solid (issue #88). + +A cut through the side of a box with a cylinder splits the cylinder wall along the cylinder's parametric seam into two +faces, and the top edge of the notch into a long and a short arc. OCCT then fillets that top edge wrongly: it returns +an invalid solid with a volume that is too small (case A), or refuses and the diagnosis blames the radius (case B). +The same geometry fillets fine once the faces are unified, which is what union already did and cut did not. +""" + +from __future__ import annotations + +import unittest + +import simplecadapi as scad +from simplecadapi import ql +from simplecadapi._cpp.occt.BRepCheck import BRepCheck_Analyzer +from simplecadapi.errors import SimpleCADError +from simplecadapi.recording.graph import GraphSession + +# the issue's two placements of the cylinder; volumes are what CadQuery and build123d (which clean by default) give +CASE_A = (0.0, 17.5) +CASE_B = (-12.5, -17.5) +VOLUME_A = 29604.46 +VOLUME_B = 29811.31 + + +def notch(cx: float, cy: float, **options): + box = scad.make_box_rsolid(width=40, height=40, depth=20, bottom_face_center=(0, 0, 0)) + cylinder = scad.make_cylinder_rsolid(radius=10, height=40, bottom_face_center=(cx, cy, 10)) + return scad.cut_rsolid(box, cylinder, **options) + + +def top_edges(solid, z: float = 20.0): + return [ + edge + for edge in ql.edges().resolve(solid) + if abs(edge.get_start_vertex().get_coordinates()[2] - z) < 1e-6 + and abs(edge.get_end_vertex().get_coordinates()[2] - z) < 1e-6 + ] + + +def is_valid(solid) -> bool: + return bool(BRepCheck_Analyzer(solid.wrapped).IsValid()) + + +def face_count(solid) -> int: + return len(ql.faces().resolve(solid)) + + +class TestCutCleans(unittest.TestCase): + def test_cut_unifies_the_wall_the_seam_split(self): + self.assertEqual(face_count(notch(*CASE_A)), 8) + self.assertEqual(face_count(notch(*CASE_A, clean=False)), 9) + + def test_the_cleaned_cut_keeps_its_volume(self): + self.assertAlmostEqual(notch(*CASE_A).get_volume(), notch(*CASE_A, clean=False).get_volume(), places=3) + + def test_cut_list_and_intersect_clean_by_default_too(self): + box = scad.make_box_rsolid(width=40, height=40, depth=20, bottom_face_center=(0, 0, 0)) + cylinder = scad.make_cylinder_rsolid(radius=10, height=40, bottom_face_center=(0, 17.5, 10)) + (piece,) = scad.cut_rsolid_list(box, cylinder) + self.assertEqual(face_count(piece), 8) + (raw,) = scad.cut_rsolid_list(box, cylinder, clean=False) + self.assertEqual(face_count(raw), 9) + # the part of the box inside the cylinder: a cylinder wall cut along its seam is two faces unless unified + inside = scad.intersect_rsolid(box, cylinder) + raw_inside = scad.intersect_rsolid(box, cylinder, clean=False) + self.assertLess(face_count(inside), face_count(raw_inside)) + self.assertAlmostEqual(inside.get_volume(), raw_inside.get_volume(), places=3) + + +class TestFilletAfterCut(unittest.TestCase): + def test_case_a_fillets_to_a_valid_solid_with_the_right_volume(self): + cut = notch(*CASE_A) + filleted = scad.fillet_rsolid(solid=cut, edges=top_edges(cut), radius=3.0) + self.assertTrue(is_valid(filleted)) + self.assertAlmostEqual(filleted.get_volume(), VOLUME_A, delta=0.05) + + def test_case_b_fillets_with_the_radius_the_diagnosis_called_too_large(self): + cut = notch(*CASE_B) + filleted = scad.fillet_rsolid(solid=cut, edges=top_edges(cut), radius=3.0) + self.assertTrue(is_valid(filleted)) + self.assertAlmostEqual(filleted.get_volume(), VOLUME_B, delta=0.05) + + def test_an_unclean_cut_no_longer_yields_an_invalid_fillet(self): + """Case A without the clean-up: OCCT's fillet is invalid. It used to be returned; now it is an error.""" + cut = notch(*CASE_A, clean=False) + with self.assertRaises(SimpleCADError) as raised: + scad.fillet_rsolid(solid=cut, edges=top_edges(cut), radius=3.0) + self.assertIn("not a valid solid", str(raised.exception)) + + def test_an_unclean_cut_names_the_split_faces_not_the_radius(self): + """Case B without the clean-up: the kernel refuses, and the diagnosis says why instead of 'reduce the radius'.""" + cut = notch(*CASE_B, clean=False) + with self.assertRaises(SimpleCADError) as raised: + scad.fillet_rsolid(solid=cut, edges=top_edges(cut), radius=3.0) + message = str(raised.exception) + self.assertIn("split on a surface", message) + self.assertIn("clean=True", message) + self.assertNotIn("Reduce the radius", message) + + def test_a_radius_that_really_is_too_large_is_still_reported_as_such(self): + cut = notch(*CASE_A) + with self.assertRaises(SimpleCADError) as raised: + scad.fillet_rsolid(solid=cut, edges=top_edges(cut), radius=50.0) + self.assertNotIn("split on a surface", str(raised.exception)) + + +class TestRecordedClean(unittest.TestCase): + """``clean`` is a recorded parameter like ``skip_non_intersecting``; replay reads it, there is no default for a + record that lacks it.""" + + def record(self, **options): + with GraphSession() as session: + notch(*CASE_A, **options) + return session + + def test_the_flag_is_recorded(self): + self.assertTrue(self.record().graph.leaf_nodes()[0].params["clean"]) + self.assertFalse(self.record(clean=False).graph.leaf_nodes()[0].params["clean"]) + + def test_replay_reproduces_the_recorded_result(self): + for options, faces in (({}, 8), ({"clean": False}, 9)): + (replayed,) = scad.replay_graph(self.record(**options).graph) # the leaf: the cut + self.assertEqual(face_count(replayed), faces, options) + + +if __name__ == "__main__": + unittest.main()