Port the sketch usability fixes: Enter/Esc, rename, stale picks

Carries snaporca 95e59289f9, faec177d42 and 20df726ecb. Parity re-verified: 17 files identical,
8 diverging by their expected counts — DesignCanvas.cpp back to 16 and DesignPanel.cpp back to
32, which is the proof each hunk landed on the right side of the FeatFlyout and TAB_ID_PREPARE
divergences rather than on top of them.

All three answer exussum12's review on OrcaSlicer PR #15238.

ENTER/ESC IN THE VALUE FIELD. The field is a borderless always-on-top frame, and whether it may
hold keyboard focus is the platform's decision — a borderless NSWindow can never be key, and
mutter refuses a re-mapped window. When focus is denied the keys reach the panel instead and the
queued-dimension chain (a line queues Length then Angle) cannot be walked. The CHAR_HOOK now
forwards Enter/Numpad-Enter/Tab/Esc to the field when it is open and unfocused, and stays out of
the way when it is focused.

ESC FROM ANYWHERE. Separately and more simply: `dismissable` is false throughout sketch mode
because m_active is the FEATURE tool, so Esc fell through to whatever widget had focus. Click
any toolbar button or the Construction checkbox first and Esc did nothing at all — the likelier
reading of "Esc hardly ever works", and platform-independent. DesignCanvas exposes
request_sketch_exit() and the hook calls it whenever a sketch is live, after the inline-field
forwarding so an open field still takes Esc first.

RENAME. wxTR_EDIT_LABELS plus the two label-edit events write through to CadFeature::name and the
recipe, with a Rename verb in the offer and F2. The rebuild is deferred with CallAfter because
refresh_tree() destroys the very wxTreeItemId wx is holding during END_LABEL_EDIT — inline, it
killed the process.

STALE PICKS. set_tool now drops the Dimension tool's first pick, the Constrain picks and
m_point_sel, and delete_selected clears the pending dimension reference that could otherwise
dereference a renumbered entity.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tommaso Bianchi
2026-08-22 12:11:53 +02:00
co-authored by Claude Opus 5
parent fbf858ba47
commit 510e63dff2
6 changed files with 133 additions and 4 deletions
+84 -2
View File
@@ -1027,6 +1027,21 @@ DesignPanel::DesignPanel(wxWindow* parent)
fadd("color", b_color);
m_verb_actions["btn:colour"] = [this] { on_set_body_color(); };
m_verb_actions["btn:delete"] = [this] { on_delete_feature(); };
// Rename exists as a slow double-click on the row too, but the offer is this tab's only
// tool vocabulary — a rename that only a double-click reveals is not discoverable, and
// the row IS the object, so it belongs in the menu (and on F2) as well as on the row.
auto rename_feature = [this] {
const int sel = tree_selection();
if (sel != wxNOT_FOUND && sel < int(m_tree_items.size())) {
m_tree->EditLabel(m_tree_items[sel]); // opens the in-place editor on that row
} else {
m_status->SetForegroundColour(wxNullColour); // "nothing selected" is not an error
set_status(_L("Select a feature first — click a sketch or feature row, then rename it"));
m_status->Refresh();
}
};
m_verb_actions["btn:rename"] = rename_feature;
m_keys_feature[WXK_F2] = rename_feature; // a function key, so no letter space spent
// The sketch's own Delete. It used to share "btn:delete" with the feature tree, so
// choosing Delete on a selected LINE ran on_delete_feature() and removed a tree row (or
// nothing) while the line stayed — the reported "I click a line and cannot remove it".
@@ -3003,7 +3018,7 @@ DesignPanel::DesignPanel(wxWindow* parent)
FromDIP(SidebarProps::TitlebarMargin()));
m_tree = new wxTreeCtrl(m_tree_box, wxID_ANY, wxDefaultPosition, wxSize(-1, 64),
wxTR_HIDE_ROOT | wxTR_SINGLE | wxTR_NO_LINES |
wxTR_FULL_ROW_HIGHLIGHT | wxBORDER_SIMPLE);
wxTR_FULL_ROW_HIGHLIGHT | wxBORDER_SIMPLE | wxTR_EDIT_LABELS);
if (!dp_dark()) m_tree->SetBackgroundColour(dp_panel_bg());
// Per-feature-type icons (indices match tree_icon_for): sketch/extrude/dressup/hole/thread.
m_tree_images = new wxImageList(16, 16);
@@ -3045,6 +3060,41 @@ DesignPanel::DesignPanel(wxWindow* parent)
// Edit button in the section header.
m_tree->Bind(wxEVT_TREE_ITEM_ACTIVATED, [this](wxTreeEvent&) { on_edit_feature(); });
// In-place rename of a feature row (slow double-click, the offer's Rename verb, or F2).
// The name is what makes a tree of eight sketches readable, and the tree row IS the object —
// so renaming belongs on the row, not in a side-panel field. Bodies are computed results,
// not named features, so a body row must never open an editor (it cannot, they live in the
// Parts list, but the guard keeps a future change from slipping a body into this tree).
auto item_index = [this](const wxTreeItemId& it) -> int {
for (size_t i = 0; i < m_tree_items.size(); ++i)
if (m_tree_items[i] == it) return int(i);
return wxNOT_FOUND;
};
m_tree->Bind(wxEVT_TREE_BEGIN_LABEL_EDIT, [this, item_index](wxTreeEvent& e) {
// The event's item is the authority for WHAT is being edited; tree_selection() is not,
// because the editor can open on a row that is not the current selection. A row that is
// not a feature (a body, or a stray id) gets the edit vetoed before it can take a name.
if (tree_body_selection() >= 0 || item_index(e.GetItem()) == wxNOT_FOUND) { e.Veto(); return; }
e.Skip();
});
m_tree->Bind(wxEVT_TREE_END_LABEL_EDIT, [this, item_index](wxTreeEvent& e) {
if (e.IsEditCancelled()) return;
const int idx = item_index(e.GetItem());
if (idx == wxNOT_FOUND) { e.Veto(); return; }
wxString label = e.GetLabel();
label.Trim(true).Trim(false);
if (label.empty()) { e.Veto(); return; } // a nameless row is worse than a badly named one
m_doc.features[idx].name = std::string(label.ToUTF8().data());
sync_recipe_to_model(); // the name is part of the recipe, so the save path persists it
e.Skip(); // let wx finish applying the label to the item it is holding
// REBUILD LATER, NOT NOW. refresh_tree() deletes and re-creates every wxTreeItemId, and
// we are inside wx's own END_LABEL_EDIT dispatch for one of them — destroying it here
// frees the item the caller is still using and takes the process down. Measured: typing
// a name and pressing Enter killed the app outright, with the keystrokes traced and no
// trace for the Return. Deferring to the next event-loop turn lets wx finish first.
CallAfter([this] { refresh_tree(); });
});
// Feature-tree edit actions: act on the selected feature (delete / reorder). These sit in the
// card header (Prepare puts its section actions there too) rather than on a loose row below.
{
@@ -3924,7 +3974,6 @@ DesignPanel::DesignPanel(wxWindow* parent)
// Delete/Ctrl+Z there must edit the text, not the model.
const bool in_text = (dynamic_cast<wxTextCtrl*>(wxWindow::FindFocus()) != nullptr)
|| (m_viewport && m_viewport->inline_busy());
if (getenv("SNAPORCA_KEYTRACE")) {
wxWindow* fw = wxWindow::FindFocus();
fprintf(stderr, "[KEYTRACE] key=%d ui_mode=%d is_sketching=%d in_text=%d inline_busy=%d focus=%s\n",
@@ -3933,6 +3982,39 @@ DesignPanel::DesignPanel(wxWindow* parent)
fw ? (const char*) fw->GetClassInfo()->GetClassName() : "(none)");
fflush(stderr);
}
// NOTE the position: this sits AFTER the KEYTRACE block on purpose. It returns early,
// and putting it first made every forwarded key invisible to the tracer — the one
// instrument that diagnosed this bug in the first place.
// The in-canvas value field is a borderless, always-on-top top-level frame, so whether it
// may take keyboard focus is the platform's decision, not ours: macOS denies key status to a
// borderless window, mutter's focus-stealing prevention refuses a re-mapped window, and the
// rig never grants it. When focus is refused, Enter/Esc/Tab are delivered HERE (to the panel)
// instead of the field, its own wxEVT_TEXT_ENTER / WXK_ESCAPE bindings never fire, and the
// queued-dimension chain (a line queues Length then Angle) becomes unwalkable. Forwarding them
// makes the field behave the same everywhere WITHOUT fighting the window manager for focus,
// which is what seven earlier attempts did unsuccessfully. When the field DOES hold focus we
// deliberately do nothing here, so its own bindings run and typing keeps working.
if (m_viewport && m_viewport->inline_busy() && !m_viewport->inline_has_focus()) {
if (key == WXK_RETURN || key == WXK_NUMPAD_ENTER || key == WXK_TAB) {
m_viewport->inline_commit();
return;
}
if (key == WXK_ESCAPE) {
m_viewport->inline_cancel();
return;
}
}
// Esc must exit the sketch wherever focus happens to be. Which widget holds focus is an
// accident of where the user last clicked (a toolbar button, the Construction checkbox),
// and Esc must not depend on it — request_exit() is the layered behaviour the GL-canvas
// path already uses, so Esc means the same thing here as it does over the viewport.
if (key == WXK_ESCAPE && m_viewport && m_viewport->is_sketching()) {
m_viewport->request_sketch_exit();
return;
}
const bool dismissable = m_active != Tool::None || (m_viewport && m_viewport->moving_body());
if (key == WXK_ESCAPE && dismissable) { tool_cancel(); return; }