From a1fdb9f21723c7be0ba194aa542a1c6f97d185b8 Mon Sep 17 00:00:00 2001 From: Tommaso Bianchi Date: Fri, 31 Jul 2026 21:10:08 +0200 Subject: [PATCH] =?UTF-8?q?Design:=20double-click=20a=20sketch=20stroke=20?= =?UTF-8?q?to=20edit=20it=20=E2=80=94=20the=20gesture=20belongs=20on=20the?= =?UTF-8?q?=20geometry?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Selecting a committed sketch line lit the right tree row and then told the user to go and press Edit in the panel. That is the side-panel dependency this tab exists to remove, and it made "selectable" true while "editable from the geometry" stayed false. on_edit_feature already does the whole job — re-open the entities in the sketch UI with handles and live quotes — and was only ever reachable from a tree row. A double-click on a committed stroke now calls it for that feature. Double-click on empty space still fits the view, so nothing is taken away. The stroke hit test is now one hit_display_sketch() shared by the click and the double-click. Two copies of "what is under the pointer" drift, and a double-click acting on a different entity than the click before it is a miserable thing to chase. It also reports the entity index, which the tracer prints, so a pick that lands on the wrong stroke can be seen rather than inferred. Verified on :11 end to end: draw an open line, commit, double-click it. The tracer prints "double-click -> edit sketch feature 0 (entity 0)", the panel reads "Editing sketch — drag a handle or click a quote to edit", and a click inside the session selects the line with endpoint handles, live quotes and "1 selected — Delete removes them". NOT delivered by this commit, found while verifying it: typing a new value into a length quote is accepted and displayed but the geometry does not move and the solver drops to "Conflicting constraints". Filed separately — it lives in the constraint layer, not in selection, and nothing here touches it. Refs snaporca-e1p. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01LyRwbuq6fjn3VV9U9UvhBM --- src/slic3r/GUI/DesignCanvas.cpp | 5 ++ src/slic3r/GUI/DesignCanvas.hpp | 1 + src/slic3r/GUI/DesignPanel.cpp | 15 +++++- src/slic3r/GUI/DesignSketchTool.cpp | 83 ++++++++++++++++++++--------- src/slic3r/GUI/DesignSketchTool.hpp | 8 +++ 5 files changed, 84 insertions(+), 28 deletions(-) diff --git a/src/slic3r/GUI/DesignCanvas.cpp b/src/slic3r/GUI/DesignCanvas.cpp index 3739e353d9..3adc615aec 100644 --- a/src/slic3r/GUI/DesignCanvas.cpp +++ b/src/slic3r/GUI/DesignCanvas.cpp @@ -494,6 +494,11 @@ void DesignCanvas::set_on_display_sketch_selected(std::function m_sketch_tool.on_display_sketch_selected = std::move(cb); } +void DesignCanvas::set_on_display_sketch_activated(std::function cb) +{ + m_sketch_tool.on_display_sketch_activated = std::move(cb); +} + std::vector DesignCanvas::selected_loop_entities() const { return m_sketch_tool.selected_loop_entities(); diff --git a/src/slic3r/GUI/DesignCanvas.hpp b/src/slic3r/GUI/DesignCanvas.hpp index 0762678343..21fefa5a21 100644 --- a/src/slic3r/GUI/DesignCanvas.hpp +++ b/src/slic3r/GUI/DesignCanvas.hpp @@ -77,6 +77,7 @@ public: void set_on_sketch_selection_changed(std::function cb); void set_on_sketch_face_selected(std::function cb); // closed loop clicked void set_on_display_sketch_selected(std::function cb); // committed loop clicked: (feature, region) + void set_on_display_sketch_activated(std::function cb); // committed sketch DOUBLE-clicked: edit it std::vector selected_loop_entities() const; // entities of the click-selected loop std::vector> region_entity_indices(const std::vector& ents) const; void clear_loop_pick(); // drop the click-selected loop highlight (e.g. after extrude) diff --git a/src/slic3r/GUI/DesignPanel.cpp b/src/slic3r/GUI/DesignPanel.cpp index e4fd483f67..4f5061c221 100644 --- a/src/slic3r/GUI/DesignPanel.cpp +++ b/src/slic3r/GUI/DesignPanel.cpp @@ -3125,11 +3125,22 @@ DesignPanel::DesignPanel(wxWindow* parent) set_tree_selection(feat); m_status->SetForegroundColour(wxNullColour); m_status->SetLabel(region >= 0 - ? _L("Loop selected — Extrude it, or Edit / Delete the sketch") - : _L("Sketch selected — Extrude it, or Edit / Delete from the tree")); + ? _L("Loop selected — Extrude it, or double-click to edit") + : _L("Sketch selected — Extrude it, or double-click to edit")); m_status->Refresh(); }); + // Double-click a committed sketch stroke: open THAT sketch for editing, where its entities + // are individually selectable and their quotes editable. on_edit_feature already does the + // whole job — it was only ever reachable from the tree, which is precisely the side-panel + // dependency being retired. The pick has already lit the right tree row by the time the + // double-click arrives, but set it again so the path does not depend on that ordering. + m_viewport->set_on_display_sketch_activated([this](int feat) { + if (feat < 0 || feat >= int(m_doc.features.size())) return; + set_tree_selection(feat); + on_edit_feature(); + }); + // F key (Prepare's Place on Face): the tool forwards it here when the Design viewport // has focus; we lay the selected body face on the bed. Returns false when no face is // selected so the key can fall through to the default handler. diff --git a/src/slic3r/GUI/DesignSketchTool.cpp b/src/slic3r/GUI/DesignSketchTool.cpp index 22bb1ced15..6b0a87dc5b 100644 --- a/src/slic3r/GUI/DesignSketchTool.cpp +++ b/src/slic3r/GUI/DesignSketchTool.cpp @@ -31,6 +31,7 @@ static void translate_entity(SketchEntity& e, const Vec2d& d); // Pick-distance helpers (defined lower down; used earlier by the edit-op gizmo). static double point_segment_dist(const Vec2d& p, const Vec2d& a, const Vec2d& b); static double entity_pick_dist(const Vec2d& p, const SketchEntity& e); +static bool point_in_poly(const Vec2d& q, const std::vector& poly); static bool ray_triangle(const Vec3d& ro, const Vec3d& rd, const Vec3d& v0, const Vec3d& v1, const Vec3d& v2, double& t); static double ray_segment_dist3(const Vec3d& ro, const Vec3d& rd, const Vec3d& a, const Vec3d& b); @@ -2696,6 +2697,37 @@ DesignSketchTool::region_entity_indices(const std::vector& ents) c return out; } +// Nearest stroke, and the enclosing region if the cursor is inside one, for ONE committed +// sketch. Factored out so the single-click and double-click paths cannot drift apart: they must +// agree about what is under the pointer or one of them will act on something else. +// +// region_loops exists to find EXTRUDABLE regions, and by design it discards open chains — its +// own walk comment says so. Using it as the pick index meant a committed sketch of open lines +// had no pickable geometry whatsoever: the strokes drew, and not one of them could be clicked, +// so there was no way to select it and therefore none to edit or delete it. Whether a stroke +// bounds a region has nothing to do with whether the user can point at it. Region membership +// decides what a hit REPORTS, not whether the hit can happen. +void DesignSketchTool::hit_display_sketch(const DisplaySketch& d, const Vec2d& p, double tol, + int& edge_feat, int& edge_reg, int& edge_ent, + double& edge_d, int& face_feat, int& face_reg) const +{ + const std::vector loops = region_loops(d.entities); + std::vector ent_region(d.entities.size(), -1); + for (int r = 0; r < int(loops.size()); ++r) + for (int ei : loops[r].ents) + if (ei >= 0 && ei < int(ent_region.size())) ent_region[ei] = r; + + for (int ei = 0; ei < int(d.entities.size()); ++ei) { + if (d.entities[ei].construction) continue; // as region_loops filters it + const double ed = entity_pick_dist(p, d.entities[ei]); + if (ed <= tol * 3.0 && ed < edge_d) { + edge_d = ed; edge_feat = d.feature; edge_reg = ent_region[ei]; edge_ent = ei; + } + } + for (int r = 0; r < int(loops.size()); ++r) + if (face_feat < 0 && point_in_poly(p, loops[r].poly)) { face_feat = d.feature; face_reg = r; } +} + std::vector DesignSketchTool::selected_loop_entities() const { if (m_display_pick < 0 || m_display_pick_region < 0) return {}; @@ -7609,9 +7641,29 @@ bool DesignSketchTool::on_mouse_impl(wxMouseEvent& evt, GLCanvas3D& canvas) // is the ONLY interaction in display-only mode; everything else (drag/move/wheel/right) // falls through (return false) so the camera can still orbit the plate. if (!m_active) { - // Double-click anywhere fits the view (the bottom navigator orb does orientation; this - // is the fit shortcut). Handled before gizmo/pick so it always works on the idle plate. - if (evt.LeftDClick()) { canvas.zoom_to_volumes(); return true; } + // Double-click on a committed sketch stroke OPENS IT FOR EDITING; on empty space it + // still fits the view. A sketch line has to be editable from the line, not from a tree + // row — selecting it already lit the feature, but "now go and press Edit in the panel" + // is the side-panel dependency this tab exists to remove. Fit keeps the rest of the + // plate, so nothing is taken away. + if (evt.LeftDClick()) { + int f = -1, r = -1, e = -1, ff = -1, fr = -1; double dbest = 1e30; + const Linef3 dray = canvas.mouse_ray(Point(evt.GetX(), evt.GetY())); + const Linef3 dray8 = canvas.mouse_ray(Point(evt.GetX() + 8, evt.GetY())); + for (const DisplaySketch& d : m_display_sketches) { + const Vec2d dp = d.plane.project(dray.a, dray.vector()); + const Vec2d dp8 = d.plane.project(dray8.a, dray8.vector()); + hit_display_sketch(d, dp, std::max(1e-3, (dp8 - dp).norm()), f, r, e, dbest, ff, fr); + } + const int target = (f >= 0) ? f : ff; + if (target >= 0 && on_display_sketch_activated) { + dp_pick_trace("double-click -> edit sketch feature %d (entity %d)", target, e); + on_display_sketch_activated(target); + return true; + } + canvas.zoom_to_volumes(); + return true; + } // Visual Extrude gizmo (C5b): while the Extrude card is open the depth arrow is // grabbable — drag changes the depth live; a click (no drag) on the arrow opens the // inline depth editor. Intercept before the early no-LeftDown bailout so Dragging/ @@ -7901,34 +7953,13 @@ bool DesignSketchTool::on_mouse_impl(wxMouseEvent& evt, GLCanvas3D& canvas) Point pos(evt.GetX(), evt.GetY()); const Linef3 ray = canvas.mouse_ray(pos); const Linef3 ray8 = canvas.mouse_ray(Point(evt.GetX() + 8, evt.GetY())); - int edge_feat = -1, edge_reg = -1; double edge_d = 1e30; // nearest loop stroke (wins) + int edge_feat = -1, edge_reg = -1, edge_ent = -1; double edge_d = 1e30; // nearest stroke int face_feat = -1, face_reg = -1; // interior (fallback) for (const DisplaySketch& d : m_display_sketches) { const Vec2d p = d.plane.project(ray.a, ray.vector()); const Vec2d p8 = d.plane.project(ray8.a, ray8.vector()); const double tol = std::max(1e-3, (p8 - p).norm()); - const std::vector loops = region_loops(d.entities); - // region_loops exists to find EXTRUDABLE regions, and by design it discards open - // chains — its own walk comment says so. Using it as the pick index meant a - // committed sketch of open lines had no pickable geometry whatsoever: you could see - // the strokes and could not click one of them, so there was no way to select it, and - // therefore no way to edit or delete it either. Whether a stroke bounds a region has - // nothing to do with whether the user can point at it. Region membership decides - // what a hit REPORTS, not whether the hit can happen. - std::vector ent_region(d.entities.size(), -1); - for (int r = 0; r < int(loops.size()); ++r) - for (int ei : loops[r].ents) - if (ei >= 0 && ei < int(ent_region.size())) ent_region[ei] = r; - - for (int ei = 0; ei < int(d.entities.size()); ++ei) { - if (d.entities[ei].construction) continue; // as region_loops filters it - const double ed = entity_pick_dist(p, d.entities[ei]); - if (ed <= tol * 3.0 && ed < edge_d) { - edge_d = ed; edge_feat = d.feature; edge_reg = ent_region[ei]; - } - } - for (int r = 0; r < int(loops.size()); ++r) - if (face_feat < 0 && point_in_poly(p, loops[r].poly)) { face_feat = d.feature; face_reg = r; } + hit_display_sketch(d, p, tol, edge_feat, edge_reg, edge_ent, edge_d, face_feat, face_reg); } // A precise hit on a loop outline wins over the solid face beneath it. if (edge_feat >= 0) { diff --git a/src/slic3r/GUI/DesignSketchTool.hpp b/src/slic3r/GUI/DesignSketchTool.hpp index 862d51fd85..55b94a5913 100644 --- a/src/slic3r/GUI/DesignSketchTool.hpp +++ b/src/slic3r/GUI/DesignSketchTool.hpp @@ -158,6 +158,9 @@ public: // Click a committed sketch overlay (no live session) -> select that loop: the Sketch // feature index + the clicked closed-region index within it (-1 = no specific loop). std::function on_display_sketch_selected; + // Double-click on a committed sketch stroke: open THAT feature for editing. Selecting a line + // and then hunting for an Edit button in a panel is the dependency this tab exists to remove. + std::function on_display_sketch_activated; // Entities forming the currently click-selected loop (for a per-loop extrude); empty // if no loop is selected. std::vector selected_loop_entities() const; @@ -918,6 +921,11 @@ private: 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 + // Nearest stroke + enclosing region of ONE committed sketch. Shared by the click and + // double-click paths so they cannot disagree about what is under the pointer. + void hit_display_sketch(const DisplaySketch& d, const Vec2d& p, double tol, + int& edge_feat, int& edge_reg, int& edge_ent, + double& edge_d, int& face_feat, int& face_reg) const; 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