Design: the Hole and Thread cards say which face they are holding

snaporca-200 asked which of the two models of "the card's face input" is right,
because clicking empty canvas now clears the selection (snaporca-od0) and made
them visibly disagree: Thicken / Shell / Draft read the LIVE selection and their
label reverts to "(pick a solid face)", while Hole / Thread LATCH the face they
were opened or picked on and keep it. The complaint was that Hole then drills a
face you can no longer see selected.

Taken to the rig, that turns out to be the wrong half of the story. With Hole
open and its face picked, a click on empty canvas leaves the Ø6.0 ghost and its
dimension gizmo drawn on that exact face — the card was never operating in
secret, it was showing its target the strongest way a CAD tool can. Meanwhile
Draft, whose behaviour was held up as the honest one, threw the pick away and
had to be told the face again.

So neither model replaces the other. They are different in kind: Thicken /
Shell / Draft are operations whose operand IS the selected face, and Hole /
Thread are placement tools with their own plane state that a pick merely seeds.
The latch is also the kinder of the two now that empty clicks are a deliberate
gesture — a stray one costs Thicken a pick and costs Hole nothing.

What was genuinely missing is that nothing in those two cards NAMED the latched
face, so after such a click the only words on screen were the viewport's
"Nothing selected" over a ghost about to drill. Both cards now carry an "On
face" row, the way the other three already do:

  Hole    Face 5 | (none — uses Hole plane)
  Thread  Face 1 | Edge 2 | (none — uses Thread plane)

Thread names an edge when the cylinder came from a circular rim rather than a
cylindrical face, which the code already distinguished internally and never
said out loud.

Verified on both rigs, every state driven through the GUI: face pick on open
and on live pick, survival across a click on empty canvas, and the fallback
after choosing XY/XZ/YZ from the plane dropdown. Thread's edge branch was
exercised on a revolved tube's rim, its face branch on the same tube's outer
wall.

Filed while here, surfaced by the new row rather than caused by it —
snaporca-uif9: re-editing a stored Hole/Thread from the feature tree restores
f.plane into the dropdown but never clears m_hole_on_face, so the re-edit
silently reuses the PREVIOUS card's latched face. Now visible by name instead
of invisible.

Fork parity unchanged: DesignPanel.cpp 30, DesignPanel.hpp 0.
This commit is contained in:
Tommaso Bianchi
2026-08-03 15:06:17 +02:00
parent 93395b7888
commit 24f4076bb5
2 changed files with 54 additions and 0 deletions
+45
View File
@@ -985,6 +985,7 @@ DesignPanel::DesignPanel(wxWindow* parent)
m_hole_on_face = false;
m_hole_face_body = -1;
m_hole_has_bounds = false;
set_hole_target_label(-1);
// Use the explicitly-picked face; otherwise default to the top face of the
// selected (or first) body so the hole lands on the surface being viewed, not
// the z=0 datum under the model. The XY/XZ/YZ dropdown still overrides.
@@ -1000,6 +1001,7 @@ DesignPanel::DesignPanel(wxWindow* parent)
m_hole_face_plane = face_plane_inward(face);
m_hole_on_face = true;
m_hole_face_body = hb;
set_hole_target_label(hf); // may be the top-face default, not a pick
// Face (u,v) extents so the hole dims read from the sides (#2 Part B).
m_hole_has_bounds = GeometryEngine::face_plane_bounds(
face, m_hole_face_plane.origin, m_hole_face_plane.x_axis,
@@ -1021,11 +1023,14 @@ DesignPanel::DesignPanel(wxWindow* parent)
// the user never types a radius. The diameter field shows what was derived.
m_thread_on_face = false;
m_thread_face_body = -1;
set_thread_target_label(-1, -1);
GeometryEngine::CylinderFace cf;
bool from_face = false; // which of the two the cylinder actually came from
if (m_sel_solid_body >= 0 && m_sel_solid_body < int(m_doc.bodies.size())) {
const TopoDS_Shape& shape = m_doc.bodies[m_sel_solid_body].shape;
if (m_sel_solid_face >= 0)
cf = GeometryEngine::cylinder_of_face(GeometryEngine::face_by_index(shape, m_sel_solid_face));
from_face = cf.ok;
if (!cf.ok && m_sel_solid_edge >= 0)
cf = GeometryEngine::circle_of_edge(GeometryEngine::edge_by_index(shape, m_sel_solid_edge));
}
@@ -1039,6 +1044,8 @@ DesignPanel::DesignPanel(wxWindow* parent)
m_thread_face_plane = p;
m_thread_on_face = true;
m_thread_face_body = m_sel_solid_body;
set_thread_target_label(from_face ? m_sel_solid_face : -1,
from_face ? -1 : m_sel_solid_edge);
infer_thread_spec(2.0 * cf.radius); // M diameter + pitch + depth from the cylinder
if (m_thread_height && cf.height > 1e-6) m_thread_height->SetValue(cf.height);
if (m_thread_internal) m_thread_internal->SetValue(cf.internal);
@@ -1719,6 +1726,7 @@ DesignPanel::DesignPanel(wxWindow* parent)
m_hole_plane->Bind(wxEVT_COMBOBOX, [this](wxCommandEvent& e) {
m_hole_on_face = false;
m_hole_has_bounds = false;
set_hole_target_label(-1);
update_hole_gizmo();
refresh_preview();
e.Skip();
@@ -1747,6 +1755,10 @@ DesignPanel::DesignPanel(wxWindow* parent)
hform->Add(new wxStaticText(m_cards, wxID_ANY, _L("Through")), 0, wxALIGN_CENTER_VERTICAL);
hform->Add(m_hole_through, 0, wxALIGN_LEFT | wxALIGN_CENTER_VERTICAL);
m_hole_target_label = new wxStaticText(m_cards, wxID_ANY, _L("(none — uses Hole plane)"));
hform->Add(new wxStaticText(m_cards, wxID_ANY, _L("On face")), 0, wxALIGN_CENTER_VERTICAL);
hform->Add(m_hole_target_label, 0, wxALIGN_CENTER_VERTICAL);
m_box_hole = new wxBoxSizer(wxVERTICAL);
m_box_hole->Add(card_header(m_cards, "design_hole", _L("Hole"), m_hdr_hole), 0, wxLEFT | wxRIGHT | wxTOP, 12);
m_box_hole->Add(new wxStaticLine(m_cards), 0, wxEXPAND | wxALL, 8);
@@ -1812,6 +1824,10 @@ DesignPanel::DesignPanel(wxWindow* parent)
tform->Add(new wxStaticText(m_cards, wxID_ANY, _L("Internal")), 0, wxALIGN_CENTER_VERTICAL);
tform->Add(m_thread_internal, 0, wxALIGN_LEFT | wxALIGN_CENTER_VERTICAL);
m_thread_target_label = new wxStaticText(m_cards, wxID_ANY, _L("(none — uses Thread plane)"));
tform->Add(new wxStaticText(m_cards, wxID_ANY, _L("On face")), 0, wxALIGN_CENTER_VERTICAL);
tform->Add(m_thread_target_label, 0, wxALIGN_CENTER_VERTICAL);
m_box_thread = new wxBoxSizer(wxVERTICAL);
m_box_thread->Add(card_header(m_cards, "design_thread", _L("Thread"), m_hdr_thread), 0, wxLEFT | wxRIGHT | wxTOP, 12);
m_box_thread->Add(new wxStaticLine(m_cards), 0, wxEXPAND | wxALL, 8);
@@ -3348,6 +3364,7 @@ DesignPanel::DesignPanel(wxWindow* parent)
m_hole_face_plane = face_plane_inward(fc);
m_hole_on_face = true;
m_hole_face_body = body;
set_hole_target_label(face);
m_hole_has_bounds = GeometryEngine::face_plane_bounds(
fc, m_hole_face_plane.origin, m_hole_face_plane.x_axis,
m_hole_face_plane.y_axis, m_hole_umin, m_hole_umax, m_hole_vmin, m_hole_vmax);
@@ -3363,6 +3380,7 @@ DesignPanel::DesignPanel(wxWindow* parent)
GeometryEngine::CylinderFace cf;
if (face >= 0)
cf = GeometryEngine::cylinder_of_face(GeometryEngine::face_by_index(shape, face));
const bool from_face = cf.ok; // which of the two the cylinder actually came from
if (!cf.ok && edge >= 0)
cf = GeometryEngine::circle_of_edge(GeometryEngine::edge_by_index(shape, edge));
if (cf.ok) {
@@ -3373,6 +3391,7 @@ DesignPanel::DesignPanel(wxWindow* parent)
m_thread_face_plane = p;
m_thread_on_face = true;
m_thread_face_body = m_sel_solid_body;
set_thread_target_label(from_face ? face : -1, from_face ? -1 : edge);
infer_thread_spec(2.0 * cf.radius); // M diameter + pitch + depth from the cylinder
if (m_thread_height && cf.height > 1e-6) m_thread_height->SetValue(cf.height);
if (m_thread_internal) m_thread_internal->SetValue(cf.internal);
@@ -4409,6 +4428,32 @@ void DesignPanel::on_add_dressup()
refresh_tree();
}
// Hole and Thread LATCH the geometry they were opened or picked on; Thicken / Shell / Draft read
// the live selection instead. Both models are right for what they are — a placement tool with its
// own plane state, versus an operation whose operand IS the selected face — and the latch is the
// kinder of the two now that a click on empty canvas clears the selection (snaporca-od0): a stray
// click costs a Thicken pick, and costs a Hole nothing. What was missing is that nothing in the
// Hole/Thread card NAMED the latched face, so after such a click the only words on screen were
// the viewport's "Nothing selected" — over a ghost still drawn on the face Confirm would drill.
// That reads as a contradiction and was filed as one (snaporca-200). The card now says what it
// holds, the way the other three cards already do. Pass -1 for "none, using the plane dropdown".
void DesignPanel::set_hole_target_label(int face)
{
if (m_hole_target_label)
m_hole_target_label->SetLabel(face >= 0 ? wxString::Format(_L("Face %d"), face)
: _L("(none — uses Hole plane)"));
}
// Thread also latches onto a circular EDGE (a cylinder's rim), so it has two kinds to name.
void DesignPanel::set_thread_target_label(int face, int edge)
{
if (!m_thread_target_label) return;
m_thread_target_label->SetLabel(
face >= 0 ? wxString::Format(_L("Face %d"), face)
: edge >= 0 ? wxString::Format(_L("Edge %d"), edge)
: _L("(none — uses Thread plane)"));
}
SketchPlane DesignPanel::hole_plane() const
{
if (m_hole_on_face) return m_hole_face_plane;
+9
View File
@@ -235,6 +235,10 @@ private:
SketchPlane hole_plane() const;
// The plane the Thread tool builds on: a picked cylindrical face (axis) or the dropdown.
SketchPlane thread_plane() const;
// Name the geometry the card has LATCHED, so it never has to be inferred from the viewport.
// Pass -1 for "none, falling back to the plane dropdown". See snaporca-200.
void set_hole_target_label(int face);
void set_thread_target_label(int face, int edge);
CadFeature build_candidate(Tool t) const;
int resolve_extrude_sketch() const;
// Plane pickers: fill a choice with XY/XZ/YZ + the document's datum planes, and
@@ -674,6 +678,10 @@ private:
// dims read as distance from the face sides (umin/vmin edges) rather than from the centre.
bool m_hole_has_bounds{false};
double m_hole_umin{0}, m_hole_umax{0}, m_hole_vmin{0}, m_hole_vmax{0};
// Says which face the latch above is holding. Thicken/Shell/Draft show theirs because their
// face IS the live selection; this one has to be shown precisely BECAUSE it is not, and the
// status line goes on saying "Nothing selected" while the ghost keeps drilling. snaporca-200.
wxStaticText* m_hole_target_label{nullptr};
ComboBox* m_thread_plane{nullptr};
ComboBox* m_thread_std{nullptr}; // standard designation (M6, 1/4-20 UNC, ...)
@@ -689,6 +697,7 @@ private:
bool m_thread_on_face{false};
SketchPlane m_thread_face_plane;
int m_thread_face_body{-1};
wxStaticText* m_thread_target_label{nullptr}; // the latched face/edge — see m_hole_target_label
wxSpinCtrlDouble* m_shell_thickness{nullptr};
wxStaticText* m_shell_face_label{nullptr}; // shows the picked face to remove