diff --git a/src/libslic3r/CadDocument.cpp b/src/libslic3r/CadDocument.cpp index de50cc1da1..ebb7fbd913 100644 --- a/src/libslic3r/CadDocument.cpp +++ b/src/libslic3r/CadDocument.cpp @@ -32,6 +32,8 @@ #include #include #include +#include // plane_of_face: reject non-planar faces +#include #include #include #include @@ -1376,6 +1378,20 @@ static SketchPlane frame_from(const Vec3d& origin, const Vec3d& normal) return p; } +bool CadDocument::plane_of_face(int body_idx, int face_idx, SketchPlane& out) const +{ + if (face_idx < 0 || body_idx < 0 || body_idx >= int(bodies.size())) return false; + const TopoDS_Face f = GeometryEngine::face_by_index(bodies[body_idx].shape, face_idx); + if (f.IsNull()) return false; + // Planar only. face_normal_world evaluates the normal at the mid parameter, which on a cylinder + // or a fillet is a tangent plane at one arbitrary point — usable for a datum offset, wrong as a + // sketch plane. Refuse rather than sketch somewhere the user did not point at. + BRepAdaptor_Surface surf(f); + if (surf.GetType() != GeomAbs_Plane) return false; + out = frame_from(GeometryEngine::face_centroid_world(f), GeometryEngine::face_normal_world(f)); + return true; +} + std::vector> CadDocument::resolve_datum_planes() const { std::vector> out; diff --git a/src/libslic3r/CadDocument.hpp b/src/libslic3r/CadDocument.hpp index f87813cf85..405a5a22fe 100644 --- a/src/libslic3r/CadDocument.hpp +++ b/src/libslic3r/CadDocument.hpp @@ -553,6 +553,13 @@ public: // Every datum plane currently in the recipe, in feature order, as (name, plane). // Used by the GUI to populate plane pickers (after the 3 base planes). std::vector> resolve_datum_planes() const; + + // World-space sketch plane lying on a body's PLANAR face, so a face picked in the viewport can + // be sketched on directly — no datum plane in between and nothing to choose from a list. + // Returns false when the indices don't resolve or the face isn't planar (a cylinder or a fillet + // has no single plane, and guessing one from a mid-parameter normal would silently sketch on a + // tangent). Same derivation the Coincident datum method uses, shared so the two cannot drift. + bool plane_of_face(int body_idx, int face_idx, SketchPlane& out) const; // Resolved datum axes in feature order. axis_err is non-empty if construction failed. struct DatumAxis { std::string name; Vec3d origin{0,0,0}; Vec3d direction{0,0,1}; std::string error; }; diff --git a/src/slic3r/GUI/DesignPanel.cpp b/src/slic3r/GUI/DesignPanel.cpp index 113f9b674c..da442aaf7a 100644 --- a/src/slic3r/GUI/DesignPanel.cpp +++ b/src/slic3r/GUI/DesignPanel.cpp @@ -341,16 +341,24 @@ DesignPanel::DesignPanel(wxWindow* parent) auto select_tool = [this](DesignSketchTool::Mode mode, const wxString& hint) { if (!m_viewport) return; if (!m_viewport->is_sketching()) { - const SketchPlane plane = plane_from_choice(m_draw_plane->GetSelection()); + wxString on; + const SketchPlane plane = sketch_plane_from_selection(on); m_viewport->begin_sketch(plane, mode); m_construction->SetValue(false); // a fresh session starts non-construction + m_sketch_on = on; // shown with the tool hint, so the target is visible + // The face has been CONSUMED as the sketch plane, so drop the pick. Leaving it live + // meant the next Extrude saw a selected face and push/pulled it instead of extruding + // the sketch just drawn — the same trap the imported-art path already guards against. + if (m_sel_solid_face >= 0) + m_sel_solid_face = m_sel_solid_edge = m_sel_solid_body = -1; } else { m_viewport->set_sketch_tool(mode); } m_viewport->set_sketch_construction(m_construction->GetValue()); show_polygon_card(mode == DesignSketchTool::Mode::Polygon); m_status->SetForegroundColour(wxNullColour); - m_status->SetLabel(hint); + m_status->SetLabel(m_sketch_on.IsEmpty() ? hint + : wxString::Format(_L("%s · on %s"), hint, m_sketch_on)); m_status->Refresh(); }; @@ -3523,6 +3531,7 @@ void DesignPanel::set_active_tool_btn(ScalableButton* b) void DesignPanel::set_ui_mode(UiMode m) { m_ui_mode = m; + if (m != UiMode::Sketch) m_sketch_on.clear(); // no stale "on the picked face" on the next hint wxSizer* s = m_toolbar->GetSizer(); s->Show(m_tb_feature, m == UiMode::Feature, true); s->Show(m_tb_sketch, m == UiMode::Sketch, true); @@ -4837,6 +4846,29 @@ SketchPlane DesignPanel::plane_from_choice(int row) const SketchPlane p = SketchPlane::XY(); p.origin += m_doc.modeling_origin; return p; } +// A sketch goes where the user pointed. A planar face picked in the viewport wins outright; only +// when nothing is picked do we fall back to the reference plane, which itself is normally set by +// clicking one of the ghost planes in 3D (on_datum_base_picked) rather than by opening the combo. +// Before this, a picked face was ignored and the only way onto it was to build a Coincident datum +// plane first and then find it in a dropdown — three steps and a junk feature in the tree for the +// most common gesture in solid modelling. snaporca-3a2. +SketchPlane DesignPanel::sketch_plane_from_selection(wxString& what) const +{ + SketchPlane p; + if (m_sel_solid_face >= 0 && m_sel_solid_body >= 0 + && m_doc.plane_of_face(m_sel_solid_body, m_sel_solid_face, p)) { + what = (m_doc.bodies.size() > 1) + ? wxString::Format(_L("the picked face of Body %d"), m_sel_solid_body + 1) + : _L("the picked face"); + return p; + } + const int row = m_draw_plane ? m_draw_plane->GetSelection() : 0; + what = (m_draw_plane && row >= 0 && row < int(m_draw_plane->GetCount())) + ? m_draw_plane->GetString(unsigned(row)) + : wxString("XY"); + return plane_from_choice(row); +} + void DesignPanel::apply_plane_refs(CadFeature& f) const { f.plane_type = (PlaneType)m_plane_type->GetSelection(); diff --git a/src/slic3r/GUI/DesignPanel.hpp b/src/slic3r/GUI/DesignPanel.hpp index db89d5aab4..b248204924 100644 --- a/src/slic3r/GUI/DesignPanel.hpp +++ b/src/slic3r/GUI/DesignPanel.hpp @@ -230,6 +230,10 @@ private: // map a choice row back to the actual SketchPlane (rows 0-2 base, 3+ datum). void populate_plane_choices(ComboBox* c) const; SketchPlane plane_from_choice(int row) const; + // Where a new sketch goes, resolved from what is SELECTED IN THE VIEWPORT rather than from a + // list: a picked planar face wins, otherwise the reference plane last clicked in 3D. `what` + // comes back as something to show the user, so the choice is visible without a combo. + SketchPlane sketch_plane_from_selection(wxString& what) const; // True when Extrude should build only the click-selected loop (a region of the // resolved sketch is selected and it carries entities). bool extrude_uses_loop() const; @@ -585,6 +589,9 @@ private: int m_sel_solid_body{-1}; // which body the face/edge selection is on int m_sel_solid_face{-1}; int m_sel_solid_edge{-1}; + // What the live sketch was actually opened on ("the picked face", "XY", a datum's name), so the + // hint can say it. Resolved from the selection at begin_sketch, not read back from a combo. + wxString m_sketch_on; // Face-as-profile extrude (Onshape): when Extrude is opened on a picked solid face with // no sketch source, this carries that global face id so the kernel extrudes the face. // -1 = ordinary sketch/loop extrude. Set when opening the Extrude card, consumed on add. diff --git a/tests/libslic3r/test_caddocument.cpp b/tests/libslic3r/test_caddocument.cpp index e35204566a..742c20e30a 100644 --- a/tests/libslic3r/test_caddocument.cpp +++ b/tests/libslic3r/test_caddocument.cpp @@ -6798,3 +6798,58 @@ TEST_CASE("An entity sketch that forms no wire fails instead of extruding a defa CHECK(doc.error.find("do not form a single closed wire") != std::string::npos); } } + +// Sketching on a picked face is the most common gesture in solid modelling, and it was impossible: +// the plane came from a combo of base + datum planes only, so the sole route onto a face was to +// build a Coincident datum plane first. plane_of_face is the shared derivation that makes the +// viewport selection usable directly. snaporca-3a2. +TEST_CASE("plane_of_face gives a sketchable plane for a planar face only", "[CadDocument]") +{ + // 20 x 20 x 20 box on XY, so its top face sits at z = 20 with +Z normal. + CadDocument doc; + int sk = doc.add_sketch(SketchShape::Rectangle, SketchPlane::XY(), 20, 20, 10, "Sketch"); + doc.add_extrude(sk, 20.0, false, BooleanMode::New, "Extrude"); + REQUIRE(doc.recompute()); + REQUIRE(doc.bodies.size() == 1); + + SECTION("every planar face of the box resolves, and its normal is a unit axis") { + int resolved = 0, top = -1; + for (int fi = 0; fi < 64; ++fi) { + SketchPlane p; + if (!doc.plane_of_face(0, fi, p)) continue; + ++resolved; + CHECK_THAT(p.normal.norm(), Catch::Matchers::WithinAbs(1.0, 1e-9)); + // The frame must be orthonormal or sketch coordinates on it would be skewed. + CHECK_THAT(p.x_axis.dot(p.y_axis), Catch::Matchers::WithinAbs(0.0, 1e-9)); + CHECK_THAT(p.x_axis.dot(p.normal), Catch::Matchers::WithinAbs(0.0, 1e-9)); + if (p.normal.z() > 0.99) top = fi; + } + CHECK(resolved == 6); // a box has exactly six planar faces + REQUIRE(top >= 0); // and one of them faces +Z + SketchPlane p; + REQUIRE(doc.plane_of_face(0, top, p)); + CHECK_THAT(p.origin.z(), Catch::Matchers::WithinAbs(20.0, 1e-6)); // the top, not the base + } + + SECTION("bad indices are refused rather than guessed") { + SketchPlane p; + CHECK_FALSE(doc.plane_of_face(0, -1, p)); + CHECK_FALSE(doc.plane_of_face(-1, 0, p)); + CHECK_FALSE(doc.plane_of_face(9, 0, p)); + CHECK_FALSE(doc.plane_of_face(0, 999, p)); + } + + SECTION("a cylindrical face is refused — it has no single sketch plane") { + CadDocument cyl; + int c = cyl.add_sketch(SketchShape::Circle, SketchPlane::XY(), 0, 0, 10.0, "Sketch"); + cyl.add_extrude(c, 20.0, false, BooleanMode::New, "Extrude"); + REQUIRE(cyl.recompute()); + int planar = 0, refused = 0; + for (int fi = 0; fi < 16; ++fi) { + SketchPlane p; + if (cyl.plane_of_face(0, fi, p)) ++planar; else ++refused; + } + CHECK(planar == 2); // the two flat caps, and NOT the barrel + CHECK(refused > 0); + } +}