Design: confine the mapped-frame workaround to GTK, so macOS stops hanging

Reported on PR #15238: on macOS, drawing a corner rectangle and pressing Enter
on the first dimension opens the second field and then wedges the entire
application — no typing, no Escape, dead menu bar, no tab switching, and the
field stays composited over the desktop after minimising. That last detail is
what identifies it: an already-drawn window keeps being composited by the
WindowServer once the process stops answering. The main thread is stuck.

4c8c93b512 is the only commit that ever changed the second-queued-field path.
It stopped unmapping the value field between two queued dimensions, and its
whole justification is mutter: focus-stealing prevention refuses keyboard focus
to a window that was just re-mapped, which left the second field visible but
dead on GNOME (snaporca-p8uw). It was applied with no platform guard, so macOS
re-activates an already-visible borderless NSWindow at NSModalPanelWindowLevel
from inside wxOSX's pending-event drain — which on macOS runs from a
CFRunLoopObserver at kCFRunLoopBeforeTimers (evtloop_cf.cpp) over an unbounded
`while (!m_handlersWithPendingEvents.IsEmpty())` (appbase.cpp).

One constexpr now carries that choice, and both halves of the contract read it,
because the failure mode of this fix is the two halves drifting apart: open()'s
reuse-if-shown and do_commit()'s deferred hide are one decision, not two.

Not a macOS guess I could not check: the non-GTK branch was exercised on the
Linux rig by forcing the constant to false and rebuilding. A corner rectangle
drew, its first field committed at 60 mm, the SECOND field opened, committed at
40 mm, and the sketch ended at 60.0 x 40.0 mm with the field closed and no
freeze — so the map-afresh path is functionally complete, not merely different.
On GTK the constant is true and every generated instruction is unchanged.

What is still unproven is the exact line where macOS wedges; the reporter has
been asked for a `sample` of the hung process. This fixes the cause the evidence
points at without waiting for that, and cannot regress the GTK behaviour.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tommaso Bianchi
2026-08-20 18:37:01 +02:00
co-authored by Claude Opus 5
parent 52d16e4218
commit eb66e45b7b
+31 -8
View File
@@ -72,6 +72,23 @@ void present_toplevel(wxFrame* frame)
}
#endif
// Between two queued dimensions (a rectangle's Width then Height) the frame is either kept
// MAPPED and merely re-titled, or unmapped and mapped again. That is a per-toolkit choice, not
// a preference:
// GTK/mutter keep it mapped. Focus-stealing prevention refuses keyboard focus to a window
// that was just re-mapped, so hiding between the two fields left the second one
// visible but dead (snaporca-p8uw).
// elsewhere map it afresh. This is what shipped before that workaround, which was applied
// with no platform guard — and it is the only difference between the first queued
// field (works everywhere) and the second (macOS wedges the whole app, PR #15238).
// A workaround for one window manager must not become a contract for all of them.
constexpr bool keep_mapped_between_fields =
#ifdef __WXGTK__
true;
#else
false;
#endif
void trace_inline_focus(wxFrame* frame, const std::string& title)
{
if (!std::getenv("SNAPORCA_KEYTRACE")) return;
@@ -136,9 +153,11 @@ void SketchInlineEditor::open(const wxPoint& screen_px, double value,
std::function<void()> on_cancel)
{
if (m_frame == nullptr || m_ctrl == nullptr) { if (on_cancel) on_cancel(); return; }
// Never close/unmap on the way in: the previous queued dimension left this frame mapped
// (see do_commit), and re-mapping a hidden toplevel is exactly what mutter refuses to
// focus. Reuse the still-mapped frame and just re-title/re-position it.
// Where the frame is kept mapped, never close/unmap on the way in: the previous queued
// dimension left it mapped (see do_commit) and re-mapping is what mutter refuses to focus,
// so reuse it and just re-title/re-position. Elsewhere, force a fresh map.
if (!keep_mapped_between_fields && m_frame->IsShown())
m_frame->Hide();
m_commit = std::move(on_commit);
m_cancel = std::move(on_cancel);
m_ctrl->ChangeValue(en_format(value));
@@ -195,14 +214,18 @@ void SketchInlineEditor::do_commit()
return;
}
auto cb = m_commit;
m_open = false; // logically closed: the frame stays MAPPED
m_open = false; // logically closed; whether it stays MAPPED is per-toolkit
m_commit = nullptr;
m_cancel = nullptr;
// Unmap BEFORE the callback where we are not keeping it mapped, so the reopen the callback
// schedules starts from a hidden frame — the ordering that shipped before the workaround.
if (!keep_mapped_between_fields)
m_frame->Hide();
if (cb) cb(v);
// The callback either re-opens us for the next queued dimension (via its own CallAfter,
// queued during cb(v), therefore BEFORE the one below) or it does not. Hiding here would
// unmap the window and mutter would refuse to focus the re-map; so hide only after the
// reopen has had its turn.
// Kept mapped: the callback either re-opens us for the next queued dimension (via its own
// CallAfter, queued during cb(v), therefore BEFORE the one below) or it does not. Hiding
// here would unmap the window and mutter would refuse to focus the re-map; so hide only
// after the reopen has had its turn. Harmless on the unmapped path — already hidden.
m_frame->CallAfter([this] { if (!m_open && m_frame) m_frame->Hide(); });
}