From 143b8906bb5478e62d0b1f8d4a16177ec031dce1 Mon Sep 17 00:00:00 2001 From: lkn Date: Mon, 22 Jun 2026 00:21:42 +0200 Subject: [PATCH] wayland: fix view_table probe chain corruption on widget reopen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The WINGs view_table (open-addressing hash, size 256) used id=0 as both "empty" and "deleted". Zeroing a slot on removal broke probe chains for entries that hashed past that slot. After two dialog open/close cycles (~40 views created and destroyed), enough chains were broken that W_GetViewForWindow/W_GetViewBacking returned NULL for valid IDs. Fix: use a tombstone marker (id=1) for deleted slots. Lookups skip tombstones (continue probing), inserts reuse them. This is the standard approach for deletion in open-addressing hash tables (Knuth Vol.3 §6.4). Also: - wl_destroy now calls view_table_remove + frees backing pixman image - wl_frame_destroy cleans up child frame_bufs - client_reparent existing path re-links backing and recreates scene_buf - client_reparent to root destroys the child frame_buf --- WINGs/wview_wl.c | 16 ++++++--- src/backend/wayland/wl_client.c | 56 ++++++++++++++++++++++++++++--- src/backend/wayland/wl_framebuf.c | 13 +++++++ 3 files changed, 76 insertions(+), 9 deletions(-) diff --git a/WINGs/wview_wl.c b/WINGs/wview_wl.c index c5a8a44..b59c067 100644 --- a/WINGs/wview_wl.c +++ b/WINGs/wview_wl.c @@ -49,6 +49,7 @@ extern void wl_view_destroy_backing_impl(unsigned long id); /* Simple view lookup table (ID → W_View*) */ #define VIEW_TABLE_SIZE 256 +#define VIEW_TABLE_TOMBSTONE ((WNativeWindow)1) /* never a valid view/fb ID */ static struct { WNativeWindow id; W_View *view; void *backing; } view_table[VIEW_TABLE_SIZE]; static void view_table_put(WNativeWindow id, W_View *view) @@ -56,7 +57,8 @@ static void view_table_put(WNativeWindow id, W_View *view) unsigned idx = ((unsigned)id) % VIEW_TABLE_SIZE; for (int i = 0; i < VIEW_TABLE_SIZE; i++) { unsigned slot = (idx + i) % VIEW_TABLE_SIZE; - if (view_table[slot].id == 0 || view_table[slot].id == id) { + if (view_table[slot].id == 0 || view_table[slot].id == id + || view_table[slot].id == VIEW_TABLE_TOMBSTONE) { view_table[slot].id = id; view_table[slot].view = view; return; @@ -71,10 +73,9 @@ W_View *W_GetViewForWindow(WNativeWindow id) unsigned slot = (idx + i) % VIEW_TABLE_SIZE; if (view_table[slot].id == id) return view_table[slot].view; - if (view_table[slot].id == 0) { - //wwarning("W_GetViewForWindow: id=0x%lx NOT FOUND (slot %u empty)", (unsigned long)id, slot); + if (view_table[slot].id == 0) return NULL; - } + /* skip tombstones */ } return NULL; } @@ -113,7 +114,7 @@ void view_table_remove(WNativeWindow id) for (int i = 0; i < VIEW_TABLE_SIZE; i++) { unsigned slot = (idx + i) % VIEW_TABLE_SIZE; if (view_table[slot].id == id) { - view_table[slot].id = 0; + view_table[slot].id = VIEW_TABLE_TOMBSTONE; view_table[slot].view = NULL; view_table[slot].backing = NULL; return; @@ -470,6 +471,11 @@ static void wl_destroy(void *vctx, W_View *view) W_WLClientToplevelDestroyFn(view); if (view->flags.topLevel && W_ViewDestroyBackingFn) W_ViewDestroyBackingFn((unsigned long)view->window); + /* Free backing image and remove from view table */ + pixman_image_t *img = (pixman_image_t *)W_GetViewBacking(view->window); + if (img) + pixman_image_unref(img); + view_table_remove(view->window); } static void wl_set_background_color(void *vctx, W_View *view, WMColor *color) diff --git a/src/backend/wayland/wl_client.c b/src/backend/wayland/wl_client.c index 38b4769..fe78045 100644 --- a/src/backend/wayland/wl_client.c +++ b/src/backend/wayland/wl_client.c @@ -93,6 +93,20 @@ wl_client_reparent(WNativeWindow client, WNativeWindow new_parent, int x, int y) + /* Reparent to root = dialog teardown: destroy the child frame_buf */ + if (client >= 0x20000000 && new_parent == (WNativeWindow)wScreenWithNumber(0)->root_win) { + struct wl_frame_buf *fb = frame_buf_find(client); + if (fb) { + extern void W_UnregisterBacking(unsigned long id); + W_UnregisterBacking(fb->id); + if (fb->scene_buf) + wlr_scene_node_destroy(&fb->scene_buf->node); + wl_list_remove(&fb->link); + wlr_buffer_drop(&fb->base); + } + return; + } + /* For WINGs internal windows: create a child frame_buf so the dialog * content is visible in the scene graph. */ if (client >= 0x20000000 && new_parent >= 0x10000000) { @@ -103,15 +117,48 @@ wl_client_reparent(WNativeWindow client, WNativeWindow new_parent, int x, int y) /* Check if we already have a frame_buf for this client */ struct wl_frame_buf *existing = frame_buf_find(client); if (existing) { - /* Just update parent and position */ + /* Update parent, position, and ensure backing is current */ existing->parent_id = new_parent; existing->rel_x = x; existing->rel_y = y; existing->x = parent_fb->x + x; existing->y = parent_fb->y + y; + + /* Resize backing image if the view size changed */ + extern W_View *W_GetViewForWindow(unsigned long id); + W_View *view = W_GetViewForWindow((unsigned long)client); + int w = view ? view->size.width : existing->width; + int h = view ? view->size.height : existing->height; + if (w <= 0) w = 1; + if (h <= 0) h = 1; + if (w != existing->width || h != existing->height) { + pixman_image_unref(existing->image); + existing->image = pixman_image_create_bits(PIXMAN_a8r8g8b8, w, h, NULL, 0); + existing->width = w; + existing->height = h; + existing->base.width = w; + existing->base.height = h; + if (existing->scene_buf) + wlr_scene_buffer_set_dest_size(existing->scene_buf, w, h); + } + + /* Re-link the view's backing to this frame_buf's image */ + extern void W_SetViewBacking(unsigned long id, void *img); + W_SetViewBacking((unsigned long)client, existing->image); + + /* Recreate scene_buf if it was destroyed (cascade from parent tree) */ + if (!existing->scene_buf && wl_state.scene) { + struct wlr_scene_tree *_wp = parent_fb->tree + ? parent_fb->tree + : (wl_state.level_trees[WL_LAYER_FLOATING] + ? wl_state.level_trees[WL_LAYER_FLOATING] + : &wl_state.scene->tree); + existing->scene_buf = wlr_scene_buffer_create(_wp, &existing->base); + if (existing->scene_buf) + fb_attach_scene_buf_listener(existing); + } + if (existing->scene_buf) { - /* Ensure it's under the parent frame's tree, then position - * at relative offset within the tree. */ if (parent_fb->tree && existing->scene_buf->node.parent != parent_fb->tree) { wlr_scene_node_reparent(&existing->scene_buf->node, @@ -122,6 +169,7 @@ wl_client_reparent(WNativeWindow client, WNativeWindow new_parent, int x, int y) else wlr_scene_node_set_position(&existing->scene_buf->node, existing->x, existing->y); + wlr_scene_node_set_enabled(&existing->scene_buf->node, false); } return; } @@ -216,7 +264,7 @@ void wl_view_commit_backing_impl(unsigned long id) if (fb) { extern void *W_GetViewBacking(unsigned long id); void *current = W_GetViewBacking((unsigned long)id); - if (current != (void *)fb->image) { + if (current && current != (void *)fb->image && !fb->parent_id) { /* Save parent relation before destroying — the new frame_buf * must be positioned within the same parent tree so dialog * content reparented via wl_client_reparent stays visible. */ diff --git a/src/backend/wayland/wl_framebuf.c b/src/backend/wayland/wl_framebuf.c index 6977b47..010384e 100644 --- a/src/backend/wayland/wl_framebuf.c +++ b/src/backend/wayland/wl_framebuf.c @@ -456,6 +456,19 @@ wl_frame_destroy(WScreen *scr, WNativeWindow win) (void)scr; struct wl_frame_buf *fb = frame_buf_find(win); if (!fb) return; + + /* Destroy child frame_bufs whose parent is this one */ + struct wl_frame_buf *child, *tmp; + wl_list_for_each_safe(child, tmp, &wl_state.frame_list, link) { + if (child->parent_id == win) { + W_UnregisterBacking(child->id); + if (child->scene_buf) + wlr_scene_node_destroy(&child->scene_buf->node); + wl_list_remove(&child->link); + wlr_buffer_drop(&child->base); + } + } + W_UnregisterBacking(fb->id); /* Destroy the scene_buf and (if any) the tree. Without destroying * the tree, leaked tree nodes accumulate in the scene graph and