Design: a verb's address is data, so the toolbar widget can stop existing

snaporca-7ih's remaining half. Both flyout factories registered their verbs INSIDE the
widget-building loop, so the ~40 retired tool buttons had to be constructed and then
Hide()n: skipping construction would have deleted 42 offer verbs (26 fly:<family>#<row>
+ 16 Shift+keys) while their rows still rendered and did nothing when picked.

Register first, build second. The addresses are pure data; the widget is one door onto
them, not their owner. A family absent from kBarKeep now returns before any wxWindow is
made. The keep-list stays a one-line data decision, not a structural one.

And close the class of bug for good: the constructor now verifies, once, that every verb
the atlas marks wired resolves to a real registration, logging each break and asserting in
debug. Rows that render and do nothing have shipped three times (edit_feature and sk_move
with action:null, then this) and are invisible from either side alone.

Verified on the snaporca rig by walking the offer, not by reading the code — all four
at-risk address kinds run with no widget behind them: fly:design_rect#2 drew an OBLIQUE
rectangle (the third variant, not the family's first), key:S+E opened Extrude with its
10 mm gizmo, fly:material#4 opened Thicken. Hover hints, icons and nesting intact. This
fork is code-identical here bar the two permitted DropDown divergences; it still owes a
build of its own (snaporca-5pl).

Two hints were wrong and are fixed: Cut said "Split the body with a plane", colliding with
the Split verb one row away and pointing at a card for a value the canvas already offers as
a draggable arrow; Split never said its plane comes from a picked face.

Also, because it blocked the verification and will block the next one: gui-session.sh
killed by full path while its own app_pid() matched by basename, so a differently-pathed
instance survived, held the single-instance lock, and got reported as a healthy session —
a Jul-30 binary nearly passed as this build. It now kills by basename and prints which
binary is actually on screen. Traps 6 and 7 documented.
This commit is contained in:
Tommaso Bianchi
2026-08-02 08:37:44 +02:00
parent 96816f725c
commit cfc2555c3a
5 changed files with 110 additions and 24 deletions
+2 -2
View File
@@ -99,8 +99,8 @@ static const OfferVerb kOfferVerbs[] = {
{"hole", "Hole", 2, "Shift+H", "key:S+H", "Pick a face or a plane to drill into", 0x00000402u, 1, 0, false, false, nullptr, "design_hole", "Drill a hole, centred on a picked face or placed on a plane"},
{"thread", "Thread", 2, "Shift+T", "key:S+T", "Pick a cylindrical surface (bore / outer) or a circular edge for a thread", 0x00000024u, 1, 0, false, false, nullptr, "design_thread", "Thread a cylindrical surface (inner bore / outer) or a circular edge"},
{"shell", "Shell", 2, "Shift+K", "key:S+K", "Shell needs a solid body", 0x00000082u, 1, 0, false, false, nullptr, "design_shell", "Hollow the body to a wall thickness, opening a picked face"},
{"cut", "Cut", 2, "Shift+X", "key:S+X", "Create a solid body to cut first", 0x00000480u, 1, 0, false, false, nullptr, "design_cut", "Split the body with a plane; set the offset in the card"},
{"split", "Split", 2, nullptr, nullptr, "Split needs a solid body", 0x00000080u, 1, 0, false, false, nullptr, nullptr, "Split the body into separate solids"},
{"cut", "Cut", 2, "Shift+X", "key:S+X", "Create a solid body to cut first", 0x00000480u, 1, 0, false, false, nullptr, "design_cut", "Trim the body with a plane — drag the offset arrow; keep one half or both"},
{"split", "Split", 2, nullptr, nullptr, "Split needs a solid body", 0x00000080u, 1, 0, false, false, nullptr, nullptr, "Split the body along a picked face into two solids"},
{"fillet", "Fillet", 3, "Shift+F", "btn:dress#0", "Pick an edge to round", 0x000000b2u, 1, 0, false, false, nullptr, "design_filletedge", "Pick an edge, then drag the radius arrow or type it"},
{"chamfer", "Chamfer", 3, nullptr, "btn:dress#1", "Pick an edge to bevel", 0x000000b2u, 1, 0, false, false, nullptr, "design_chamfer", "Pick an edge, then drag the distance arrow or type it"},
{"draft", "Draft", 3, "Shift+D", "key:S+D", "Pick a face to taper", 0x0000000au, 1, 0, false, false, nullptr, "design_draft", "Tilt a picked face by a draft angle"},
+65 -19
View File
@@ -8,8 +8,10 @@
#include <boost/algorithm/string/case_conv.hpp>
#include <boost/filesystem/path.hpp>
#include <boost/log/trivial.hpp> // the offer/atlas join check reports on the log
#include <Standard_Failure.hxx>
#include <cassert>
#include <map>
#include <set>
#include <wx/sizer.h>
@@ -477,6 +479,21 @@ DesignPanel::DesignPanel(wxWindow* parent)
FeatFlyout() : drop(items) {}
};
auto feat_dropdown = [&](const char* id, const char* def_icon, const wxString& grp, std::vector<FeatVar> vars) {
// REGISTER FIRST, BUILD SECOND. A verb's two addresses — "fly:<family>#<row>" for the
// offer and its Shift+key — are data. The widget is one door onto them, not their
// owner. Doing this before any wxWindow exists is what lets the build below be skipped
// outright for a family the bar no longer carries. It used to sit INSIDE the build
// loop, so a retired family still had to be constructed and then Hide()n: skipping it
// would have deleted 42 verbs from the offer while their rows still rendered and did
// nothing when picked. snaporca-7ih.
// Keyed on "fly:<family>#<row>" so the generated table can name a variant without the
// item struct growing a field at 26 call sites.
for (size_t i = 0; i < vars.size(); ++i) {
if (vars[i].key) m_keys_feature[vars[i].key] = vars[i].action; // key runs the same action
m_verb_actions["fly:" + std::string(id) + "#" + std::to_string(i)] = vars[i].action;
}
if (kBarKeep.count(id) == 0)
return; // reached from the offer alone — no button, no chevron, no popup
auto* b = icon_btn(def_icon, grp);
b->SetFont(Label::Body_14); // measure popup labels in the popup's font (no truncation)
auto fo = std::make_shared<FeatFlyout>();
@@ -488,12 +505,6 @@ DesignPanel::DesignPanel(wxWindow* parent)
fo->items.push_back(it);
fo->actions.push_back(std::move(v.action));
fo->icon_names.emplace_back(v.icon);
if (v.key) m_keys_feature[v.key] = fo->actions.back(); // key runs the same action
// …and the offer reaches the same action by its ratified address. Keyed on
// "fly:<family>#<row>" so the generated table can name it without the item
// struct growing a field at 26 call sites.
m_verb_actions["fly:" + std::string(id) + "#" +
std::to_string(fo->actions.size() - 1)] = fo->actions.back();
}
fo->btn = b;
fo->drop.Create(b);
@@ -524,7 +535,6 @@ DesignPanel::DesignPanel(wxWindow* parent)
chev->SetForegroundColour(dp_sec_text());
chev->SetFont(Label::Body_9);
fadd(id, chev);
return b;
};
auto* b_sketch = icon_btn("design_sketch", _L("Sketch"));
@@ -1088,6 +1098,19 @@ DesignPanel::DesignPanel(wxWindow* parent)
ToolFlyout() : drop(items) {}
};
auto dropdown = [&](const char* def_icon, const wxString& grp, std::vector<SkVar> vars) {
// Register first, build second — the same law as feat_dropdown, for the same reason.
// The offer reaches each tool by its ratified address; without these the offer could
// name a family but only ever arm its FIRST tool: picking "Rectangle" ran key:R and
// gave you a corner rectangle, with oblique and rounded unreachable. Keyed on the icon
// id (already unique per family) so no call site grows an argument. snaporca-6vs.
for (size_t i = 0; i < vars.size(); ++i) {
const DesignSketchTool::Mode mode = vars[i].mode;
const wxString hint = vars[i].hint;
m_verb_actions["fly:" + std::string(def_icon) + "#" + std::to_string(i)] =
[mode, hint, select_tool] { select_tool(mode, hint); };
}
if (kBarKeep.count(def_icon) == 0)
return; // the drawing tools live in the offer; nothing of this family is built
auto* b = icon_btn(def_icon, grp);
// messureSize() measures labels with the PARENT's font (this button) but the
// popup draws them in Body_14 — so an under-sized button font truncates rows.
@@ -1104,17 +1127,6 @@ DesignPanel::DesignPanel(wxWindow* parent)
fo->modes.push_back(v.mode);
fo->hints.push_back(v.hint);
fo->icon_names.emplace_back(v.icon);
// …and the offer reaches the same tool by its ratified address, exactly as
// feat_dropdown does for the model verbs. Without this the offer could name a
// family but only ever arm its FIRST tool: picking "Rectangle" ran key:R and
// gave you a corner rectangle, with oblique and rounded unreachable.
// Keyed on the icon id (already unique per family) so no call site grows an
// argument. snaporca-6vs.
const DesignSketchTool::Mode mode = v.mode;
const wxString hint = v.hint;
m_verb_actions["fly:" + std::string(def_icon) + "#" +
std::to_string(fo->modes.size() - 1)] =
[mode, hint, select_tool] { select_tool(mode, hint); };
}
fo->btn = b;
fo->drop.Create(b);
@@ -1148,7 +1160,6 @@ DesignPanel::DesignPanel(wxWindow* parent)
chev->SetForegroundColour(dp_sec_text());
chev->SetFont(Label::Body_9);
sadd(chev); // follows its button off the bar
return b;
};
skbtn("design_select", DesignSketchTool::Mode::Select, _L("Select"),
_L("Click to select; Shift to add; double-click for a whole loop"));
@@ -3660,6 +3671,41 @@ DesignPanel::DesignPanel(wxWindow* parent)
outer->Add(body, 1, wxEXPAND);
SetSizer(outer);
// The atlas says a verb is wired; the registrations above say what it runs. Nothing checks
// that the two agree, and a broken join is INVISIBLE — the row renders, is enabled, and does
// nothing when picked. That defect has shipped three times (edit_feature and sk_move carrying
// action:null, then a whole flyout family the moment its widget stopped being built), and it
// cannot be seen by reading either side alone. Verify the join once, here, where every
// registration is complete. Cheap: 86 map lookups, once per panel.
{
std::vector<std::string> dead;
for (int i = 0; i < kOfferVerbCount; ++i) {
const OfferVerb& v = kOfferVerbs[i];
if (v.action == nullptr || *v.action == '\0')
continue; // kernel support, no GUI path — the row shows disabled
const std::string a(v.action);
bool ok = false;
if (a.rfind("key:", 0) == 0) { // resolved exactly as run_offer_action() resolves it
const std::string k = a.substr(4);
if (k.size() >= 3 && k[0] == 'S' && k[1] == '+') {
auto it = m_keys_feature.find(int(k[2]) | SC_SHIFT);
ok = it != m_keys_feature.end() && bool(it->second);
} else if (!k.empty()) {
auto it = m_keys_sketch.find(int(k[0]));
ok = it != m_keys_sketch.end() && bool(it->second);
}
} else {
auto it = m_verb_actions.find(a);
ok = it != m_verb_actions.end() && bool(it->second);
}
if (!ok)
dead.emplace_back(std::string(v.id) + " -> " + a);
}
for (const std::string& d : dead)
BOOST_LOG_TRIVIAL(error) << "Design offer: wired verb has no action: " << d;
assert(dead.empty()); // debug builds stop here; release ships the log line
}
set_ui_mode(UiMode::Feature);
}