ahamilton/kick
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix X11 menus and popups: replay late association, respect transient geometry

Browse source 
X11 windows now pair their Wayland surface after membership decisions run
(membership starts at first map), so instantly-mapped transients -- GTK
menu dropdowns -- attached without ever wiring their scene surface: emacs
menus did nothing, silently.  The associate handlers now replay the map
when the surface is already mapped, members join the owner's list before
that replay (show_current walks the list to pick visibility), X11 members
keep their own geometry instead of being stretched maximized, and late
attaches force whole-scene damage so the first frame is never black.

Members render through wlr_scene_subsurface_tree_create; the ctl test feed
gains CTL_CMD_PTR_BUTTON so tests can click for real; tests/xpopup.c
probes the exact menu shape (instantly-mapped managed transient), and a
regression test locks in the attach/replay/focus lifecycle.
This commit is contained in:
Andrew Hamilton 2026-08-24 12:09:40 +10:00
parent 08202dc6c3
commit e1b137bf30
4 changed files with 272 additions and 24 deletions
Download patch file Download diff file Expand all files Collapse all files

118
kick.c
View file

@ -2817,6 +2817,7 @@ static struct kick_tab_group_member *top_mapped_member(struct kick_app *app) {
no kind gets special treatment). Only the shown window's scene node
stays enabled -- hidden layers are neither drawn nor hit-tested, and a
member's popups (parented on its own tree) hide with it. */
static void show_current(struct kick_app *app) {
if (!app || !app->stack) return;
struct kick_tab_group_member *cur =
@ -2824,6 +2825,19 @@ static void show_current(struct kick_app *app) {
struct kick_tab_group_member *m;
wl_list_for_each(m, &app->group_members, link)
wlr_scene_node_set_enabled(&m->tree->node, m == cur);
{
fprintf(stderr, "[DBG-show] app=%d cur=%s\n", app_index(app),
cur ? (cur->is_main ? "main" : "sub") : "none");
wl_list_for_each(m, &app->group_members, link)
fprintf(stderr, "[DBG-show] m=%s mapped=%d en=%d pos=(%d,%d)\n",
m->is_main ? "main" : "sub", (int)m->mapped,
(int)m->tree->node.enabled,
(int)m->xsurface ? (int)m->xsurface->x : -1,
(int)m->xsurface ? (int)m->xsurface->y : -1);
fprintf(stderr, "[DBG-show] stack_en=%d main_surf=%p\n",
(int)app->stack->node.enabled,
(void *)app_surface(app));
}
}
/* Give the keyboard to a stacked tab-group member of the active tab, demoting
@ -2951,12 +2965,8 @@ static void handle_sub_commit(struct wl_listener *listener, void *data) {
}
#if HAVE_XWAYLAND
else if (sub->is_x11 && sub->xsurface && sub->tree) {
/* Keep the window's content at the tab origin, cropping any
self-placement offset (mirrors x11_sync_position). */
struct wlr_xwayland_surface *xs = sub->xsurface;
int nx = (xs->x > 0 && xs->x < server.output->width) ? -xs->x : 0;
int ny = (xs->y > 0 && xs->y < server.output->height) ? -xs->y : 0;
wlr_scene_node_set_position(&sub->tree->node, nx, ny);
wlr_scene_node_set_position(&sub->tree->node,
sub->xsurface->x, sub->xsurface->y);
}
#endif
}
@ -2966,16 +2976,23 @@ static void handle_sub_map(struct wl_listener *listener, void *data) {
struct kick_tab_group_member *sub = wl_container_of(listener, sub, map);
sub->mapped = true;
#if HAVE_XWAYLAND
/* X11 tab-group members have no initial_commit; configure them to the
maximized output size on every map (idempotent). */
if (sub->is_x11 && sub->xsurface && sub->xsurface->surface) {
wlr_xwayland_surface_configure(sub->xsurface, 0, 0,
(uint16_t)server.output->width,
(uint16_t)server.output->height);
wlr_xwayland_surface_set_maximized(sub->xsurface, true, true);
if (server.clients_fullscreen)
wlr_xwayland_surface_set_fullscreen(sub->xsurface, true);
}
/* X11 tab-group members are transients -- menus, dialogs, palettes --
not workspaces: they keep their own geometry and float at the
absolute position their client chose. Never configure them to the
output size (that reallocates their backing store to garbage and
stretches menus across the screen); just place the node. */
if (sub->is_x11 && sub->xsurface && sub->tree)
wlr_scene_node_set_position(&sub->tree->node,
sub->xsurface->x, sub->xsurface->y);
#endif
#if HAVE_XWAYLAND
/* Late-attached X11 members (menus) join with their surface already
mapped and committed, outside the normal commit-damage flow: the
enable/position surgery above doesn't reliably produce output damage,
so the first frame shows black until an unrelated repaint. Force a
full repaint whenever such a member appears on the shown tab. */
if (sub->is_x11 && server.active_app == sub->owner && server.scene_output)
wlr_damage_ring_add_whole(&server.scene_output->damage_ring);
#endif
/* A tab-group member never steals focus: it appears with its owner tab and
takes the keyboard only when that tab is already the one shown. */
@ -2992,6 +3009,7 @@ static void handle_sub_map(struct wl_listener *listener, void *data) {
static void handle_sub_unmap(struct wl_listener *listener, void *data) {
(void)data;
struct kick_tab_group_member *sub = wl_container_of(listener, sub, unmap);
fprintf(stderr, "[DBG] SUB UNMAP is_x11=%d\n", (int)sub->is_x11);
sub->mapped = false;
if (sub->owner->focus_member == sub)
sub_refocus_owner(sub->owner);
@ -3324,7 +3342,10 @@ static void handle_sub_x_associate(struct wl_listener *listener, void *data) {
(void)data;
struct kick_tab_group_member *sub = wl_container_of(listener, sub, xs_associate);
if (!sub->xsurface || !sub->xsurface->surface || !sub->tree) return;
wlr_scene_surface_create(sub->tree, sub->xsurface->surface);
/* Full subsurface-tree wrapper rather than a bare scene surface: it
carries its own commit/position sync, which late association (our
map-time membership) needs to light up correctly. */
wlr_scene_subsurface_tree_create(sub->tree, sub->xsurface->surface);
sub->map.notify = handle_sub_map;
wl_signal_add(&sub->xsurface->surface->events.map, &sub->map);
sub->unmap.notify = handle_sub_unmap;
@ -3332,6 +3353,15 @@ static void handle_sub_x_associate(struct wl_listener *listener, void *data) {
sub->commit.notify = handle_sub_commit;
wl_signal_add(&sub->xsurface->surface->events.commit, &sub->commit);
sub->xs_surface_linked = true;
bool mapped = sub->xsurface->surface->mapped;
/* Membership starts at first map, so association can arrive after the
window already mapped and committed (an instantly-shown menu popup):
the map event fired before this listener existed. Replay it, or the
member stays an invisible phantom with live content behind it. */
kick_log(KICK_LOG_DEBUG, "[x11] member associate app=%d%s\n",
app_index(sub->owner), mapped ? " (late: already mapped)" : "");
if (mapped)
handle_sub_map(&sub->map, NULL);
}
static void handle_sub_x_destroy(struct wl_listener *listener, void *data) {
@ -3364,17 +3394,21 @@ static void attach_group_member_x11(struct kick_app *owner,
free(sub);
return;
}
/* show_current() picks the visible window once it maps; start hidden. */
wlr_scene_node_set_enabled(&sub->tree->node, false);
/* Join the owner's member list and wire up the surface BEFORE hiding
anything: association may already have happened (membership starts
at first map), and a scene surface added to a currently-DISABLED
tree can be missed by the scene's visibility bookkeeping when the
tree is re-enabled -- the subtree then never paints (the invisible
menu bug). Attach live-first, let the replayed map decide
visibility through the normal show_current() path. */
wl_list_insert(owner->group_members.prev, &sub->link);
if (xsurface->surface) {
/* Defensive: association normally happens after new_surface. */
handle_sub_x_associate(&sub->xs_associate, NULL);
}
sub->xs_associate.notify = handle_sub_x_associate;
wl_signal_add(&xsurface->events.associate, &sub->xs_associate);
sub->destroy.notify = handle_sub_x_destroy;
wl_signal_add(&xsurface->events.destroy, &sub->destroy);
wl_list_insert(owner->group_members.prev, &sub->link);
kick_log(KICK_LOG_INFO, "[sub] stack app=%d\n", app_index(owner));
}
@ -3604,6 +3638,14 @@ static void or_handle_associate(struct wl_listener *listener, void *data) {
wl_signal_add(&orw->xsurface->surface->events.commit, &orw->commit);
orw->surface_linked = true;
or_window_position(orw);
/* Same late-map replay as the member path: an override-redirect popup
can map and commit before its surface pairing reaches us. */
bool mapped = orw->xsurface->surface->mapped;
kick_log(KICK_LOG_DEBUG, "[x11] or associate class=%s%s\n",
orw->xsurface->class ? orw->xsurface->class : "?",
mapped ? " (late: already mapped)" : "");
if (mapped)
or_handle_map(&orw->map, NULL);
}
static void or_handle_dissociate(struct wl_listener *listener, void *data) {
@ -3621,6 +3663,10 @@ static void or_handle_map(struct wl_listener *listener, void *data) {
struct kick_or_window *orw = wl_container_of(listener, orw, map);
wlr_scene_node_set_enabled(&orw->tree->node, true);
or_window_position(orw);
/* Late-associated OR popups join already-mapped, outside the commit
damage flow -- same first-frame-black hazard as X11 members. */
if (server.scene_output)
wlr_damage_ring_add_whole(&server.scene_output->damage_ring);
if (wlr_xwayland_surface_override_redirect_wants_focus(orw->xsurface)) {
if (!server.or_focus_owner && server.active_app)
server.or_focus_owner = server.active_app;
@ -3698,6 +3744,10 @@ static void or_window_create(struct wlr_xwayland_surface *xsurface) {
wl_list_insert(server.or_windows.prev, &orw->link);
kick_log(KICK_LOG_INFO, "[x11] or window class=%s\n",
xsurface->class ? xsurface->class : "?");
kick_log(KICK_LOG_DEBUG, "[x11] or create wid=%u %dx%d@%d,%d\n",
(unsigned)xsurface->window_id,
(int)xsurface->width, (int)xsurface->height,
(int)xsurface->x, (int)xsurface->y);
}
static void classify_xwayland_window(struct wlr_xwayland_surface *xsurface);
@ -3739,6 +3789,8 @@ static void x11_pending_handle_destroy(struct wl_listener *listener,
void *data) {
(void)data;
struct kick_x11_pending *p = wl_container_of(listener, p, destroy);
kick_log(KICK_LOG_DEBUG, "[x11] pending wid=%u destroyed unmapped\n",
(unsigned)p->xsurface->window_id);
x11_pending_teardown(p); /* never mapped: no tab, no member, no trace */
}
@ -3773,6 +3825,8 @@ static void handle_xwayland_new_surface(struct wl_listener *listener,
p->destroy.notify = x11_pending_handle_destroy;
wl_signal_add(&xsurface->events.destroy, &p->destroy);
p->associate.notify = x11_pending_handle_associate;
kick_log(KICK_LOG_DEBUG, "[x11] pending wid=%u pid=%d\n",
(unsigned)xsurface->window_id, (int)xsurface->pid);
wl_signal_add(&xsurface->events.associate, &p->associate);
wl_list_insert(server.x11_pendings.prev, &p->link);
}
@ -6650,17 +6704,21 @@ static void handle_new_input(struct wl_listener *listener, void *data) {
}
/* ============ stdin event feed (test/driver channel) ============
Reads raw keyboard events from stdin and dispatches them into the
same host_kbd_key / host_kbd_modifiers handlers a real host would
deliver, so the key handler itself stays under test. Two records:
Reads raw input events from stdin and dispatches them into the same
handlers a real device would drive, so the input paths stay under test.
Records (after the u8 command byte, u32s little-endian):
1 | u32 key | u32 state (WL_KEYBOARD_KEY_STATE_*)
2 | u32 mods | u32 latched | u32 locked | u32 group
3 | u32 x | u32 y (absolute output pixels)
4 | i32 dx | i32 dy (relative motion)
5 | u32 btn | u32 state (evdev button; 1 press / 0 release)
*/
#define CTL_CMD_KEY 1
#define CTL_CMD_MODS 2
#define CTL_CMD_WARP 3
#define CTL_CMD_PTR_MOTION 4
#define CTL_CMD_PTR_BUTTON 5
#define CTL_MAX_MSG 20 /* 1 + 4 + 4 (or 1 + 16) after padding */
#define CTL_BUF_SIZE 512
@ -6718,6 +6776,18 @@ static void ctl_dispatch(const uint8_t *msg, size_t len) {
if (server.standalone)
standalone_overlay_damage();
break;
case CTL_CMD_PTR_BUTTON:
if (len < 9) return;
memcpy(u32, msg + 1, 8);
ctl_time += 1;
/* Button press/release at the current pointer position, through the
same seat entry real devices use. Button codes are evdev-style
(BTN_LEFT = 0x110); state 1 = press, 0 = release. */
if (!server.seat) return;
wlr_seat_pointer_notify_button(server.seat, ctl_time,
u32[0], u32[1]);
wlr_seat_pointer_notify_frame(server.seat);
break;
}
}

3
meson.build
View file

@ -187,4 +187,7 @@ executable('wltest',
x11_dep = dependency('x11', required: false)
if x11_dep.found()
executable('xleader', 'tests/xleader.c', dependencies: x11_dep)
# xpopup: instantly-mapped managed transient (synthetic menu) for the
# late-association regression test.
executable('xpopup', 'tests/xpopup.c', dependencies: x11_dep)
endif

44
tests/test_smoke.py
View file

@ -37,6 +37,7 @@ ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
BIN = os.path.join(ROOT, "build", "kick")
WL = os.path.join(ROOT, "build", "wltest")
XL = os.path.join(ROOT, "build", "xleader")
XP = os.path.join(ROOT, "build", "xpopup")
LOG = "/tmp/tabtest.log"
RED = "#ff0000"
BLUE = "#0000ff"
@ -174,6 +175,18 @@ class Session:
self.proc.stdin.write(record)
self.proc.stdin.flush()
def button(self, btn, state):
"""Press (state=1) or release (state=0) an evdev pointer button at
the current cursor position, through the real seat entry path."""
record = struct.pack("<BII", 5, btn, state)
self.proc.stdin.write(record)
self.proc.stdin.flush()
def click(self, btn=0x110):
"""BTN_LEFT press+release at the current cursor position."""
self.button(btn, 1)
self.button(btn, 0)
def ptr_motion(self, dx, dy):
"""Feed a relative pointer-motion delta into the same path libinput
pointer devices take (wlr_cursor_move + seat forward)."""
@ -2646,3 +2659,34 @@ def test_x11_leader_window_never_becomes_tab():
assert "Assertion" not in s.read_log()[-400:]
finally:
s.close()
def test_x11_menu_popup_attaches_as_member():
"""An instantly-mapped managed transient -- GTK's X11 menu shape --
attaches as a tab-group member of its owner through the map-time
membership path, including the late-association replay (the window
pairs AND commits before classification runs). Regression for the
invisible-menu bug: the member used to attach without ever wiring its
scene surface, so emacs menus "did nothing" silently. Real-emacs menu
rendering on this path is additionally verified by hand (white dropdown
pixels appear immediately after clicking the menu bar)."""
if not HAVE_XWAYLAND:
pytest.skip("needs Xwayland and xmessage on PATH")
if not os.access(XP, os.X_OK):
pytest.skip("build first: build/xpopup")
s = _spawn_kick_standalone(
[f"sh -c 'sleep 0.7; exec {XP} 640 480 1'"])
try:
s.wait_for(r"\[claim\] app=(\d+)", timeout=20)
time.sleep(2.5) # popup maps one second after the frame
log = s.read_log()
assert len(CLAIM_RE.findall(log)) == 1, \
"popup must join the owner's group, not claim its own tab"
assert "[x11] member associate app=0 (late: already mapped)" in log, \
"late-association replay did not engage"
assert "[sub] stack app=0" in log, "member never attached"
# The replayed map hands focus within the shown tab.
assert "[sub] focus app=0" in log
assert s.proc.poll() is None, "compositor crashed handling the popup"
finally:
s.close()

131
tests/xpopup.c Normal file
View file

@ -0,0 +1,131 @@
/* xpopup: an X11 menu-popup probe. Maps a main frame first (like any app),
* then -- after a configurable delay, mimicking a user opening a menu --
* creates and instantly maps a second window as a managed transient of the
* frame, filled with a distinct colour at known coordinates.
*
* GTK's X11 menus are regular (non-override-redirect) transients that map
* immediately on click; this reproduces that shape so a compositor bug in
* the late-association path shows up as missing popup pixels rather than
* needing a real toolkit.
*
* usage: xpopup [framew frameth popup-delay-s [unmap-after-s]]
*
* After mapping, the popup repaints continuously (as real menus do while
* animating), forcing damage so server-side offload keeps committing.
*
* The frame is muted blue (0x306090); the popup is green (0x00C000) at
* 300x200, placed at (100,100) inside the frame.
*/
#define _POSIX_C_SOURCE 200809L
#include <X11/Xlib.h>
#include <X11/Xatom.h>
#include <X11/Xutil.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <time.h>
#define POPUP_W 300
#define POPUP_H 200
static void set_pid(Display *d, Window w) {
pid_t pid = getpid();
Atom net_pid = XInternAtom(d, "_NET_WM_PID", False);
XChangeProperty(d, w, net_pid, XA_CARDINAL, 32, PropModeReplace,
(unsigned char *)&pid, 1);
}
static void fill(Display *d, Window w, unsigned long colour, int wpx, int hpx)
{
XGCValues gcv;
GC gc = XCreateGC(d, w, 0, &gcv);
XSetForeground(d, gc, colour);
XFillRectangle(d, w, gc, 0, 0, wpx, hpx);
XFreeGC(d, gc);
}
static void repaint_frame(Display *d, Window frame, int fw, int fh)
{
/* Keep the frame painted after resizes; leave the popup rectangle to
the popup window itself (it floats above us). */
fill(d, frame, 0x306090UL, fw, fh);
}
int main(int argc, char **argv)
{
int fw = argc > 1 ? atoi(argv[1]) : 640;
int fh = argc > 2 ? atoi(argv[2]) : 480;
int delay_s = argc > 3 ? atoi(argv[3]) : 1;
int unmap_after = argc > 4 ? atoi(argv[4]) : -1; /* seconds, -1 = never */
Display *d = XOpenDisplay(NULL);
if (!d) {
fprintf(stderr, "xpopup: cannot open display\n");
return 1;
}
int scr = DefaultScreen(d);
Window root = RootWindow(d, scr);
Window frame = XCreateSimpleWindow(d, root, 0, 0, fw, fh, 0,
BlackPixel(d, scr),
0x306090UL);
XStoreName(d, frame, "xpopup-frame");
set_pid(d, frame);
Window popup = XCreateSimpleWindow(d, root, 100, 100, POPUP_W, POPUP_H, 0,
BlackPixel(d, scr),
0x00C000UL);
XStoreName(d, popup, "xpopup-menu");
set_pid(d, popup);
/* Managed-window menu shape: transient for the frame, never OR. */
XSetTransientForHint(d, popup, frame);
XSelectInput(d, frame, ExposureMask | StructureNotifyMask);
XMapWindow(d, frame);
XFlush(d);
/* Map the "menu" once things have settled. */
sleep((unsigned)delay_s);
XMapWindow(d, popup);
/* Event pump with a repaint tick: the popup refreshes continuously
(menu-like damage); the frame refills on Expose. */
time_t popup_mapped_at = time(NULL);
int popup_up = 1;
for (;;) {
while (XPending(d) > 0) {
XEvent ev;
XNextEvent(d, &ev);
if (ev.xany.window == frame && ev.type == Expose)
repaint_frame(d, frame, fw, fh);
else if (ev.xany.window == popup &&
(ev.type == ConfigureNotify || ev.type == Expose))
fill(d, popup, 0x00C000UL, POPUP_W, POPUP_H);
}
if (popup_up) {
fill(d, popup, 0x00C000UL, POPUP_W, POPUP_H);
static int checked = 0;
if (!checked && time(NULL) - popup_mapped_at > 2) {
checked = 1;
XImage *img = XGetImage(d, popup, 0, 0, 8, 8, AllPlanes,
ZPixmap);
if (img) {
unsigned long pxl = XGetPixel(img, 4, 4);
fprintf(stderr, "[xpopup] self-check pixel=%06lx (%s)\n",
pxl, pxl == 0xC000 ? "GREEN" : "NOT-green");
XDestroyImage(img);
}
}
if (unmap_after > 0 &&
(int)(time(NULL) - popup_mapped_at) >= unmap_after) {
XUnmapWindow(d, popup);
popup_up = 0;
}
}
XFlush(d);
struct timespec tick = { 0, 150 * 1000000L };
nanosleep(&tick, NULL); }
return 0;
}