Design: right-click a body row opens the offer, and taking a body always means the same thing

The third door onto the offer, after the viewport right-click and the Menu key. A body ROW is
an unambiguous body, so the offer reports BodySolid and the body verbs act on the row you can
see highlighted — the confirmation a face pick cannot give, since pointing at a face lights the
face and never the body the verb will change. The status line has been promising exactly this
("Body N selected — right-click for what applies to it") since before any handler existed on
that list; the product was advertising a gesture that did nothing.

WHAT THE RIG CAUGHT THAT THE BUILD DID NOT. The first version hung the state normalisation off
wxEVT_TREE_SEL_CHANGED. But SelectItem() on a row that is ALREADY selected fires no selection
event, so a stale vertex from an earlier viewport pick survived — and offer_selection_kind()
tests vertex FIRST, so right-clicking the body row served the VERTEX offer while the row sat
highlighted: Fillet/chamfer/draft greyed, Mirror standing where Repeat belongs, "vertex
selected" still in the status line and the cyan marker still on screen. The happy path (fresh
row, nothing else picked) looked perfect, which is why only the deliberate stale-state sequence
exposed it. Reading the code would not have shown it — SelectItem looks like it selects.

So the normalisation is no longer a selection handler. apply_body_row() is called
UNCONDITIONALLY by both doors, because taking a body from the list means the same state change
however it was asked for. It also clears m_sel_solid_vertex, which the original handler never
did — latent while nothing opened the offer from that list, and immediately fatal once
something did.

Verified on both rigs with the failing sequence itself: pick a vertex, then right-click the
already-selected row. Fillet/chamfer/draft enabled, Repeat back in place, status reads "Body 1
selected", vertex marker gone.

Does NOT touch the feature tree. That needs new selection kinds (offer_selection_kind has no
notion of "a feature is selected") plus verbs the atlas does not contain — Suppress, Rename,
Reorder, Roll back — and is filed separately.
This commit is contained in:
Tommaso Bianchi
2026-08-02 10:31:21 +02:00
parent 6c59898ac0
commit 7e5994b8cb
+34 -7
View File
@@ -2970,21 +2970,48 @@ DesignPanel::DesignPanel(wxWindow* parent)
// tree rebuild — until then an empty box would sit under the header.
m_parts->Hide();
m_parts_label->Hide();
m_parts->Bind(wxEVT_TREE_SEL_CHANGED, [this](wxTreeEvent&) {
if (!m_viewport) return;
const int b = tree_body_selection();
if (b < 0) return;
// Taking a body from the list means the SAME state change however it was asked for, so the
// normalisation lives here and not inside a selection handler. That distinction is not
// pedantry: SelectItem() on a row that is ALREADY selected fires no SEL_CHANGED at all, so a
// version of this that only ran on selection left a stale vertex/edge from an earlier
// viewport pick in place — and offer_selection_kind() tests vertex FIRST, so right-clicking
// the body row served the VERTEX offer (Fillet greyed, Mirror in place of Repeat) while the
// row sat highlighted. Measured on the rig 2026-08-02; it is invisible from the code alone.
auto apply_body_row = [this](int b) {
if (!m_viewport || b < 0) return;
// One selection at a time: a body row and a feature row mean different things to the
// op bar, so clear the feature tree's highlight when a body takes over.
if (m_tree) m_tree->UnselectAll();
m_viewport->set_body_highlight(false); // the per-body overlay does the tint
m_viewport->select_body(b);
m_sel_solid_body = b;
m_sel_solid_face = m_sel_solid_edge = -1;
m_viewport->select_body(b); // also drops the vertex/edge marker
m_sel_solid_body = b;
m_sel_solid_face = m_sel_solid_edge = -1;
m_sel_solid_vertex = false;
m_pick_face = m_pick_face_body = -1; // chosen from the list, no face was pointed at
m_status->SetForegroundColour(wxNullColour);
set_status(wxString::Format(_L("Body %d selected — right-click for what applies to it"), b + 1));
m_status->Refresh();
};
m_parts->Bind(wxEVT_TREE_SEL_CHANGED, [this, apply_body_row](wxTreeEvent&) {
apply_body_row(tree_body_selection());
});
// The third door onto the offer, after the viewport right-click and the Menu key. A body ROW
// is an unambiguous body, so the offer reports BodySolid and the body verbs act on the row you
// can see highlighted. That is the confirmation a face pick cannot give: pointing at a face
// lights the face, never the body the verb will actually change. The status line above has
// been promising this right-click since before it existed.
m_parts->Bind(wxEVT_TREE_ITEM_MENU, [this, apply_body_row](wxTreeEvent& e) {
if (e.GetItem().IsOk())
m_parts->SelectItem(e.GetItem()); // the row under the cursor, never a stale one
apply_body_row(tree_body_selection()); // unconditional — see above, SelectItem on an
// already-selected row raises no event
// GetPoint() is tree-client; it is (-1,-1) when the KEYBOARD menu key raised this, so fall
// back to the shared anchor rather than popping the menu at a garbage coordinate.
const wxPoint p = e.GetPoint();
const wxPoint screen = (p.x >= 0 && p.y >= 0) ? m_parts->ClientToScreen(p) : offer_anchor();
// Let the modal menu take the loop after this handler returns — same CallAfter as the
// sketch path, which learned it the hard way.
CallAfter([this, screen] { show_offer_menu(screen); });
});
// --- Variables (document-scope named expressions) ---