diff --git a/docs/ux/interaction-model.md b/docs/ux/interaction-model.md new file mode 100644 index 0000000000..e6d5428253 --- /dev/null +++ b/docs/ux/interaction-model.md @@ -0,0 +1,111 @@ +# Design tab — interaction model + +The contract for Esc, the right mouse button, and the states between them. Code that changes any +of the three changes this file in the same commit. + +## 1. The state machine + +`src/slic3r/GUI/CAD/DesignInteraction.hpp` — a four-level LIFO stack. The enum value *is* the +depth, so "which level does this press belong to" is a comparison rather than a chain of +special cases spread over three files. + +```cpp +enum class CadLevel : int { + Idle = 0, // nothing transient is up: Esc clears the selection + Tool = 1, // a feature card / armed sketch tool / constrain session: Esc exits it + Gesture = 2, // an uncommitted delta (entity being drawn, body being dragged): Esc reverts it + Transient = 3, // a value field or a popup menu: Esc closes just that +}; + +struct CadInteractionState { // the four bits routing actually needs + bool value_field_open{false}; + bool gesture_active{false}; + bool tool_armed{false}; + bool has_selection{false}; +}; + +constexpr CadLevel cad_escape_level(const CadInteractionState& s) +{ + if (s.value_field_open) return CadLevel::Transient; + if (s.gesture_active) return CadLevel::Gesture; + if (s.tool_armed) return CadLevel::Tool; + return CadLevel::Idle; +} +``` + +The rule is a `constexpr` free function over a POD, not a method on the panel, so the ordering +that is the entire contract is checkable without a window, a GL context or an event loop. Five +`static_assert`s in the header do exactly that, at compile time. + +**Strict invariant.** No level of Esc deletes a feature, discards a sketch that holds geometry, +or rolls history back. Destroying work needs a gesture that says so: + +| To destroy | Gesture | +|---|---| +| a feature | Delete / Backspace on an explicit selection | +| a drawn sketch | the ribbon's ✗ Cancel, which asks first | +| the last committed change | Ctrl+Z | + +## 2. Event routing + +**`OnKeyDown(WXK_ESCAPE)`** — `DesignPanel`'s `wxEVT_CHAR_HOOK`, one line: + +```cpp +if (key == WXK_ESCAPE) { escape(); return; } +``` + +Every Esc in the tab goes through it, whatever holds focus. `DesignPanel::escape_level()` answers +the four questions of `CadInteractionState` about this panel; `DesignPanel::escape()` acts on the +one level that answer names, and on no other: + +| Level | What one press does | What it must not touch | +|---|---|---| +| `Transient` | close the value field (`cancel_value` / `inline_cancel`) | the tool, which stays armed | +| `Gesture` | drop the clicks of the entity being drawn, or put a moved body back at the pose it had when the gizmo appeared | everything already committed | +| `Tool` | discard a feature card's *candidate*; drop an armed sketch tool to Select; end Constrain | committed features; entities already drawn | +| `Idle` | clear the selection (model and sketch); leave a sketch session **only if it is empty** | a sketch holding geometry — it is left through Finish or Cancel | + +A sketch *session* is deliberately not a `Tool`. It is the environment the Idle level lives in, +which is what makes the destructive path unrepresentable rather than merely unlikely. + +**`OnRightDown` / `OnRightUp`** — `DesignCanvas::set_on_context_menu`, bound after `GLCanvas3D`'s +own handlers so it can consume the event before them: + +```cpp +RIGHT_DOWN: remember the press position and the clock, then Skip() // the canvas still seeds the orbit + +RIGHT_UP: terminated = sketch_tool.take_right_consumed(); // read-and-clear, always + is_click = drift <= 3 px && dt <= 200 ms; // both budgets, or it was navigation + if (callback && !terminated && !inline_busy && is_click) { + select_at_screen(press.x, press.y); // raycast at the PRESS, not the release + on_context_menu(ClientToScreen(press)); + return; // consumed + } + Skip(); // orbit / pan / the handlers underneath +``` + +Two independent budgets because the two failure modes are independent: drift alone still popped a +menu at the end of a slow, careful orbit. `take_right_consumed()` is how a right-click that +already meant something to the armed sketch tool (terminate a chain, drop an edit-op) declines to +also mean "open a menu". + +## 3. Transition table + +`sel` = something is picked. Blank = the input does nothing at that state. + +| State | Left-click | Right-click | Esc | Enter | +|---|---|---|---|---| +| **Idle — model view** | pick / escalate the pick | offer menu for what is under the cursor | clear the selection | — | +| **Idle — sketch, empty** | pick | sketch offer menu | leave the session (nothing to lose) | Finish sketch | +| **Idle — sketch, drawn** | pick | sketch offer menu | clear the selection; status says the sketch is kept | Finish sketch | +| **Tool — feature card** | pick the card's next reference | offer menu | discard the candidate, close the card | commit the feature | +| **Tool — sketch tool armed** | place the first point | drop the tool to Select | drop the tool to Select | — | +| **Tool — constrain** | pick an entity | offer menu | end the session | apply | +| **Gesture — drawing** | place the next point | terminate the chain (keep what is drawn) | drop the in-progress entity, tool stays armed | commit the entity as drawn | +| **Gesture — moving a body** | drop the body here | end the move | revert to the pose at move-start | keep the placement | +| **Transient — value field** | — | — | close the field, tool stays armed | commit the value, advance the chain | +| **Transient — popup menu** | run the entry | — | close the menu | run the highlighted entry | +| **any** | — | — | *never* deletes, discards or rolls back | — | + +Right-hold-and-drag is not in the table on purpose: past 3 px or 200 ms it is navigation, and +navigation does not transition the state machine. diff --git a/src/slic3r/CMakeLists.txt b/src/slic3r/CMakeLists.txt index b6e50af6bf..5b336b44f2 100644 --- a/src/slic3r/CMakeLists.txt +++ b/src/slic3r/CMakeLists.txt @@ -785,6 +785,7 @@ if (SLIC3R_CAD) GUI/CAD/DesignSketchTool.cpp GUI/CAD/DesignSketchTool.hpp GUI/CAD/DesignOffer.hpp + GUI/CAD/DesignInteraction.hpp GUI/CAD/SketchInlineEditor.cpp GUI/CAD/SketchInlineEditor.hpp GUI/CAD/McpControl.cpp diff --git a/src/slic3r/GUI/CAD/DesignCanvas.cpp b/src/slic3r/GUI/CAD/DesignCanvas.cpp index b65223dc57..1d0ea603c6 100644 --- a/src/slic3r/GUI/CAD/DesignCanvas.cpp +++ b/src/slic3r/GUI/CAD/DesignCanvas.cpp @@ -1,6 +1,7 @@ #include "slic3r/GUI/CAD/DesignCanvas.hpp" #include "slic3r/GUI/CAD/SketchInlineEditor.hpp" +#include "slic3r/GUI/CAD/DesignInteraction.hpp" #include "slic3r/GUI/GLCanvas3D.hpp" #include "slic3r/GUI/OpenGLManager.hpp" #include "slic3r/GUI/3DBed.hpp" @@ -16,6 +17,7 @@ #include #include +#include // wxGetLocalTimeMillis: the right-click vs right-hold budget #include #include #include @@ -1038,23 +1040,33 @@ void DesignCanvas::set_on_context_menu(std::function cb) // the view. The offer is the release of a STATIONARY right-click, at the same 8 px budget // the left-click pick uses. m_canvas_widget->Bind(wxEVT_RIGHT_DOWN, [this](wxMouseEvent& e) { - m_ctx_press = e.GetPosition(); - e.Skip(); // the canvas still needs the press to seed the pan + m_ctx_press = e.GetPosition(); + m_ctx_press_ms = wxGetLocalTimeMillis().GetValue(); + e.Skip(); // the canvas still needs the press to seed the orbit }); m_canvas_widget->Bind(wxEVT_RIGHT_UP, [this](wxMouseEvent& e) { - const wxPoint d = e.GetPosition() - m_ctx_press; + const wxPoint d = e.GetPosition() - m_ctx_press; + const long long dt = wxGetLocalTimeMillis().GetValue() - m_ctx_press_ms; // 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) { - // The menu belongs to what you POINTED AT. Pick first, so a right-click on a line - // offers that line's verbs instead of the empty-selection vocabulary. Selecting an - // entity that is already selected is a no-op, so a multi-entity pick survives a + // Click, or navigation? Both budgets must hold: a press that travelled orbited, and a + // press that was HELD was aiming to orbit even if the hand never quite moved. Two + // independent budgets because the two failure modes are independent — the drift one + // alone still popped a menu at the end of a slow, careful orbit. + const bool is_click = std::max(std::abs(d.x), std::abs(d.y)) <= kCadRightClickDriftPx + && dt <= kCadRightClickMs; + if (m_on_context_menu && !terminated && !inline_busy() && is_click) { + // The menu belongs to what you POINTED AT — and pointing happened at the PRESS, not + // at the release, so the raycast uses the press position. Within a 3 px budget the + // two are the same pixel in practice; using the press is what makes that a + // guarantee rather than a coincidence. Pick first, so a right-click on a line offers + // that line's verbs instead of the empty-selection vocabulary. Selecting an entity + // that is already selected is a no-op, so a multi-entity pick survives a // right-click on one of its members. - if (m_canvas && m_sketch_tool.select_at_screen(*m_canvas, e.GetX(), e.GetY())) + if (m_canvas && m_sketch_tool.select_at_screen(*m_canvas, m_ctx_press.x, m_ctx_press.y)) request_repaint(); - m_on_context_menu(m_canvas_widget->ClientToScreen(e.GetPosition())); + m_on_context_menu(m_canvas_widget->ClientToScreen(m_ctx_press)); return; // consumed } e.Skip(); @@ -1474,6 +1486,48 @@ int DesignCanvas::sketch_selection_count() const return int(m_sketch_tool.selection().size()); } +bool DesignCanvas::sketch_abort_gesture() +{ + if (!m_sketch_tool.abort_gesture()) return false; + request_repaint(); // the rubber band is gone; the canvas must stop drawing it + return true; +} + +bool DesignCanvas::sketch_disarm_tool() +{ + if (!m_sketch_tool.disarm_tool()) return false; + request_repaint(); + return true; +} + +bool DesignCanvas::drawing_in_progress() const +{ + return m_sketch_tool.pending_points() > 0; +} + +bool DesignCanvas::has_any_selection() const +{ + return m_sketch_tool.has_solid_selection() || m_sketch_tool.sketch_has_selection(); +} + +bool DesignCanvas::clear_any_selection() +{ + if (!has_any_selection()) return false; + // Both, unconditionally: which of the two is live depends on the mode, and Esc at idle means + // "nothing is picked" in either of them. clear_selection() reports through the tool's own + // on_selection_changed; the solid side has no such notification, so the panel refreshes what + // depends on it (see DesignPanel::escape). + m_sketch_tool.clear_selection(); + m_sketch_tool.clear_solid_selection(); + // clear_solid_selection() is silent by design (recomputes call it while ids are invalid), but + // the panel mirrors the pick to aim Extrude and the dress-up tools. An Esc that cleared the + // highlight without telling the panel would leave those aimed at a body nothing points to. + if (m_sketch_tool.on_solid_selection_changed) + m_sketch_tool.on_solid_selection_changed(0, -1, -1, -1); + request_repaint(); + return true; +} + bool DesignCanvas::sketch_first_selected_type(SketchEntity::Type& out) const { return m_sketch_tool.first_selected_type(out); diff --git a/src/slic3r/GUI/CAD/DesignCanvas.hpp b/src/slic3r/GUI/CAD/DesignCanvas.hpp index 6a2153d00d..b6ceca2390 100644 --- a/src/slic3r/GUI/CAD/DesignCanvas.hpp +++ b/src/slic3r/GUI/CAD/DesignCanvas.hpp @@ -293,6 +293,14 @@ public: // Sketch selection, for the offer menu: how many entities are selected and what the first // one is. Returns 0 when nothing is selected. int sketch_selection_count() const; + // Esc routing (DesignInteraction.hpp). The panel decides WHICH level one press belongs to; + // these are the levels it can act on inside the canvas. Each returns whether it did anything, + // so the panel can fall through to the next level without asking twice. + bool sketch_abort_gesture(); // CadLevel::Gesture — drop the entity being drawn + bool sketch_disarm_tool(); // CadLevel::Tool — armed sketch tool falls back to Select + bool drawing_in_progress() const;// an entity has clicks down but is not committed + bool has_any_selection() const; // model pick or sketch pick + bool clear_any_selection(); // CadLevel::Idle — drop both; true if anything was dropped bool sketch_first_selected_type(SketchEntity::Type& out) const; // Live sketch session (Fase 4.2 live constraint path): the panel reads the in-session // selection and entities, and commits a planned constraint through the tool's @@ -355,7 +363,8 @@ private: std::function m_on_context_menu; bool m_ctx_bound{false}; // bind the RIGHT_UP handler once, however often the cb is set - wxPoint m_ctx_press{0, 0}; // right-press origin: a right-DRAG pans, it must not offer + wxPoint m_ctx_press{0, 0}; // right-press origin: a right-DRAG orbits, it must not offer + long long m_ctx_press_ms{0}; // and a right-HOLD is navigation too, however still it is held Bed3D m_bed; // The half of the camera swap above that is NOT on screen: the editor tabs' view while diff --git a/src/slic3r/GUI/CAD/DesignInteraction.hpp b/src/slic3r/GUI/CAD/DesignInteraction.hpp new file mode 100644 index 0000000000..d849b779e4 --- /dev/null +++ b/src/slic3r/GUI/CAD/DesignInteraction.hpp @@ -0,0 +1,59 @@ +#ifndef slic3r_GUI_DesignInteraction_hpp_ +#define slic3r_GUI_DesignInteraction_hpp_ + +namespace Slic3r { namespace GUI { + +// The Design tab's interaction stack, and the ONE rule Esc obeys. +// +// Esc unwinds exactly one level per press, deepest first, and never more. The enum value IS +// the LIFO depth, so "which level does this press belong to" is a comparison, not a chain of +// special cases scattered over three files — which is what it was, and why two presses in a +// row could reach past a tool and destroy the sketch underneath it. +// +// STRICT INVARIANT (the bug this exists to make unrepresentable): no level of Esc deletes a +// feature, discards a sketch that holds geometry, or rolls history back. Destroying work needs +// a gesture that says so — Delete/Backspace on an explicit selection, the banner's Cancel, or +// Ctrl+Z. An Esc that can destroy is an Esc nobody can press with confidence, and being the +// safe key is the whole point of it. +enum class CadLevel : int { + Idle = 0, // nothing transient is up: Esc clears the selection + Tool = 1, // a feature card / armed sketch tool / constrain session: Esc exits it + Gesture = 2, // an uncommitted delta (entity being drawn, body being dragged): Esc reverts it + Transient = 3, // a value field or a popup menu: Esc closes just that +}; + +// What the tab is doing, reduced to the four bits the routing actually needs. Kept as a POD of +// answers rather than a pointer to the panel so the rule below is decidable — and checkable — +// without a window, a GL context or an event loop. +struct CadInteractionState { + bool value_field_open{false}; // in-canvas value field, or the panel's value card + bool gesture_active{false}; // in-progress entity points, or a body being moved + bool tool_armed{false}; // feature card open, sketch draw tool armed, constrain session + bool has_selection{false}; // something is picked (model or sketch) +}; + +// The whole routing rule. Deepest live level wins; Idle is the floor. +constexpr CadLevel cad_escape_level(const CadInteractionState& s) +{ + if (s.value_field_open) return CadLevel::Transient; + if (s.gesture_active) return CadLevel::Gesture; + if (s.tool_armed) return CadLevel::Tool; + return CadLevel::Idle; +} + +// The ordering is the entire contract, so it is checked where it is defined, at compile time. +static_assert(cad_escape_level({true, true, true, true}) == CadLevel::Transient, "value field is deepest"); +static_assert(cad_escape_level({false, true, true, true}) == CadLevel::Gesture, "gesture beats tool"); +static_assert(cad_escape_level({false, false, true, true}) == CadLevel::Tool, "tool beats idle"); +static_assert(cad_escape_level({false, false, false, true}) == CadLevel::Idle, "selection is idle-level"); +static_assert(cad_escape_level({false, false, false, false}) == CadLevel::Idle, "empty is idle"); + +// Right-click vs. right-hold-orbit. A press that stays put and is let go promptly is a click and +// summons the offer; anything longer or further was navigation, and navigation must never be +// rewarded with a menu over wherever the camera happened to stop. +inline constexpr int kCadRightClickMs = 200; // press->release budget +inline constexpr int kCadRightClickDriftPx = 3; // cursor drift budget, max(|dx|,|dy|) + +}} // namespace Slic3r::GUI + +#endif // slic3r_GUI_DesignInteraction_hpp_ diff --git a/src/slic3r/GUI/CAD/DesignPanel.cpp b/src/slic3r/GUI/CAD/DesignPanel.cpp index e140bbe38d..c1ff2df5c3 100644 --- a/src/slic3r/GUI/CAD/DesignPanel.cpp +++ b/src/slic3r/GUI/CAD/DesignPanel.cpp @@ -4118,9 +4118,10 @@ DesignPanel::DesignPanel(wxWindow* parent) refresh_preview(); }); - // Esc exits the active sketch tool: drop the live session, restore Feature mode + - // the committed-sketch overlay (an in-progress draw is discarded). The tool's layered - // request_exit only calls this once it's an idle Select session. + // Leaving an EMPTY sketch session: restore Feature mode + the committed-sketch overlay. + // request_exit() calls this only for an idle Select session that holds no geometry, so + // nothing a user drew can reach here — discarding drawn work goes through tool_cancel's + // confirmation instead. m_viewport->set_on_sketch_exit([this]() { // While placing imported Text/SVG art, right-click = Confirm (keep the art) — the // Insert card is the explicit gate, this is the in-canvas shortcut to it. @@ -4135,13 +4136,13 @@ DesignPanel::DesignPanel(wxWindow* parent) m_status->Refresh(); }); - // The tool refuses the exit layer of Esc when the sketch still has unsaved geometry (a - // second consecutive Esc is let through). The refusal lives in the tool's request_exit(); - // only the STATUS LINE lives here, so the tool reports via this callback instead of - // writing text itself. + // The tool declined to leave because the session holds geometry. There is no "press it again" + // any more — the answer is a deliberate Finish or Cancel — so this is a plain statement of + // where you are, not a warning shot. Only the STATUS LINE lives here; the tool reports via + // this callback instead of writing text itself. m_viewport->set_on_sketch_exit_refused([this]() { - m_status->SetForegroundColour(wxColour(235, 110, 110)); - set_status(_L("Sketch has unsaved geometry — use Confirm to keep it, or Cancel to discard")); + m_status->SetForegroundColour(wxNullColour); + set_status(_L("Sketch kept — Finish to commit it, Cancel to discard")); m_status->Refresh(); }); @@ -4194,32 +4195,18 @@ DesignPanel::DesignPanel(wxWindow* parent) m_viewport->inline_commit(); return; } - if (key == WXK_ESCAPE) { - m_viewport->inline_cancel(); - return; - } + // Esc is NOT special-cased here any more: escape() routes it, and the open field is + // exactly what CadLevel::Transient means, so it closes the field and stops there. } - // Esc must exit the sketch wherever focus happens to be. Which widget holds focus is an - // accident of where the user last clicked (a toolbar button, the Construction checkbox), - // and Esc must not depend on it — request_exit() is the layered behaviour the GL-canvas - // path already uses, so Esc means the same thing here as it does over the viewport. - // The predicate is the SESSION, not the armed tool. is_sketching() is - // DesignSketchTool::is_active(), true only while a draw tool is armed, and the state you - // are left in after committing an entity is ui_mode=Sketch with no tool armed -- so this - // branch used to be skipped exactly when a user reaches for Esc, and the Cancel button - // was the only way out ("Esc hardly ever works", exussum12 on PR #15238). Same mistake as - // snaporca-0ud, which gated the sketch key MAP on is_sketching() thirty lines above. - // request_exit() is layered and already handles the idle case, so widening the gate - // costs nothing: in-progress entity -> drop to Select -> exit the session. - if (key == WXK_ESCAPE && m_viewport - && (m_ui_mode == UiMode::Sketch || m_viewport->is_sketching())) { - m_viewport->request_sketch_exit(); - return; - } - - const bool dismissable = m_active != Tool::None || (m_viewport && m_viewport->moving_body()); - if (key == WXK_ESCAPE && dismissable) { tool_cancel(); return; } + // ONE Esc, ONE route, whatever holds focus. Which widget has focus is an accident of where + // the user last clicked (a toolbar button, the Construction checkbox), and Esc must not + // depend on it. It used to be answered in four places — the inline field above, a sketch + // branch, a feature-card branch, and the canvas — none of which could see the others, so + // a press aimed at one of them fell through to the next and the SECOND press reached a + // layer that discarded the live sketch. escape() picks the single deepest live level and + // acts on that one only; see DesignInteraction.hpp for the ladder and its invariant. + if (key == WXK_ESCAPE) { escape(); return; } // The offer from the keyboard (charter 4.1): the Menu key, or Shift+F10 for keyboards that // do not have one. Same menu the right-click opens — show_offer_menu already decides which @@ -11277,17 +11264,18 @@ void DesignPanel::tool_cancel() if (m_active == Tool::Insert) { cancel_insert(); return; } if (m_active != Tool::None) { cancel_tool(); return; } if (m_ui_mode == UiMode::Sketch) { - // Escape is how anyone dismisses the inline dimension field, and it used to cascade - // straight through to here: first press disarmed the tool, second press dropped the - // whole live session — a drawn rectangle gone, silently, with no undo prompt. That is - // the "I cannot add the circle after the rectangle" report: the sketch was already - // destroyed. Discarding real work needs the explicit Cancel button, not a key people - // press to close a text field. + // THE explicit discard. Esc no longer arrives here at all (it routes through escape(), + // which cannot destroy anything), so this button is now the only way a drawn sketch is + // thrown away — and being the only way, it has to ask. It used to refuse instead, telling + // the user to press the very button they had just pressed: a sketch could be kept but + // never discarded. if (m_viewport && m_viewport->live_sketch_has_work()) { - m_status->SetForegroundColour(wxNullColour); - set_status(_L("Sketch kept — use Confirm to keep it, Cancel to discard")); - m_status->Refresh(); - return; + wxMessageDialog dlg(this, + _L("Discard this sketch and everything drawn in it?"), + _L("Discard sketch"), + wxYES_NO | wxNO_DEFAULT | wxICON_EXCLAMATION); + dlg.SetYesNoLabels(_L("Discard"), _L("Keep drawing")); + if (dlg.ShowModal() != wxID_YES) return; } if (m_viewport) m_viewport->cancel_sketch(); // drop the live session (committed art stays) m_edit_index = -1; @@ -11310,6 +11298,98 @@ void DesignPanel::tool_cancel() } } +// Which level of the interaction stack one Esc press belongs to. The rule itself lives in +// DesignInteraction.hpp, decidable without a window; this only answers the four questions it +// asks about THIS panel. +CadLevel DesignPanel::escape_level() const +{ + CadInteractionState st; + // A value being typed is the deepest thing on screen, whether it is the in-canvas floating + // field or the panel's value card: both are "a number you are in the middle of entering". + st.value_field_open = (m_value_cont != nullptr) + || (m_viewport && m_viewport->inline_busy()); + // An uncommitted delta: clicks are down on an entity that does not exist yet, or a body is + // being moved by a gizmo that has not been confirmed. + st.gesture_active = m_viewport + && (m_viewport->drawing_in_progress() + || (m_active == Tool::None && m_viewport->moving_body())); + // Something is armed and waiting for input: a feature card, a sketch draw tool, Constrain. + // A sketch SESSION is deliberately not in this list — see escape(). + st.tool_armed = (m_active != Tool::None) + || m_ui_mode == UiMode::Constrain + || (m_ui_mode == UiMode::Sketch && m_viewport && !m_viewport->sketch_is_selecting()); + st.has_selection = m_viewport && m_viewport->has_any_selection(); + return cad_escape_level(st); +} + +// Esc: unwind exactly one level. Nothing here deletes a feature, discards geometry or touches +// history — those need Delete/Backspace on a selection, the sketch banner's Cancel, or Ctrl+Z. +void DesignPanel::escape() +{ + switch (escape_level()) { + case CadLevel::Transient: + // Close just the field. The tool stays armed and the geometry is untouched, which is the + // whole reason this level exists: dismissing a number people press Esc for reflexively + // used to cascade down the stack and take the sketch with it. + if (m_value_cont) { cancel_value(); return; } + if (m_viewport) { m_viewport->inline_cancel(); return; } + return; + + case CadLevel::Gesture: + // Restore the state from before the uncommitted delta. Drawing: drop the clicks, keep the + // tool armed so the next click starts a fresh entity. Moving: tool_cancel's move branch + // puts the body back at the pose it had when the gizmo appeared. + if (m_viewport && m_viewport->drawing_in_progress()) { + m_viewport->sketch_abort_gesture(); + m_status->SetForegroundColour(wxNullColour); + set_status(wxString()); + m_status->Refresh(); + return; + } + tool_cancel(); + return; + + case CadLevel::Tool: + // Back to the idle state of whatever environment we are in. A feature card discards its + // CANDIDATE (never a committed feature — in edit mode reset_edit_state only forgets which + // feature was being edited, the feature itself is untouched); an armed sketch tool falls + // back to Select, leaving every entity already drawn exactly where it is. + if (m_active != Tool::None || m_ui_mode == UiMode::Constrain) { tool_cancel(); return; } + if (m_viewport && m_viewport->sketch_disarm_tool()) { + m_status->SetForegroundColour(wxNullColour); + set_status(_L("Select")); + m_status->Refresh(); + } + return; + + case CadLevel::Idle: + // Deselect. In a sketch this is the floor: the session is left through Finish or Cancel, + // both of which say which one they are, and never through a key pressed on the way out of + // something else. + if (m_viewport && m_viewport->clear_any_selection()) { + m_sel_sketch_region = -1; + m_sel_sketch_feat = -1; + m_status->SetForegroundColour(wxNullColour); + set_status(wxString()); + m_status->Refresh(); + return; + } + // Nothing selected and nothing to unwind. An EMPTY sketch session may as well close — + // there is no work to lose, so this cannot be the destructive case, and being unable to + // leave a sketch you have not drawn in yet is its own small trap. + if (m_ui_mode == UiMode::Sketch && m_viewport && !m_viewport->live_sketch_has_work()) { + m_viewport->request_sketch_exit(); + return; + } + if (m_ui_mode == UiMode::Sketch) { + m_status->SetForegroundColour(wxNullColour); + set_status(_L("Sketch kept — Finish to commit it, Cancel to discard")); + m_status->Refresh(); + } + return; + } +} + void DesignPanel::update_undo_redo_buttons() { // Grey Undo/Redo to mirror exactly what do_undo_redo will do: it acts only in Feature diff --git a/src/slic3r/GUI/CAD/DesignPanel.hpp b/src/slic3r/GUI/CAD/DesignPanel.hpp index 441e46b081..fe6d7d30a9 100644 --- a/src/slic3r/GUI/CAD/DesignPanel.hpp +++ b/src/slic3r/GUI/CAD/DesignPanel.hpp @@ -11,6 +11,7 @@ #include #include "libslic3r/CAD/CadDocument.hpp" +#include "slic3r/GUI/CAD/DesignInteraction.hpp" // CadLevel: what one Esc press means class ComboBox; // Orca dropdown (Widgets/ComboBox.hpp) — replaces wxChoice everywhere here class StaticBox; // Orca rounded card frame (Widgets/StaticBox.hpp) @@ -106,7 +107,14 @@ private: void apply_dof_status(int dof, bool ok, bool has_constraints); // Unified action-bar dispatch: one Confirm / one Cancel for every tool and mode. void tool_confirm(); // ✓ : commit the active feature / sketch / constrain session - void tool_cancel(); // ✗ / Esc : cancel the active feature / discard / exit + void tool_cancel(); // ✗ : cancel the active feature / discard / exit + // Esc. ONE press unwinds ONE level of the interaction stack (DesignInteraction.hpp), and no + // level of it destroys committed work. escape_level() answers which level the press belongs + // to; escape() acts on exactly that one. Every Esc in the tab routes through here — the key + // used to be handled in four places that could not see each other, and that is how two + // presses in a row reached past a tool and discarded the sketch under it. + CadLevel escape_level() const; + void escape(); void update_action_bar(); // show the ✓/✗ bar iff a tool or mode is active void on_shape_changed(); diff --git a/src/slic3r/GUI/CAD/DesignSketchTool.cpp b/src/slic3r/GUI/CAD/DesignSketchTool.cpp index 4d4aa02fec..817ff7dc8c 100644 --- a/src/slic3r/GUI/CAD/DesignSketchTool.cpp +++ b/src/slic3r/GUI/CAD/DesignSketchTool.cpp @@ -225,7 +225,6 @@ void DesignSketchTool::rebuild_features_from_entities() void DesignSketchTool::set_tool(Mode mode) { - m_exit_refused = false; // any new action re-arms the one-shot exit refusal // A READY edit-op carries the user's typed or dragged value, so switching tools commits it // rather than dropping it — the same rule Tab follows in the dimension editor. Discarding it // here is most of why Fillet looked like it simply did not work: every documented route (type @@ -335,28 +334,44 @@ void DesignSketchTool::cancel() reset_tf(); } -// Esc while active: layered exit (Onshape-like). Abort an in-progress entity first, then -// drop a draw tool back to Select; only an idle Select session exits to Feature mode. +// CadLevel::Gesture inside a sketch: drop the entity being drawn, keep the tool armed. +bool DesignSketchTool::abort_gesture() +{ + if (m_points.empty()) return false; + m_points.clear(); + m_has_cursor = false; + return true; +} + +// CadLevel::Tool inside a sketch: an armed draw/edit tool falls back to Select. +// Drop any pending edit-op BEFORE the downgrade: set_tool commits a ready one, and Esc must +// cancel it, never apply it. Right-click already discards it through its own branch. +bool DesignSketchTool::disarm_tool() +{ + if (m_mode == Mode::Select) return false; + reset_op(); + set_tool(Mode::Select); + return true; +} + +// Esc / right-click while active: layered exit (Onshape-like), and layered is where it stops. +// Abort an in-progress entity first, then drop a draw tool back to Select, then — only if the +// session holds NOTHING a user could mourn — leave it. +// +// It used to have a fourth layer: refuse once, and let the SECOND consecutive request destroy a +// drawn sketch. That is the "I pressed Esc twice and my rectangle was gone" report, and no +// warning makes it acceptable, because the two presses are never deliberate — the first is aimed +// at a value field or a tool and the second at the tool underneath it. A session holding geometry +// is now left ONLY through Finish (keep) or Cancel (discard), both of which say which they are. +// This is the single decision point for every caller, keyboard and mouse alike, so the guarantee +// cannot be re-opened by adding a route. void DesignSketchTool::request_exit() { - if (!m_points.empty()) { m_points.clear(); m_has_cursor = false; return; } - // Drop any pending edit-op BEFORE the downgrade: set_tool commits a ready one, and Esc must - // cancel it, never apply it. Right-click already discards it through its own branch. - if (m_mode != Mode::Select) { reset_op(); set_tool(Mode::Select); return; } - if (on_exit) { - // This is the layer that would destroy a drawn-but-uncommitted sketch. That is the one - // thing Esc must not do silently: refuse the FIRST time work exists, and only let a - // second consecutive Esc through. The panel reports the refusal; the tool only decides. - if (live_sketch_has_work() && !m_exit_refused) { - m_exit_refused = true; - if (on_exit_refused) on_exit_refused(); - return; - } - m_exit_refused = false; - on_exit(); - } else { - cancel(); - } + if (abort_gesture()) return; + if (disarm_tool()) return; + if (live_sketch_has_work()) { if (on_exit_refused) on_exit_refused(); return; } + if (on_exit) on_exit(); + else cancel(); } void DesignSketchTool::request_undo_redo(bool redo) @@ -375,7 +390,6 @@ void DesignSketchTool::clear_selection() void DesignSketchTool::delete_selected() { if (m_selection.empty()) return; - m_exit_refused = false; // deleting is an action; re-arm the exit refusal const int n = int(m_entities.size()); std::vector del(n, false); for (int i : m_selection) @@ -9625,10 +9639,6 @@ bool DesignSketchTool::on_mouse(wxMouseEvent& evt, GLCanvas3D& canvas) bool DesignSketchTool::on_mouse_impl(wxMouseEvent& evt, GLCanvas3D& canvas) { - // Re-arm the one-shot exit refusal on a BUTTON press only, never on motion: moving the - // mouse between the two Esc presses is what anyone would do, and re-arming there would - // make the second Esc refuse again — an Esc that can never exit while the hand moves. - if (evt.LeftDown() || evt.RightDown() || evt.MiddleDown()) m_exit_refused = false; // 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 // projection — the design canvas's viewport isn't valid outside its own paint). diff --git a/src/slic3r/GUI/CAD/DesignSketchTool.hpp b/src/slic3r/GUI/CAD/DesignSketchTool.hpp index 761a629117..d73cfce6a8 100644 --- a/src/slic3r/GUI/CAD/DesignSketchTool.hpp +++ b/src/slic3r/GUI/CAD/DesignSketchTool.hpp @@ -185,6 +185,7 @@ public: // Survives set_solid_pick() — it is owned by the panel, not by the mesh feed. void set_pick_only_body(int b) { m_pick_only_body = b; } void clear_solid_selection(); + bool has_solid_selection() const { return m_solid_sel != SolidSel::None; } // Select a whole body by index (from the Parts list) — Whole-level highlight, no face/edge. // body < 0 or out of range clears the selection. void select_body(int body); @@ -455,6 +456,9 @@ public: // Selection (Mode::Select): pick points/lines/arcs/circles of the in-session // sketch; Shift/Ctrl extends, double-click grabs the whole connected loop. const std::vector& selection() const { return m_selection; } + // Entities OR bare points: clear_selection() drops both, so "is anything picked" must ask + // about both, or Esc at idle would report nothing to do while a point sat highlighted. + bool sketch_has_selection() const { return !m_selection.empty() || !m_point_sel.empty(); } // Type of the first selected entity. False when nothing is selected, so the offer menu can // tell a line from an arc from a point and stop collapsing every sketch selection to "none". bool first_selected_type(SketchEntity::Type& out) const { @@ -620,14 +624,17 @@ public: // Emitted when a closed-loop face is clicked in Select mode (Onshape: a region // becomes a selectable face → extrude). The panel commits the sketch + extrudes. std::function on_face_selected; // region index into region_loops(m_entities) - // Esc pressed while the tool is active: exit/cancel the session (the panel restores - // Feature mode). Layered: an in-progress entity or a non-Select draw tool is dropped - // first; a second Esc exits the session. + // Esc pressed while the tool is active with nothing left to unwind and no geometry to + // lose: leave the session (the panel restores Feature mode). std::function on_exit; - // Esc refusal: request_exit() declined to destroy a sketch that still has geometry. The - // panel owns the status line, so the tool reports through this instead of writing text itself. + // request_exit() declined to leave because the session holds geometry. The panel owns the + // status line, so the tool reports through this instead of writing text itself. std::function on_exit_refused; std::function on_move_exit; // right-click finished the move-body gizmo + // The two inner Esc levels, callable on their own so the panel can route one press to one + // level (see DesignInteraction.hpp). Each returns whether it had anything to unwind. + bool abort_gesture(); // CadLevel::Gesture — drop the entity being drawn + bool disarm_tool(); // CadLevel::Tool — armed draw/edit tool falls back to Select void request_exit(); // Ctrl+Z / Ctrl+Shift+Z (Ctrl+Y) while the Design canvas is focused: undo/redo the // committed feature history. The tool just forwards to the host, which owns the @@ -1174,10 +1181,6 @@ private: double& edge_d, int& face_feat, int& face_reg) const; bool m_right_consumed{false}; // last RightDown was a gesture terminator, not a menu bool m_escalate_repick{true}; // re-picking the same sub-element takes the whole body - // One-shot exit confirmation: request_exit() refused once because the sketch has unsaved - // geometry. The NEXT exit request (with nothing in between) is allowed through; any other - // action re-arms the refusal, so the warning is never a permanent block. - bool m_exit_refused{false}; void render_solid_highlight(); // The shared body of the above: one highlight from explicit arguments, so the committed // selection and the hover pre-highlight cannot drift apart in how they look.