Sketch where the user pointed: a picked face is the sketch plane

Selecting a face and sketching on it is the most common gesture in solid
modelling, and it was impossible. The plane came from a combo holding
XY/XZ/YZ plus datums, and plane_from_choice had no face branch at all, so
the only route onto a face was to build a Coincident datum plane on it
first, confirm that, reopen the sketch and find the datum in the
dropdown. Three extra steps and a junk feature in the tree.

The fix is not another combo row. A new sketch now takes its plane from
what is SELECTED IN THE VIEWPORT: a picked planar face wins outright, and
only when nothing is picked does it fall back to the reference plane —
which is itself normally set by clicking one of the ghost planes in 3D,
not by opening the combo. The tool hint names the target ("Circle — click
center, then radius · on the picked face") so the choice is visible on the
geometry side rather than needing a control to read back.

CadDocument::plane_of_face is the shared derivation, so the sketch path
and the Coincident datum method cannot drift apart. It refuses
non-planar faces: face_normal_world evaluates at the mid parameter, which
on a cylinder or a fillet is a tangent plane at one arbitrary point —
fine for offsetting a datum, wrong as a sketch plane, and silently
sketching on a tangent is worse than declining.

Picking the face also CONSUMES it. Leaving the pick 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.

Verified on :10 end to end with no combo interaction: build a box, click
its top face twice to cycle whole -> face, press S then C, and the circle
is drawn in the plane of that face with its Radius tab on the geometry
(artifacts/shots/f3a2-03-face.png, f3a2-04-sketch-on-face.png,
f3a2-05-circle-drawn.png). Kernel side: 154 cases / 2125 assertions green
on both forks, including that a cylinder resolves exactly its two flat
caps and refuses the barrel.

Still side-panel-shaped and to be dealt with separately: the Plane combo
remains on the card and now merely displays a stale row when a face is
the real target. It should show the actual target or go away.

snaporca-3a2.

src/libslic3r/CadDocument.cpp

@@ -32,6 +32,8 @@
 #include <BRepCheck_Analyzer.hxx>
 #include <BRepLib.hxx>
 #include <BRepAdaptor_Curve.hxx>
+#include <BRepAdaptor_Surface.hxx>   // plane_of_face: reject non-planar faces
+#include <GeomAbs_SurfaceType.hxx>
 #include <GeomAbs_CurveType.hxx>
 #include <BRep_Tool.hxx>
 #include <Geom_CylindricalSurface.hxx>
@@ -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<std::pair<std::string, SketchPlane>> CadDocument::resolve_datum_planes() const
 {
     std::vector<std::pair<std::string, SketchPlane>> out;

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<std::pair<std::string, SketchPlane>> 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; };

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();

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.

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);
+    }
+}