From 1274d97983ff6c01251d6c6590a76be93b22ef47 Mon Sep 17 00:00:00 2001 From: Tommaso Bianchi Date: Sat, 22 Aug 2026 23:03:47 +0200 Subject: [PATCH] Sketch: closing a polyline is now a previewed snap, not an invisible bubble Ported from snaporca 982968b1af. --- docs/ux/tool_atlas.json | 2 +- src/slic3r/GUI/CAD/DesignOffer.hpp | 2 +- src/slic3r/GUI/CAD/DesignSketchTool.cpp | 36 +++++++++++++++++++------ src/slic3r/GUI/CAD/DesignSketchTool.hpp | 5 +++- 4 files changed, 34 insertions(+), 11 deletions(-) diff --git a/docs/ux/tool_atlas.json b/docs/ux/tool_atlas.json index 9aaf1400c4..177775b336 100644 --- a/docs/ux/tool_atlas.json +++ b/docs/ux/tool_atlas.json @@ -1181,7 +1181,7 @@ "mode": "sketch", "family": "Line", "icon": "design_polyline", - "hint": "Click points; click first / right-click to close the loop" + "hint": "Click points; click the first point to close the loop, right-click to end it open" }, { "id": "sk_rect", diff --git a/src/slic3r/GUI/CAD/DesignOffer.hpp b/src/slic3r/GUI/CAD/DesignOffer.hpp index de13eb6752..82ef068291 100644 --- a/src/slic3r/GUI/CAD/DesignOffer.hpp +++ b/src/slic3r/GUI/CAD/DesignOffer.hpp @@ -127,7 +127,7 @@ static const OfferVerb kOfferVerbs[] = { {"delete", "Delete", 7, "Del", "btn:delete", nullptr, 0x000f7c00u, 0, 0, false, false, nullptr, "design_delete", "Delete what is selected"}, {"delete_body", "Delete Body", 7, nullptr, "btn:delete_body", nullptr, 0x000001feu, 1, 0, false, false, nullptr, "design_delete", "Delete this whole body — removes the feature it was made from"}, {"sk_line_t", "Line", 0, "L", "key:L", nullptr, 0x000f8000u, 0, 0, false, true, "Line", "design_line", "Line — click start, then end"}, - {"sk_polyline", "Polyline", 0, nullptr, "fly:design_line#1", nullptr, 0x000f8000u, 0, 0, false, true, "Line", "design_polyline", "Click points; click first / right-click to close the loop"}, + {"sk_polyline", "Polyline", 0, nullptr, "fly:design_line#1", nullptr, 0x000f8000u, 0, 0, false, true, "Line", "design_polyline", "Click points; click the first point to close the loop, right-click to end it open"}, {"sk_rect", "Corner rectangle", 0, "R", "key:R", nullptr, 0x000f8000u, 0, 0, false, true, "Rectangle", "design_rect", "Rectangle — click two opposite corners"}, {"sk_rect_center", "Centre rectangle", 0, nullptr, "fly:design_rect#1", nullptr, 0x000f8000u, 0, 0, false, true, "Rectangle", "design_crect", "Click center, then a corner"}, {"sk_rect_oblique", "Oblique rectangle", 0, nullptr, "fly:design_rect#2", nullptr, 0x000f8000u, 0, 0, false, true, "Rectangle", "design_rect_oblique", "Click two corners of one edge, then a point for the width"}, diff --git a/src/slic3r/GUI/CAD/DesignSketchTool.cpp b/src/slic3r/GUI/CAD/DesignSketchTool.cpp index a9265b5376..2472957719 100644 --- a/src/slic3r/GUI/CAD/DesignSketchTool.cpp +++ b/src/slic3r/GUI/CAD/DesignSketchTool.cpp @@ -2206,9 +2206,25 @@ bool DesignSketchTool::screen_to_plane(GLCanvas3D& canvas, const wxMouseEvent& e return true; } -bool DesignSketchTool::near_first(const Vec2d& p) const +// Closing the loop is THE outcome this tab exists for — a sketch that is not closed cannot be +// extruded — so it must be as easy and as visible as any other snap, and it must behave the +// same at every zoom. This used to be a bare `squaredNorm() < 4.0`, i.e. a fixed 2 mm bubble in +// plane units: a pixel hunt zoomed out, an over-eager magnet zoomed in, and invisible either +// way — nothing told the user their next click would close the loop rather than place another +// vertex. Now the chain's first point is a real snap target on the same 8 px screen tolerance +// as every endpoint snap: hovering it moves the cursor exactly onto it, which makes the rubber +// band draw the closing segment as a preview and lights the existing snap marker. The click +// then closes only because it was SNAPPED, never because it was merely nearby. +bool DesignSketchTool::snap_chain_start(GLCanvas3D& canvas, const wxMouseEvent& evt, Vec2d& p) const { - return m_points.size() >= 3 && (p - m_points[0]).squaredNorm() < 4.0; + if (m_mode != Mode::Polyline || m_points.size() < 3) return false; + if ((p - m_points[0]).norm() > screen_tol(canvas, evt, p)) return false; + p = m_points[0]; + InferenceSnap s; + s.kind = InferenceSnap::Kind::Endpoint; // renders the same hint as any endpoint snap + s.point = m_points[0]; + const_cast(this)->m_cursor_snap = s; + return true; } Vec2d DesignSketchTool::snap_dir(const Vec2d& anchor, const Vec2d& raw, bool& locked) const @@ -10156,6 +10172,9 @@ bool DesignSketchTool::on_mouse_impl(wxMouseEvent& evt, GLCanvas3D& canvas) m_cursor_locked = false; bool vsnap = false; m_cursor = snap_vertex(canvas, evt, m_cursor, vsnap); // preview-snap to endpoints + // The chain's own start is not an entity yet, so snap_vertex cannot see it. Offer it + // here: the cursor lands exactly on it, so the rubber band below IS the closing segment. + if (snap_chain_start(canvas, evt, m_cursor)) vsnap = true; const bool line_like = (m_mode == Mode::Polyline || m_mode == Mode::Line); if (line_like && !m_points.empty() && !vsnap) m_cursor = snap_dir(m_points.back(), m_cursor, m_cursor_locked); @@ -10175,7 +10194,7 @@ bool DesignSketchTool::on_mouse_impl(wxMouseEvent& evt, GLCanvas3D& canvas) m_snap_off = evt.ShiftDown(); bool vsnap = false; p = snap_vertex(canvas, evt, p, vsnap); // snap onto an existing endpoint - if (near_first(p)) { // closing the current chain back to its start + if (snap_chain_start(canvas, evt, p)) { // clicked the previewed close target const int base = int(m_entities.size()); push_closed_lines(m_points); // close the loop infer_auto_constraints(base); // loop self-closes via auto Coincident + H/V @@ -10204,11 +10223,12 @@ bool DesignSketchTool::on_mouse_impl(wxMouseEvent& evt, GLCanvas3D& canvas) // or more points, i.e. it drew a final segment from the last point back to the // first that the user never asked for, and did it silently. No mainstream sketcher // does that: FreeCAD, Fusion and Onshape all end an open chain on right-click and - // require the close to be EXPLICIT. Ours already has that gesture — click back on - // the start point, which near_first() picks up in the LeftDown branch above — so - // the fabricated edge was not even the only way to get a loop, just the one the - // user could not see coming. A closed loop is this tab's goal, but an invented - // segment is not "precise definition of every aspect", it is a guess. + // require the close to be EXPLICIT. Ours has that gesture — click the chain's first + // point, which snap_chain_start() previews and snaps onto in the LeftDown branch + // above, so the closing segment is on screen BEFORE it is committed. A closed loop + // is this tab's goal and it stays a four-action triangle either way; what changes + // is that the closing edge is now something the user saw and chose, not one the + // app appended on their behalf. const int base = int(m_entities.size()); if (m_points.size() >= 2) push_open_chain(m_points); diff --git a/src/slic3r/GUI/CAD/DesignSketchTool.hpp b/src/slic3r/GUI/CAD/DesignSketchTool.hpp index a5d0835887..0e2bf6a4a7 100644 --- a/src/slic3r/GUI/CAD/DesignSketchTool.hpp +++ b/src/slic3r/GUI/CAD/DesignSketchTool.hpp @@ -601,7 +601,10 @@ public: private: bool screen_to_plane(GLCanvas3D& canvas, const wxMouseEvent& evt, Vec2d& out) const; - bool near_first(const Vec2d& p) const; + // True when the cursor is close enough to the open chain's FIRST point to close the loop, + // on the same screen tolerance as every other snap; snaps `p` exactly onto that point so + // the rubber band previews the closing segment and the snap marker lights. + bool snap_chain_start(GLCanvas3D& canvas, const wxMouseEvent& evt, Vec2d& p) const; // Onshape-style angle inference: snap the direction anchor->raw to the nearest // of {0,30,45,60,90} deg (replicated every 90 deg) when within tolerance, keeping