diff --git a/src/libslic3r/CadDocument.cpp b/src/libslic3r/CadDocument.cpp index b6787b9923..caf46280ce 100644 --- a/src/libslic3r/CadDocument.cpp +++ b/src/libslic3r/CadDocument.cpp @@ -3092,6 +3092,16 @@ void CadDocument::apply_mate(std::vector& bodies, const CadFeature& f) bodies[tgt_body].shape = xform.Shape(); } +// Volume of a shape, 0 for anything that isn't a solid we can measure. Used to catch a +// subtraction that removed nothing (snaporca-daf). +static double solid_volume(const TopoDS_Shape& s) +{ + if (s.IsNull()) return 0.0; + GProp_GProps props; + BRepGProp::VolumeProperties(s, props); + return std::abs(props.Mass()); +} + void CadDocument::route_feature(std::vector& bodies, const CadFeature& f) const { if (f.type == CadFeatureType::Plane) return; // datum plane: not part of the body pipeline @@ -3132,7 +3142,30 @@ void CadDocument::route_feature(std::vector& bodies, const CadFeature& if (t < 0) throw std::runtime_error("feature needs a body"); TopoDS_Shape result = bodies[t].shape; // shallow handle; apply_feature mutates it bool have_body = true; + // A subtraction whose tool misses the target is a legal boolean that removes nothing, so + // OCCT reports IsDone() and the feature lands in the recipe reporting success. A caller — + // an agent especially — then has no signal at all that the hole it asked for was never + // drilled: same body, same volume, ok:true. Measure the volume across the op and refuse + // the no-op. Only for removals: every other feature type may legitimately leave the volume + // alone (a Transform certainly does). snaporca-daf. + const bool removes = f.type == CadFeatureType::Hole + || f.type == CadFeatureType::Thread + || ((f.type == CadFeatureType::Extrude || f.type == CadFeatureType::Revolve + || f.type == CadFeatureType::Sweep || f.type == CadFeatureType::Loft) + && f.mode == BooleanMode::Cut); + const double before = removes ? solid_volume(result) : 0.0; apply_feature(result, have_body, context, f); + if (removes && before > 0.0) { + const double after = solid_volume(result); + // Relative tolerance: a cut that shaves a numerically invisible sliver is a miss too, + // and an absolute epsilon would be wrong across the mm-to-metre range of real parts. + if (after >= before - 1e-9 * std::max(1.0, before)) + throw std::runtime_error(std::string( + f.type == CadFeatureType::Hole ? "hole" : + f.type == CadFeatureType::Thread ? "thread" : "cut") + + " removed no material — the tool does not intersect the target body" + " (coordinates are in the sketch plane's frame, not world)"); + } bodies[t].shape = result; } } diff --git a/src/slic3r/GUI/McpControl.cpp b/src/slic3r/GUI/McpControl.cpp index c242964bfd..609f03476c 100644 --- a/src/slic3r/GUI/McpControl.cpp +++ b/src/slic3r/GUI/McpControl.cpp @@ -159,8 +159,8 @@ json describe_tools() json{{"name", "diameter"}, {"type", "number"}, {"unit", "mm"}, {"default", 5}, {"min", 0.01}}, json{{"name", "depth"}, {"type", "number"}, {"unit", "mm"}, {"default", 10}, {"min", 0.01}}, json{{"name", "through"}, {"type", "boolean"}, {"default", false}}, - json{{"name", "x"}, {"type", "number"}, {"unit", "mm"}, {"default", 0}}, - json{{"name", "y"}, {"type", "number"}, {"unit", "mm"}, {"default", 0}}, + json{{"name", "x"}, {"type", "number"}, {"unit", "mm"}, {"default", 0}, {"description", "in the sketch plane's frame (origin = describe_scene.modeling_origin), NOT world"}}, + json{{"name", "y"}, {"type", "number"}, {"unit", "mm"}, {"default", 0}, {"description", "in the sketch plane's frame (origin = describe_scene.modeling_origin), NOT world"}}, json{{"name", "plane"}, {"type", "string"}, {"enum", json::array({"XY", "XZ", "YZ"})}, {"default", "XY"}}, })}}, json{{"name", "hole_styled"}, {"summary", "Drill a hole with optional counterbore (style=1) or countersink (style=2) at (x,y) on a plane."}, @@ -168,8 +168,8 @@ json describe_tools() json{{"name", "diameter"}, {"type", "number"}, {"unit", "mm"}, {"default", 5}, {"min", 0.01}}, json{{"name", "depth"}, {"type", "number"}, {"unit", "mm"}, {"default", 10}, {"min", 0.01}}, json{{"name", "through"}, {"type", "boolean"}, {"default", true}}, - json{{"name", "x"}, {"type", "number"}, {"unit", "mm"}, {"default", 0}}, - json{{"name", "y"}, {"type", "number"}, {"unit", "mm"}, {"default", 0}}, + json{{"name", "x"}, {"type", "number"}, {"unit", "mm"}, {"default", 0}, {"description", "in the sketch plane's frame (origin = describe_scene.modeling_origin), NOT world"}}, + json{{"name", "y"}, {"type", "number"}, {"unit", "mm"}, {"default", 0}, {"description", "in the sketch plane's frame (origin = describe_scene.modeling_origin), NOT world"}}, json{{"name", "plane"}, {"type", "string"}, {"enum", json::array({"XY", "XZ", "YZ"})}, {"default", "XY"}}, json{{"name", "style"}, {"type", "integer"}, {"default", 0}, {"description", "0=simple, 1=counterbore, 2=countersink"}}, json{{"name", "cbore_diameter"}, {"type", "number"}, {"unit", "mm"}, {"default", 0}}, @@ -184,8 +184,8 @@ json describe_tools() json{{"name", "style"}, {"type", "integer"}, {"default", 0}}, json{{"name", "through"}, {"type", "boolean"}, {"default", true}}, json{{"name", "depth"}, {"type", "number"}, {"unit", "mm"}, {"default", 10}, {"min", 0.01}}, - json{{"name", "x"}, {"type", "number"}, {"unit", "mm"}, {"default", 0}}, - json{{"name", "y"}, {"type", "number"}, {"unit", "mm"}, {"default", 0}}, + json{{"name", "x"}, {"type", "number"}, {"unit", "mm"}, {"default", 0}, {"description", "in the sketch plane's frame (origin = describe_scene.modeling_origin), NOT world"}}, + json{{"name", "y"}, {"type", "number"}, {"unit", "mm"}, {"default", 0}, {"description", "in the sketch plane's frame (origin = describe_scene.modeling_origin), NOT world"}}, json{{"name", "plane"}, {"type", "string"}, {"enum", json::array({"XY", "XZ", "YZ"})}, {"default", "XY"}}, })}}, json{{"name", "boolean"}, {"summary", "Combine two bodies: union | subtract (tool from target) | intersect."}, diff --git a/tests/libslic3r/test_caddocument.cpp b/tests/libslic3r/test_caddocument.cpp index 79f76e176f..e90f3d2013 100644 --- a/tests/libslic3r/test_caddocument.cpp +++ b/tests/libslic3r/test_caddocument.cpp @@ -6697,3 +6697,58 @@ TEST_CASE("Failed sketch solve leaves geometry untouched", "[CadDocument]") CHECK((ents[b].p1 - ents[xi].p1).norm() == Approx(0.0).margin(1e-6)); } } + +// A subtraction whose tool misses the target is a perfectly legal boolean that removes nothing, +// so OCCT reports success and the feature lands in the recipe with ok:true and an unchanged body. +// That is how a hole placed with world coordinates instead of plane-frame ones read as "drilled" +// three times in a row while the volume never moved. snaporca-daf. +TEST_CASE("A cut that removes no material is an error, not a silent success", "[CadDocument]") +{ + // 20 x 20 box, 20 tall, centred on the origin of the XY plane. + auto box = [] { + CadDocument d; + int sk = d.add_sketch(SketchShape::Rectangle, SketchPlane::XY(), 20, 20, 10, "Sketch"); + d.add_extrude(sk, 20.0, false, BooleanMode::New, "Extrude"); + return d; + }; + + SECTION("hole placed clear of the body is rejected") { + CadDocument doc = box(); + REQUIRE(doc.recompute()); + const double solid = doc.body_mass_properties(0).volume; + + // 135 mm away — exactly the failure the issue reported (world centre passed for a + // plane-frame coordinate). The old behaviour: recompute() true, volume unchanged. + doc.add_hole(8.0, 20.0, true, 135.0, 0.0, SketchPlane::XY(), "Hole"); + CHECK_FALSE(doc.recompute()); + CHECK(doc.error.find("removed no material") != std::string::npos); + + // And the body is left as it was, not half-applied. + CadDocument again = box(); + REQUIRE(again.recompute()); + CHECK(again.body_mass_properties(0).volume == Approx(solid).margin(1e-6)); + } + + SECTION("the same hole on the body still works") { + CadDocument doc = box(); + REQUIRE(doc.recompute()); + const double solid = doc.body_mass_properties(0).volume; + doc.add_hole(8.0, 20.0, true, 0.0, 0.0, SketchPlane::XY(), "Hole"); + REQUIRE(doc.recompute()); + CHECK(doc.body_mass_properties(0).volume + == Approx(solid - M_PI * 16.0 * 20.0).epsilon(0.01)); + } + + SECTION("a cut-mode extrude that misses is rejected too") { + CadDocument doc = box(); + REQUIRE(doc.recompute()); + // Same trick, on the sketch plane's origin: the profile sits far outside the box, + // so the subtraction is a legal no-op. + SketchPlane far_plane = SketchPlane::XY(); + far_plane.origin = Vec3d(200.0, 0.0, 0.0); + int sk = doc.add_sketch(SketchShape::Rectangle, far_plane, 5, 5, 10, "Tool"); + doc.add_extrude(sk, 30.0, false, BooleanMode::Cut, "Cut"); + CHECK_FALSE(doc.recompute()); + CHECK(doc.error.find("removed no material") != std::string::npos); + } +}