From 1addba6ea0de5c435cd8d80a2624cbb94094d58d Mon Sep 17 00:00:00 2001 From: Tommaso Bianchi Date: Fri, 31 Jul 2026 21:35:41 +0200 Subject: [PATCH] Design: editing a quote UPDATES its dimension instead of appending a rival to it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Type a new length into a sketch quote and the number changed while the geometry sat still, with the solver dropping to "Conflicting constraints" — a broken constraint state the user never asked for, arrived at by doing the one thing the status line invited. An experiment separated the two candidate causes. Same gesture, one variable: a line committed WITH a driving length refused the edit and conflicted; a line committed with none (Esc keeps it as drawn) accepted it and visibly shrank. So the value was never the problem and the solver was not wrong — it was being handed two contradicting constraints and correctly declining to choose. The quote-edit path appended unconditionally: a.con = int(m_constraints.size()); m_constraints.push_back(constraint_for(a)); Accepting the length at draw time creates Distance(P0,P1) = 64.9. Clicking the quote later creates a SECOND Distance on the same two points asking for 30. Over -constrained by construction. A line with no dimension yet only ever gets one constraint, which is exactly why this looked intermittent rather than total. upsert_constraint() finds an existing constraint with the same type and operands, overwrites its value and returns its index; it appends only when there is none. Operand order is ignored — a Distance from A to B is the same constraint as B to A, and so is an Angle. Returning the index matters as much as the update: it keeps the annotation's `con` pointing at the constraint that is actually live, so the NEXT edit is an update too rather than reverting to appending after one good round. upsert_dimension() applies the same rule to the visible quote, which had been stacking labels reading different values on the same pixel, and keeps the existing label position so a placed quote does not teleport. set_dimension_value already did the right thing through a.con. The machinery existed; these two call sites never consulted it. Verified on :11 on the exact failing case — draw a line, Return to lock the length, Confirm, double-click to re-open, click the line, type 30 into the quote: the line shrinks, the quote reads 30.0 mm, one label not two, and the solver stays at "3 degrees of freedom" with no conflict. NOT included: record_dimension_constraint() has the same unconditional push_back in all six of its branches. It belongs to the legacy Constrain mode with different selection semantics and I could not exercise it, so it is flagged rather than changed blind. Refs snaporca-e1p. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01LyRwbuq6fjn3VV9U9UvhBM --- src/slic3r/GUI/DesignSketchTool.cpp | 57 +++++++++++++++++++++++++---- src/slic3r/GUI/DesignSketchTool.hpp | 4 ++ 2 files changed, 54 insertions(+), 7 deletions(-) diff --git a/src/slic3r/GUI/DesignSketchTool.cpp b/src/slic3r/GUI/DesignSketchTool.cpp index 6b0a87dc5b..9ca55025d8 100644 --- a/src/slic3r/GUI/DesignSketchTool.cpp +++ b/src/slic3r/GUI/DesignSketchTool.cpp @@ -912,6 +912,51 @@ double DesignSketchTool::measure_dim(const DimAnnot& a) const } } +// One driving constraint per (kind, operands) — update it in place rather than appending a +// second one every time its value is edited. +// +// Re-typing a quote used to push a duplicate alongside the original: draw a line and accept its +// length, and you get Distance(P0,P1)=64.9; click the quote later and type 30, and you get a +// SECOND Distance on the same two points asking for 30. That is over-constrained by +// construction, so the solver reported "Conflicting constraints" and refused to move anything — +// the number on screen changed and the geometry did not, which reads as the edit being ignored. +// A line carrying no dimension yet was unaffected, which is what made it look intermittent. +// +// Operand order is ignored: a Distance from A to B is the same constraint as B to A, and so is +// an Angle. Returns the index, so callers can keep the annotation's `con` link pointing at the +// constraint that is actually live — which is what makes the next edit an update too. +int DesignSketchTool::upsert_constraint(const SketchEntityConstraintDef& c) +{ + auto same_operands = [&](const SketchEntityConstraintDef& o) { + if (o.ea == c.ea && o.ra == c.ra && o.eb == c.eb && o.rb == c.rb) return true; + return o.ea == c.eb && o.ra == c.rb && o.eb == c.ea && o.rb == c.ra; + }; + for (int i = 0; i < int(m_constraints.size()); ++i) + if (m_constraints[i].type == c.type && same_operands(m_constraints[i])) { + m_constraints[i] = c; + return i; + } + m_constraints.push_back(c); + return int(m_constraints.size()) - 1; +} + +// The same rule for the visible annotation: one quote per (kind, operands), so repeated edits +// do not stack labels on top of each other reading different values. +int DesignSketchTool::upsert_dimension(const DimAnnot& a) +{ + for (int i = 0; i < int(m_dimensions.size()); ++i) { + const DimAnnot& o = m_dimensions[i]; + if (o.kind == a.kind && ((o.ea == a.ea && o.eb == a.eb) || (o.ea == a.eb && o.eb == a.ea))) { + const Vec2d keep = m_dimensions[i].label_pos; // don't teleport a placed label + m_dimensions[i] = a; + m_dimensions[i].label_pos = keep; + return i; + } + } + m_dimensions.push_back(a); + return int(m_dimensions.size()) - 1; +} + SketchEntityConstraintDef DesignSketchTool::constraint_for(const DimAnnot& a) const { SketchEntityConstraintDef c; @@ -942,11 +987,10 @@ SketchEntityConstraintDef DesignSketchTool::constraint_for(const DimAnnot& a) co int DesignSketchTool::place_dimension(DimAnnot a) { a.value = measure_dim(a); - a.con = int(m_constraints.size()); - m_constraints.push_back(constraint_for(a)); - m_dimensions.push_back(a); + a.con = upsert_constraint(constraint_for(a)); + const int di = upsert_dimension(a); resolve_live(); - open_value_editor(int(m_dimensions.size()) - 1); + open_value_editor(di); return m_pending_dim; } @@ -1158,9 +1202,8 @@ void DesignSketchTool::open_primary_autoedit() m_autoedit_dims.push_back({ a.label_pos, a.value, [this, a](double v) mutable { a.value = v; - a.con = int(m_constraints.size()); - m_constraints.push_back(constraint_for(a)); - m_dimensions.push_back(a); + a.con = upsert_constraint(constraint_for(a)); + upsert_dimension(a); resolve_live(); }, { a.ea, a.eb }, dimtype_title(a.kind) }); } diff --git a/src/slic3r/GUI/DesignSketchTool.hpp b/src/slic3r/GUI/DesignSketchTool.hpp index 55b94a5913..d72c149974 100644 --- a/src/slic3r/GUI/DesignSketchTool.hpp +++ b/src/slic3r/GUI/DesignSketchTool.hpp @@ -620,6 +620,10 @@ private: double measure_dim(const DimAnnot& a) const; // value from geometry std::string dimtype_title(DimType k) const; SketchEntityConstraintDef constraint_for(const DimAnnot& a) const; // driving def + // One driving constraint (and one visible quote) per kind+operands: re-typing a value must + // UPDATE it, not append a rival asking for something else. Both return the index. + int upsert_constraint(const SketchEntityConstraintDef& c); + int upsert_dimension(const DimAnnot& a); int place_dimension(DimAnnot a); // create+drive+notify std::string dim_text(const DimAnnot& a) const; // rendered label string void render_dimensions(double unit_per_px); // quote lines + labels