Anchor menus on Wayland and load Motif Xft fonts - #73
Merged
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
jserv
added a commit
that referenced
this pull request
Jul 24, 2026
The new wayland job failed at SDL3 configure with "could not find X11 or Wayland development libraries": SDL3's Wayland video driver needs EGL, which the job did not install, so with no usable X11 set either SDL3 disabled both backends and aborted. Install libegl-dev and libgles-dev, and build SDL3 with SDL_X11=OFF since this path only exercises the Wayland driver: the incomplete system X11 set is irrelevant and turning X11 off keeps the configure honest. Also reflow scripts/validate-menu-rendering.sh and scripts/build-sdl3.sh through shfmt so the lint job's `shfmt -d` gate passes, and pick up the clang-format reflow of the popup unmap comment and the parent-unmap test.
jserv
added a commit
that referenced
this pull request
Jul 24, 2026
Address the PR #73 review: popup-parent-unmap.c printed PARENT_UNMAP_OK after the map/unmap/remap cycle regardless of what happened to the popup, so the gate passed on mere absence of an ASan crash. It now reads the popup's map_state after XUnmapWindow(main) and fails unless the parent-unmap sweep carried it to IsUnmapped, so the check validates the lifecycle behavior rather than only teardown cleanliness. While validating that, the teardown check turned out not to be re-runnable: its ASan rebuild instruments both build/src and the staged upstream libX11 objects under build/upstream/src, but restore_library wiped only build/src, so it relinked the leftover instrumented upstream objects and left libX11-compat.so carrying __asan_* references that break the ordinary test compiles on the next run. A new wipe_first_party_objs drops the upstream object files too (keeping their staged .c sources) before both the ASan build and the restore, so each rebuild is clean and the workspace is usable afterward. Confirmed on node1: two back-to-back runs both report 4/4 and the library is left uninstrumented.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Significantly better-- the menus are in the right places, and the font is legible. It's not the font specified in the .Xdefaults file, but it's usable. Some other menu oddities I noticed on this version:
It also sometimes goes the other way-- a large menu is cropped to a smaller size with parts missing. If you open a second-level menu and keep the button held down as you enter it (for example, click "Preferences", then click on "Default Settings" and hold the mouse button down until you enter the secondary menu), the highlight is somewhat off from the mouse cursor. This seems to happen when the second-level menu starts "above" the first-level menu on the screen. I tried to press the accelerator commands for menus (Alt-F, Alt-E, etc) to see if the menu sizing thing responded differently and they don't register. |
|
Oh, another fun menu behaviour: Pop up a menu, and then go over to a Firefox window and hover over the tab bar. After a short time, The menu will suddenly move over to be there! I was also able to get the menu to jump by switching to a Galculator or Qmmp window and pressing that menu bar. |
Contributor
Author
|
Positive: The menus now seem to be the right size, no weird clipping. The accelerator keys open the menus as expected, but you can't navigate with arrows reliably (left and right move from menu to menu, but down just causes the menu to close) It still does the thing where a popped out menu will jump to another window if you activate its menu bar. In general, you can still trigger inconsistent behaviour by holding down the mouse button while navigating the menu. For example, if I do it with Preferences and highlight Default Settings, sometimes the submenu pops up as its own bordered window. (see screenshot) This doesn't even have a tearoff option. :) It seems like it's more prone to do this when the window is moved away from the top left corner of a screen Some submenus (File -> Open Previous) or sub-submenus (Preferences -> Default Settings -> Auto Indent) won't open until you release the mouse button. Even if you click at each phase, sometimes sub-submenus are placed oddly-- it seems this is more prominent when the window is not at the top of the screen. 
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
jserv
added a commit
that referenced
this pull request
Jul 31, 2026
Cubic review (PR #73): - Move-echo token: make moveEchoValid atomic so the event filter reads the valid flag and its coordinates consistently across the main and SDL event threads (they were plain fields); a torn read could otherwise misroute a MOVED into a duplicate ConfigureNotify. - ReplayPointer: consume the cached press and release the grab up front so a later resolve failure cannot leave a stale press that a subsequent ReplayPointer would deliver a second time; emit the ungrab in releasePointerGrab so the replay path keeps the grab-transition timeline balanced (matching XUngrabPointer). - keycodeForSdlKeycode: return 0 for an untabulated scancode or a Latin-1 key whose low byte lands in the 128+ range reserved for tabulated scancode indices, instead of colliding (Volume Up no longer reports as XK_Delete). - osfPrevField: bind to XK_Tab; XK_ISO_Left_Tab has no SDL key and left it unbound (keycode 0). The backward direction comes from the Shift modifier. - menu-nav-test buildFileMenu: return void; callers ignore the result. Wayland nested-popup check: - Keep the parent popup on screen. SDL3's Wayland backend cannot give an xdg_popup role to a child whose parent popup lies off screen on a non-constraining compositor (weston headless does not slide it back on), so the off-screen parent made the cascade submenu fall back with no popup role and fail. The parent-selection and offset logic under test is unchanged; the compositor-slide divergence is covered separately.
A Wayland client cannot self-position a top-level, so override-redirect menu created as an ordinary window floats wherever the compositor drops it, detached from the widget that spawned it. When a mapped parent contains the popup's anchor, realize it as an SDL3 xdg_popup positioned relative to that parent surface. findPopupParentToplevel picks the innermost containing window (smallest area wins, so a cascade submenu anchors to its parent menu, not the shell); a zero-area transient is skipped so it cannot misanchor. Anchored popups are torn down before their parent's SDL surface through a single drainAnchoredPopups helper shared by the unmap, unrealize, and destroy paths. It unmaps each child through the full path (grab release, UnmapNotify, mapState flip) and forces the unrealize on any child left holding an sdlWindow, so the drain always converges and never leaves a popup stranded with a surface SDL would free out from under it.
Motif's FONT_IS_XFT render tables need a working Xft. The bundled
thentenaar/motif was built --without-xft, so those tables fell through
to an invalid rendition type and painted as a tiny crushed block. Build
it --with-xft against the in-tree Xft-over-SDL layer and add the entry
points that path links against: FcInit/FcFini, which Motif's Display.c
calls at display open and close, and XftDrawCreateBitmap, the XmRenderT
fallback that must at least resolve. Bump MOTIF_COMMIT to the matching
source revision.
The app differentials build the same --with-xft Motif, so keep them
green: install lib{fontconfig1,xft}-dev in CI, flip motif/mosaic/violawww
differential drivers to --with-xft with a pkg-config preflight,
recalibrate the demo comparison thresholds now that both sides render
through Xft, and widen the GIMP-Motif startup window-wait. GIMP's
heavier --with-xft cold start, captured concurrently with the system
side, now maps its toolbox past the old five-second bound on a two-core
runner; the wait is a max, so the single-process path is unaffected.
tests/menu-rendering covers the two behaviors from the detached-menu report. A raw override-redirect popup and a real Motif XmPulldownMenu must each emit a Wayland xdg_popup anchored to their parent rather than a detached second toplevel; a FONT_IS_XFT render table must measure a legible height; and tearing a live popup's parent down, by both destroy and unmap, must carry the popup through the full unmap path (its own UnmapNotify, IsUnmapped, a clean remap) with no leak or use-after-free under AddressSanitizer. scripts/validate-menu-rendering.sh drives them under weston headless, building a Wayland-enabled SDL3 via scripts/build-sdl3.sh (pinned source with published checksums). For the teardown check it rebuilds the library with the sanitizer and restores an uninstrumented one afterward; the restore wipes the staged upstream objects the sanitizer instrumented too, so the harness leaves a clean tree and is re-runnable. Wayland checks skip with a reason on an X11-only host.
Add a wayland job: it builds a Wayland-enabled SDL3 and SDL3_ttf from source (with EGL/GLES dev libs so SDL3's Wayland video driver actually configures), builds the compat stack and the --with-xft Motif, and runs scripts/validate-menu-rendering.sh against weston headless with CI_REQUIRE=1. It is the only job that exercises compositor-dependent override-redirect xdg_popup path; the SDL2 jobs and the console-only sdl3 job cannot see it.
Override-redirect popups on the SDL3/Wayland backend had two defects. An xdg_popup's geometry is fixed by its positioner at creation, so SDL_SetWindowSize only grew the surface buffer while the compositor kept clipping a popup resized after mapping to its creation rectangle: a Motif menu bar switching the armed pulldown from a short menu to a taller one rendered the taller menu cropped. configureWindow now re-runs the positioner once, after the move and resize blocks have settled the geometry, through repositionAnchoredPopup, so the compositor grows the popup to the final size. A constraint-enforcing compositor slides or flips a popup that would fall off screen and reports the new position only through SDL's MOVED event, which onSdlEvent dropped. ws->x/y then kept the requested position, so pointer-to-root translation for a grabbed submenu was offset by the slide and the highlight drifted from the cursor. onSdlEvent now lets an anchored popup's MOVED through; convertEvent records the real origin (parent origin plus the parent-relative offset) and emits no ConfigureNotify, so the toolkit's own menu tracking is undisturbed.
On SDL2 with Xvnc, tearing down or moving a top-level does not make the X server deliver Expose to the siblings it uncovered, so their stale pixels linger as a residue trail. Force a present on every mapped top-level overlapping the vacated rect on the three paths that shrink a top-level's on-screen footprint: unmap, destroy, and a window-manager opaque drag. The same MOVED path records the host origin configureWindow wrote for a client XMoveWindow as an atomic coordinate token and drops the matching SDL echo, so a bare move no longer escapes as a second ConfigureNotify. clampToIntRange moves into window-internal, non-static, so the repaint rects and the popup geometry share one saturating clamp.
An override-redirect popup on SDL2 needs the X11 driver to actually set override_redirect, which BORDERLESS alone does not; request the TOOLTIP window type so the popup is not managed or focus-stolen by the window manager. Split the popup's client-requested X11 origin (popupAnchorX/Y) from ws->x/y, which may later track the compositor-adjusted actual position: parent selection, the create-time offset, and the reposition all read the requested origin through popupAnchorOriginX/Y so they cannot disagree and drift the anchor. findPopupParentToplevel gains a stacking tie-break for overlapping candidates and clamps its geometry math through clampToIntRange so a hostile coordinate saturates instead of wrapping.
resolvePointerHit gains a captureHeld flag so a synchronous grab routes the event to the grabbed window instead of the deepest child under the pointer. A button uses the hovered-window root coordinates only when the grab that redirected it is the event's own top-level or a descendant widget (a Motif menu grabs the shell), and takes the ordinary scale/translate path otherwise. ReplayPointer caches only a press frozen by a sync grab, the one press it can replay, and releasePointerGrab now releases the grab and reprocesses the cached press, emitting the ungrab timeline tap so the record stays balanced. The key event's subwindow is derived from the same pointer-hit resolution since it shares the API.
X11 KeyCode is 8-bit while SDL keycodes span ASCII plus a separate scancode range, so keycodeForSdlKeycode maps tabulated scancode keys into a distinct range above 127 and returns 0 for an untabulated scancode or a Latin-1 code whose low byte lands past 127, instead of truncating Volume Up onto Delete. XKeysymToKeycode prefers the ASCII SDL keycode so a round-trippable keysym like XK_Return resolves to 13, and osfXK_PrevField maps to XK_Tab rather than an unbound keysym so Motif field navigation works. convertEvent adopts the SDL-reported keyboard focus for a born-focused single-window client that never calls XSetInputFocus, and an owner_events XGrabKeyboard tries ordinary focus delivery first so a Motif menu keeps receiving keys after it moves focus into a posted pulldown.
The sans probe lists put the Narrow (condensed) URW and Liberation faces first, so a plain Helvetica or sans request on Linux mapped to a squished condensed font that did not match the regular-width Helvetica macOS renders. Order the regular-width faces first and keep the Narrow variants only as a last resort for hosts that ship nothing else.
macOS already resolves the in-tree compat libs through @rpath, so ./xnedit starts with no environment setup; Linux did not. Emit DT_RPATH instead of the default DT_RUNPATH (--disable-new-dtags): DT_RUNPATH is not consulted when the loader resolves a dependency's own NEEDED entries, so the transitive compat libs (libXft-compat is NEEDED by libXm, not by xnedit) went unfound and the bare binary needed LD_LIBRARY_PATH. DT_RPATH is searched for the whole dependency tree. Mirror the SDL2 shims into the lib-aliases directory too, since libX11-compat is loaded through the libX11.so alias there and its RUNPATH resolves relative to that dir.
GNU make predefines CC=cc, and the tree forced CC=clang, so a host without clang could not build without the caller passing CC=. Probe PATH for clang and fall back to the system cc when it is absent, while explicit command-line and environment overrides still win. Add --disable-dependency-tracking to the Motif configure flags so its build does not emit per-object depfiles the vendored copy never consumes.
libx11-compat's ConnectionNumber is a wake pipe, so Xt's poll() can wake readable after the SDL pump drains a timer byte without producing an X event. Upstream Xt treats that as impossible and restarts the wait without refreshing wt.cur_time, which can starve Xt timers forever under a steady pump-wake cadence. Add a NextEvent.c source patch that refreshes the clock before the spurious-ready retry.
menu-nav-test.c drives a Motif menu bar through keyboard navigation and popup-nested.c checks that a cascade submenu anchors to its parent popup rather than the app shell; the menu-rendering validator gains a nested check that runs the latter under Wayland. The JSONL timeline taps now expose the event subwindow, root coordinates, and the decoded keysym so the menu-nav assertions can match on them, run-ui-replay maps the navigation keys (arrows, Escape, Return, F10) for its xdotool backend, and the X11/Intrinsic.h wrapper predefines XMSTRINGDEFINES so Motif clients build against the vendored headers.
Cover the de-aliased keycode mapping (XKeysymToKeycode(XK_Return) resolves to 13, XK_Down does not alias onto a letter, keycode 13 round-trips through XkbKeycodeToKeysym), the pointer-hit routing that reports the covered child window and its offset coordinates, a synchronous grab with ReplayPointer, LeaveNotify crossing under a grab, and the overlapping top-level pointer routing path.
Pure formatting: put each aggregate initializer field on its own line with a trailing comma and remove braces around single-statement ifs, so the reformatted blocks match the tree's clang-format style. No test behavior changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motif drop-down menus rendered as free-standing windows and FONT_IS_XFT render tables came out as tiny unreadable blocks on the SDL3 backend under a Wayland compositor.
Menus: an override-redirect popup is a top-level, and a Wayland client cannot self-position a top-level, so each menu shell floated wherever the compositor dropped it. Under the SDL3 backend these popups are now created with SDL_CreatePopupWindow anchored to the parent surface whose rectangle contains the menu anchor. findPopupParentToplevel picks the innermost containing realized window (cascade submenus anchor to their parent menu), and the popup is placed at a parent-relative offset. SDL owns the popup as a child window, so parent teardown unrealizes any anchored popup first to avoid a dangling SDL_Window double free. The SDL2 path is unchanged: popupParent stays None and the absolute-position top-level path is preserved.
Fonts: libXm was built --without-xft, so a FONT_IS_XFT rendition took the INVALID_TYPE branch and loaded no font. Build Motif --with-xft against the in-tree Xft, adding Fc{Init,Fini} and XftDrawCreateBitmap the Motif Xft path links. The differential reference builds flip to --with-xft as well, with matching libxft/libfontconfig deps, so both sides render the same font stack; measured baselines are unchanged.
Add a wayland CI job that builds a Wayland-enabled SDL3 from source and runs scripts/validate-menu-rendering.sh under weston headless: a real Motif XmPulldownMenu, a raw override-redirect popup, a FONT_IS_XFT rendition, and a parent-first teardown under AddressSanitizer.
Close #68
Summary by cubic
Fix Wayland menus by anchoring override‑redirect popups as parent‑anchored
SDL3xdg_popups and make Motif Xft fonts render at readable sizes. Also fix SDL2 WM decoration/residue and keyboard mapping/focus so menu navigation and popup behavior work correctly.Bug Fixes
SDL_CreatePopupWindow;findPopupParentToplevelpicks the innermost mapped parent (skips zero‑area transients, stacking tie‑break, overflow‑safe); accept anchored popupMOVEDto track compositor slides and record the real origin; re‑run the xdg positioner after resize so grown menus aren’t clipped; drop invalid always‑on‑top; safe teardown viadrainAnchoredPopups.SDL_WINDOW_TOOLTIPfor override‑redirect windows so WMs don’t decorate menus; repaint overlapping siblings on unmap/destroy/move to remove residue; suppress move‑echoes with an atomic coordinate token.XKeysymToKeycodeprefers ASCII keycodes; adopt SDL focus for born‑focused apps and honor owner_events so posted menus keep receiving keys;XTestderives modifiers; replay pointer consumes the cached press before ungrab and emits a matching ungrab; safer hover fallback; OSF keysyms map to physical keys, withosfPrevFieldbound to Tab.--with-xftand addFcInit/FcFiniandXftDrawCreateBitmap; prefer regular‑width faces before Narrow so Linux menus aren’t squished.XGetWindowAttributesfailure; clamp popup geometry math; wrap<X11/Intrinsic.h>to force literal resource macros.New Features
SDL3/SDL3_ttfand runsscripts/validate-menu-rendering.shunder headlesswestonwithCI_REQUIRE=1.XmPulldownMenuanchoring, popup resize growth and slide offsets, nested popups (keep parent on‑screen), parent unmap/destroy teardown, and menu keyboard navigation; timeline taps now include subwindow/root coords and decoded keysym; extend xdotool mapping inscripts/run-ui-replay.pywith named SDL keycodes; useDT_RPATHsoxneditruns withoutLD_LIBRARY_PATH; toolchain auto‑detectsclang; Motif builds with--disable-dependency-tracking.Written for commit 3c792d1. Summary will update on new commits.