mirror of
https://github.com/OrcaSlicer/OrcaSlicer.git
synced 2026-09-27 10:51:22 +00:00
Sketch value fields: content-based key arbiter + the gate that can judge it
The reported defect: sketch dimension labels are "not editable" — you draw a
rectangle, its Width field opens, you type, and the as-drawn number is committed
instead. It affects every sketch tool, not just the rounded rectangle.
WHAT THIS ADDS
1. The arbiter (DesignPanel CHAR_HOOK -> DesignCanvas::inline_type_char ->
SketchInlineEditor::type_char). Routes a key by what it IS, not by who the
window manager focused: digits, sign, decimal separator and Backspace/Delete
go to the open value field, Enter/Tab commit, letters stay tool shortcuts.
This is FreeCAD Sketcher's rule (DrawSketchKeyboardManager::
detectKeyboardEventHandlingMode), and the reason its sketcher behaves the same
on every desktop: it never asks who has focus.
2. The [UX] trace (SNAPORCA_UXTRACE) in SketchInlineEditor: open/commit/refused/
cancel, with the prefill and what the control actually held at Enter. It did
not exist — the ladder below was written against a surface no build emitted,
so it could only ever report "nothing opened". typed == prefill on a commit is
the defect's signature and nothing else makes it visible.
3. A draw-then-edit trace in DesignSketchTool: four early returns can swallow the
value-field chain and from outside they are indistinguishable.
4. scripts/CAD/check-gui-click-edit.py — types WITHOUT clicking the field, as a
person does, across Line/Rectangle/Circle/Slot/Polygon/Ellipse/Arc plus label
click-to-edit, and asserts committed == typed != prefill.
5. scripts/CAD/focus-loop.sh — sync/build/assert on behemoth. NOT the orcacad-gui
rig: its image pins deps 216 non-CAD files behind cad-mainline, so today's CAD
sources cannot build there without a deps rebuild.
WHAT IS PROVEN, AND WHAT IS NOT
Green under openbox: 28 checks, every tool, committed == typed != prefill.
But openbox CANNOT adjudicate this bug and the ladder says so in place. There the
field always wins the keyboard, so the same ladder also passes against a binary
with the arbiter compiled out — measured twice. Two ways of removing the keyboard
were tried and both are recorded as dead ends: XSetInputFocus loses to the field's
own re-focus CallAfter, and XSendEvent (xdotool --window) is dropped by GTK, which
made every run red regardless of the code.
Under metacity — same focus-stealing-prevention lineage as the user's mutter — the
mechanism appears in the WM's own log:
Buggy client sent a _NET_ACTIVE_WINDOW message with a timestamp of 0
That is the activation being refused, which is exactly the reported symptom.
present_toplevel() already asks for a server timestamp, so a path is still falling
through to frame->Raise(), which sends time 0. That is the next thing to fix, and
it is tracked; the arbiter alone does not close it. metacity also aborts on this
window (frames.c:1239), so the gate needs a WM that survives before it can return
a verdict.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011FbJKJAJxxkhDTs9XdZzKA
This commit is contained in:
co-authored by
Claude Opus 5
parent
af4bbe0217
commit
9134299233
@@ -90,6 +90,23 @@ constexpr bool keep_mapped_between_fields =
|
||||
false;
|
||||
#endif
|
||||
|
||||
// The click-edit contract, made observable. scripts/CAD/check-gui-click-edit.py grades a build
|
||||
// on these four lines and nothing else, because they are the only place the distinction it cares
|
||||
// about is visible: a field that is on screen but deaf commits its PREFILL, and every other
|
||||
// signal — the field drew, a constraint appeared, the solve succeeded — looks perfectly healthy
|
||||
// either way. `typed` is what the control actually held when Enter arrived; `prefill` is what
|
||||
// open() put there. typed == prefill on a commit means the keyboard never reached the field.
|
||||
//
|
||||
// stderr, one line, no buffering, only under SNAPORCA_UXTRACE: this is a test surface, not
|
||||
// logging, and it must cost nothing in a normal run.
|
||||
void trace_ux(const char* event, const std::string& title, const std::string& kv)
|
||||
{
|
||||
if (!std::getenv("SNAPORCA_UXTRACE")) return;
|
||||
fprintf(stderr, "[UX] %s title=%s%s%s\n", event, title.c_str(),
|
||||
kv.empty() ? "" : " ", kv.c_str());
|
||||
fflush(stderr);
|
||||
}
|
||||
|
||||
void trace_inline_focus(wxFrame* frame, const std::string& title)
|
||||
{
|
||||
if (!std::getenv("SNAPORCA_KEYTRACE")) return;
|
||||
@@ -214,6 +231,8 @@ void SketchInlineEditor::open(const wxPoint& screen_px, double value,
|
||||
m_ctrl->SetFocus();
|
||||
m_ctrl->SelectAll();
|
||||
m_open = true;
|
||||
m_prefill = m_ctrl->GetValue();
|
||||
trace_ux("open", title, "prefill=" + std::string(m_prefill.utf8_str()));
|
||||
trace_inline_focus(m_frame, title);
|
||||
// Re-assert on the next tick too: the GL canvas can reclaim focus while it finishes
|
||||
// handling the click/render that opened us, so a single immediate SetFocus may be stolen.
|
||||
@@ -235,6 +254,8 @@ void SketchInlineEditor::do_commit()
|
||||
// Silence here read as a freeze: Enter did nothing, the text re-selected itself, and
|
||||
// nothing on screen said the value had been refused or what would be accepted. Every
|
||||
// other CAD names the problem in place; so do we.
|
||||
trace_ux("refused", std::string(m_title_text.utf8_str()),
|
||||
"typed=" + std::string(m_ctrl->GetValue().utf8_str()));
|
||||
flag_invalid(m_ctrl->GetValue().Strip(wxString::both).IsEmpty()
|
||||
? _L("Enter a number")
|
||||
: _L("Not a number"));
|
||||
@@ -242,6 +263,17 @@ void SketchInlineEditor::do_commit()
|
||||
m_ctrl->SelectAll();
|
||||
return;
|
||||
}
|
||||
{
|
||||
// LOCALE-INVARIANT on purpose. printf honours the app's locale, which on an Italian
|
||||
// desktop makes this "61,0000" — and the ladder that reads it does float(), which raises
|
||||
// on a comma and takes the whole run down one check after the first success. A machine
|
||||
// surface must not change shape with the user's regional settings.
|
||||
char buf[64];
|
||||
snprintf(buf, sizeof(buf), "%.4f", v);
|
||||
for (char* c = buf; *c; ++c) if (*c == ',') *c = '.';
|
||||
trace_ux("commit", std::string(m_title_text.utf8_str()),
|
||||
"typed=" + std::string(m_ctrl->GetValue().utf8_str()) + " value=" + buf);
|
||||
}
|
||||
auto cb = m_commit;
|
||||
m_open = false; // logically closed; whether it stays MAPPED is per-toolkit
|
||||
m_commit = nullptr;
|
||||
@@ -262,6 +294,53 @@ void SketchInlineEditor::do_commit()
|
||||
});
|
||||
}
|
||||
|
||||
// Deliver one character into the field without the window manager's permission.
|
||||
//
|
||||
// This is the whole content-based-routing idea in one function: the caller has already decided,
|
||||
// from the KEY ITSELF, that this keystroke belongs to a number field, so the field takes it —
|
||||
// whether or not any window manager saw fit to give it focus. FreeCAD's sketcher works exactly
|
||||
// this way and never asks who is focused.
|
||||
bool SketchInlineEditor::type_char(int key)
|
||||
{
|
||||
if (!m_open || m_ctrl == nullptr) return false;
|
||||
|
||||
if (key == WXK_BACK || key == WXK_DELETE) {
|
||||
long from = 0, to = 0;
|
||||
m_ctrl->GetSelection(&from, &to);
|
||||
if (from != to) {
|
||||
m_ctrl->Remove(from, to);
|
||||
} else {
|
||||
const long ip = m_ctrl->GetInsertionPoint();
|
||||
if (key == WXK_BACK) { if (ip > 0) m_ctrl->Remove(ip - 1, ip); }
|
||||
else { if (ip < m_ctrl->GetLastPosition()) m_ctrl->Remove(ip, ip + 1); }
|
||||
}
|
||||
clear_invalid();
|
||||
return true;
|
||||
}
|
||||
|
||||
// The numeric keypad reports its own key codes, and a keypad is exactly what someone typing
|
||||
// dimensions all day uses.
|
||||
int ch = key;
|
||||
if (key >= WXK_NUMPAD0 && key <= WXK_NUMPAD9) ch = '0' + (key - WXK_NUMPAD0);
|
||||
else if (key == WXK_NUMPAD_DECIMAL) ch = '.';
|
||||
else if (key == WXK_NUMPAD_SUBTRACT) ch = '-';
|
||||
|
||||
const bool numeric = (ch >= '0' && ch <= '9') || ch == '-' || ch == '+' || ch == '.' || ch == ',';
|
||||
if (!numeric) return false;
|
||||
|
||||
// A decimal COMMA is normalised to a point on the way in: this field feeds a CAD kernel and
|
||||
// the rest of the file already promises a point whatever the locale (see fmt_value).
|
||||
if (ch == ',') ch = '.';
|
||||
|
||||
// WriteText replaces the current selection — and open() left the whole prefill selected, so
|
||||
// the FIRST character typed replaces the as-drawn value and the rest append. That is the
|
||||
// behaviour a person expects from a pre-selected field, obtained for free rather than
|
||||
// reimplemented.
|
||||
m_ctrl->WriteText(wxString(wxUniChar(ch)));
|
||||
clear_invalid();
|
||||
return true;
|
||||
}
|
||||
|
||||
void SketchInlineEditor::cancel()
|
||||
{
|
||||
if (m_open) do_cancel();
|
||||
@@ -345,6 +424,7 @@ void SketchInlineEditor::clear_invalid()
|
||||
void SketchInlineEditor::do_cancel()
|
||||
{
|
||||
if (!m_open) return;
|
||||
trace_ux("cancel", std::string(m_title_text.utf8_str()), "");
|
||||
auto cb = m_cancel;
|
||||
close();
|
||||
if (cb) cb();
|
||||
|
||||
Reference in New Issue
Block a user