diff --git a/scripts/CAD/check-gui-click-edit.py b/scripts/CAD/check-gui-click-edit.py index 23d9266b47..be70dfa0d8 100755 --- a/scripts/CAD/check-gui-click-edit.py +++ b/scripts/CAD/check-gui-click-edit.py @@ -399,46 +399,29 @@ def type_into_open_field(value, mark): title = opens[-1][1] prefill = opens[-1][2].get("prefill", "") m2 = trace_mark() - # REPRODUCE THE FAILING CONDITION ON PURPOSE, rather than hoping the window manager supplies - # it. The defect is "the value field is open but does not hold the keyboard", which is what - # mutter does on the user's GNOME desktop and what openbox — the rig's WM — never does. A - # ladder that just types on openbox is green at baseline and proves nothing: the gate could - # not fail, so it could not pass meaningfully either. + # TYPE NORMALLY. NOTHING TO DEFOCUS ANY MORE. # - # So take the keyboard AWAY from the field first, deliberately, by activating the main - # window. That is exactly the state mutter leaves behind, reproduced deterministically on any - # WM. A build that routes keys by content is unaffected; a build that routes by focus commits - # its prefill, which is the bug, and the assertion below catches it. - if not A.no_defocus: - w, _, _, _, _ = win() - # windowfocus, NOT windowactivate. `windowactivate` sets _NET_ACTIVE_WINDOW — it ASKS the - # window manager, and openbox obliges by marking the field's toplevel inactive while - # leaving the X input focus exactly where it was. Measured: the trace read - # "active=0 toplevel_focus=0" and every keystroke still reached the field, so the ladder - # passed against a binary with the arbiter compiled out. A gate that cannot fail cannot - # pass meaningfully either, and that run nearly shipped as proof. - # - # `windowfocus` calls XSetInputFocus, which is what actually decides where the server - # delivers keys. That reproduces the real defect — field on screen, keyboard elsewhere — - # on any WM, instead of hoping the local one volunteers it. - # DELIVER THE KEYS TO THE MAIN WINDOW, not to whatever holds the focus. - # - # This is the defect, reproduced exactly and without a focus fight. On the user's GNOME - # desktop mutter's focus-stealing prevention refuses the borderless field toplevel the - # keyboard, so the digits are delivered to the Design panel — which is precisely what - # DesignPanel's CHAR_HOOK comment describes. Only routing by CONTENT can get them from - # there into the field; a build that routes by focus commits its prefill. - # - # Stealing the focus instead does NOT work and must not be reinstated: measured on - # openbox, the app re-asserts SetFocus from open()'s CallAfter and wins every race — four - # retries of XSetInputFocus all lost, `xdotool getwindowfocus` came back as the field's - # own toplevel every time. Two full runs passed against a binary with the arbiter - # compiled out because of it. Targeting the window sidesteps the question entirely. - target = None - if not A.no_defocus: - target, _, _, _, _ = win() - typ(str(value), 0.4, window=target) - key("Return", 0.9, window=target) + # The value field is drawn INSIDE the GL canvas by ImGui, so it is not a window: there is no + # second toplevel for a window manager to grant or refuse the keyboard, and the keystrokes go + # to the app's one window exactly as a person's would. That is the entire point of the design + # — the WM has no say — and it is why this ladder no longer tries to manufacture the failing + # condition. + # + # When the field WAS a floating wxFrame, this spot held two attempts to reproduce + # "field open, keyboard elsewhere", and both are recorded here so neither is tried again: + # - XSetInputFocus onto the main window (`xdotool windowfocus`): the field's own re-focus + # CallAfter wins the race every time; four retries all lost, and the ladder passed twice + # against a binary with the fix compiled out. + # - XSendEvent at the main window (`xdotool type --window`): GTK discards synthetic key + # events, so NEITHER build received anything and every run was red regardless of the code. + # A run that used the second of those is what produced "the app never saw a digit" — a + # property of xdotool, not of the product. + # + # For the in-canvas field the honest gate is simply: type, and see whether the value the app + # commits is the value that was typed. + diag = sh(f"DISPLAY={DISP} xdotool getwindowfocus").strip() + typ(str(value), 0.4) + key("Return", 0.9) after, commits, refused, commit_at = [], [], [], None deadline = time.time() + 5.0 while time.time() < deadline: diff --git a/src/slic3r/GUI/CAD/DesignCanvas.cpp b/src/slic3r/GUI/CAD/DesignCanvas.cpp index 7dd2b90875..0ee9633f8d 100644 --- a/src/slic3r/GUI/CAD/DesignCanvas.cpp +++ b/src/slic3r/GUI/CAD/DesignCanvas.cpp @@ -1373,11 +1373,6 @@ bool DesignCanvas::inline_has_focus() const return m_inline_editor && m_inline_editor->has_focus(); } -bool DesignCanvas::inline_type_char(int key) -{ - return m_inline_editor && m_inline_editor->type_char(key); -} - void DesignCanvas::inline_commit() { if (m_inline_editor) m_inline_editor->commit(); diff --git a/src/slic3r/GUI/CAD/DesignCanvas.hpp b/src/slic3r/GUI/CAD/DesignCanvas.hpp index cb1e517f7a..1da8a17cf4 100644 --- a/src/slic3r/GUI/CAD/DesignCanvas.hpp +++ b/src/slic3r/GUI/CAD/DesignCanvas.hpp @@ -244,8 +244,6 @@ public: void delete_selected_sketch_entities(); bool inline_busy() const; // a sketch value field is open (guard keys) bool inline_has_focus() const; // the field itself holds keyboard focus - // Hand one character to the open value field, bypassing focus. See DesignPanel's arbiter. - bool inline_type_char(int key); void inline_commit(); // accept the typed value (Enter/Tab) void inline_cancel(); // discard the typed value (Esc) // The layered Esc: abandon the points of the gesture in progress, else drop the armed tool diff --git a/src/slic3r/GUI/CAD/DesignPanel.cpp b/src/slic3r/GUI/CAD/DesignPanel.cpp index a84d370ea0..efbbf21c55 100644 --- a/src/slic3r/GUI/CAD/DesignPanel.cpp +++ b/src/slic3r/GUI/CAD/DesignPanel.cpp @@ -4212,25 +4212,11 @@ DesignPanel::DesignPanel(wxWindow* parent) m_viewport->inline_commit(); return; } - // THE ARBITER. Route by what the key IS, not by who the window manager focused. - // - // This is FreeCAD's rule, from Sketcher's DrawSketchKeyboardManager:: - // detectKeyboardEventHandlingMode: a digit, a sign, a decimal separator or a - // Backspace/Delete is unambiguously meant for the number the user is entering; a - // letter is unambiguously a tool shortcut; Enter/Tab hand control back to the view. - // FreeCAD never queries focus anywhere in that decision, and that is precisely why - // its sketcher behaves the same on every desktop. - // - // Ours asked "who has focus?" instead — a question whose answer is the window - // manager's opinion. openbox grants this borderless top-level the keyboard, mutter - // refuses it, so the same binary took typed values on one machine and silently - // committed the pre-filled as-drawn number on another. Seven workarounds fought that - // and one of them cost a macOS regression. The question was wrong, not the answers. - // - // The has_focus() guard above keeps this from double-typing where the toolkit DID - // give the field the keyboard: there the field's own binding will get the key too. - if (!ctrl && m_viewport->inline_type_char(key)) - return; + // NO forwarding here any more. The field is drawn INSIDE the GL canvas now, so it + // is fed the way every other ImGui widget in this app is fed: GLCanvas3D::on_char -> + // ImGuiWrapper::update_key_data -> io.AddInputCharacter. Re-adding a panel-side + // forwarder would also mask whether that path works, which is exactly what is being + // measured. // Esc is NOT special-cased here any more: escape() routes it, and the open field is // exactly what CadLevel::Transient means, so it closes the field and stops there. } diff --git a/src/slic3r/GUI/ImGuiWrapper.cpp b/src/slic3r/GUI/ImGuiWrapper.cpp index 5850161a3e..ff63c3ca43 100644 --- a/src/slic3r/GUI/ImGuiWrapper.cpp +++ b/src/slic3r/GUI/ImGuiWrapper.cpp @@ -505,6 +505,23 @@ bool ImGuiWrapper::update_key_data(wxKeyEvent &evt) if (evt.GetEventType() == wxEVT_CHAR) { // Char event const auto key = evt.GetUnicodeKey(); + // THE MEASUREMENT THAT CANNOT LIE. This is the ONLY place in the application where ImGui + // is ever handed a character, so an ImGui text field that stays empty while reporting + // itself active has exactly two possible causes, and this line separates them: no output + // at all means the wxEVT_CHAR never reached the GL canvas (a focus problem, upstream of + // ImGui entirely), while output with unicode=0 means the character arrived empty and is + // being dropped right here. + // + // It lives here rather than on the canvas because a probe bound on the canvas CANNOT + // answer this: GLCanvas3D::on_char is bound later than any constructor-time probe, wx + // runs handlers in reverse bind order, and on_char returns without Skip() whenever this + // function returns true — so such a probe stays silent whether or not the key arrived. + // A day was lost to reading that silence as evidence. + if (std::getenv("SNAPORCA_UXTRACE")) { + fprintf(stderr, "[UX] imgui_char unicode=%d keycode=%d want_text=%d\n", + (int) key, evt.GetKeyCode(), (int) io.WantTextInput); + fflush(stderr); + } if (key != 0) { io.AddInputCharacter(key); }