Sketch on the face you clicked, not the one you clicked twice

The previous commit made a picked face the sketch plane and I verified it
by clicking the face TWICE. That was the wrong test. handle_solid_click
cycles whole -> face -> edge, and "First click on a (new) body/face
selects the WHOLE solid; refine on repeat clicks" — so at level 1
m_sel_solid_face is -1, and one click on a face, which is what selecting
a face means to anyone, still fell through to the plane combo. The fix
was real and unreachable, which from the outside is indistinguishable
from no fix at all.

The face id was never missing. handle_solid_click resolves it by ray on
the FIRST click and passes it to on_solid_selection_changed regardless of
the cycle level; the panel simply discarded it whenever level < 2. Keep
it in m_pick_face/m_pick_face_body and let a sketch use it, preferring an
explicit face-level selection when there is one. Nothing about the cycle
changes, so body operations that rely on whole-body selection are
untouched.

The new state is dropped wherever the existing picks are, so a stale face
cannot come back: choosing a body from the Bodies list (an explicit
choice with nothing pointed at), picking a committed sketch loop (last
pick wins), undo/redo (recompute invalidates topology ids), and when a
sketch consumes the face.

Verified on :10 with ONE click, which is the flow that was broken: build
a box, single-click its top face, S then C, and the hint reads "Circle —
click center, then radius · on the picked face" with the circle drawn in
that face's plane (artifacts/shots/g3a2-01-one-click.png,
g3a2-02-sketch.png, g3a2-03-drawn.png).

GUI-only, so the kernel suite is unaffected — plane_of_face and its 154
cases / 2125 assertions are unchanged from the previous commit.

snaporca-3a2.
This commit is contained in:
Tommaso Bianchi
2026-07-30 14:09:23 +02:00
parent 6ce20d78c3
commit 3c0843c68c
2 changed files with 24 additions and 4 deletions
+18 -4
View File
@@ -349,8 +349,10 @@ DesignPanel::DesignPanel(wxWindow* parent)
// 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)
if (m_sel_solid_face >= 0 || m_pick_face >= 0) {
m_sel_solid_face = m_sel_solid_edge = m_sel_solid_body = -1;
m_pick_face = m_pick_face_body = -1;
}
} else {
m_viewport->set_sketch_tool(mode);
}
@@ -2849,6 +2851,7 @@ DesignPanel::DesignPanel(wxWindow* parent)
m_viewport->select_body(b);
m_sel_solid_body = b;
m_sel_solid_face = m_sel_solid_edge = -1;
m_pick_face = m_pick_face_body = -1; // chosen from the list, no face was pointed at
m_status->SetForegroundColour(wxNullColour);
m_status->SetLabel(wxString::Format(_L("Body %d selected — next Extrude / Fillet acts on it"), b + 1));
m_status->Refresh();
@@ -3097,6 +3100,7 @@ DesignPanel::DesignPanel(wxWindow* parent)
// Last pick wins (symmetric with the solid-pick handler): selecting a sketch loop drops
// any stale solid face/edge pick so Extrude treats this loop as the profile.
m_sel_solid_face = m_sel_solid_edge = -1;
m_pick_face = m_pick_face_body = -1;
set_tree_selection(feat);
m_status->SetForegroundColour(wxNullColour);
m_status->SetLabel(region >= 0
@@ -3119,6 +3123,10 @@ DesignPanel::DesignPanel(wxWindow* parent)
m_sel_solid_body = (level >= 1) ? body : -1;
m_sel_solid_face = (level >= 2) ? face : -1;
m_sel_solid_edge = (level == 3) ? edge : -1;
// Keep the hit face even at whole-body level: the cycle's first click means "this body",
// but the user pointed AT a face and a sketch should be able to use it. snaporca-3a2.
m_pick_face_body = (level >= 1) ? body : -1;
m_pick_face = (level >= 1) ? face : -1;
// Last pick wins: selecting a solid drops any stale committed-sketch loop selection.
// Otherwise a leftover loop keeps `m_sel_sketch_region >= 0`, which blocks the face
// push/pull branch in Extrude (`m_sel_solid_face >= 0 && m_sel_sketch_region < 0`) and
@@ -4855,10 +4863,15 @@ SketchPlane DesignPanel::plane_from_choice(int row) const
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)) {
// An explicitly face-level selection first, then the face merely CLICKED ON while the whole
// body was selected. The second case is the common one: one click on a face is what a user
// means by "select this face", and requiring the cycle's second click to make it count is the
// whole reason this looked unfixed.
const int fb = (m_sel_solid_face >= 0 && m_sel_solid_body >= 0) ? m_sel_solid_body : m_pick_face_body;
const int fi = (m_sel_solid_face >= 0 && m_sel_solid_body >= 0) ? m_sel_solid_face : m_pick_face;
if (fb >= 0 && fi >= 0 && m_doc.plane_of_face(fb, fi, p)) {
what = (m_doc.bodies.size() > 1)
? wxString::Format(_L("the picked face of Body %d"), m_sel_solid_body + 1)
? wxString::Format(_L("the picked face of Body %d"), fb + 1)
: _L("the picked face");
return p;
}
@@ -8892,6 +8905,7 @@ void DesignPanel::do_undo_redo(bool redo)
// The solid whole/face/edge pick and any in-place edit reference ids that recompute()
// invalidates — drop them before refreshing from the restored document.
m_sel_solid_body = m_sel_solid_face = m_sel_solid_edge = -1;
m_pick_face = m_pick_face_body = -1; // recompute() invalidated the face ids too
reset_edit_state();
after_tree_edit(true); // refresh tree + viewport meshes + status from the restored doc
m_status->SetForegroundColour(wxNullColour);
+6
View File
@@ -589,6 +589,12 @@ 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};
// The face actually under the last solid click, INDEPENDENT of the whole/face/edge cycle level.
// The first click on a solid selects the WHOLE body, but the ray has already resolved which face
// it hit and the callback passes it. "Sketch on the face I clicked" must not require discovering
// that a second click refines the selection, so keep it instead of throwing it away. snaporca-3a2.
int m_pick_face_body{-1};
int m_pick_face{-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;