From e8306a6e9a111ab3cad87955fddbd76542c636c2 Mon Sep 17 00:00:00 2001 From: Tommaso Bianchi Date: Wed, 12 Aug 2026 20:40:21 +0200 Subject: [PATCH] Helix: draw the thing, then let the numbers be dragged MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit grep -i helix over the viewport code returned nothing at all. The tool was four coupled numbers and a Confirm button — you typed radius, pitch, height and taper blind and pressed OK to find out what you had made. So this is not only the charter's L2 failure; the tool had no visible state whatsoever while it was open. Adds a plane-anchored helix gizmo built on the datum-plane gizmo as its template, being the closest existing thing: also plane-anchored, also driven by a card while the sketch tool is inactive, also a render / hit-test / drag triad. It draws the live curve and the axis, and puts a handle on each of the three lengths: radius on the base circle, height at the top of the axis, pitch at the end of the first turn — which is exactly where one pitch of rise lands, so the handle means what it is standing on. Below one full turn the pitch handle moves to the end of the curve rather than floating off a curve that does not exist yet. Taper and handedness stay on the card. One is a shape modifier and the other a flag; L2 governs numbers you can point at. A drag reports the whole (radius, pitch, height) triple rather than one value, because pitch and height are coupled through the turn count and writing one alone would redraw a stale curve. The callback re-feeds the gizmo directly instead of going through refresh_preview(), since Helix takes the produces-no-solid early return and refresh_preview would rewrite the status line on every mouse move. REVIEW CATCH, fixed here: the first cut read taper as a fraction of the radius consumed over the turn count. It is an ANGLE IN DEGREES — helix_spine() builds a Geom_ConicalSurface of half-angle taper and takes the top radius as R+H*tan(taper), growing with the height risen. The wrong reading drew a preview that collapsed to a point for any non-zero taper while the committed feature was perfectly fine. A preview that lies is worse than no preview, which is what this commit replaced. Reviewed and compiled (libslic3r_gui, RC=0); not exercised. snaporca-i3jc. --- src/slic3r/GUI/DesignCanvas.cpp | 18 +++ src/slic3r/GUI/DesignCanvas.hpp | 4 + src/slic3r/GUI/DesignPanel.cpp | 25 ++++ src/slic3r/GUI/DesignPanel.hpp | 1 + src/slic3r/GUI/DesignSketchTool.cpp | 192 ++++++++++++++++++++++++++++ src/slic3r/GUI/DesignSketchTool.hpp | 28 +++- 6 files changed, 267 insertions(+), 1 deletion(-) diff --git a/src/slic3r/GUI/DesignCanvas.cpp b/src/slic3r/GUI/DesignCanvas.cpp index f17445f8ec..166b5ecfb9 100644 --- a/src/slic3r/GUI/DesignCanvas.cpp +++ b/src/slic3r/GUI/DesignCanvas.cpp @@ -843,6 +843,24 @@ void DesignCanvas::set_on_datum_offset_changed(std::function cb) m_sketch_tool.on_datum_offset_changed = std::move(cb); } +void DesignCanvas::set_helix_gizmo(const SketchPlane& plane, double radius, double pitch, + double height, double taper, bool left_handed) +{ + m_sketch_tool.set_helix_gizmo(plane, radius, pitch, height, taper, left_handed); + request_repaint(); +} + +void DesignCanvas::clear_helix_gizmo() +{ + m_sketch_tool.clear_helix_gizmo(); + request_repaint(); +} + +void DesignCanvas::set_on_helix_changed(std::function cb) +{ + m_sketch_tool.on_helix_changed = std::move(cb); +} + void DesignCanvas::set_base_pick(std::vector planes, std::vector bases, std::vector labels) { diff --git a/src/slic3r/GUI/DesignCanvas.hpp b/src/slic3r/GUI/DesignCanvas.hpp index 41bd2460c5..2d4c7b31be 100644 --- a/src/slic3r/GUI/DesignCanvas.hpp +++ b/src/slic3r/GUI/DesignCanvas.hpp @@ -170,6 +170,10 @@ public: void clear_datum_gizmo(); void set_on_datum_size_changed(std::function cb); void set_on_datum_offset_changed(std::function cb); + void set_helix_gizmo(const SketchPlane& plane, double radius, double pitch, double height, + double taper, bool left_handed); // helix curve + 3 drag handles + void clear_helix_gizmo(); + void set_on_helix_changed(std::function cb); void set_base_pick(std::vector planes, std::vector bases, std::vector labels = {}); // clickable labelled reference planes void clear_base_pick(); diff --git a/src/slic3r/GUI/DesignPanel.cpp b/src/slic3r/GUI/DesignPanel.cpp index c95ac0209b..a83e2b8811 100644 --- a/src/slic3r/GUI/DesignPanel.cpp +++ b/src/slic3r/GUI/DesignPanel.cpp @@ -3534,6 +3534,16 @@ DesignPanel::DesignPanel(wxWindow* parent) m_viewport->request_repaint(); }); + // Helix handles: a drag reports the whole triple, since pitch and height are coupled + // through the turn count and reading one without the others would show a stale curve. + m_viewport->set_on_helix_changed([this](double radius, double pitch, double height) { + if (m_helix_radius) m_helix_radius->SetValue(radius); + if (m_helix_pitch) m_helix_pitch->SetValue(pitch); + if (m_helix_height) m_helix_height->SetValue(height); + update_helix_gizmo(); // re-feed so the curve follows the drag + m_viewport->request_repaint(); + }); + // Clicking a ghost base plane sets the base graphically (replaces the dropdown). A base pick // drops any offset-from-face choice so the picked base plane wins, then re-resolves the preview. m_viewport->set_on_datum_base_picked([this](int base) { @@ -9100,6 +9110,18 @@ void DesignPanel::update_datum_gizmo() update_reference_planes(); // base ghosts (origins + datums) follow the tool/model state } +void DesignPanel::update_helix_gizmo() +{ + if (!m_viewport) return; + if (m_active != Tool::Helix) { m_viewport->clear_helix_gizmo(); return; } + m_viewport->set_helix_gizmo(plane_from_choice(m_helix_plane->GetSelection()), + m_helix_radius ? m_helix_radius->GetValue() : 0.0, + m_helix_pitch ? m_helix_pitch->GetValue() : 1.0, + m_helix_height ? m_helix_height->GetValue() : 0.0, + m_helix_taper ? m_helix_taper->GetValue() : 0.0, + m_helix_left_handed && m_helix_left_handed->GetValue()); +} + // Onshape default planes: the XY/XZ/YZ reference planes are persistent, transparent, labelled, and // larger than the bed — shown as the FALLBACK when there is no object yet. When the Plane tool is // open they additionally surface existing datums so a base can be picked. Single authority for the @@ -9166,6 +9188,7 @@ void DesignPanel::refresh_preview() for (wxButton* b : m_confirm_btns) if (b) b->Enable(true); m_status->Refresh(); update_datum_gizmo(); // Plane card: show/refresh the in-canvas resize handles + update_helix_gizmo(); // Helix card: draw the live curve + drag handles (no solid ghost) return; } @@ -9272,6 +9295,8 @@ void DesignPanel::refresh_preview() update_pattern_gizmo(); // Datum-plane resize handles (self-gates: only while the Plane card is open). update_datum_gizmo(); + // Helix curve + handles (self-gates: only while the Helix card is open). + update_helix_gizmo(); update_operand_highlight(); } diff --git a/src/slic3r/GUI/DesignPanel.hpp b/src/slic3r/GUI/DesignPanel.hpp index 82f0abc017..a66d12efae 100644 --- a/src/slic3r/GUI/DesignPanel.hpp +++ b/src/slic3r/GUI/DesignPanel.hpp @@ -272,6 +272,7 @@ private: void update_operand_highlight(); // Boolean/Sweep/Loft operand tinting on the canvas void update_pattern_gizmo(); // linear spacing arrow / circular angle-arc (Pattern card) void update_datum_gizmo(); // resize handles on the datum plane being created/edited (C3) + void update_helix_gizmo(); // live helix curve + radius/height/pitch handles (Helix card) void refresh_datum_planes(); // push resolved datum frames + per-plane u/v extents to viewport void refresh_mate_connectors(); // push connector frames so verse + polarity are visible void update_reference_planes(); // persistent XY/XZ/YZ reference planes (fallback when no object) diff --git a/src/slic3r/GUI/DesignSketchTool.cpp b/src/slic3r/GUI/DesignSketchTool.cpp index 836ff1b28c..81a755515c 100644 --- a/src/slic3r/GUI/DesignSketchTool.cpp +++ b/src/slic3r/GUI/DesignSketchTool.cpp @@ -3738,6 +3738,182 @@ void DesignSketchTool::drag_datum_handle(GLCanvas3D& canvas, const wxMouseEvent& if (on_datum_size_changed) on_datum_size_changed(m_dz_usize, m_dz_vsize); } +// ---- Helix gizmo (plane-anchored curve + 3 drag handles) -------------------------------- +void DesignSketchTool::set_helix_gizmo(const SketchPlane& plane, double radius, double pitch, + double height, double taper, bool left_handed) +{ + m_hx_active = true; + m_hx_plane = plane; + m_hx_radius = radius; + m_hx_pitch = pitch; + m_hx_height = height; + m_hx_taper = taper; + m_hx_left = left_handed; +} + +void DesignSketchTool::clear_helix_gizmo() +{ + m_hx_active = false; + m_hx_drag = -1; +} + +// Curve point at parameter t (t in turns): angle 2*pi*t, negated when left-handed, rising one +// pitch per turn along the plane normal. +// +// Taper is an ANGLE IN DEGREES and must be read the way the kernel reads it. CadDocument's +// helix_spine() builds a Geom_ConicalSurface of half-angle taper and computes the top radius as +// R + H*tan(taper), so the radius grows with the HEIGHT RISEN, not as a fraction of R consumed +// over the turn count. Getting that wrong draws a preview that collapses to a point for any +// non-zero taper while the committed feature is fine — a preview that lies is worse than none. +Vec3d DesignSketchTool::helix_point(double t) const +{ + const Vec3d O = m_hx_plane.origin; + const Vec3d n = m_hx_plane.normal.normalized(); + const Vec3d u = m_hx_plane.x_axis.normalized(); + const Vec3d v = m_hx_plane.y_axis.normalized(); + const double a = (m_hx_left ? -1.0 : 1.0) * 2.0 * M_PI * t; + const double z = m_hx_pitch * t; // height risen at this parameter + double rt = m_hx_radius + z * std::tan(m_hx_taper * M_PI / 180.0); + // The kernel REFUSES a taper that drives the radius negative before the full height; the + // preview shows it collapsing instead, so the user can see which value did it. + if (rt < 0.0) rt = 0.0; + return O + u * (rt * std::cos(a)) + v * (rt * std::sin(a)) + n * z; +} + +// Draw the live helix as a connected camera-facing ribbon, a dim axis line, and three square +// handles (radius on the base circle, height on the axis top, pitch at the end of the first turn). +void DesignSketchTool::render_helix_gizmo() +{ + if (!m_hx_active) return; + using EPT = GLModel::Geometry::EPrimitiveType; + using EVL = GLModel::Geometry::EVertexLayout; + const Camera& cam = wxGetApp().plater()->get_camera(); + const Vec3d right = cam.get_dir_right().normalized(); + const Vec3d up = cam.get_dir_up().normalized(); + const Vec3d vd = cam.get_dir_forward(); + const double upp = 1.0 / std::max(cam.get_zoom(), 1e-6); + const double hs = 6.0 * upp; // handle half-size (~6 px) + const double hw = 1.5 * upp; // ribbon half-width + const Vec3d O = m_hx_plane.origin; + const Vec3d n = m_hx_plane.normal.normalized(); + const double turns = m_hx_pitch > 1e-9 ? m_hx_height / m_hx_pitch : 0.0; + + // Curve ribbon: connected thin camera-facing quads over t in [0, turns]. + const int segs = std::min(2048, std::max(48, int(turns * 32.0))); + GLModel::Geometry curve; curve.format = { EPT::Triangles, EVL::P3 }; + unsigned int cb = 0; + auto add_seg = [&](const Vec3d& a, const Vec3d& b) { + Vec3d dir = b - a; if (dir.norm() < 1e-9) return; dir.normalize(); + Vec3d off = dir.cross(vd); + if (off.norm() < 1e-9) off = dir.cross(up); + if (off.norm() < 1e-9) return; + off.normalize(); off *= hw; + curve.add_vertex((Vec3f)(a + off).cast()); curve.add_vertex((Vec3f)(b + off).cast()); + curve.add_vertex((Vec3f)(b - off).cast()); curve.add_vertex((Vec3f)(a - off).cast()); + curve.add_triangle(cb, cb + 1, cb + 2); curve.add_triangle(cb, cb + 2, cb + 3); cb += 4; + }; + for (int i = 0; i < segs; ++i) + add_seg(helix_point(turns * i / segs), helix_point(turns * (i + 1) / segs)); + + // Axis: a dim line from the plane origin to the top of the helix. + const Vec3d top = O + n * m_hx_height; + GLModel::Geometry axis; axis.format = { EPT::Triangles, EVL::P3 }; + { + Vec3d dir = top - O; + if (dir.norm() > 1e-9) { + dir.normalize(); + Vec3d off = dir.cross(vd); + if (off.norm() < 1e-9) off = dir.cross(up); + if (off.norm() > 1e-9) { + off.normalize(); off *= hw * 0.6; + axis.add_vertex((Vec3f)(O + off).cast()); axis.add_vertex((Vec3f)(top + off).cast()); + axis.add_vertex((Vec3f)(top - off).cast()); axis.add_vertex((Vec3f)(O - off).cast()); + axis.add_triangle(0, 1, 2); axis.add_triangle(0, 2, 3); + } + } + } + + // Three handles: 0=radius (t=0), 1=height (axis top), 2=pitch (end of first turn, or the + // whole curve if shorter than one turn so the handle never floats off a missing curve). + const Vec3d hpts[3] = { helix_point(0.0), top, + helix_point(m_hx_height < m_hx_pitch ? turns : 1.0) }; + const ColorRGBA hcol[3] = { ColorRGBA(1.0f, 0.72f, 0.28f, 1.0f), // radius — amber + ColorRGBA(0.45f, 0.86f, 1.0f, 1.0f), // height — cyan + ColorRGBA(0.30f, 0.80f, 0.34f, 1.0f) }; // pitch — green + const ColorRGBA hot(1.0f, 0.85f, 0.2f, 1.0f); + + glsafe(::glDisable(GL_DEPTH_TEST)); + if (cb > 0) { + GLModel m; m.init_from(std::move(curve)); + m.set_color(ColorRGBA(1.0f, 0.62f, 0.16f, 0.9f)); // CAD amber helix curve + m.render(); + } + if (!axis.vertices.empty()) { + GLModel m; m.init_from(std::move(axis)); + m.set_color(ColorRGBA(0.42f, 0.46f, 0.52f, 0.55f)); // dim grey axis + m.render(); + } + for (int i = 0; i < 3; ++i) { + const Vec3d ctr = hpts[i]; + const Vec3d q0 = ctr - right * hs - up * hs, q1 = ctr + right * hs - up * hs, + q2 = ctr + right * hs + up * hs, q3 = ctr - right * hs + up * hs; + GLModel::Geometry sq; sq.format = { EPT::Triangles, EVL::P3 }; + sq.add_vertex((Vec3f)q0.cast()); sq.add_vertex((Vec3f)q1.cast()); + sq.add_vertex((Vec3f)q2.cast()); sq.add_vertex((Vec3f)q3.cast()); + sq.add_triangle(0, 1, 2); sq.add_triangle(0, 2, 3); + GLModel m; m.init_from(std::move(sq)); + m.set_color(m_hx_drag == i ? hot : hcol[i]); + m.render(); + } +} + +bool DesignSketchTool::hit_test_helix_handle(GLCanvas3D& canvas, const wxMouseEvent& evt, int& which) const +{ + if (!m_hx_active) return false; + const Linef3 r = canvas.mouse_ray(Point(evt.GetX(), evt.GetY())); + const Vec3d ro = r.a, rd = r.b - r.a; + const Camera& cam = wxGetApp().plater()->get_camera(); + const double tol = 9.0 / std::max(cam.get_zoom(), 1e-6); // ~9 px in world units + const Vec3d O = m_hx_plane.origin; + const Vec3d n = m_hx_plane.normal.normalized(); + const double turns = m_hx_pitch > 1e-9 ? m_hx_height / m_hx_pitch : 0.0; + const Vec3d hpts[3] = { helix_point(0.0), O + n * m_hx_height, + helix_point(m_hx_height < m_hx_pitch ? turns : 1.0) }; + double best = tol; which = -1; + for (int i = 0; i < 3; ++i) { + const Vec3d w = hpts[i] - ro; + const double t = w.dot(rd) / std::max(rd.dot(rd), 1e-12); + const double d = (w - rd * t).norm(); + if (d < best) { best = d; which = i; } + } + return which >= 0; +} + +// Drag a handle: radius = cursor's in-plane distance from the origin, height/pitch = the signed +// distance of the cursor's closest axis point. All fire the full (radius, pitch, height) triple. +void DesignSketchTool::drag_helix_handle(GLCanvas3D& canvas, const wxMouseEvent& evt, int which) +{ + const Linef3 r = canvas.mouse_ray(Point(evt.GetX(), evt.GetY())); + const Vec3d ro = r.a, rd = r.b - r.a; + const SketchPlane& p = m_hx_plane; + const Vec3d O = p.origin, n = p.normal.normalized(); + + if (which == 0) { // radius: in-plane distance from O + const Vec2d lp = p.project(ro, rd); + m_hx_radius = std::max(0.01, lp.norm()); + } else { // height/pitch: signed distance along n + const Vec3d e = n; + const Vec3d w0 = O - ro; + const double a = e.dot(e), b = e.dot(rd), c = rd.dot(rd), dd = e.dot(w0), ee = rd.dot(w0); + const double denom = a * c - b * b; + if (std::abs(denom) < 1e-7) return; // camera ∥ axis: leave value as-is + const double s = (b * ee - c * dd) / denom; // signed distance along the axis + if (which == 1) m_hx_height = std::max(0.0, s); + else m_hx_pitch = std::max(0.01, s); // zero/negative pitch divides by zero + } + if (on_helix_changed) on_helix_changed(m_hx_radius, m_hx_pitch, m_hx_height); +} + // ---- Reference/base planes (Onshape-style default planes) ----------------------------- void DesignSketchTool::set_base_pick(std::vector planes, std::vector bases, std::vector labels) @@ -7159,6 +7335,7 @@ void DesignSketchTool::render(GLCanvas3D& canvas) render_solid_highlight(); if (m_dbp_active) render_base_pick(); if (m_dz_active) render_datum_gizmo(); + if (m_hx_active) render_helix_gizmo(); if (m_ex_active) render_extrude_gizmo(); if (m_mv_active) render_move_gizmo(); if (m_fl_active) render_fillet_gizmo(); @@ -7966,6 +8143,21 @@ bool DesignSketchTool::on_mouse_impl(wxMouseEvent& evt, GLCanvas3D& canvas) } } } + // Helix gizmo: radius/height/pitch handles on the live curve, while the Helix card is open. + if (m_hx_active) { + if (m_hx_drag >= 0 && evt.Dragging() && evt.LeftIsDown()) { + drag_helix_handle(canvas, evt, m_hx_drag); + return true; + } + if (evt.LeftUp() && m_hx_drag >= 0) { m_hx_drag = -1; return true; } + if (evt.LeftDown()) { + int which = -1; + if (hit_test_helix_handle(canvas, evt, which)) { + m_hx_drag = which; m_hx_press_x = evt.GetX(); m_hx_press_y = evt.GetY(); + return true; + } + } + } // Datum base picker: HOVER highlight only here. The CLICK is handled at the very end of the // selection fall-through (below), so picking existing geometry (committed sketch loops, // solid faces/edges) always wins over a base-plane click — the planes never block selection. diff --git a/src/slic3r/GUI/DesignSketchTool.hpp b/src/slic3r/GUI/DesignSketchTool.hpp index 6f0af7d9c7..33879da896 100644 --- a/src/slic3r/GUI/DesignSketchTool.hpp +++ b/src/slic3r/GUI/DesignSketchTool.hpp @@ -119,7 +119,7 @@ public: || m_show_planes || m_show_axes || m_ex_active || m_mv_active || m_fl_active || m_hl_active || m_th_active || m_sh_active - || m_dr_active || m_ct_active || m_dz_active || m_dbp_active; } + || m_dr_active || m_ct_active || m_dz_active || m_dbp_active || m_hx_active; } // View helpers: the 3 world origin planes (XY/XZ/YZ) and the world axis triad, each // shown/hidden by a toggle (keys P / A). Off by default so the idle scene stays clean. @@ -204,6 +204,16 @@ public: std::function on_datum_size_changed; std::function on_datum_offset_changed; + // Visual Helix gizmo. The Helix tool is a DesignPanel docked card (sketch tool NOT active), + // so the panel resolves the axis plane and feeds the live parameters here; the tool draws + // the helix curve itself plus three handles — radius on the base circle, height at the top + // of the axis, pitch at the end of the first turn. Taper and handedness stay on the card: + // one is a shape modifier and the other is a flag, and L2 governs numbers you can point at. + void set_helix_gizmo(const SketchPlane& plane, double radius, double pitch, double height, + double taper, bool left_handed); + void clear_helix_gizmo(); + std::function on_helix_changed; + // Graphical base/origin pick: while the Plane card is open, the candidate base planes // (XY/XZ/YZ origin planes + existing datums) draw as translucent clickable ghosts. A click // on one fires on_datum_base_picked(base) with that plane's base index (0/1/2 or 3+N). @@ -1013,6 +1023,22 @@ private: void render_datum_gizmo(); bool hit_test_datum_handle(GLCanvas3D& canvas, const wxMouseEvent& evt, int& which) const; void drag_datum_handle(GLCanvas3D& canvas, const wxMouseEvent& evt, int which); + // Helix gizmo state (plane-anchored curve + 3 drag handles). Fed by the panel while the + // Helix card is open (sketch tool NOT active); the tool draws the live helix plus a handle + // on each length parameter (radius/height/pitch). Taper and handedness stay on the card. + bool m_hx_active{false}; + SketchPlane m_hx_plane; // axis = plane normal, base circle in the plane + double m_hx_radius{10.0}; + double m_hx_pitch{2.0}; + double m_hx_height{20.0}; + double m_hx_taper{0.0}; // DEGREES (cone half-angle), as the kernel reads it + bool m_hx_left{false}; + int m_hx_drag{-1}; // 0=radius, 1=height, 2=pitch, -1 none + int m_hx_press_x{0}, m_hx_press_y{0}; + Vec3d helix_point(double t) const; // curve point at parameter t (shared render/hit/drag) + void render_helix_gizmo(); + bool hit_test_helix_handle(GLCanvas3D& canvas, const wxMouseEvent& evt, int& which) const; + void drag_helix_handle(GLCanvas3D& canvas, const wxMouseEvent& evt, int which); // Datum base picker (translucent clickable origin/datum planes) bool m_dbp_active{false}; std::vector m_dbp_planes;