juli/OrcaSlicer
1
1
Fork 0
mirror of https://github.com/OrcaSlicer/OrcaSlicer.git synced 2026-09-19 15:03:05 +00:00
Code Issues Packages Projects Releases Wiki Activity

A subtraction that removes nothing is an error, not a silent success

Browse Source 
A boolean cut whose tool misses the target is a perfectly legal
operation: OCCT reports IsDone(), the shape comes back unchanged, and the
feature lands in the recipe reporting ok:true. Driving the MCP socket,
that produced two consecutive {ok: true, bodies: 1, error: ''} responses
for a hole that was never drilled -- same viewport, same 46939.11 mm3 --
and the tree grew two Hole features that will never cut anything. A
caller, an agent especially, has no signal at all that the thing it asked
for did not happen.

Measure the volume across the op in route_feature's in-place branch and
refuse the no-op. Only for removals: Hole, Thread, and Extrude / Revolve
/ Sweep / Loft in Cut mode. Everything else may legitimately leave the
volume alone -- a Transform certainly does. The tolerance is relative,
because an absolute epsilon is wrong across the mm-to-metre range of real
parts, and a cut that shaves a numerically invisible sliver is a miss
too. The existing rollback in the MCP actions already preserves the
reason, so a missed hole now answers ok:false with the error and undoes
the feature.

The confusion underneath was not itself a bug: hole's x/y are in the
sketch plane's frame, whose origin is describe_scene's modeling_origin,
and describe_tools documented them only as "number, unit mm". Passing the
world centre put the hole 135 mm clear of the solid. All six x/y params
on hole / hole_styled / hole_standard now say which frame they are in,
since the wrong guess was silent.

Regression test drives the reported failure directly: a hole at x=135 on
a 20x20x20 box is rejected and leaves the body untouched, the same hole
at the origin still removes exactly pi*4^2*20, and a cut-mode extrude
whose profile sits at x=200 is rejected too. 152 cases / 2083 assertions
green on both forks.

snaporca-daf.
This commit is contained in:
Tommaso Bianchi
2026-07-27 07:21:06 +02:00
parent 5d05ca30ca
commit 5f1811c2c5
3 changed files with 94 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/libslic3r/CadDocument.cpp
+33
View File
@@ -3092,6 +3092,16 @@ void CadDocument::apply_mate(std::vector<CadBody>& 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<CadBody>& 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<CadBody>& 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;
}
}
src/slic3r/GUI/McpControl.cpp
+6 -6
View File
@@ -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."},
tests/libslic3r/test_caddocument.cpp
+55
View File
@@ -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);
}
}