From 9858080aa05b9c9bbb3463ae1313df19db0c9a7b Mon Sep 17 00:00:00 2001 From: Tommaso Bianchi Date: Tue, 1 Sep 2026 06:45:18 +0200 Subject: [PATCH] The constraint list follows you into a live sketch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rows, their ✗ buttons and the click-to-highlight were all built against m_doc.features[m_constrain_feat].entity_constraints — a COMMITTED feature. A live sketch has no committed feature, so the card was hidden for the whole session and the list it would have shown was empty by construction. Every constraint applied while drawing was nameless: the badge said one existed, nothing said which. rebuild_constraint_list now picks its source by scope. live_constraint_scope() is the same discriminator apply_constraint already used to route to apply_live_constraint — both Constrain modes set m_active, so is_sketching() alone would claim the live scope while the committed manager is open. delete_constraint and highlight_constraint_entities branch on it too, and the card shows in Sketch mode as well as Constrain. Keeping the rows in step needed a signal that did not exist: on_solve_state fires on every frame of a drag, so rebuilding from it would rebuild the list continuously. The tool now fires on_constraints_changed only when the constraint SET changes — one added by try_add_constraints, one removed by remove_constraint (the indexed form the badge click and the ✗ row now share). The rebuild is deferred through CallAfter. One of its callers is the ✗ button's own click handler, and rebuild_constraint_list destroys those buttons: deleting the window whose handler is still on the stack is a use-after-free. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011FbJKJAJxxkhDTs9XdZzKA --- src/slic3r/GUI/CAD/DesignCanvas.cpp | 17 +++++++++ src/slic3r/GUI/CAD/DesignCanvas.hpp | 3 ++ src/slic3r/GUI/CAD/DesignPanel.cpp | 50 +++++++++++++++++++++---- src/slic3r/GUI/CAD/DesignPanel.hpp | 5 +++ src/slic3r/GUI/CAD/DesignSketchTool.cpp | 13 ++++++- src/slic3r/GUI/CAD/DesignSketchTool.hpp | 7 ++++ 6 files changed, 86 insertions(+), 9 deletions(-) diff --git a/src/slic3r/GUI/CAD/DesignCanvas.cpp b/src/slic3r/GUI/CAD/DesignCanvas.cpp index 6234fa6f61..b65223dc57 100644 --- a/src/slic3r/GUI/CAD/DesignCanvas.cpp +++ b/src/slic3r/GUI/CAD/DesignCanvas.cpp @@ -1494,6 +1494,23 @@ int DesignCanvas::sketch_constraint_count() const return int(m_sketch_tool.constraints().size()); } +const std::vector& DesignCanvas::sketch_constraints() const +{ + return m_sketch_tool.constraints(); +} + +bool DesignCanvas::remove_sketch_constraint(int idx) +{ + const bool removed = m_sketch_tool.remove_constraint(idx); + if (removed) request_repaint(); + return removed; +} + +void DesignCanvas::set_on_sketch_constraints_changed(std::function cb) +{ + m_sketch_tool.on_constraints_changed = std::move(cb); +} + bool DesignCanvas::try_add_sketch_constraints(const std::vector& defs) { return m_sketch_tool.try_add_constraints(defs); diff --git a/src/slic3r/GUI/CAD/DesignCanvas.hpp b/src/slic3r/GUI/CAD/DesignCanvas.hpp index fe87936f8b..6a2153d00d 100644 --- a/src/slic3r/GUI/CAD/DesignCanvas.hpp +++ b/src/slic3r/GUI/CAD/DesignCanvas.hpp @@ -302,6 +302,9 @@ public: // How many constraints the LIVE session holds. Only a count: the hint line needs to know // whether any badge is on screen to talk about, nothing more. int sketch_constraint_count() const; + const std::vector& sketch_constraints() const; + bool remove_sketch_constraint(int idx); + void set_on_sketch_constraints_changed(std::function cb); bool try_add_sketch_constraints(const std::vector& defs); // In-canvas bbox transform of imported Text/SVG art (replaces the Move/Scale dialog). diff --git a/src/slic3r/GUI/CAD/DesignPanel.cpp b/src/slic3r/GUI/CAD/DesignPanel.cpp index cddecfbbb1..e140bbe38d 100644 --- a/src/slic3r/GUI/CAD/DesignPanel.cpp +++ b/src/slic3r/GUI/CAD/DesignPanel.cpp @@ -3591,6 +3591,14 @@ DesignPanel::DesignPanel(wxWindow* parent) on_sketch_step(int(mode), step, picks); }); + // A live constraint appeared or went away: refill the rows. CallAfter, not a direct call — + // one of the callers is the ✗ button's own click handler, and rebuild_constraint_list + // destroys those buttons; deleting the window whose handler is still on the stack is a + // use-after-free. Deferring to the next event-loop turn lets the handler return first. + m_viewport->set_on_sketch_constraints_changed([this]() { + CallAfter([this]() { rebuild_constraint_list(); }); + }); + // DoF feedback (P3): after each live solve, report constraint state on its own // line. Green = fully constrained, red = conflicting, neutral = N remaining DoF. m_viewport->set_on_solve_state([this](int dof, bool ok, bool has_constraints) { @@ -4448,8 +4456,8 @@ void DesignPanel::set_ui_mode(UiMode m) } // Constraint-manager card follows Constrain mode; rebuilt from the active feature. if (m_box_constraints != nullptr && m_form != nullptr && m_cards->GetSizer() != nullptr) { - if (m == UiMode::Constrain) - rebuild_constraint_list(); + if (m == UiMode::Constrain || m == UiMode::Sketch) + rebuild_constraint_list(); // Sketch too: a live session has its own constraints else m_cards->GetSizer()->Show(m_box_constraints, false, true); update_cards_frame(); m_form->Layout(); @@ -8002,7 +8010,16 @@ wxString DesignPanel::constraint_label(const SketchEntityConstraintDef& d) const return _L("Constraint"); } -// Rebuild the constraint-row list from the constrained feature's entity_constraints. +bool DesignPanel::live_constraint_scope() const +{ + return m_viewport != nullptr && m_viewport->is_sketching() && + !m_viewport->is_constraining() && !m_viewport->is_constraining_entities(); +} + +// Rebuild the constraint-row list. Source depends on scope: a LIVE sketch session owns its +// constraints inside the sketch tool and has no committed feature yet, so the list read only +// the feature's and stayed empty for the whole session — constraints applied while drawing had +// no row, no ✗, and no name anywhere in the UI. void DesignPanel::rebuild_constraint_list() { if (m_constraint_rows == nullptr || m_form == nullptr) @@ -8010,10 +8027,12 @@ void DesignPanel::rebuild_constraint_list() m_constraint_rows->Clear(true /* delete windows */); m_constraint_sel = -1; + const bool live = live_constraint_scope(); const bool active = (m_constrain_feat >= 0 && m_constrain_feat < int(m_doc.features.size())); const std::vector empty; const std::vector& cons = - active ? m_doc.features[m_constrain_feat].entity_constraints : empty; + live ? m_viewport->sketch_constraints() + : active ? m_doc.features[m_constrain_feat].entity_constraints : empty; if (m_hdr_constraints) m_hdr_constraints->SetLabel(wxString::Format(_L("Constraints (%d)"), int(cons.size()))); @@ -8044,7 +8063,8 @@ void DesignPanel::rebuild_constraint_list() if (m_viewport) m_viewport->set_constraint_glyphs(cons); - m_cards->GetSizer()->Show(m_box_constraints, m_ui_mode == UiMode::Constrain, true); + m_cards->GetSizer()->Show(m_box_constraints, + m_ui_mode == UiMode::Constrain || live, true); update_cards_frame(); m_form->Layout(); m_form->FitInside(); } @@ -8053,9 +8073,13 @@ void DesignPanel::rebuild_constraint_list() // highlight (toggle off if the same row is clicked again). void DesignPanel::highlight_constraint_entities(int idx) { - if (!m_viewport || m_constrain_feat < 0 || m_constrain_feat >= int(m_doc.features.size())) + if (!m_viewport) return; - const auto& cons = m_doc.features[m_constrain_feat].entity_constraints; + const bool live = live_constraint_scope(); + if (!live && (m_constrain_feat < 0 || m_constrain_feat >= int(m_doc.features.size()))) + return; + const auto& cons = live ? m_viewport->sketch_constraints() + : m_doc.features[m_constrain_feat].entity_constraints; if (idx < 0 || idx >= int(cons.size())) return; if (m_constraint_sel == idx) { // second click clears @@ -8074,6 +8098,18 @@ void DesignPanel::highlight_constraint_entities(int idx) // Drop constraint `idx`, re-solve the feature, and refresh viewport + list + DoF. void DesignPanel::delete_constraint(int idx) { + // Live session: the constraint lives in the sketch tool, not in any feature. Removing it + // re-solves and fires on_constraints_changed, which rebuilds these rows — so this branch + // deliberately does NOT call rebuild_constraint_list() itself (it would run twice, and the + // second run would delete the wxButton whose click handler is still on the stack). + if (live_constraint_scope()) { + if (!m_viewport->remove_sketch_constraint(idx)) + return; + m_status->SetForegroundColour(wxNullColour); + set_status(_L("Constraint deleted")); + m_status->Refresh(); + return; + } if (m_constrain_feat < 0 || m_constrain_feat >= int(m_doc.features.size()) || !m_viewport) return; CadFeature& feat = m_doc.features[m_constrain_feat]; diff --git a/src/slic3r/GUI/CAD/DesignPanel.hpp b/src/slic3r/GUI/CAD/DesignPanel.hpp index 3b8e091bae..441e46b081 100644 --- a/src/slic3r/GUI/CAD/DesignPanel.hpp +++ b/src/slic3r/GUI/CAD/DesignPanel.hpp @@ -240,6 +240,11 @@ private: // entity-constraints with per-row select (highlight the referenced entities in // the viewport) and delete (drop the constraint + re-solve). Shown in Constrain // mode only; operates on m_doc.features[m_constrain_feat].entity_constraints. + // True when the constraint UI must address the LIVE sketch session rather than a committed + // feature. Same discriminator apply_constraint uses to choose apply_live_constraint: both + // Constrain modes set m_active too, so is_sketching() alone would claim the live scope while + // the committed manager is open. + bool live_constraint_scope() const; void rebuild_constraint_list(); // refill m_constraint_rows void delete_constraint(int idx); // erase + re-solve + refresh void highlight_constraint_entities(int idx); // push referenced entities to viewport diff --git a/src/slic3r/GUI/CAD/DesignSketchTool.cpp b/src/slic3r/GUI/CAD/DesignSketchTool.cpp index 68c2196912..9ef927cd42 100644 --- a/src/slic3r/GUI/CAD/DesignSketchTool.cpp +++ b/src/slic3r/GUI/CAD/DesignSketchTool.cpp @@ -2359,8 +2359,10 @@ bool DesignSketchTool::try_add_constraints(const std::vector= 0 && g.con < int(m_constraints.size())) { best_d = d; best = g.con; } } if (best < 0) return false; - m_constraints.erase(m_constraints.begin() + best); + return remove_constraint(best); +} + +bool DesignSketchTool::remove_constraint(int idx) +{ + if (idx < 0 || idx >= int(m_constraints.size())) return false; + m_constraints.erase(m_constraints.begin() + idx); // resolve_live(), not a bare solve: it is the path that recomputes the DoF, clears the // per-entity conflict flags and fires on_solve_state. Solving directly would relax the // geometry while leaving the DoF readout and any red over-constrained tint stale — the // readout would still describe the constraint that was just deleted. resolve_live(); m_glyph_hits.clear(); // stale until the next render rebuilds them + if (on_constraints_changed) on_constraints_changed(); return true; } diff --git a/src/slic3r/GUI/CAD/DesignSketchTool.hpp b/src/slic3r/GUI/CAD/DesignSketchTool.hpp index cce003a74d..761a629117 100644 --- a/src/slic3r/GUI/CAD/DesignSketchTool.hpp +++ b/src/slic3r/GUI/CAD/DesignSketchTool.hpp @@ -442,6 +442,10 @@ public: // already down (Mirror: 0 = no axis, 1 = axis down, 2 = ready to apply); picks = size of the // set the gesture accumulates (mirror targets, transform targets, Select's selection). std::function on_step_changed; + // Fired when the live constraint SET changes (one added or removed), never on a re-solve. + // The panel rebuilds its constraint rows from this; binding it to on_solve_state instead + // would rebuild the whole list on every frame of a drag. + std::function on_constraints_changed; // DoF feedback (P3): solver state after each live solve. dof>0 = under-constrained, // dof==0 = fully constrained, ok==false = conflicting/inconsistent constraints. @@ -488,6 +492,9 @@ public: // Click-to-delete on a constraint badge: drop the constraint whose glyph sits under `p` // and re-solve. Returns true if one was removed (the caller then repaints). bool remove_constraint_near(const Vec2d& p); + // Drop constraint `idx` from the live session and re-solve. Same operation the badge click + // performs, addressed by index instead of by position — the panel list needs the index form. + bool remove_constraint(int idx); // The loop report: what is CLOSED, what its internal voids are, and where a chain is still // open. This is the answer to "is my profile buildable", and it is the one question the