From 9d47280a192d2095ae609fae294e6407affa1320 Mon Sep 17 00:00:00 2001 From: Tommaso Bianchi Date: Sun, 2 Aug 2026 09:02:52 +0200 Subject: [PATCH] Design: a card opened from a face must use, and show, that face MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit snaporca-y7q. Thicken's opener cleared m_sel_solid_face outright. That was right when the only door was a toolbar button — a button carries no selection, so pressing Thicken had to clear and ask you to point at something. The offer inverted it: the verb is now invoked ON a face, and the same line threw away the only thing the user had said. The card opened reading "(pick a solid face)" over an immediate "thicken: face not found" — you pointed at the face and were told none could be found. Keep the pick when the body combo landed on the body it came from (the index is per-body, and selected_body_default() returns exactly that body when it is valid). Two neighbours had the mirror-image flaw, both invisible for the same reason — the value was right and the ghost updated, so only the label lied: - Thicken had NO live label update at all. Nothing outside the opener ever wrote m_thicken_face_label, so while the card was open you could pick face after face and it still read "(pick a solid face)". - Shell and Draft wrote theirs ONLY from the pick handler, which runs while a card is already open — so opened from a selection they showed the previous pick, or the placeholder over a face they were about to use. So the label is now written once in open_tool(), which every door goes through. The edit-feature path already restores m_sel_solid_face from the stored feature BEFORE calling open_tool, so it agrees rather than fights. Verified on the snaporca rig: face 4 of an extruded plate, offer > Add material > Thicken now opens "Face: Face 4" with "Preview — 24 triangles" and confirms to a real Body 2. Draft opened from a face shows "Face 3" and previews the taper. This fork is code-identical here bar the two permitted DropDown divergences; it still owes a build of its own (snaporca-5pl). Project keeps its clear: there "(all edges)" is a legitimate default mode rather than a failure, so changing it would alter behaviour with no reported problem behind it. --- src/slic3r/GUI/DesignPanel.cpp | 40 +++++++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/src/slic3r/GUI/DesignPanel.cpp b/src/slic3r/GUI/DesignPanel.cpp index c57cd365fb..b5ea3d3483 100644 --- a/src/slic3r/GUI/DesignPanel.cpp +++ b/src/slic3r/GUI/DesignPanel.cpp @@ -649,9 +649,17 @@ DesignPanel::DesignPanel(wxWindow* parent) m_thicken_body->SetSelection(std::min(selected_body_default(), int(m_thicken_body->GetCount()) - 1)); } - m_sel_solid_face = -1; - m_thicken_face_label->SetLabel(_L("(pick a solid face)")); - open_tool(Tool::Thicken); + // KEEP a face the user has already picked. Clearing it unconditionally was right + // when the only door was a toolbar button, which carries no selection: you pressed + // Thicken and were then asked to point at something. Reached from the offer the + // verb is invoked ON a face, so discarding it opened the card reading "(pick a + // solid face)" over an immediate "thicken: face not found" — the user pointed at + // the face and the card said it could not find one. snaporca-y7q. + // The index is per-body, so it only survives if the body combo landed on the body + // it came from; selected_body_default() above returns exactly that when valid. + if (m_thicken_body->GetSelection() != m_sel_solid_body) + m_sel_solid_face = -1; + open_tool(Tool::Thicken); // syncs m_thicken_face_label from m_sel_solid_face }, 0}, {"design_rib", _L("Rib"), _L("Grow a thin wall from an open sketch line, fused to a body"), [this] { @@ -3283,6 +3291,16 @@ DesignPanel::DesignPanel(wxWindow* parent) : _L("(all faces — closed hollow)")); refresh_preview(); // rebuilds the shell ghost + re-anchors the thickness gizmo } + // Thicken card open: the face pick IS the feature's input, and until now nothing here + // wrote its label — the value reached m_sel_solid_face and Confirm worked, but the card + // read "(pick a solid face)" however many faces you had picked. Invisible because the + // ghost did update, so the card looked wrong while behaving right. + if (m_active == Tool::Thicken) { + m_thicken_face_label->SetLabel(m_sel_solid_face >= 0 + ? wxString::Format(_L("Face %d"), m_sel_solid_face) + : _L("(pick a solid face)")); + refresh_preview(); + } // Draft card open: a face pick chooses the face to taper; update label + ghost live. if (m_active == Tool::Draft) { m_draft_face_label->SetLabel(m_sel_solid_face >= 0 @@ -9144,6 +9162,22 @@ void DesignPanel::open_tool(Tool t) case Tool::None: break; } + // A card that consumes a face must say WHICH face the moment it appears. These labels used to + // be written only by the pick handler, which runs while a card is already open — so a card + // opened FROM a selection (the offer's whole premise) showed the previous pick, or the "(pick + // one)" placeholder over a face it was in fact about to use. Same law as the Construction + // toggle: a control that carries state has to show it. One writer, on open, for all three. + { + const bool have = m_sel_solid_face >= 0; + const wxString face_name = have ? wxString::Format(_L("Face %d"), m_sel_solid_face) : wxString(); + if (t == Tool::Thicken && m_thicken_face_label) + m_thicken_face_label->SetLabel(have ? face_name : _L("(pick a solid face)")); + if (t == Tool::Shell && m_shell_face_label) + m_shell_face_label->SetLabel(have ? face_name : _L("(all faces — closed hollow)")); + if (t == Tool::Draft && m_draft_face_label) + m_draft_face_label->SetLabel(have ? face_name : _L("(pick a side face)")); + } + if (editing) { populate_expr_fields(t); // field-name combo for this feature type // Display current expression bindings on the edited feature