Design: sketch means a tool — the offer works inside a sketch, and the app stops

contradicting itself about the plane

Reported from the rig: pick a plane, press Sketch, and you are told to pick a
plane. The app prescribed a sequence and then refused to acknowledge that you
had followed it. Right-click did nothing, so there was no way to reach a
drawing tool except the toolbar this tab exists to retire.

act_sketch was two lines: set the mode, then print "Click a face or a reference
plane in the viewport, then a sketch tool" — unconditionally, without ever
asking whether a plane was already chosen. The plane was never lost; m_ref_plane
held it and begin_sketch captures it when the first tool is armed. The sentence
was simply false.

It now asks. sketch_plane_target() is a companion to sketch_plane_from_selection
that distinguishes "the user chose XZ" from "nothing chosen, falling back to XY"
— a distinction m_ref_plane cannot express on its own, being always a valid
index, so m_plane_picked carries it. With a target the readout names it and the
offer opens on the Create row; without one the old prompt stands, because then
it is true. The card above the status line was a local wxStaticText that nothing
could update, so it went on asking for a plane two inches from a line saying the
plane was chosen. It is a member now and the two are written together.

Right-click inside a sketch was excluded wholesale so that it could end a
polyline chain, abandon an anchor, exit a tool. That made every sketch row in
the atlas unreachable. The honest test is not which mode we are in but whether
the tool actually USED this right-click, and only the tool knows: on_mouse now
wraps on_mouse_impl and records that once, for every terminator, instead of
threading a flag through the twenty-odd sites that consume a RightDown. The
canvas read-and-clears it on the matching release.

Underneath all of it was one confusion — MODE versus SESSION — at four sites.
begin_sketch does not run until the first tool is armed, so is_sketching() is
false for exactly the interval between "press Sketch" and "pick a tool", which
is precisely when the drawing tools must be on offer. The keyboard learned this
once already (snaporca-0ud, whose comment states the rule) and I reintroduced it
in offer_selection_kind and again in show_offer_menu, where the offer built from
the FEATURE map and rendered nine rows that all refused the sketch selection.
Both now call sketch_map_applies(), so they cannot drift apart again. The
keyboard keeps its own split: its "sketching" gates undo and delete-last-entity,
which genuinely need a live session.

Verified on :11 against a fresh build. Pick XZ, press Sketch: card reads
"Drawing on XZ", status reads "Sketching on XZ — pick a tool", offer opens with
Create and Reference live and the six rows needing geometry greyed. Right-click
while idle opens the offer. Right-click as a terminator does NOT — the chain
ends, the line lands on XZ, its length field arms at 49.36 mm. That last one is
the regression the blanket exclusion was buying and the reason this shape of fix
was chosen over a mode test.

Refs snaporca-6vs.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LyRwbuq6fjn3VV9U9UvhBM
This commit is contained in:
Tommaso Bianchi
2026-07-31 19:21:42 +02:00
co-authored by Claude Opus 5
parent bb403b82cf
commit 9f2b2bc511
5 changed files with 96 additions and 7 deletions
+4 -1
View File
@@ -816,7 +816,10 @@ void DesignCanvas::set_on_context_menu(std::function<void(const wxPoint&)> cb)
});
m_canvas_widget->Bind(wxEVT_RIGHT_UP, [this](wxMouseEvent& e) {
const wxPoint d = e.GetPosition() - m_ctx_press;
if (m_on_context_menu && !is_sketching() && !inline_busy()
// Always read-and-clear, even when another guard already rules the offer out, or a
// terminator recorded under one condition would still be pending under the next.
const bool terminated = m_sketch_tool.take_right_consumed();
if (m_on_context_menu && !terminated && !inline_busy()
&& std::max(std::abs(d.x), std::abs(d.y)) <= 8) {
m_on_context_menu(m_canvas_widget->ClientToScreen(e.GetPosition()));
return; // consumed
+60 -6
View File
@@ -516,11 +516,33 @@ DesignPanel::DesignPanel(wxWindow* parent)
};
auto* b_sketch = icon_btn("design_sketch", _L("Sketch"));
// Sketch means a tool. Pressing it used to set the mode and then ask, unconditionally,
// for the very thing the user had just done — click XZ, read "XZ plane selected, press
// Sketch", press Sketch, and be told to click a reference plane. The plane was never
// lost (m_ref_plane holds it and begin_sketch captures it when the first tool is armed);
// the sentence was simply false, and with right-click excluded in sketch mode there was
// no door to the tools at all, so the only way on was the toolbar this tab is retiring.
std::function<void()> act_sketch = [this] {
set_ui_mode(UiMode::Sketch);
wxString where;
const bool have_plane = sketch_plane_target(where);
m_status->SetForegroundColour(wxNullColour);
m_status->SetLabel(_L("Click a face or a reference plane in the viewport, then a sketch tool"));
m_status->SetLabel(have_plane
? wxString::Format(_L("Sketching on %s — pick a tool"), where)
: _L("Click a face or a reference plane in the viewport, then a sketch tool"));
m_status->Refresh();
if (m_sketch_hint) { // the card must agree with the status line, not argue with it
m_sketch_hint->SetLabel(have_plane
? wxString::Format(_L("Drawing on %s.\nPick a tool, or right-click for the list."), where)
: _L("Click a face or a reference plane, then a sketch tool."));
m_sketch_hint->Refresh();
m_cards->Layout();
}
// Hand over the tools rather than naming them in a status line. CallAfter so the
// mode change has settled before a modal menu takes the loop; the menu carries each
// tool's shortcut, so pressing the key instead of picking a row costs nothing.
if (have_plane)
CallAfter([this] { show_offer_menu(wxGetMousePosition()); });
};
b_sketch->Bind(wxEVT_BUTTON, [act_sketch](wxCommandEvent&) { act_sketch(); });
m_keys_feature[SHIFT('S')] = act_sketch;
@@ -2681,10 +2703,13 @@ DesignPanel::DesignPanel(wxWindow* parent)
// looking and pointing. A combo duplicated that decision somewhere the geometry could not
// see it, and once a face could be picked it went further and displayed a stale row that
// contradicted the real target. snaporca-e1p.
auto* hint = new wxStaticText(m_cards, wxID_ANY,
// Kept as a member, not a local: the card has to be able to STOP saying this. It asked
// for a plane even when one had just been picked, directly contradicting the status line
// two inches below it, which by then read "Sketching on XZ".
m_sketch_hint = new wxStaticText(m_cards, wxID_ANY,
_L("Click a face or a reference plane, then a sketch tool."));
hint->SetForegroundColour(dp_sec_text());
m_box_sketch_session->Add(hint, 0, wxLEFT | wxRIGHT | wxTOP | wxBOTTOM, 12);
m_sketch_hint->SetForegroundColour(dp_sec_text());
m_box_sketch_session->Add(m_sketch_hint, 0, wxLEFT | wxRIGHT | wxTOP | wxBOTTOM, 12);
}
cards->Add(m_box_sketch_session, 0, wxEXPAND);
@@ -3291,6 +3316,7 @@ DesignPanel::DesignPanel(wxWindow* parent)
// while the committed feature landed on the new one.
if (base >= 0) {
m_ref_plane = base;
m_plane_picked = true; // chosen, not merely defaulted to
m_pick_face = m_pick_face_body = -1; // last pick wins: a plane beats a stale face
if (m_viewport && m_viewport->is_sketching())
m_viewport->set_sketch_plane(plane_from_choice(m_ref_plane));
@@ -4890,6 +4916,25 @@ SketchPlane DesignPanel::sketch_plane_from_selection(wxString& what) const
return plane_from_choice(m_ref_plane);
}
// Is there a sketch target the USER chose, and what is it called? Distinct from
// sketch_plane_from_selection, which always answers because it falls back to m_ref_plane —
// a caller that wants to say "sketching on X" needs to know whether there is anything to fall
// back FROM, so it can ask for a plane instead of claiming one the user never picked.
bool DesignPanel::sketch_plane_target(wxString& what) const
{
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;
SketchPlane p;
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"), fb + 1)
: _L("the picked face");
return true;
}
if (m_plane_picked) { what = ref_plane_name(m_ref_plane); return true; }
return false;
}
// ---------------------------------------------------------------------------------------------
// The object-driven offer (charter §4.1). Right-click the geometry and get a vertical list in the
// ratified row order, with the verbs that do not apply DISABLED IN PLACE carrying their reason.
@@ -4897,6 +4942,11 @@ SketchPlane DesignPanel::sketch_plane_from_selection(wxString& what) const
// mockups are drawn from, so a drawing and the product cannot drift apart.
// ---------------------------------------------------------------------------------------------
bool DesignPanel::sketch_map_applies() const
{
return m_ui_mode == UiMode::Sketch || (m_viewport && m_viewport->is_sketching());
}
// Which kind of thing is selected, as an OfferSel. Classified by the level the pick cycle has
// actually REACHED, so the menu describes what is highlighted — a header that names a face while
// the whole body is lit would be lying, and this menu's whole value is that it tells the truth
@@ -4904,7 +4954,7 @@ SketchPlane DesignPanel::sketch_plane_from_selection(wxString& what) const
// sketch_plane_from_selection, which deliberately uses m_pick_face. snaporca-3a2.)
int DesignPanel::offer_selection_kind() const
{
if (m_viewport && m_viewport->is_sketching())
if (sketch_map_applies())
return int(OfferSel::SkNone);
const int nb = int(m_doc.bodies.size());
@@ -4959,7 +5009,11 @@ void DesignPanel::show_offer_menu(const wxPoint& screen_pos)
{
const int kind = offer_selection_kind();
const uint32_t bit = offer_bit(OfferSel(kind));
const bool sketching = m_viewport && m_viewport->is_sketching();
// Which verb MAP applies is a question about the MODE, not about whether a session is
// running — the same distinction the keyboard already had to learn (snaporca-0ud). Gated on
// is_sketching() the offer opened on entering a sketch showing the FEATURE rows, every one
// of them refusing the sketch selection, so it read as a menu of nine dead entries.
const bool sketching = sketch_map_applies();
const int bodies = int(m_doc.bodies.size());
int sketches = 0;
+12
View File
@@ -235,6 +235,9 @@ private:
// 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;
// Whether that resolution has anything the USER picked behind it, rather than the default
// reference plane. Lets a caller say "sketching on XZ" only when it is actually true.
bool sketch_plane_target(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;
@@ -328,6 +331,7 @@ private:
// persists until Finish (Phase 3).
wxSizer* m_box_sketch_session{nullptr};
wxStaticText* m_hdr_sketch_session{nullptr};
wxStaticText* m_sketch_hint{nullptr}; // "click a plane" / "drawing on X" — must match the status
wxStaticText* m_hdr_extrude{nullptr};
wxStaticText* m_hdr_dressup{nullptr};
wxStaticText* m_hdr_hole{nullptr};
@@ -397,6 +401,9 @@ private:
// >=3 indexes resolve_datum_planes(). Set by CLICKING a ghost plane in the viewport — there is
// deliberately no dropdown for it. snaporca-e1p.
int m_ref_plane{0};
// m_ref_plane is always a VALID plane, so it cannot itself distinguish "the user chose XY"
// from "nobody has chosen anything yet". This does.
bool m_plane_picked{false};
ComboBox* m_shape{nullptr};
ComboBox* m_plane{nullptr};
ComboBox* m_mode{nullptr};
@@ -611,6 +618,11 @@ private:
std::map<std::string, std::function<void()>> m_verb_actions;
void show_offer_menu(const wxPoint& screen_pos);
int offer_selection_kind() const; // an OfferSel, as int to keep the header light
// Does the SKETCH half of the map apply? A mode question, not a session one: begin_sketch
// does not run until the first tool is armed, so between "press Sketch" and "pick a tool"
// is_sketching() is still false — precisely when the drawing tools must be on offer. The
// is_sketching() arm covers re-opening a committed sketch, which enters the session first.
bool sketch_map_applies() const;
void run_offer_action(const char* action);
// 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.
+14
View File
@@ -7563,7 +7563,21 @@ std::vector<int> DesignSketchTool::connected_loop(int seed) const
return out;
}
// Right-click has two jobs in a sketch, and they were resolved by giving one of them everything:
// the offer was excluded in sketch mode wholesale so a right-click could end a polyline chain,
// abandon an anchor or exit a tool. That made every sketch row in the atlas unreachable.
// The honest test is not "which mode are we in" but "did the tool actually USE this right-click",
// and only the tool knows. Wrapping on_mouse records that once, for every terminator, instead of
// threading a flag through the twenty-odd sites that consume a RightDown.
bool DesignSketchTool::on_mouse(wxMouseEvent& evt, GLCanvas3D& canvas)
{
const bool consumed = on_mouse_impl(evt, canvas);
if (evt.RightDown())
m_right_consumed = consumed;
return consumed;
}
bool DesignSketchTool::on_mouse_impl(wxMouseEvent& evt, GLCanvas3D& canvas)
{
// Track the cursor in canvas client px so the in-canvas value editor can open right
// where the user clicked (Onshape places the field at the click, not via a camera
+6
View File
@@ -89,6 +89,10 @@ public:
bool is_active() const { return m_active; }
bool has_entities() const { return !m_entities.empty(); }
bool on_mouse(wxMouseEvent& evt, GLCanvas3D& canvas);
// True if the LAST right-press was consumed as a gesture terminator (end a polyline chain,
// abandon an anchor, exit a tool). Read-and-clear: the canvas asks on the matching release to
// decide whether that right-click was the user's, in which case it opens the offer.
bool take_right_consumed() { const bool b = m_right_consumed; m_right_consumed = false; return b; }
void render(GLCanvas3D& canvas);
// Persistent committed sketches to draw even when no session is active (e.g. an
@@ -913,6 +917,8 @@ private:
// button is free for it, which is the CAD convention (Onshape/SolidWorks).
GLSelectionRectangle m_rubber;
void pick_bodies_in_rectangle(); // resolve the swept rectangle -> whole-body selection
bool on_mouse_impl(wxMouseEvent& evt, GLCanvas3D& canvas); // the body; on_mouse wraps it
bool m_right_consumed{false}; // last RightDown was a gesture terminator, not a menu
void render_solid_highlight();
void render_datum_planes(); // translucent rectangles for datum/reference planes
void render_view_helpers(); // world origin planes + axis triad (P / A toggles)