From 1cb80f7f9f4bd44f08ce95ab5c667eeb0e7d84ea Mon Sep 17 00:00:00 2001 From: Tommaso Bianchi Date: Sun, 26 Jul 2026 07:04:15 +0200 Subject: [PATCH] Project: implement "(all edges)"; stop discarding the failure reason MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects found while driving the tools that Phase B wired but nobody had exercised yet. apply_project had no all-edges branch: with no face picked and no explicit edge list it threw "no edges or face selected". That is precisely the state the Project card opens in, and its label reads "(all edges)" — so the card's default could never be confirmed. It now projects every edge of the source body. Edges perpendicular to the target plane collapse to a point when projected, so segments whose endpoints coincide are dropped instead of being emitted as zero-length lines that would poison the sketch downstream. The second defect is why the first one was invisible. 29 of the 31 rollback sites in McpControl ran `if (!ok) doc.undo();`, and undo() recomputes the restored feature list — which succeeds and clears doc.error. Every failing command therefore reported `error: ""`. Yesterday's fix covered 2 sites and I treated the file as done; it was not. All 31 now capture the reason before the rollback and restore it after. Failures that read as `""` now read as "rib: bad entity" / "surface-revolve: revolve failed". Verified on the running GUI through the control socket: the Project call that previously returned ok:false now returns ok:true, and failures carry a reason. Co-Authored-By: Claude Opus 5 (1M context) --- src/libslic3r/CadDocument.cpp | 38 ++++++++++-------- src/slic3r/GUI/McpControl.cpp | 58 ++++++++++++++-------------- tests/libslic3r/test_caddocument.cpp | 25 ++++++++++++ 3 files changed, 76 insertions(+), 45 deletions(-) diff --git a/src/libslic3r/CadDocument.cpp b/src/libslic3r/CadDocument.cpp index aed5834133..f7b7e965ca 100644 --- a/src/libslic3r/CadDocument.cpp +++ b/src/libslic3r/CadDocument.cpp @@ -2831,7 +2831,11 @@ void CadDocument::apply_project(const std::vector& bodies, CadFeature& if (fc.IsNull()) throw std::runtime_error("project: face not found"); edges = GeometryEngine::edges_of_face(fc); } else { - throw std::runtime_error("project: no edges or face selected"); + // No face and no explicit selection means "all edges" — the state the Project card + // starts in, and its label says so. Edges perpendicular to the target plane collapse + // to a point when projected; they are dropped below rather than emitted as + // zero-length lines. + edges = GeometryEngine::edges_of(shape); } if (edges.empty()) throw std::runtime_error("project: no edges to project"); @@ -2840,15 +2844,23 @@ void CadDocument::apply_project(const std::vector& bodies, CadFeature& return Vec2d(d.dot(f.plane.x_axis), d.dot(f.plane.y_axis)); }; + // A segment whose endpoints coincide after projection carries no geometry: that is what + // an edge perpendicular to the target plane becomes. Emitting it as a zero-length line + // would poison the sketch downstream, so drop it here. + auto push_line = [&](const Vec2d& a, const Vec2d& b) { + if ((b - a).norm() < 1e-7) return; + SketchEntity se; se.type = SketchEntity::Type::Line; + se.p0 = a; se.p1 = b; + f.entities.push_back(se); + }; + for (const TopoDS_Edge& e : edges) { BRepAdaptor_Curve ac(e); const GeomAbs_CurveType ct = ac.GetType(); if (ct == GeomAbs_Line) { gp_Pnt a = ac.Value(ac.FirstParameter()); gp_Pnt b = ac.Value(ac.LastParameter()); - SketchEntity se; se.type = SketchEntity::Type::Line; - se.p0 = to2d(a); se.p1 = to2d(b); - f.entities.push_back(se); + push_line(to2d(a), to2d(b)); } else if (ct == GeomAbs_Circle) { gp_Circ c = ac.Circle(); gp_Dir cn = c.Axis().Direction(); @@ -2876,20 +2888,14 @@ void CadDocument::apply_project(const std::vector& bodies, CadFeature& continue; } std::vector pts = GeometryEngine::sample_edge_world(e); - for (size_t i = 1; i < pts.size(); ++i) { - SketchEntity se; se.type = SketchEntity::Type::Line; - se.p0 = to2d(gp_Pnt(pts[i-1].x(), pts[i-1].y(), pts[i-1].z())); - se.p1 = to2d(gp_Pnt(pts[i].x(), pts[i].y(), pts[i].z())); - f.entities.push_back(se); - } + for (size_t i = 1; i < pts.size(); ++i) + push_line(to2d(gp_Pnt(pts[i-1].x(), pts[i-1].y(), pts[i-1].z())), + to2d(gp_Pnt(pts[i].x(), pts[i].y(), pts[i].z()))); } else { std::vector pts = GeometryEngine::sample_edge_world(e); - for (size_t i = 1; i < pts.size(); ++i) { - SketchEntity se; se.type = SketchEntity::Type::Line; - se.p0 = to2d(gp_Pnt(pts[i-1].x(), pts[i-1].y(), pts[i-1].z())); - se.p1 = to2d(gp_Pnt(pts[i].x(), pts[i].y(), pts[i].z())); - f.entities.push_back(se); - } + for (size_t i = 1; i < pts.size(); ++i) + push_line(to2d(gp_Pnt(pts[i-1].x(), pts[i-1].y(), pts[i-1].z())), + to2d(gp_Pnt(pts[i].x(), pts[i].y(), pts[i].z()))); } } if (f.entities.empty()) throw std::runtime_error("project: produced no entities"); diff --git a/src/slic3r/GUI/McpControl.cpp b/src/slic3r/GUI/McpControl.cpp index 98cd4cc638..c242964bfd 100644 --- a/src/slic3r/GUI/McpControl.cpp +++ b/src/slic3r/GUI/McpControl.cpp @@ -642,7 +642,7 @@ json import_step(DesignPanel* panel, const json& params) doc.features.push_back(f); } bool ok = doc.recompute(); - if (!ok) doc.undo(); + if (!ok) { const std::string why = doc.error; doc.undo(); doc.error = why; } panel->mcp_after_change(); return json{{"ok", ok}, {"imported", int(solids.size())}, {"first_feature", first}, {"bodies", int(doc.bodies.size())}, {"error", doc.error}}; @@ -687,7 +687,7 @@ json import_mesh(DesignPanel* panel, const json& params) doc.features.push_back(f); const bool ok = doc.recompute(); - if (!ok) doc.undo(); + if (!ok) { const std::string why = doc.error; doc.undo(); doc.error = why; } panel->mcp_after_change(); return json{{"ok", ok}, {"first_feature", first}, {"bodies", int(doc.bodies.size())}, {"input_triangles", st.input_tris}, {"kept_triangles", st.kept_tris}, @@ -788,7 +788,7 @@ json action_extrude(DesignPanel* panel, const json& params) else if (end == "up_to_face") { fe.extrude_end = ExtrudeEnd::UpToFace; fe.up_to_face = params.value("up_to_face", -1); } else fe.extrude_end = ExtrudeEnd::Blind; bool ok = doc.recompute(); - if (!ok) doc.undo(); + if (!ok) { const std::string why = doc.error; doc.undo(); doc.error = why; } panel->mcp_after_change(); return json{{"ok", ok}, {"sketch_index", s}, {"extrude_index", e}, {"end", end}, {"bodies", int(doc.bodies.size())}, {"error", doc.error}}; @@ -815,7 +815,7 @@ json action_revolve(DesignPanel* panel, const json& params) : doc.add_sketch(SketchShape::Rectangle, pl, w, h, 0.0, "Sketch"); int r = doc.add_revolve(s, angle, axis, flip, mode, "Revolve"); bool ok = doc.recompute(); - if (!ok) doc.undo(); + if (!ok) { const std::string why = doc.error; doc.undo(); doc.error = why; } panel->mcp_after_change(); return json{{"ok", ok}, {"sketch_index", s}, {"revolve_index", r}, {"bodies", int(doc.bodies.size())}, {"error", doc.error}}; @@ -843,7 +843,7 @@ json action_fillet(DesignPanel* panel, const json& params) int f = doc.add_fillet(radius, params["edge"].get(), "Fillet"); if (bi >= 0) doc.features[f].target_body = bi; // edge id resolved against THIS body's shape bool ok = doc.recompute(); - if (!ok) doc.undo(); + if (!ok) { const std::string why = doc.error; doc.undo(); doc.error = why; } panel->mcp_after_change(); return json{{"ok", ok}, {"fillet_index", f}, {"body", bi < 0 ? int(doc.bodies.size()) - 1 : bi}, {"bodies", int(doc.bodies.size())}, {"error", doc.error}}; @@ -861,7 +861,7 @@ json action_chamfer(DesignPanel* panel, const json& params) int c = doc.add_chamfer(dist, params["edge"].get(), "Chamfer"); if (bi >= 0) doc.features[c].target_body = bi; // edge id resolved against THIS body's shape bool ok = doc.recompute(); - if (!ok) doc.undo(); + if (!ok) { const std::string why = doc.error; doc.undo(); doc.error = why; } panel->mcp_after_change(); return json{{"ok", ok}, {"chamfer_index", c}, {"body", bi < 0 ? int(doc.bodies.size()) - 1 : bi}, {"bodies", int(doc.bodies.size())}, {"error", doc.error}}; @@ -880,7 +880,7 @@ json action_hole(DesignPanel* panel, const json& params) doc.checkpoint(); int h = doc.add_hole(dia, depth, thru, x, y, pl, "Hole"); bool ok = doc.recompute(); - if (!ok) doc.undo(); + if (!ok) { const std::string why = doc.error; doc.undo(); doc.error = why; } panel->mcp_after_change(); return json{{"ok", ok}, {"hole_index", h}, {"bodies", int(doc.bodies.size())}, {"error", doc.error}}; } @@ -906,7 +906,7 @@ json action_hole_styled(DesignPanel* panel, const json& params) cbore_diameter, cbore_depth, csink_diameter, csink_angle, standard, "Hole"); bool ok = doc.recompute(); - if (!ok) doc.undo(); + if (!ok) { const std::string why = doc.error; doc.undo(); doc.error = why; } panel->mcp_after_change(); return json{{"ok", ok}, {"hole_index", h}, {"bodies", int(doc.bodies.size())}, {"error", doc.error}}; } @@ -926,7 +926,7 @@ json action_hole_standard(DesignPanel* panel, const json& params) try { int h = doc.add_hole_standard(desig, style, thru, depth, x, y, pl, "Hole"); bool ok = doc.recompute(); - if (!ok) doc.undo(); + if (!ok) { const std::string why = doc.error; doc.undo(); doc.error = why; } panel->mcp_after_change(); return json{{"ok", ok}, {"hole_index", h}, {"bodies", int(doc.bodies.size())}, {"error", doc.error}}; } catch (const std::exception& ex) { @@ -952,7 +952,7 @@ json action_boolean(DesignPanel* panel, const json& params) doc.checkpoint(); int b = doc.add_boolean(m, target, tool, keep, tol, -1, -1, "Boolean"); bool ok = doc.recompute(); - if (!ok) doc.undo(); + if (!ok) { const std::string why = doc.error; doc.undo(); doc.error = why; } panel->mcp_after_change(); return json{{"ok", ok}, {"boolean_index", b}, {"bodies", int(doc.bodies.size())}, {"error", doc.error}}; } @@ -972,7 +972,7 @@ json action_pattern(DesignPanel* panel, const json& params) int p = doc.add_pattern(circular, count, spacing, dir, angle, bi, "Pattern"); doc.features[p].plane = plane_from(params, doc); // axis (circular) / step dirs (linear) bool ok = doc.recompute(); - if (!ok) doc.undo(); + if (!ok) { const std::string why = doc.error; doc.undo(); doc.error = why; } panel->mcp_after_change(); return json{{"ok", ok}, {"pattern_index", p}, {"bodies", int(doc.bodies.size())}, {"error", doc.error}}; } @@ -991,7 +991,7 @@ json action_pattern_on_curve(DesignPanel* panel, const json& params) doc.checkpoint(); int p = doc.add_pattern_on_curve(count, sketch, entity, bi, "PatternOnCurve"); bool ok = doc.recompute(); - if (!ok) doc.undo(); + if (!ok) { const std::string why = doc.error; doc.undo(); doc.error = why; } panel->mcp_after_change(); return json{{"ok", ok}, {"pattern_index", p}, {"bodies", int(doc.bodies.size())}, {"error", doc.error}}; } @@ -1007,7 +1007,7 @@ json action_shell(DesignPanel* panel, const json& params) doc.checkpoint(); int s = doc.add_shell(thickness, face, bi, "Shell"); bool ok = doc.recompute(); - if (!ok) doc.undo(); + if (!ok) { const std::string why = doc.error; doc.undo(); doc.error = why; } panel->mcp_after_change(); return json{{"ok", ok}, {"shell_index", s}, {"bodies", int(doc.bodies.size())}, {"error", doc.error}}; } @@ -1026,7 +1026,7 @@ json action_rib(DesignPanel* panel, const json& params) doc.checkpoint(); int idx = doc.add_rib(sketch, entity, thickness, depth, bi, "Rib"); bool ok = doc.recompute(); - if (!ok) doc.undo(); + if (!ok) { const std::string why = doc.error; doc.undo(); doc.error = why; } panel->mcp_after_change(); return json{{"ok", ok}, {"rib_index", idx}, {"bodies", int(doc.bodies.size())}, {"error", doc.error}}; } @@ -1040,7 +1040,7 @@ json action_surface_extrude(DesignPanel* panel, const json& params) doc.checkpoint(); int idx = doc.add_surface_extrude(sketch, distance, "SurfaceExtrude"); bool ok = doc.recompute(); - if (!ok) doc.undo(); + if (!ok) { const std::string why = doc.error; doc.undo(); doc.error = why; } panel->mcp_after_change(); return json{{"ok", ok}, {"feature_index", idx}, {"bodies", int(doc.bodies.size())}, {"error", doc.error}}; } @@ -1055,7 +1055,7 @@ json action_surface_revolve(DesignPanel* panel, const json& params) doc.checkpoint(); int idx = doc.add_surface_revolve(sketch, angle, axis, "SurfaceRevolve"); bool ok = doc.recompute(); - if (!ok) doc.undo(); + if (!ok) { const std::string why = doc.error; doc.undo(); doc.error = why; } panel->mcp_after_change(); return json{{"ok", ok}, {"feature_index", idx}, {"bodies", int(doc.bodies.size())}, {"error", doc.error}}; } @@ -1070,7 +1070,7 @@ json action_thicken_surface(DesignPanel* panel, const json& params) doc.checkpoint(); int idx = doc.add_thicken_surface(bi, thickness, flip, "ThickenSurface"); bool ok = doc.recompute(); - if (!ok) doc.undo(); + if (!ok) { const std::string why = doc.error; doc.undo(); doc.error = why; } panel->mcp_after_change(); return json{{"ok", ok}, {"feature_index", idx}, {"bodies", int(doc.bodies.size())}, {"error", doc.error}}; } @@ -1084,7 +1084,7 @@ json action_surface_offset(DesignPanel* panel, const json& params) doc.checkpoint(); int idx = doc.add_surface_offset(bi, offset, "SurfaceOffset"); bool ok = doc.recompute(); - if (!ok) doc.undo(); + if (!ok) { const std::string why = doc.error; doc.undo(); doc.error = why; } panel->mcp_after_change(); return json{{"ok", ok}, {"feature_index", idx}, {"bodies", int(doc.bodies.size())}, {"error", doc.error}}; } @@ -1100,7 +1100,7 @@ json action_surface_loft(DesignPanel* panel, const json& params) doc.checkpoint(); int idx = doc.add_surface_loft(profiles, ruled, "SurfaceLoft"); bool ok = doc.recompute(); - if (!ok) doc.undo(); + if (!ok) { const std::string why = doc.error; doc.undo(); doc.error = why; } panel->mcp_after_change(); return json{{"ok", ok}, {"feature_index", idx}, {"bodies", int(doc.bodies.size())}, {"error", doc.error}}; } @@ -1113,7 +1113,7 @@ json action_surface_fill(DesignPanel* panel, const json& params) doc.checkpoint(); int idx = doc.add_surface_fill(sketch, "SurfaceFill"); bool ok = doc.recompute(); - if (!ok) doc.undo(); + if (!ok) { const std::string why = doc.error; doc.undo(); doc.error = why; } panel->mcp_after_change(); return json{{"ok", ok}, {"feature_index", idx}, {"bodies", int(doc.bodies.size())}, {"error", doc.error}}; } @@ -1128,7 +1128,7 @@ json action_draft(DesignPanel* panel, const json& params) doc.checkpoint(); int d = doc.add_draft(angle, params["face"].get(), bi, "Draft"); bool ok = doc.recompute(); - if (!ok) doc.undo(); + if (!ok) { const std::string why = doc.error; doc.undo(); doc.error = why; } panel->mcp_after_change(); return json{{"ok", ok}, {"draft_index", d}, {"bodies", int(doc.bodies.size())}, {"error", doc.error}}; } @@ -1145,7 +1145,7 @@ json action_mirror(DesignPanel* panel, const json& params) int idx = doc.add_mirror(plane_from(params, doc), bi, m, "Mirror"); doc.features[idx].mirror_keep_original = keep; bool ok = doc.recompute(); - if (!ok) doc.undo(); + if (!ok) { const std::string why = doc.error; doc.undo(); doc.error = why; } panel->mcp_after_change(); return json{{"ok", ok}, {"mirror_index", idx}, {"bodies", int(doc.bodies.size())}, {"error", doc.error}}; } @@ -1163,7 +1163,7 @@ json action_transform(DesignPanel* panel, const json& params) doc.checkpoint(); int idx = doc.add_transform(bi, translate, axis, pivot, angle, copy, "Transform"); bool ok = doc.recompute(); - if (!ok) doc.undo(); + if (!ok) { const std::string why = doc.error; doc.undo(); doc.error = why; } panel->mcp_after_change(); return json{{"ok", ok}, {"transform_index", idx}, {"bodies", int(doc.bodies.size())}, {"error", doc.error}}; } @@ -1180,7 +1180,7 @@ json action_thicken(DesignPanel* panel, const json& params) doc.checkpoint(); int idx = doc.add_thicken(bi, face, thickness, flip, "Thicken"); bool ok = doc.recompute(); - if (!ok) doc.undo(); + if (!ok) { const std::string why = doc.error; doc.undo(); doc.error = why; } panel->mcp_after_change(); return json{{"ok", ok}, {"thicken_index", idx}, {"bodies", int(doc.bodies.size())}, {"error", doc.error}}; } @@ -1198,7 +1198,7 @@ json action_split(DesignPanel* panel, const json& params) doc.checkpoint(); int idx = doc.add_split_by_face(bi, face_body, face, keep_upper, keep_lower, "Split"); bool ok = doc.recompute(); - if (!ok) doc.undo(); + if (!ok) { const std::string why = doc.error; doc.undo(); doc.error = why; } panel->mcp_after_change(); return json{{"ok", ok}, {"split_index", idx}, {"bodies", int(doc.bodies.size())}, {"error", doc.error}}; } @@ -1216,7 +1216,7 @@ json action_project(DesignPanel* panel, const json& params) doc.checkpoint(); int idx = doc.add_project_edges(source_body, edges, face, pl, "Project"); bool ok = doc.recompute(); - if (!ok) doc.undo(); + if (!ok) { const std::string why = doc.error; doc.undo(); doc.error = why; } panel->mcp_after_change(); return json{{"ok", ok}, {"project_index", idx}, {"bodies", int(doc.bodies.size())}, {"error", doc.error}}; } @@ -1233,7 +1233,7 @@ json action_delete_face(DesignPanel* panel, const json& params) doc.checkpoint(); int idx = doc.add_delete_face(bi, faces, "DeleteFace"); bool ok = doc.recompute(); - if (!ok) doc.undo(); + if (!ok) { const std::string why = doc.error; doc.undo(); doc.error = why; } panel->mcp_after_change(); return json{{"ok", ok}, {"feature_index", idx}, {"bodies", int(doc.bodies.size())}, {"error", doc.error}}; } @@ -1252,7 +1252,7 @@ json action_bridge(DesignPanel* panel, const json& params) doc.checkpoint(); int ei = doc.add_bridge(sketch, ent_a, end_a, ent_b, end_b, "Bridge"); bool ok = doc.recompute(); - if (!ok) doc.undo(); + if (!ok) { const std::string why = doc.error; doc.undo(); doc.error = why; } panel->mcp_after_change(); return json{{"ok", ok}, {"sketch_index", sketch}, {"entity_index", ei}, {"bodies", int(doc.bodies.size())}, {"error", doc.error}}; @@ -1343,7 +1343,7 @@ json action_mate(DesignPanel* panel, const json& params) doc.checkpoint(); int idx = doc.add_mate(kind, cs_a, cs_b, offset, angle, flip, "Mate"); bool ok = doc.recompute(); - if (!ok) doc.undo(); + if (!ok) { const std::string why = doc.error; doc.undo(); doc.error = why; } panel->mcp_after_change(); return json{{"ok", ok}, {"mate_index", idx}, {"bodies", int(doc.bodies.size())}, {"error", doc.error}}; } diff --git a/tests/libslic3r/test_caddocument.cpp b/tests/libslic3r/test_caddocument.cpp index d8029be725..8db9ad5a24 100644 --- a/tests/libslic3r/test_caddocument.cpp +++ b/tests/libslic3r/test_caddocument.cpp @@ -3365,6 +3365,31 @@ TEST_CASE("project a cylinder top edge to 1 circle, extrudable", "[CadDocument][ REQUIRE_THAT(v, WithinRel(M_PI * 36.0 * 4.0, 1e-2)); } +TEST_CASE("project with no face and no edge selection projects every edge", "[CadDocument][project]") +{ + // This is the state the Project card opens in — its label reads "(all edges)". Before the + // all-edges branch existed it threw "no edges or face selected", so the card's default + // could never be confirmed. + CadDocument doc; + int sk = doc.add_sketch(SketchShape::Rectangle, SketchPlane::XY(), 20, 20, 10, "Box"); + doc.add_extrude(sk, 10.0, false, BooleanMode::New, "Ext"); + REQUIRE(doc.recompute()); + + int proj = doc.add_project_edges(0, {}, -1, SketchPlane::XY(), "ProjAll"); + REQUIRE(proj >= 0); + REQUIRE(doc.recompute()); + REQUIRE(doc.error.empty()); + + // A box has 12 edges; the 4 running along Z collapse to points on XY and are dropped, + // leaving the 4 bottom and 4 top edges. + const auto& pf = doc.features[proj]; + REQUIRE(pf.entities.size() == 8); + for (const auto& e : pf.entities) { + REQUIRE(e.type == SketchEntity::Type::Line); + REQUIRE((e.p1 - e.p0).norm() > 1e-6); + } +} + TEST_CASE("project bad face id returns error", "[CadDocument][project]") { CadDocument doc;