From c8d638b35ae7715cc3494c5270817983145a5b9b Mon Sep 17 00:00:00 2001 From: Marius Vlad Date: Fri, 30 Apr 2021 21:53:16 +0300 Subject: [PATCH 01/46] kiosk-shell: Check if app_ids have been set after initial commit Some applications would set-up the app_id after the initial commit (without a buffer) which is too late to correctly assign the application to the corresponding output set-up in the configuration file. This patch fixes that by checking one more time, after a buffer has been attached, if indeed there's an output with an app_id set. Fixes: #469 Signed-off-by: Marius Vlad (cherry picked from commit 8a1849db8a3e1d1e3bc5da33d5b78f600fe19fe7) --- kiosk-shell/kiosk-shell.c | 25 ++++++++++++++++++++++++- kiosk-shell/kiosk-shell.h | 2 ++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/kiosk-shell/kiosk-shell.c b/kiosk-shell/kiosk-shell.c index 5995baea..477c85c3 100644 --- a/kiosk-shell/kiosk-shell.c +++ b/kiosk-shell/kiosk-shell.c @@ -173,8 +173,10 @@ kiosk_shell_surface_find_best_output(struct kiosk_shell_surface *shsurf) app_id = weston_desktop_surface_get_app_id(shsurf->desktop_surface); if (app_id) { wl_list_for_each(shoutput, &shsurf->shell->output_list, link) { - if (kiosk_shell_output_has_app_id(shoutput, app_id)) + if (kiosk_shell_output_has_app_id(shoutput, app_id)) { + shsurf->appid_output_assigned = true; return shoutput->output; + } } } @@ -354,6 +356,7 @@ kiosk_shell_surface_create(struct kiosk_shell *shell, shsurf->desktop_surface = desktop_surface; shsurf->view = view; shsurf->shell = shell; + shsurf->appid_output_assigned = false; weston_desktop_surface_set_user_data(desktop_surface, shsurf); @@ -721,6 +724,8 @@ desktop_surface_committed(struct weston_desktop_surface *desktop_surface, weston_desktop_surface_get_user_data(desktop_surface); struct weston_surface *surface = weston_desktop_surface_get_surface(desktop_surface); + const char *app_id = + weston_desktop_surface_get_app_id(desktop_surface); bool is_resized; bool is_fullscreen; @@ -729,6 +734,24 @@ desktop_surface_committed(struct weston_desktop_surface *desktop_surface, if (surface->width == 0) return; + if (!shsurf->appid_output_assigned && app_id) { + struct weston_output *output = NULL; + + /* reset previous output being set in _added() as the output is + * being cached */ + shsurf->output = NULL; + output = kiosk_shell_surface_find_best_output(shsurf); + + kiosk_shell_surface_set_output(shsurf, output); + weston_desktop_surface_set_size(shsurf->desktop_surface, + shsurf->output->width, + shsurf->output->height); + /* even if we couldn't find an appid set for a particular + * output still flag the shsurf as to a avoid changing the + * output every time */ + shsurf->appid_output_assigned = true; + } + /* TODO: When the top-level surface is committed with a new size after an * output resize, sometimes the view appears scaled. What state are we not * updating? diff --git a/kiosk-shell/kiosk-shell.h b/kiosk-shell/kiosk-shell.h index 9f680806..070ba1ab 100644 --- a/kiosk-shell/kiosk-shell.h +++ b/kiosk-shell/kiosk-shell.h @@ -73,6 +73,8 @@ struct kiosk_shell_surface { int32_t x; int32_t y; } xwayland; + + bool appid_output_assigned; }; struct kiosk_shell_seat { From 52c924ad7e7dae027ba6197ea1ac04b24d69cbd5 Mon Sep 17 00:00:00 2001 From: Marius Vlad Date: Thu, 9 Dec 2021 12:22:53 +0200 Subject: [PATCH 02/46] kiosk-shell: Don't occlude shsurf on other outputs This adds an additional check to make sure the current focus surface is on the same output as the surface that is going to be activated. This is necessary in order to avoid placing the currently focused one in the inactive layer, which shouldn't happen in situations where the new surface is going to be placed on a different output than the currently focused one. Signed-off-by: Marius Vlad (cherry picked from commit f3ad5939255daf0d39c9d53605e678f4e0ece968) --- kiosk-shell/kiosk-shell.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/kiosk-shell/kiosk-shell.c b/kiosk-shell/kiosk-shell.c index 477c85c3..b63116c8 100644 --- a/kiosk-shell/kiosk-shell.c +++ b/kiosk-shell/kiosk-shell.c @@ -390,8 +390,10 @@ kiosk_shell_surface_activate(struct kiosk_shell_surface *shsurf, /* removes it from the normal_layer and move it to inactive * one, without occluding the top-level window if the new one - * is a child to that */ - if (!shsurf->parent) { + * is a child to that. Also, do not occlude another view + * (currently focused one) on a different output when activating + * a new one. */ + if (!shsurf->parent && (shsurf->output == current_focus->output)) { weston_layer_entry_remove(¤t_focus->view->layer_link); weston_layer_entry_insert(&shsurf->shell->inactive_layer.view_list, ¤t_focus->view->layer_link); From a08be33890a47da2da9487f3008a6e6b99371c24 Mon Sep 17 00:00:00 2001 From: Marius Vlad Date: Wed, 22 Dec 2021 16:52:12 +0200 Subject: [PATCH 03/46] kiosk-shell: Favor out views on same output In multiple output cases, finding the succesor from the inactive layer might result in picking the wrong view when there are multiple views being stacked in the inactive layer. This adds two additional checks to favor views on the same output as the one being destroyed/removed. Signed-off-by: Marius Vlad (cherry picked from commit f3221832c504fe2d3bb0aa7c2ecc1d55a0154e0a) --- kiosk-shell/kiosk-shell.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/kiosk-shell/kiosk-shell.c b/kiosk-shell/kiosk-shell.c index b63116c8..14857ecc 100644 --- a/kiosk-shell/kiosk-shell.c +++ b/kiosk-shell/kiosk-shell.c @@ -650,12 +650,18 @@ find_focus_successor(struct weston_layer *layer, struct weston_view *top_view = NULL; struct weston_view *view; + /* we need to take into account that the surface being destroyed it not * always the same as the focus_surface, which could result in picking * and *activating* the wrong window, so avoid returning a view for * that case. A particular case is when a top-level child window, would - * pick a parent window below the focused_surface. */ - if (focused_surface != shsurf->view->surface) + * pick a parent window below the focused_surface. + * + * Apply that only on the same output to avoid incorrectly returning an + * invalid/empty view, which could happen if the view being destroyed + * is on a output different than the focused_surface output */ + if (focused_surface && focused_surface != shsurf->view->surface && + shsurf->output == focused_surface->output) return top_view; wl_list_for_each(view, &layer->view_list.link, layer_link.link) { @@ -665,6 +671,10 @@ find_focus_successor(struct weston_layer *layer, if (!view->is_mapped || view == shsurf->view) continue; + /* pick views only on the same output */ + if (view->output != shsurf->output) + continue; + view_shsurf = get_kiosk_shell_surface(view->surface); if (!view_shsurf) continue; From 673943d730a5800c033af119a98dff0112e9ed0e Mon Sep 17 00:00:00 2001 From: Robert Mader Date: Fri, 31 Dec 2021 15:49:34 +0100 Subject: [PATCH 04/46] libweston/compositor: Cache buffer damage for synced subsurfaces The spec states: > Because buffer transformation changes and damage requests may be > interleaved in the protocol stream, it is impossible to determine > the actual mapping between surface and buffer damage until > wl_surface.commit time. Therefore, compositors wishing to take both > kinds of damage into account will have to accumulate damage from the > two requests separately and only transform from one to the other after > receiving the wl_surface.commit. For subsurfaces in sync mode, arguably the same is the case until the cached state gets applied eventually. Thus, in order to keep complexity to a sane level, just accumulate buffer damage and convert it only when the cached state gets applied. This mirrors how other compositors like Mutter implement cached damage and what the spec arguably should demand. Closes https://gitlab.freedesktop.org/wayland/weston/-/issues/446 Signed-off-by: Robert Mader (cherry picked from commit 933290e6eadb37a984dba78562f1e499a5f41679) --- libweston/compositor.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/libweston/compositor.c b/libweston/compositor.c index 1670c500..452d32f7 100644 --- a/libweston/compositor.c +++ b/libweston/compositor.c @@ -4211,6 +4211,11 @@ weston_subsurface_commit_to_cache(struct weston_subsurface *sub) &surface->pending.damage_surface); pixman_region32_clear(&surface->pending.damage_surface); + pixman_region32_union(&sub->cached.damage_buffer, + &sub->cached.damage_buffer, + &surface->pending.damage_buffer); + pixman_region32_clear(&surface->pending.damage_buffer); + if (surface->pending.newly_attached) { sub->cached.newly_attached = 1; weston_surface_state_set_buffer(&sub->cached, @@ -4233,8 +4238,6 @@ weston_subsurface_commit_to_cache(struct weston_subsurface *sub) sub->cached.sx += surface->pending.sx; sub->cached.sy += surface->pending.sy; - apply_damage_buffer(&sub->cached.damage_surface, surface, &surface->pending); - sub->cached.buffer_viewport.changed |= surface->pending.buffer_viewport.changed; sub->cached.buffer_viewport.buffer = From 2fa690a9a96431e76242fed834b287555e1cfcea Mon Sep 17 00:00:00 2001 From: Robert Mader Date: Tue, 25 Jan 2022 17:56:46 +0100 Subject: [PATCH 05/46] tests: Add test for synced subsurfaces and buffer damage Changing `wl_surface_damage()` to `wl_surface_damage_buffer()` should not have an effect on the existing tests. The new test will fail without the commit "libweston/compositor: Cache buffer damage for synced subsurfaces" Signed-off-by: Robert Mader (cherry picked from commit dc3b3493259ee55df2429af30d1ddf9e5e9a45d2) --- .../subsurface_sync_damage_buffer-00.png | Bin 0 -> 825 bytes .../subsurface_sync_damage_buffer-01.png | Bin 0 -> 858 bytes .../subsurface_sync_damage_buffer-02.png | Bin 0 -> 857 bytes tests/subsurface-shot-test.c | 69 +++++++++++++++++- 4 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 tests/reference/subsurface_sync_damage_buffer-00.png create mode 100644 tests/reference/subsurface_sync_damage_buffer-01.png create mode 100644 tests/reference/subsurface_sync_damage_buffer-02.png diff --git a/tests/reference/subsurface_sync_damage_buffer-00.png b/tests/reference/subsurface_sync_damage_buffer-00.png new file mode 100644 index 0000000000000000000000000000000000000000..f127154cb68edf8e0148ac0de00ea5ca360b371b GIT binary patch literal 825 zcmeAS@N?(olHy`uVBq!ia0y~yU~~YoKX5Ps$$$P@Hb9Ck$=lt9;Xep2*t>i(0|V1L zPZ!6KiaBquI`T3(inusVQ%IX=T;|45HlbP8=FQXRsjEf4JYCvf{FeQ~mpO6_G3p5% z!Uipz5{FoL5}Dcz7@cP{DEb@_P@})_w^?!lpMKXZQ%~7*ecrqRZn;6=rU3{BqO9ET rIHtOaxnLsufW{b6PNij%+Fi(0|PU^ zr;B4q#hkZyHu4^F5Mg!f(Kt5cahZ>wwO)IX!Xn1x!l)T1{i{yCN;f}yx{7Jmxy9@Z zzd0oivG61^wHYuv&uCEeIUt~xz#(kVG62G{Zu}aRIo~e|8?8HSRdp!7pph)+d{)0U z-Dmmp7o7LEzOA>?-~PDlkA?W4@g&W{b$5?kKvC?qzx_ZDADGQeR+Qc_ssD6d*kB5I hx(7J9rfODhL8pZOKY^Kr!PC{xWt~$(69BBr`2GL@ literal 0 HcmV?d00001 diff --git a/tests/reference/subsurface_sync_damage_buffer-02.png b/tests/reference/subsurface_sync_damage_buffer-02.png new file mode 100644 index 0000000000000000000000000000000000000000..9ef82e06c45ebdd1e1ad0689517c71ef17752649 GIT binary patch literal 857 zcmeAS@N?(olHy`uVBq!ia0y~yU~~YoKX5Ps$$$P@Hb9Ck$=lt9;Xep2*t>i(0|PUk zr;B4q#hkZyHu4^F5Mg!f(Kt3G`BWieUVD)SyHZig1b^+cPm|*|{krMimage, color); wl_surface_attach(surface, buf->proxy, 0, 0); - wl_surface_damage(surface, 0, 0, width, height); + wl_surface_damage_buffer(surface, 0, 0, width, height); wl_surface_commit(surface); return buf; @@ -213,3 +213,70 @@ TEST(subsurface_z_order) wl_subcompositor_destroy(subco); client_destroy(client); } + +TEST(subsurface_sync_damage_buffer) +{ + struct client *client; + struct wl_subcompositor *subco; + struct buffer *bufs[2] = { 0 }; + struct wl_surface *surf[2] = { 0 }; + struct wl_subsurface *sub[2] = { 0 }; + struct rectangle clip = { 40, 40, 280, 200 }; + int fail = 0; + unsigned i; + pixman_color_t red; + pixman_color_t blue; + pixman_color_t green; + + color_rgb888(&red, 255, 0, 0); + color_rgb888(&blue, 0, 0, 255); + color_rgb888(&green, 0, 255, 0); + + client = create_client_and_test_surface(100, 50, 100, 100); + assert(client); + subco = get_subcompositor(client); + + /* move the pointer clearly away from our screenshooting area */ + weston_test_move_pointer(client->test->weston_test, 0, 1, 0, 2, 30); + + /* make the parent surface red */ + surf[0] = client->surface->wl_surface; + client->surface->wl_surface = NULL; /* we stole it and destroy it */ + bufs[0] = surface_commit_color(client, surf[0], &red, 100, 100); + /* sub[0] is not used */ + + fail += check_screen(client, "subsurface_sync_damage_buffer", 0, &clip, 0); + + /* create a blue sub-surface above red */ + surf[1] = wl_compositor_create_surface(client->wl_compositor); + sub[1] = wl_subcompositor_get_subsurface(subco, surf[1], surf[0]); + bufs[1] = surface_commit_color(client, surf[1], &blue, 100, 100); + + wl_subsurface_set_position(sub[1], 20, 20); + wl_surface_commit(surf[0]); + + fail += check_screen(client, "subsurface_sync_damage_buffer", 1, &clip, 1); + + buffer_destroy(bufs[1]); + bufs[1] = surface_commit_color(client, surf[1], &green, 100, 100); + wl_surface_commit(surf[0]); + + fail += check_screen(client, "subsurface_sync_damage_buffer", 2, &clip, 2); + + assert(fail == 0); + + for (i = 0; i < ARRAY_LENGTH(sub); i++) + if (sub[i]) + wl_subsurface_destroy(sub[i]); + + for (i = 0; i < ARRAY_LENGTH(surf); i++) + if (surf[i]) + wl_surface_destroy(surf[i]); + + for (i = 0; i < ARRAY_LENGTH(bufs); i++) + if (bufs[i]) + buffer_destroy(bufs[i]); + + wl_subcompositor_destroy(subco); + client_destroy(client); +} From 7c30ab2dbdb6075e9368098123e966792ddafd69 Mon Sep 17 00:00:00 2001 From: Robert Mader Date: Wed, 26 Jan 2022 22:04:52 +0100 Subject: [PATCH 06/46] libweston/compositor: Do not map subsurfaces without buffer We can end in `subsurface_committed()` in different scenarios without the surface having an attached buffer. While setting the mapped state to `true` in that case doesn't matter for that (sub)surface itself, it triggers its own child subsurfaces to get mapped when they shouldn't. Closes https://gitlab.freedesktop.org/wayland/weston/-/issues/426 Signed-off-by: Robert Mader (cherry picked from commit 8b04534c76390cda00059e804b6f3f0bf92a56b8) --- libweston/compositor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libweston/compositor.c b/libweston/compositor.c index 452d32f7..3a838ec7 100644 --- a/libweston/compositor.c +++ b/libweston/compositor.c @@ -4364,7 +4364,7 @@ subsurface_committed(struct weston_surface *surface, int32_t dx, int32_t dy) */ if (!weston_surface_is_mapped(surface)) { - surface->is_mapped = true; + surface->is_mapped = surface->buffer_ref.buffer != NULL; /* Cannot call weston_view_update_transform(), * because that would call it also for the parent surface, From 7818b054da4b069fdff059ae55c4ab1723590172 Mon Sep 17 00:00:00 2001 From: Robert Mader Date: Wed, 26 Jan 2022 12:53:31 +0100 Subject: [PATCH 07/46] tests: Add test for subsurfaces mapping hierachies Test different scenarios where child subsurfaces of unmapped subsurfaces would get mapped. This test will fail in various ways without the commit "libweston/compositor: Do not map subsurfaces without buffer" Also try to test potential regressions of that patch. Signed-off-by: Robert Mader (cherry picked from commit c83f0a153927d8d775e093b6a8a99bd42e16a755) --- .../reference/subsurface_empty_mapping-00.png | Bin 0 -> 825 bytes .../reference/subsurface_empty_mapping-01.png | Bin 0 -> 887 bytes tests/subsurface-shot-test.c | 129 ++++++++++++++++++ 3 files changed, 129 insertions(+) create mode 100644 tests/reference/subsurface_empty_mapping-00.png create mode 100644 tests/reference/subsurface_empty_mapping-01.png diff --git a/tests/reference/subsurface_empty_mapping-00.png b/tests/reference/subsurface_empty_mapping-00.png new file mode 100644 index 0000000000000000000000000000000000000000..f127154cb68edf8e0148ac0de00ea5ca360b371b GIT binary patch literal 825 zcmeAS@N?(olHy`uVBq!ia0y~yU~~YoKX5Ps$$$P@Hb9Ck$=lt9;Xep2*t>i(0|V1L zPZ!6KiaBquI`T3(inusVQ%IX=T;|45HlbP8=FQXRsjEf4JYCvf{FeQ~mpO6_G3p5% z!Uipz5{FoL5}Dcz7@cP{DEb@_P@})_w^?!lpMKXZQ%~7*ecrqRZn;6=rU3{BqO9ET rIHtOaxnLsufW{b6PNij%+Fi(0|T>; zr;B4q#hkZyHs&675Mgz^7+@G;`MhYwowqAwR!dtJF`FDbe9HJIqkiO{H`7XU#236w z{>G4C&*(g(LDA=cfLa2Fut5u_#32@*M5Z4^3a+25v=P|zcmHh{{K++_YU9fzz0l&CA0`d zP}yAD1y9Vj{q2`O{{ca}tB6tDnm{r-UW|ZA1@J literal 0 HcmV?d00001 diff --git a/tests/subsurface-shot-test.c b/tests/subsurface-shot-test.c index f9b967ea..16534497 100644 --- a/tests/subsurface-shot-test.c +++ b/tests/subsurface-shot-test.c @@ -280,3 +280,132 @@ TEST(subsurface_sync_damage_buffer) wl_subcompositor_destroy(subco); client_destroy(client); } + +TEST(subsurface_empty_mapping) +{ + struct client *client; + struct wl_subcompositor *subco; + struct wp_viewporter *viewporter; + struct buffer *bufs[3] = { 0 }; + struct wl_surface *surf[3] = { 0 }; + struct wl_subsurface *sub[3] = { 0 }; + struct wp_viewport *viewport; + struct rectangle clip = { 40, 40, 280, 200 }; + int fail = 0; + unsigned i; + pixman_color_t red; + pixman_color_t blue; + pixman_color_t green; + + color_rgb888(&red, 255, 0, 0); + color_rgb888(&blue, 0, 0, 255); + color_rgb888(&green, 0, 255, 0); + + client = create_client_and_test_surface(100, 50, 100, 100); + assert(client); + subco = get_subcompositor(client); + viewporter = bind_to_singleton_global(client, + &wp_viewporter_interface, 1); + + /* move the pointer clearly away from our screenshooting area */ + weston_test_move_pointer(client->test->weston_test, 0, 1, 0, 2, 30); + + /* make the parent surface red */ + surf[0] = client->surface->wl_surface; + client->surface->wl_surface = NULL; /* we stole it and destroy it */ + bufs[0] = surface_commit_color(client, surf[0], &red, 100, 100); + /* sub[0] is not used */ + + fail += check_screen(client, "subsurface_empty_mapping", 0, &clip, 0); + + /* create an empty subsurface on top */ + surf[1] = wl_compositor_create_surface(client->wl_compositor); + sub[1] = wl_subcompositor_get_subsurface(subco, surf[1], surf[0]); + wl_subsurface_set_desync (sub[1]); + + wl_subsurface_set_position(sub[1], 20, 20); + wl_surface_commit(surf[0]); + + fail += check_screen(client, "subsurface_empty_mapping", 0, &clip, 1); + + /* create a green subsurface on top */ + surf[2] = wl_compositor_create_surface(client->wl_compositor); + sub[2] = wl_subcompositor_get_subsurface(subco, surf[2], surf[1]); + wl_subsurface_set_desync (sub[2]); + bufs[2] = surface_commit_color(client, surf[2], &green, 100, 100); + + wl_subsurface_set_position(sub[2], 20, 20); + wl_surface_commit(surf[1]); + + fail += check_screen(client, "subsurface_empty_mapping", 0, &clip, 2); + + wl_surface_attach(surf[1], NULL, 0, 0); + wl_surface_commit(surf[1]); + + fail += check_screen(client, "subsurface_empty_mapping", 0, &clip, 3); + + wl_surface_set_buffer_scale (surf[1], 1); + wl_surface_commit(surf[1]); + + fail += check_screen(client, "subsurface_empty_mapping", 0, &clip, 4); + + viewport = wp_viewporter_get_viewport(viewporter, surf[1]); + wp_viewport_set_destination(viewport, 5, 5); + wl_surface_commit(surf[1]); + + fail += check_screen(client, "subsurface_empty_mapping", 0, &clip, 5); + + wp_viewport_set_destination(viewport, -1, -1); + wl_surface_commit(surf[1]); + + fail += check_screen(client, "subsurface_empty_mapping", 0, &clip, 6); + + /* map the previously empty middle surface with a blue buffer */ + bufs[1] = surface_commit_color(client, surf[1], &blue, 100, 100); + + fail += check_screen(client, "subsurface_empty_mapping", 1, &clip, 7); + + /* try to trigger a recomputation of the buffer size with the + * shm-buffer potentially being released already */ + wl_surface_set_buffer_scale (surf[1], 1); + wl_surface_commit(surf[1]); + + fail += check_screen(client, "subsurface_empty_mapping", 1, &clip, 8); + + /* try more */ + wp_viewport_set_destination(viewport, 100, 100); + wl_surface_commit(surf[1]); + + fail += check_screen(client, "subsurface_empty_mapping", 1, &clip, 9); + + /* unmap the middle surface again to ensure recursive unmapping */ + wl_surface_attach(surf[1], NULL, 0, 0); + wl_surface_commit(surf[1]); + + fail += check_screen(client, "subsurface_empty_mapping", 0, &clip, 10); + + /* remap middle surface to ensure recursive mapping */ + bufs[1] = surface_commit_color(client, surf[1], &blue, 100, 100); + + fail += check_screen(client, "subsurface_empty_mapping", 1, &clip, 11); + + assert(fail == 0); + + wp_viewport_destroy(viewport); + + for (i = 0; i < ARRAY_LENGTH(sub); i++) + if (sub[i]) + wl_subsurface_destroy(sub[i]); + + for (i = 0; i < ARRAY_LENGTH(surf); i++) + if (surf[i]) + wl_surface_destroy(surf[i]); + + for (i = 0; i < ARRAY_LENGTH(bufs); i++) + if (bufs[i]) + buffer_destroy(bufs[i]); + + wp_viewporter_destroy(viewporter); + wl_subcompositor_destroy(subco); + client_destroy(client); +} From a40ae68853d1ce18f5cdb201a83f91e7734eb114 Mon Sep 17 00:00:00 2001 From: James Le Cuirot Date: Fri, 4 Feb 2022 23:43:50 +0000 Subject: [PATCH 08/46] meson.build: Fix -Dbackend-default=auto following fbdev deprecation Signed-off-by: James Le Cuirot (cherry picked from commit 89587db3cba43a057714a4d4e5593ae56939f789) --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index 7b80214d..d95f82d5 100644 --- a/meson.build +++ b/meson.build @@ -121,7 +121,7 @@ config_h.set10('TEST_GL_RENDERER', get_option('test-gl-renderer')) backend_default = get_option('backend-default') if backend_default == 'auto' - foreach b : [ 'headless', 'fbdev', 'x11', 'wayland', 'drm' ] + foreach b : [ 'headless', 'x11', 'wayland', 'drm' ] if get_option('backend-' + b) backend_default = b endif From e4522507ad76a56e6838825e8f92779d234144f1 Mon Sep 17 00:00:00 2001 From: Marius Vlad Date: Mon, 7 Feb 2022 11:05:14 +0200 Subject: [PATCH 09/46] libweston: Assert if ref-count balance is wrong Calling weston_surface_destroy() one time too many could be a sign we haven't correctly increased the ref count for it. Also, if we don't have a surface being passed, do no attempt to use it. Signed-off-by: Marius Vlad (cherry picked from commit d3ed2eb34533e901f4d02c45058318bc3f922cb2) --- libweston/compositor.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libweston/compositor.c b/libweston/compositor.c index 3a838ec7..ff3fa45b 100644 --- a/libweston/compositor.c +++ b/libweston/compositor.c @@ -2306,6 +2306,10 @@ weston_surface_destroy(struct weston_surface *surface) struct weston_pointer_constraint *constraint, *next_constraint; struct weston_paint_node *pnode, *pntmp; + if (!surface) + return; + + assert(surface->ref_count > 0); if (--surface->ref_count > 0) return; From 8df487e0ff3b79e7411a19d302a475217f7d1016 Mon Sep 17 00:00:00 2001 From: Marius Vlad Date: Mon, 14 Feb 2022 22:42:22 +0200 Subject: [PATCH 10/46] libweston, desktop-shell: Add a wrapper for weston_surface reference Similar to how we do it with drm_fb ref counts, increase a reference count and return the same object. Plug-in in desktop-shell when we map up the view in order to survive a weston_surface destruction. Astute readers will notice that this patch removes weston_view_destroy() while keeping the balance between removing and adding a weston_surface_unref() call in desktop_shell_destroy_surface(). The reason is to let weston_surface_unref() handle destruction on its own. If multiple references are taken, then weston_surface_unref() doesn't destroy the view, it just decreases the reference, with a latter call to weston_surface_unref() to determine if the view should be destroyed as well. This situation happens if we have close animation enabled, were we have more than one reference taken: one when mapping the view/surface and when when the surface itself was created, (what we call, a weak reference). If only a single reference is taken (for instance if we don't have close animations enabled) then this weston_surface_unref() call is inert as that reference is not set-up, leaving libweston to handle the view destruction. Following that with a weston_view_destroy() explicit call would cause a UAF as the view was previous destroyed by a weston_surface_unref() call. A side-effect of not keeping the weston_view_destroy() call would happen when tearing down the compositor. If close animations are enabled, weston_surface_unref() would not destroy the view, and because weston_view_destroy() no longer exists, we would still have the view in the other layers by the time we check-up if layers have views present. Signed-off-by: Marius Vlad (cherry picked from commit bd8314078de81cf19bfc75e23c12d730dc18c63e) While the original patch was adding a weston_surface_ref() wrapper, this patch doesn't add any wrapper to avoid a minor version bump, but instead achieves a similar outcome by inlining the wrapper version being added by the original commit. Further more, as the original patch was dependent on another patch, any mention of weston_surface_unref() in the commit description should instead be replaced with weston_surface_destroy(). Signed-off-by: Marius Vlad --- desktop-shell/shell.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c index 4ca78979..31258e81 100644 --- a/desktop-shell/shell.c +++ b/desktop-shell/shell.c @@ -101,6 +101,7 @@ struct shell_surface { struct weston_desktop_surface *desktop_surface; struct weston_view *view; + struct weston_surface *wsurface_anim_fade; int32_t last_width, last_height; struct desktop_shell *shell; @@ -263,8 +264,8 @@ desktop_shell_destroy_surface(struct shell_surface *shsurf) wl_list_remove(&shsurf->children_link); wl_signal_emit(&shsurf->destroy_signal, shsurf); + weston_surface_destroy(shsurf->wsurface_anim_fade); - weston_view_destroy(shsurf->view); if (shsurf->output_destroy_listener.notify) { wl_list_remove(&shsurf->output_destroy_listener.link); shsurf->output_destroy_listener.notify = NULL; @@ -2376,8 +2377,6 @@ desktop_surface_removed(struct weston_desktop_surface *desktop_surface, weston_fade_run(shsurf->view, 1.0, 0.0, 300.0, fade_out_done, shsurf); return; - } else { - weston_surface_destroy(surface); } } @@ -2500,8 +2499,12 @@ desktop_surface_committed(struct weston_desktop_surface *desktop_surface, if (!weston_surface_is_mapped(surface)) { map(shell, shsurf, sx, sy); surface->is_mapped = true; - if (shsurf->shell->win_close_animation_type == ANIMATION_FADE) - ++surface->ref_count; + /* as we need to survive the weston_surface destruction we'll + * need to take another reference */ + if (shsurf->shell->win_close_animation_type == ANIMATION_FADE) { + surface->ref_count++; + shsurf->wsurface_anim_fade = surface; + } return; } From e34bb982990def85d3f7c4da39387f358bb21439 Mon Sep 17 00:00:00 2001 From: Marius Vlad Date: Mon, 14 Feb 2022 22:57:08 +0200 Subject: [PATCH 11/46] desktop-shell: Create a distinct view for the fade-out close anim Creates a distinct view, separated from the one created by libweston-desktop, in order to avoid a potential ownership fight with libweston-desktop upon destroying the view. Upon weston_desktop_surface destruction, libweston-desktop inflicts damage on the view it creates, so we need the view to be alive at that time. This wasn't such an issue before because we had different destruction paths but with commit 'desktop-shell: Do not leave views in layers upon shell destruction' all of the destruction paths now land in the same spot + handle compositor tear down. Note as we still use the same weston_surface we'll keep the previous construct where we were taking a reference to keep it alive. The original view is destroyed when releasing the ownership, while for the view created in this patch we handle the destruction directly upon compositor tear down. Signed-off-by: Marius Vlad (cherry picked from commit 9cf602840d0c49d882d84b6cfedfd44274f1c966) --- desktop-shell/shell.c | 53 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 50 insertions(+), 3 deletions(-) diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c index 31258e81..2d3a9633 100644 --- a/desktop-shell/shell.c +++ b/desktop-shell/shell.c @@ -102,6 +102,7 @@ struct shell_surface { struct weston_desktop_surface *desktop_surface; struct weston_view *view; struct weston_surface *wsurface_anim_fade; + struct weston_view *wview_anim_fade; int32_t last_width, last_height; struct desktop_shell *shell; @@ -195,6 +196,10 @@ struct shell_seat { }; +static struct weston_view * +shell_fade_create_fade_out_view(struct shell_surface *shsurf, + struct weston_surface *surface); + static struct desktop_shell * shell_surface_get_shell(struct shell_surface *shsurf); @@ -2254,8 +2259,8 @@ fade_out_done(struct weston_view_animation *animation, void *data) loop = wl_display_get_event_loop(shsurf->shell->compositor->wl_display); - if (weston_view_is_mapped(shsurf->view)) { - weston_view_unmap(shsurf->view); + if (weston_view_is_mapped(shsurf->wview_anim_fade)) { + weston_view_unmap(shsurf->wview_anim_fade); wl_event_loop_add_idle(loop, fade_out_done_idle_cb, shsurf); } } @@ -2374,8 +2379,25 @@ desktop_surface_removed(struct weston_desktop_surface *desktop_surface, pixman_region32_init(&surface->pending.input); pixman_region32_fini(&surface->input); pixman_region32_init(&surface->input); - weston_fade_run(shsurf->view, 1.0, 0.0, 300.0, + + /* its location might have changed, but also might've + * migrated to a different output, so re-compute this + * as the animation requires having the same output as + * the view */ + weston_view_set_output(shsurf->wview_anim_fade, + shsurf->view->output); + weston_view_set_position(shsurf->wview_anim_fade, + shsurf->view->geometry.x, + shsurf->view->geometry.y); + + weston_layer_entry_insert(&shsurf->view->layer_link, + &shsurf->wview_anim_fade->layer_link); + + /* unmap the "original" view */ + weston_view_unmap(shsurf->view); + weston_fade_run(shsurf->wview_anim_fade, 1.0, 0.0, 300.0, fade_out_done, shsurf); + return; } } @@ -2504,6 +2526,8 @@ desktop_surface_committed(struct weston_desktop_surface *desktop_surface, if (shsurf->shell->win_close_animation_type == ANIMATION_FADE) { surface->ref_count++; shsurf->wsurface_anim_fade = surface; + shsurf->wview_anim_fade = + shell_fade_create_fade_out_view(shsurf, surface); } return; } @@ -3995,6 +4019,29 @@ shell_fade_create_surface_for_output(struct desktop_shell *shell, struct shell_o return view; } +static struct weston_view * +shell_fade_create_fade_out_view(struct shell_surface *shsurf, + struct weston_surface *surface) +{ + struct weston_view *view; + struct weston_output *woutput; + + view = weston_view_create(surface); + if (!view) + return NULL; + + woutput = get_focused_output(surface->compositor); + /* set the initial position and output just in case we happen to not + * move it around and just destroy it */ + weston_view_set_output(view, woutput); + weston_view_set_position(view, + shsurf->view->geometry.x, + shsurf->view->geometry.y); + view->is_mapped = true; + + return view; +} + static void shell_fade(struct desktop_shell *shell, enum fade_type type) { From 1d95c0992b62a8f7c3b647c18fbb9f64ef4a8ac3 Mon Sep 17 00:00:00 2001 From: Marius Vlad Date: Thu, 5 May 2022 15:38:14 +0300 Subject: [PATCH 12/46] desktop-shell: Rename destroy_layer functions No functional change. Makes it obvious that we also call weston_layer_fini(). Signed-off-by: Marius Vlad (cherry picked from commit be5b6f9cdc9fe7a8b21f6a085845fac7972e5dd0) --- desktop-shell/shell.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c index 2d3a9633..a4a0f3db 100644 --- a/desktop-shell/shell.c +++ b/desktop-shell/shell.c @@ -883,7 +883,7 @@ animate_focus_change(struct desktop_shell *shell, struct workspace *ws, } static void -desktop_shell_destroy_views_on_layer(struct weston_layer *layer); +desktop_shell_destroy_layer(struct weston_layer *layer); static void workspace_destroy(struct workspace *ws) @@ -898,7 +898,7 @@ workspace_destroy(struct workspace *ws) if (ws->fsurf_back) focus_surface_destroy(ws->fsurf_back); - desktop_shell_destroy_views_on_layer(&ws->layer); + desktop_shell_destroy_layer(&ws->layer); free(ws); } @@ -4955,7 +4955,7 @@ setup_output_destroy_handler(struct weston_compositor *ec, } static void -desktop_shell_destroy_views_on_layer(struct weston_layer *layer) +desktop_shell_destroy_layer(struct weston_layer *layer) { struct weston_view *view, *view_next; @@ -5020,12 +5020,12 @@ shell_destroy(struct wl_listener *listener, void *data) workspace_destroy(*ws); wl_array_release(&shell->workspaces.array); - desktop_shell_destroy_views_on_layer(&shell->panel_layer); - desktop_shell_destroy_views_on_layer(&shell->background_layer); - desktop_shell_destroy_views_on_layer(&shell->lock_layer); - desktop_shell_destroy_views_on_layer(&shell->input_panel_layer); - desktop_shell_destroy_views_on_layer(&shell->minimized_layer); - desktop_shell_destroy_views_on_layer(&shell->fullscreen_layer); + desktop_shell_destroy_layer(&shell->panel_layer); + desktop_shell_destroy_layer(&shell->background_layer); + desktop_shell_destroy_layer(&shell->lock_layer); + desktop_shell_destroy_layer(&shell->input_panel_layer); + desktop_shell_destroy_layer(&shell->minimized_layer); + desktop_shell_destroy_layer(&shell->fullscreen_layer); free(shell->client); free(shell); From 8c9dff014642a6e71b4890c54c1fb211344b0488 Mon Sep 17 00:00:00 2001 From: Marius Vlad Date: Thu, 5 May 2022 15:32:27 +0300 Subject: [PATCH 13/46] desktop-shell: Migrate surface_unlink_view Moving weston_desktop_surface_unlink_view() to desktop_shell_destroy_surface() makes sure we don't leak the underlying weston_desktop_view when tearing/shutting down the compositor. Signed-off-by: Marius Vlad (cherry picked from commit c41cdcabb48eb763bbff78e5d0433b6a193db62d) --- desktop-shell/shell.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c index a4a0f3db..e4ffdc38 100644 --- a/desktop-shell/shell.c +++ b/desktop-shell/shell.c @@ -267,6 +267,7 @@ desktop_shell_destroy_surface(struct shell_surface *shsurf) wl_list_init(&shsurf_child->children_link); } wl_list_remove(&shsurf->children_link); + weston_desktop_surface_unlink_view(shsurf->view); wl_signal_emit(&shsurf->destroy_signal, shsurf); weston_surface_destroy(shsurf->wsurface_anim_fade); @@ -2370,7 +2371,6 @@ desktop_surface_removed(struct weston_desktop_surface *desktop_surface, weston_desktop_surface_set_user_data(shsurf->desktop_surface, NULL); shsurf->desktop_surface = NULL; - weston_desktop_surface_unlink_view(shsurf->view); if (weston_surface_is_mapped(surface) && shsurf->shell->win_close_animation_type == ANIMATION_FADE) { From 975e03b83a508810cd38d7e465f3482995883b88 Mon Sep 17 00:00:00 2001 From: Marius Vlad Date: Fri, 18 Mar 2022 22:49:29 +0200 Subject: [PATCH 14/46] desktop-shell: Check for a valid desktop_surface This patch fixes the following trace: #0 0x7f07d1bcecfa in weston_desktop_surface_get_surface ../libweston-desktop/surface.c:585 #1 0x7f07d1bfc9b8 in move_grab_motion ../desktop-shell/shell.c:1499 #2 0x7f07e539f841 in notify_motion ../libweston/input.c:1794 #3 0x7f07e1e8ace4 in handle_pointer_motion ../libweston/libinput-device.c:132 #4 0x7f07e1e8cad5 in evdev_device_process_event ../libweston/libinput-device.c:535 #5 0x7f07e1e89311 in udev_input_process_event ../libweston/libinput-seat.c:208 #6 0x7f07e1e8932f in process_event ../libweston/libinput-seat.c:218 #7 0x7f07e1e8935f in process_events ../libweston/libinput-seat.c:228 #8 0x7f07e1e8940a in udev_input_dispatch ../libweston/libinput-seat.c:239 #9 0x7f07e1e89437 in libinput_source_dispatch ../libweston/libinput-seat.c:249 #10 0x7f07e53122b1 in wl_event_loop_dispatch ../src/event-loop.c:1027 #11 0x7f07e5310114 in wl_display_run ../src/wayland-server.c:1408 #12 0x7f07e579c7ba in wet_main ../compositor/main.c:3589 #13 0x555611d6b17d in main ../compositor/executable.c:33 #14 0x7f07e55be7fc in __libc_start_main ../csu/libc-start.c:332 #15 0x555611d6b099 in _start (/home/mvlad/install-amd64/bin/weston+0x1099) A highly unlikely, but still valid operation, is to close/destroy the window while still having it grabbed and moved around, basically having an in-flight destruction of grabbed moving window. Another situation would be that the client terminates abruptly (crashing for instance), while being dragged which might take down the compositor. This could happen for both touch/pointer grab operations and could cause a NULL pointer access while accessing desktop_surface when being used to retrieve the underlying weston_surface. With this patch we check for a valid desktop_surface and return early if that's not the case. Signed-off-by: Marius Vlad (cherry picked from commit d03f01377a4b5e7c80c81957141a57f2319300aa) --- desktop-shell/shell.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c index e4ffdc38..bdba56df 100644 --- a/desktop-shell/shell.c +++ b/desktop-shell/shell.c @@ -1388,7 +1388,7 @@ touch_move_grab_motion(struct weston_touch_grab *grab, int dx = wl_fixed_to_int(grab->touch->grab_x + move->dx); int dy = wl_fixed_to_int(grab->touch->grab_y + move->dy); - if (!shsurf || !move->active) + if (!shsurf || !shsurf->desktop_surface || !move->active) return; es = weston_desktop_surface_get_surface(shsurf->desktop_surface); @@ -1520,7 +1520,7 @@ move_grab_motion(struct weston_pointer_grab *grab, int cx, cy; weston_pointer_move(pointer, event); - if (!shsurf) + if (!shsurf || !shsurf->desktop_surface) return; surface = weston_desktop_surface_get_surface(shsurf->desktop_surface); From 41864644897651616a7697e4b41af3ae25dc22fa Mon Sep 17 00:00:00 2001 From: Marius Vlad Date: Thu, 31 Mar 2022 23:20:45 +0300 Subject: [PATCH 15/46] desktop-shell: Clarify weston_view destruction at tear down This documents the fact that other views are handled implictly by libweston-desktop, and we shouldn't attempt to destroy indiscriminately views that aren't created by desktop-shell. Signed-off-by: Marius Vlad (cherry picked from commit 299f87f0739ac3c73d71fb4000f18d455fcf4a4b) --- desktop-shell/shell.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c index bdba56df..be3971f9 100644 --- a/desktop-shell/shell.c +++ b/desktop-shell/shell.c @@ -4966,9 +4966,17 @@ desktop_shell_destroy_layer(struct weston_layer *layer) * additional black_view created and added to its layer_link * fullscreen view. See shell_ensure_fullscreen_black_view() * - * As that black_view it is not a weston_desktop_surface - * we can't have a shsurf for it so we just destroy it like - * we do it in desktop_surface_removed() */ + * Note that we do not choose to destroy all other potential + * views we find in the layer, but instead we explicitly verify + * if the view in question was explicitly created by + * desktop-shell, rather than libweston-desktop (in + * desktop_surface_added()). + * + * This is particularly important because libweston-desktop + * could create additional views, which are managed implicitly, + * but which are still being added to the layer list. + * + */ if (shsurf) desktop_shell_destroy_surface(shsurf); else From ec9997960021ed432130468a7bf6d3c59992ee7f Mon Sep 17 00:00:00 2001 From: Marius Vlad Date: Fri, 13 May 2022 20:11:46 +0300 Subject: [PATCH 16/46] desktop-shell: Add missing weston_view_destroy() This fixes the following weston_view leak at compositor shutdown: #0 0x7f4250247987 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154 #1 0x7f424fd37aa3 in zalloc ../include/libweston/zalloc.h:38 #2 0x7f424fd3a05f in weston_view_create ../libweston/compositor.c:386 #3 0x7f423be7be6a in weston_desktop_surface_create_desktop_view ../libweston-desktop/surface.c:364 #4 0x7f423be7c0a8 in weston_desktop_surface_create_view ../libweston-desktop/surface.c:404 #5 0x7f423beae91d in desktop_surface_added ../desktop-shell/shell.c:2273 #6 0x7f423be77db1 in weston_desktop_api_surface_added ../libweston-desktop/libweston-desktop.c:138 #7 0x7f423be80c73 in weston_desktop_xdg_toplevel_ensure_added ../libweston-desktop/xdg-shell.c:362 #8 0x7f423be8207a in weston_desktop_xdg_toplevel_committed ../libweston-desktop/xdg-shell.c:697 #9 0x7f423be84d45 in weston_desktop_xdg_surface_committed ../libweston-desktop/xdg-shell.c:1374 #10 0x7f423be7b382 in weston_desktop_surface_surface_committed ../libweston-desktop/surface.c:174 #11 0x7f424fd378a6 in wl_signal_emit /home/mvlad/install-amd64/include/wayland-server-core.h:481 #12 0x7f424fd510e2 in weston_surface_commit_state ../libweston/compositor.c:4062 #13 0x7f424fd51161 in weston_surface_commit ../libweston/compositor.c:4068 #14 0x7f424fd516ef in surface_commit ../libweston/compositor.c:4146 #15 0x7f424fb597e9 (/usr/lib/x86_64-linux-gnu/libffi.so.8+0x77e9) With commit 'libweston, desktop-shell: Add a wrapper for weston_surface reference' we've removed an explicit weston_view_destroy() call due to a UAF which would've happen if we had close animations enabled, upon terminating a client. In that patch I've incorrectly wrote this happened when animations are off, but in fact it happened when they're on, see the following trace: READ of size 8 at 0x616000026498 thread T0 #0 0x7f757fba8797 in weston_signal_emit_mutable ../shared/signal.c:52 #1 0x7f757fb4bba1 in weston_view_destroy ../libweston/compositor.c:2269 #2 0x7f756bca89c0 in desktop_shell_destroy_surface ../desktop-shell/shell.c:275 #3 0x7f756bcb379e in fade_out_done_idle_cb ../desktop-shell/shell.c:2228 #4 0x7f757faec1da in wl_event_loop_dispatch_idle ../src/event-loop.c:969 #5 0x7f757faec31d in wl_event_loop_dispatch ../src/event-loop.c:1032 #6 0x7f757faea114 in wl_display_run ../src/wayland-server.c:1408 #7 0x7f757ff777ba in wet_main ../compositor/main.c:3589 #8 0x55f765c8d17d in main ../compositor/executable.c:33 #9 0x7f757fd997fc in __libc_start_main ../csu/libc-start.c:332 #10 0x55f765c8d099 in _start (/home/mvlad/install-amd64/bin/weston+0x1099) 0x616000026498 is located 24 bytes inside of 608-byte region [0x616000026480,0x6160000266e0) freed by thread T0 here: #0 0x7f758004c4d7 in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:127 #1 0x7f757fb4bdc8 in weston_view_destroy ../libweston/compositor.c:2295 #2 0x7f757fb4c14d in weston_surface_unref ../libweston/compositor.c:2334 #3 0x7f756bca898b in desktop_shell_destroy_surface ../desktop-shell/shell.c:273 #4 0x7f756bcb379e in fade_out_done_idle_cb ../desktop-shell/shell.c:2228 #5 0x7f757faec1da in wl_event_loop_dispatch_idle ../src/event-loop.c:969 This patch re-introduces it to avoid leaking the view upon compositor shutdown, but it does it in tandem with weston_desktop_surface_unlink_view(), (which was added in a later patch) and before weston_surface_unref() call. This way we should be safe to terminate/close clients with additional views created by libweston-desktop (pop-ups), but also in other different situations. Verified it in the following circumstances: - terminating a client with close animation on - terminating a client with close animations off - shutting down the compositor with clients running, with and without close animations - terminating top-level clients with additional pop-ups with both with and without close animations Signed-off-by: Marius Vlad (cherry picked from commit ab42159bf307b0794156676e8451b0ffc89039b4) --- desktop-shell/shell.c | 1 + 1 file changed, 1 insertion(+) diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c index be3971f9..63e14311 100644 --- a/desktop-shell/shell.c +++ b/desktop-shell/shell.c @@ -268,6 +268,7 @@ desktop_shell_destroy_surface(struct shell_surface *shsurf) } wl_list_remove(&shsurf->children_link); weston_desktop_surface_unlink_view(shsurf->view); + weston_view_destroy(shsurf->view); wl_signal_emit(&shsurf->destroy_signal, shsurf); weston_surface_destroy(shsurf->wsurface_anim_fade); From b58344b207989ada9f33b71b8194eb792457b221 Mon Sep 17 00:00:00 2001 From: Marius Vlad Date: Wed, 27 Apr 2022 17:07:57 +0300 Subject: [PATCH 17/46] simple-egl: Add start as maximized Just like start as fullscreen, let us add a start as maximized as well. It tests out the maximized state and with clients geometry checks. Signed-off-by: Marius Vlad (cherry picked from commit 01ef3746a2b0219135cab89480cdd4d91f4f95f7) --- clients/simple-egl.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/clients/simple-egl.c b/clients/simple-egl.c index a0f418a5..4c3ad682 100644 --- a/clients/simple-egl.c +++ b/clients/simple-egl.c @@ -404,6 +404,8 @@ create_surface(struct window *window) if (window->fullscreen) xdg_toplevel_set_fullscreen(window->xdg_toplevel, NULL); + else if (window->maximized) + xdg_toplevel_set_maximized(window->xdg_toplevel); } static void @@ -806,6 +808,7 @@ usage(int error_code) fprintf(stderr, "Usage: simple-egl [OPTIONS]\n\n" " -d \tBuffer swap delay in microseconds\n" " -f\tRun in fullscreen mode\n" + " -m\tRun in maximized mode\n" " -o\tCreate an opaque surface\n" " -s\tUse a 16 bpp EGL config\n" " -b\tDon't sync to compositor redraw (eglSwapInterval 0)\n" @@ -836,6 +839,8 @@ main(int argc, char **argv) window.delay = atoi(argv[++i]); else if (strcmp("-f", argv[i]) == 0) window.fullscreen = 1; + else if (strcmp("-m", argv[i]) == 0) + window.maximized = 1; else if (strcmp("-o", argv[i]) == 0) window.opaque = 1; else if (strcmp("-s", argv[i]) == 0) From a158b4353c7b608a0c1fac42f39704043f62376b Mon Sep 17 00:00:00 2001 From: Marius Vlad Date: Thu, 28 Apr 2022 10:14:00 +0300 Subject: [PATCH 18/46] libweston-desktop: Replace buffer with geometry A previous patch modified this for fullscreen, but missed out the maximized state. As we check the geometry rather than the buffer dimensions use the same terminology. Signed-off-by: Marius Vlad (cherry picked from commit cc877d4b779bab1ebd67663dbfe7f3b26cdddfd8) --- libweston-desktop/xdg-shell.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libweston-desktop/xdg-shell.c b/libweston-desktop/xdg-shell.c index ff76c39c..6cbf55e4 100644 --- a/libweston-desktop/xdg-shell.c +++ b/libweston-desktop/xdg-shell.c @@ -713,7 +713,7 @@ weston_desktop_xdg_toplevel_committed(struct weston_desktop_xdg_toplevel *toplev wl_resource_post_error(client_resource, XDG_WM_BASE_ERROR_INVALID_SURFACE_STATE, - "xdg_surface buffer (%" PRIi32 " x %" PRIi32 ") " + "xdg_surface geometry (%" PRIi32 " x %" PRIi32 ") " "does not match the configured maximized state (%" PRIi32 " x %" PRIi32 ")", geometry.width, geometry.height, toplevel->next.size.width, From 870abb15259afc7f59f9c0493db5186c357f9d45 Mon Sep 17 00:00:00 2001 From: Marius Vlad Date: Wed, 27 Apr 2022 17:11:58 +0300 Subject: [PATCH 19/46] simple-egl: Remove uneeded check display->wm_base is checked right after handling registry object, and with it the globals, so there's no to perform and additional check for xwm_base. Signed-off-by: Marius Vlad (cherry picked from commit c15699b7f811d21528f04085655943fe75d2a79b) --- clients/simple-egl.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/clients/simple-egl.c b/clients/simple-egl.c index 4c3ad682..93ff3e59 100644 --- a/clients/simple-egl.c +++ b/clients/simple-egl.c @@ -399,9 +399,6 @@ create_surface(struct window *window) if (!window->frame_sync) eglSwapInterval(display->egl.dpy, 0); - if (!display->wm_base) - return; - if (window->fullscreen) xdg_toplevel_set_fullscreen(window->xdg_toplevel, NULL); else if (window->maximized) From ac83c29e7a2502f578f1513a2548ecf1959a1b96 Mon Sep 17 00:00:00 2001 From: Marius Vlad Date: Fri, 29 Apr 2022 12:28:17 +0300 Subject: [PATCH 20/46] simple-egl: Defer EGL surface/window creation Rather than creating the wl_egl_window at the same time as wl_surface, do it after we get the first configure event. With it, we also defer eglMakeCurrent() as according to the spec, the first time a OpenGL or OpenGL ES context is made current, the viewport and scissor dimensions are set to the size of the draw surface. This is particulary important when attempting to start simple-egl either as fullscreen or as maximized, as not doing so will either incorrectly commit a buffer with the original dimensions, and later on to resize to the correct dimensions (which is the case for fullscreen), or it will terminate the wayland connection abruptly due to xdg-shell protocol violation, with a mismatch for the client's geometry (the case for maximized). Signed-off-by: Marius Vlad Suggested-by: Daniel Stone (cherry picked from commit 0277046a1da6058ed574b5b788ebd2a2592a84ff) --- clients/simple-egl.c | 51 ++++++++++++++++++++++---------------------- 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/clients/simple-egl.c b/clients/simple-egl.c index 93ff3e59..e48dbc69 100644 --- a/clients/simple-egl.c +++ b/clients/simple-egl.c @@ -262,6 +262,19 @@ init_gl(struct window *window) GLuint frag, vert; GLuint program; GLint status; + EGLBoolean ret; + + window->native = wl_egl_window_create(window->surface, + window->geometry.width, + window->geometry.height); + window->egl_surface = + weston_platform_create_egl_surface(window->display->egl.dpy, + window->display->egl.conf, + window->native, NULL); + + ret = eglMakeCurrent(window->display->egl.dpy, window->egl_surface, + window->egl_surface, window->display->egl.ctx); + assert(ret == EGL_TRUE); frag = create_shader(window, frag_shader_text, GL_FRAGMENT_SHADER); vert = create_shader(window, vert_shader_text, GL_VERTEX_SHADER); @@ -362,19 +375,9 @@ static void create_surface(struct window *window) { struct display *display = window->display; - EGLBoolean ret; window->surface = wl_compositor_create_surface(display->compositor); - window->native = - wl_egl_window_create(window->surface, - window->geometry.width, - window->geometry.height); - window->egl_surface = - weston_platform_create_egl_surface(display->egl.dpy, - display->egl.conf, - window->native, NULL); - window->xdg_surface = xdg_wm_base_get_xdg_surface(display->wm_base, window->surface); xdg_surface_add_listener(window->xdg_surface, @@ -392,10 +395,6 @@ create_surface(struct window *window) window->wait_for_configure = true; wl_surface_commit(window->surface); - ret = eglMakeCurrent(window->display->egl.dpy, window->egl_surface, - window->egl_surface, window->display->egl.ctx); - assert(ret == EGL_TRUE); - if (!window->frame_sync) eglSwapInterval(display->egl.dpy, 0); @@ -866,7 +865,17 @@ main(int argc, char **argv) init_egl(&display, &window); create_surface(&window); - init_gl(&window); + + /* we already have wait_for_configure set after create_surface() */ + while (running && ret != -1 && window.wait_for_configure) { + ret = wl_display_dispatch(display.display); + + /* wait until xdg_surface::configure acks the new dimensions */ + if (window.wait_for_configure) + continue; + + init_gl(&window); + } display.cursor_surface = wl_compositor_create_surface(display.compositor); @@ -876,17 +885,9 @@ main(int argc, char **argv) sigint.sa_flags = SA_RESETHAND; sigaction(SIGINT, &sigint, NULL); - /* The mainloop here is a little subtle. Redrawing will cause - * EGL to read events so we can just call - * wl_display_dispatch_pending() to handle any events that got - * queued up as a side effect. */ while (running && ret != -1) { - if (window.wait_for_configure) { - ret = wl_display_dispatch(display.display); - } else { - ret = wl_display_dispatch_pending(display.display); - redraw(&window, NULL, 0); - } + ret = wl_display_dispatch_pending(display.display); + redraw(&window, NULL, 0); } fprintf(stderr, "simple-egl exiting\n"); From f573612d379503c85f106a2f19924bf8f21a11cf Mon Sep 17 00:00:00 2001 From: Marius Vlad Date: Thu, 28 Apr 2022 10:32:15 +0300 Subject: [PATCH 21/46] simple-egl: Move set_fullscreen/set_maximized before initial commit Rather than setting the fullscreen/maximized before initial wl_surface.commit, make it part of the initial window state. Signed-off-by: Marius Vlad Suggested-by: Pekka Paalanen (cherry picked from commit 054aaa5a8b4b8a7ba9404eb35d86a1b4f992be15) --- clients/simple-egl.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/clients/simple-egl.c b/clients/simple-egl.c index e48dbc69..2c7059c0 100644 --- a/clients/simple-egl.c +++ b/clients/simple-egl.c @@ -392,16 +392,16 @@ create_surface(struct window *window) xdg_toplevel_set_app_id(window->xdg_toplevel, "org.freedesktop.weston.simple-egl"); + if (window->fullscreen) + xdg_toplevel_set_fullscreen(window->xdg_toplevel, NULL); + else if (window->maximized) + xdg_toplevel_set_maximized(window->xdg_toplevel); + window->wait_for_configure = true; wl_surface_commit(window->surface); if (!window->frame_sync) eglSwapInterval(display->egl.dpy, 0); - - if (window->fullscreen) - xdg_toplevel_set_fullscreen(window->xdg_toplevel, NULL); - else if (window->maximized) - xdg_toplevel_set_maximized(window->xdg_toplevel); } static void From d2d5c666a3926d2e7924b597546942e364189c97 Mon Sep 17 00:00:00 2001 From: Leandro Ribeiro Date: Mon, 24 Jan 2022 18:35:03 -0300 Subject: [PATCH 22/46] clients/simple-dmabuf-feedback: do not use buffer before compositor's response This fixes an issue when running simple-dmabuf-feedback: "wl_display@1: error 1: invalid arguments for wl_surface@3.attach". As we are not using create_immed request from zwp_linux_dmabuf_v1, we can't start to use a dma-buf buffer before we process compositor's event telling us that the creation succeeded. This was causing problems in the following scenario: 1. buffer is marked to be recreated (because of dma-buf feedback); 2. in buffer_release() event, we destroy the buffer and recreate it; 3. after we recreate it, roundtrip is not called, as we don't want to block during the drawing loop; 4. buffer status is not being properly tracked, so we are trying to use a buffer before receiving the event from the compositor telling us that the creation succeeded. To fix this, this patch improves buffer status tracking. Now we only pick a buffer in the drawing loop when it is available. Also, if we have no buffers available we perform a roundtrip and try again, as we may have recreated all of them but still didn't have the chance to process compositor's events telling us that creation succeeded. Signed-off-by: Leandro Ribeiro (cherry picked from commit 7724c5ea380da617883ce8f644276e75f7e5bb7f) --- clients/simple-dmabuf-feedback.c | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/clients/simple-dmabuf-feedback.c b/clients/simple-dmabuf-feedback.c index dd2e64a5..94d734b9 100644 --- a/clients/simple-dmabuf-feedback.c +++ b/clients/simple-dmabuf-feedback.c @@ -140,7 +140,11 @@ struct display { struct buffer { struct window *window; struct wl_buffer *buffer; - bool busy; + enum { + NOT_CREATED, + IN_USE, + AVAILABLE + } status; bool recreate; int dmabuf_fds[4]; struct gbm_bo *bo; @@ -469,7 +473,7 @@ buffer_release(void *data, struct wl_buffer *buffer) { struct buffer *buf = data; - buf->busy = false; + buf->status = AVAILABLE; if (buf->recreate) buffer_recreate(buf); @@ -485,6 +489,7 @@ create_succeeded(void *data, struct zwp_linux_buffer_params_v1 *params, { struct buffer *buf = data; + buf->status = AVAILABLE; buf->buffer = new_buffer; wl_buffer_add_listener(buf->buffer, &buffer_listener, buf); zwp_linux_buffer_params_v1_destroy(params); @@ -516,6 +521,7 @@ create_dmabuf_buffer(struct window *window, struct buffer *buf, uint32_t width, struct zwp_linux_buffer_params_v1 *params; int i; + buf->status = NOT_CREATED; buf->window = window; buf->width = width; buf->height = height; @@ -573,8 +579,22 @@ window_next_buffer(struct window *window) unsigned int i; for (i = 0; i < NUM_BUFFERS; i++) - if (!window->buffers[i].busy) + if (window->buffers[i].status == AVAILABLE) return &window->buffers[i]; + + /* In this client, we sometimes have to recreate the buffers. As we are + * not using the create_immed request from zwp_linux_dmabuf_v1, we need + * to wait an event from the server (what leads to create_succeeded() + * being called in this client). So if all buffers are busy, it may be + * the case in which all the buffers were recreated but the server still + * didn't send the events. This is very unlikely to happen, but a + * roundtrip() guarantees that we receive and process the events. */ + wl_display_roundtrip(window->display->display); + + for (i = 0; i < NUM_BUFFERS; i++) + if (window->buffers[i].status == AVAILABLE) + return &window->buffers[i]; + return NULL; } @@ -639,7 +659,7 @@ redraw(void *data, struct wl_callback *callback, uint32_t time) window->callback = wl_surface_frame(window->surface); wl_callback_add_listener(window->callback, &frame_listener, window); wl_surface_commit(window->surface); - buf->busy = true; + buf->status = IN_USE; region = wl_compositor_create_region(window->display->compositor); wl_region_add(region, 0, 0, window->display->output.width, From 89c965ebd7c1838f0b4caf1d2a5f5f4981e43b0e Mon Sep 17 00:00:00 2001 From: Robert Mader Date: Mon, 25 Apr 2022 18:33:12 +0200 Subject: [PATCH 23/46] clients/simple-dmabuf-feedback: Support multi-tranche feedbacks Compositors may choose to send multiple scanout or non-scanout tranches. So instead of assuming that the first respective tranche contains the format/modifier we're looking for, check all tranches. While on it, make sure that in case a compositor sends scanout tranches on the initial feedback, `pick_format_from_scanout_tranche()` does not unintentionally pick `INITIAL_BUFFER_FORMAT`. Signed-off-by: Robert Mader (cherry picked from commit 46a6b5b448fc49b7909245a248d0135fbe914f11) --- clients/simple-dmabuf-feedback.c | 47 ++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 18 deletions(-) diff --git a/clients/simple-dmabuf-feedback.c b/clients/simple-dmabuf-feedback.c index 94d734b9..43e5670f 100644 --- a/clients/simple-dmabuf-feedback.c +++ b/clients/simple-dmabuf-feedback.c @@ -1123,7 +1123,7 @@ dmabuf_feedback_tranche_done(void *data, dmabuf_feedback_tranche_init(&feedback->pending_tranche); } -static void +static bool pick_initial_format_from_renderer_tranche(struct window *window, struct dmabuf_feedback_tranche *tranche) { @@ -1137,13 +1137,12 @@ pick_initial_format_from_renderer_tranche(struct window *window, window->format.format = fmt->format; wl_array_copy(&window->format.modifiers, &fmt->modifiers); - return; + return true; } - - assert(0 && "error: INITIAL_BUFFER_FORMAT not supported by the hardware"); + return false; } -static void +static bool pick_format_from_scanout_tranche(struct window *window, struct dmabuf_feedback_tranche *tranche) { @@ -1152,8 +1151,9 @@ pick_format_from_scanout_tranche(struct window *window, wl_array_for_each(fmt, &tranche->formats.arr) { - /* Ignore format that we're already using. */ - if (fmt->format == window->format.format) + /* Ignore the format that we want to pick from the render + * tranche. */ + if (fmt->format == INITIAL_BUFFER_FORMAT) continue; /* Format should be supported by the compositor. */ @@ -1167,10 +1167,9 @@ pick_format_from_scanout_tranche(struct window *window, window->format.format = fmt->format; wl_array_copy(&window->format.modifiers, &fmt->modifiers); - return; + return true; } - - assert(0 && "error: no valid pair of format/modifier in the scanout tranche"); + return false; } static void @@ -1178,25 +1177,37 @@ dmabuf_feedback_done(void *data, struct zwp_linux_dmabuf_feedback_v1 *dmabuf_fee { struct window *window = data; struct dmabuf_feedback_tranche *tranche; + bool got_scanout_tranche = false; unsigned int i; fprintf(stderr, "└end of dma-buf feedback\n\n"); /* The first time that we receive dma-buf feedback for a surface it - * contains only the renderer tranche. We pick the INITIAL_BUFFER_FORMAT + * contains only the renderer tranches. We pick the INITIAL_BUFFER_FORMAT * from there. Then the compositor should detect that the format is * unsupported by the underlying hardware (not actually, but you should - * have faked this in the DRM-backend) and send the scanout tranche. We - * use the formats/modifiers of the scanout tranche to reallocate our + * have faked this in the DRM-backend) and send the scanout tranches. We + * use the formats/modifiers of the scanout tranches to reallocate our * buffers. */ wl_array_for_each(tranche, &window->pending_dmabuf_feedback.tranches) { if (tranche->is_scanout_tranche) { - pick_format_from_scanout_tranche(window, tranche); - for (i = 0; i < NUM_BUFFERS; i++) - window->buffers[i].recreate = true; - break; + got_scanout_tranche = true; + if (pick_format_from_scanout_tranche(window, tranche)) { + for (i = 0; i < NUM_BUFFERS; i++) + window->buffers[i].recreate = true; + break; + } } - pick_initial_format_from_renderer_tranche(window, tranche); + if (pick_initial_format_from_renderer_tranche(window, tranche)) + break; + } + + if (got_scanout_tranche) { + assert(window->format.format != INITIAL_BUFFER_FORMAT && + "error: no valid pair of format/modifier in the scanout tranches"); + } else { + assert(window->format.format == INITIAL_BUFFER_FORMAT && + "error: INITIAL_BUFFER_FORMAT not supported by the hardware"); } dmabuf_feedback_fini(&window->dmabuf_feedback); From f5d5114d9da2e9bea5f54500d2747bdcf185142d Mon Sep 17 00:00:00 2001 From: Robert Mader Date: Thu, 17 Feb 2022 11:35:30 +0100 Subject: [PATCH 24/46] clients/simple-dmabuf-*: Increase buffer limit to four In certain situations these clients crash a lot due to the low buffer limit. Four buffers is also what EGL allows without blocking and what is arguably the upper limit of what a compositor should demand. Signed-off-by: Robert Mader (cherry picked from commit 3e6ef529f812ee1d21f7cf92279e29421cf87200) --- clients/simple-dmabuf-egl.c | 2 +- clients/simple-dmabuf-feedback.c | 2 +- clients/simple-dmabuf-v4l.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/clients/simple-dmabuf-egl.c b/clients/simple-dmabuf-egl.c index 33df4cf9..ef0d9de6 100644 --- a/clients/simple-dmabuf-egl.c +++ b/clients/simple-dmabuf-egl.c @@ -131,7 +131,7 @@ struct buffer { int release_fence_fd; }; -#define NUM_BUFFERS 3 +#define NUM_BUFFERS 4 struct window { struct display *display; diff --git a/clients/simple-dmabuf-feedback.c b/clients/simple-dmabuf-feedback.c index 43e5670f..7a58594c 100644 --- a/clients/simple-dmabuf-feedback.c +++ b/clients/simple-dmabuf-feedback.c @@ -47,7 +47,7 @@ #include #include -#define NUM_BUFFERS 3 +#define NUM_BUFFERS 4 /* We have to hack the DRM-backend to pretend that planes of the underlying * hardware don't support this format. If you change the value of this constant, diff --git a/clients/simple-dmabuf-v4l.c b/clients/simple-dmabuf-v4l.c index 85dd7a3d..a19570f9 100644 --- a/clients/simple-dmabuf-v4l.c +++ b/clients/simple-dmabuf-v4l.c @@ -127,7 +127,7 @@ struct buffer { int data_offsets[VIDEO_MAX_PLANES]; }; -#define NUM_BUFFERS 3 +#define NUM_BUFFERS 4 struct window { struct display *display; From 7b390f1dae43cf3c9845c0b78d9ba87cadefd740 Mon Sep 17 00:00:00 2001 From: Robert Mader Date: Wed, 16 Feb 2022 00:23:53 +0100 Subject: [PATCH 25/46] clients/simple-dmabuf-feedback: Add fallback print method for unknown formats Using `pixel_format_get_info()` can result in formats being reported as `UNKNOWN` when used on compositors other than Weston. As `weston-simple-dmabuf-feedback` somewhat succeeds `wayland-info` as tool for `zwp_linux_dmabuf_v1` debugging from version 4 on, copy the approach from the later for these cases. Signed-off-by: Robert Mader (cherry picked from commit 26698535625ebf971fa73b9c35f6a9fd9a5b77e3) --- clients/simple-dmabuf-feedback.c | 41 ++++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/clients/simple-dmabuf-feedback.c b/clients/simple-dmabuf-feedback.c index 7a58594c..0100c0a5 100644 --- a/clients/simple-dmabuf-feedback.c +++ b/clients/simple-dmabuf-feedback.c @@ -26,6 +26,7 @@ #include "config.h" #include +#include #include #include #include @@ -1070,18 +1071,54 @@ dmabuf_feedback_tranche_formats(void *data, } } +static char +bits2graph(uint32_t value, unsigned bitoffset) +{ + int c = (value >> bitoffset) & 0xff; + + if (isgraph(c) || isspace(c)) + return c; + + return '?'; +} + +static void +fourcc2str(uint32_t format, char *str, int len) +{ + int i; + + assert(len >= 5); + + for (i = 0; i < 4; i++) + str[i] = bits2graph(format, i * 8); + str[i] = '\0'; +} + static void print_tranche_format_modifier(uint32_t format, uint64_t modifier) { const struct pixel_format_info *fmt_info; + char *format_str; char *mod_name; + int len; - fmt_info = pixel_format_get_info(format); mod_name = pixel_format_get_modifier(modifier); + fmt_info = pixel_format_get_info(format); + + if (fmt_info) { + len = asprintf(&format_str, "%s", fmt_info->drm_format_name); + } else { + char fourcc_str[5]; + + fourcc2str(format, fourcc_str, sizeof(fourcc_str)); + len = asprintf(&format_str, "0x%08x (%s)", format, fourcc_str); + } + assert(len > 0); fprintf(stderr, "│ ├────────tranche format/modifier pair - format %s, modifier %s\n", - fmt_info ? fmt_info->drm_format_name : "UNKNOWN", mod_name); + format_str, mod_name); + free(format_str); free(mod_name); } From 35c6a4b814dc179ed28456cac17fd7510da6e6e0 Mon Sep 17 00:00:00 2001 From: Robert Mader Date: Sun, 24 Apr 2022 02:36:58 +0200 Subject: [PATCH 26/46] backend-drm: Add failure reasons for failing gbm_bo_import And add it to the list of failures triggering a resend of dmabuf feedback scanout tranches. Closes https://gitlab.freedesktop.org/wayland/weston/-/issues/614 Signed-off-by: Robert Mader (cherry picked from commit 29d480813ad64cfde3f1c8bd64310db08bdfba28) --- libweston/backend-drm/drm-internal.h | 1 + libweston/backend-drm/fb.c | 6 +++++- libweston/backend-drm/state-propose.c | 3 ++- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/libweston/backend-drm/drm-internal.h b/libweston/backend-drm/drm-internal.h index 55e9414b..48600880 100644 --- a/libweston/backend-drm/drm-internal.h +++ b/libweston/backend-drm/drm-internal.h @@ -236,6 +236,7 @@ enum try_view_on_plane_failure_reasons { FAILURE_REASONS_FB_FORMAT_INCOMPATIBLE = (1 << 1), FAILURE_REASONS_DMABUF_MODIFIER_INVALID = (1 << 2), FAILURE_REASONS_ADD_FB_FAILED = (1 << 3), + FAILURE_REASONS_GBM_BO_IMPORT_FAILED = (1 << 4) }; /** diff --git a/libweston/backend-drm/fb.c b/libweston/backend-drm/fb.c index ffe2cc52..ba0c177e 100644 --- a/libweston/backend-drm/fb.c +++ b/libweston/backend-drm/fb.c @@ -276,8 +276,12 @@ drm_fb_get_from_dmabuf(struct linux_dmabuf_buffer *dmabuf, fb->bo = gbm_bo_import(backend->gbm, GBM_BO_IMPORT_FD_MODIFIER, &import_mod, GBM_BO_USE_SCANOUT); - if (!fb->bo) + if (!fb->bo) { + if (try_view_on_plane_failure_reasons) + *try_view_on_plane_failure_reasons |= + FAILURE_REASONS_GBM_BO_IMPORT_FAILED; goto err_free; + } fb->width = dmabuf->attributes.width; fb->height = dmabuf->attributes.height; diff --git a/libweston/backend-drm/state-propose.c b/libweston/backend-drm/state-propose.c index 72e4db93..7b350aa4 100644 --- a/libweston/backend-drm/state-propose.c +++ b/libweston/backend-drm/state-propose.c @@ -602,7 +602,8 @@ dmabuf_feedback_maybe_update(struct drm_backend *b, struct weston_view *ev, if (try_view_on_plane_failure_reasons & (FAILURE_REASONS_ADD_FB_FAILED | FAILURE_REASONS_FB_FORMAT_INCOMPATIBLE | - FAILURE_REASONS_DMABUF_MODIFIER_INVALID)) + FAILURE_REASONS_DMABUF_MODIFIER_INVALID | + FAILURE_REASONS_GBM_BO_IMPORT_FAILED)) action_needed |= ACTION_NEEDED_ADD_SCANOUT_TRANCHE; assert(action_needed != (ACTION_NEEDED_REMOVE_SCANOUT_TRANCHE | From 01af0e4e0ef9e61e70a0b9413c7697f285258b27 Mon Sep 17 00:00:00 2001 From: Robert Mader Date: Wed, 4 May 2022 17:57:27 +0200 Subject: [PATCH 27/46] clients/simple-dmabuf-feedback: use time instead of redraws Weston uses a timeout of 2 seconds before it sends scanout tranches to clients in order to not trigger excessive buffer reallocations in clients. `simple-dmabuf-feedback` in turn counts redraws (200) before exiting. That doesn't work on e.g. 144Hz screens, thus use a timer here as well. Signed-off-by: Robert Mader (cherry picked from commit 34f7e01c2bb2cd19be00f7dcd1340390e1bf901a) --- clients/simple-dmabuf-feedback.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/clients/simple-dmabuf-feedback.c b/clients/simple-dmabuf-feedback.c index 0100c0a5..516cdbc8 100644 --- a/clients/simple-dmabuf-feedback.c +++ b/clients/simple-dmabuf-feedback.c @@ -32,6 +32,7 @@ #include #include #include +#include #include "shared/helpers.h" #include "shared/platform.h" @@ -667,8 +668,6 @@ redraw(void *data, struct wl_callback *callback, uint32_t time) window->display->output.height); wl_surface_set_opaque_region(window->surface, region); wl_region_destroy(region); - - window->n_redraws++; } static const struct wl_callback_listener frame_listener = { @@ -1451,6 +1450,9 @@ main(int argc, char **argv) struct display *display; struct window *window; int ret = 0; + struct timespec start_time, current_time; + const time_t MAX_TIME_SECONDS = 3; + time_t delta_time = 0; fprintf(stderr, "This client was written with the purpose of manually test " \ "Weston's dma-buf feedback implementation. See main() " \ @@ -1459,9 +1461,14 @@ main(int argc, char **argv) display = create_display(); window = create_window(display); + clock_gettime(CLOCK_MONOTONIC, &start_time); + redraw(window, NULL, 0); - while (ret != -1 && window->n_redraws < 200) + while (ret != -1 && delta_time < MAX_TIME_SECONDS) { ret = wl_display_dispatch(display->display); + clock_gettime(CLOCK_MONOTONIC, ¤t_time); + delta_time = current_time.tv_sec - start_time.tv_sec; + } destroy_window(window); destroy_display(display); From f7c5fa175efe6dcb088e36e4c887fc42fe20eb9d Mon Sep 17 00:00:00 2001 From: Robert Mader Date: Wed, 4 May 2022 18:04:55 +0200 Subject: [PATCH 28/46] libweston/linux-dmabuf: create surface feedback on demand Unconditionally creating a surface feedback for each surface creates unnecessary overhead and noise in the logs. Thus create it when the first surface feedback resource for a surface is requested and delete it again once all those resources have been destroyed. Signed-off-by: Robert Mader (cherry picked from commit 53a221ccaaaa888a0efcda0ecadc45a51f03ac92) --- libweston/compositor.c | 36 ++------------------- libweston/linux-dmabuf.c | 67 +++++++++++++++++++++++++++++++++++----- 2 files changed, 62 insertions(+), 41 deletions(-) diff --git a/libweston/compositor.c b/libweston/compositor.c index ff3fa45b..f87eb371 100644 --- a/libweston/compositor.c +++ b/libweston/compositor.c @@ -4048,54 +4048,22 @@ static const struct wl_surface_interface surface_interface = { surface_damage_buffer }; -static int -create_surface_dmabuf_feedback(struct weston_compositor *ec, - struct weston_surface *surface) -{ - struct weston_dmabuf_feedback_tranche *tranche; - dev_t main_device = ec->default_dmabuf_feedback->main_device; - uint32_t flags = 0; - - surface->dmabuf_feedback = weston_dmabuf_feedback_create(main_device); - if (!surface->dmabuf_feedback) - return -1; - - tranche = weston_dmabuf_feedback_tranche_create(surface->dmabuf_feedback, - ec->dmabuf_feedback_format_table, - main_device, flags, - RENDERER_PREF); - if (!tranche) { - weston_dmabuf_feedback_destroy(surface->dmabuf_feedback); - surface->dmabuf_feedback = NULL; - return -1; - } - - return 0; -} - static void compositor_create_surface(struct wl_client *client, struct wl_resource *resource, uint32_t id) { struct weston_compositor *ec = wl_resource_get_user_data(resource); struct weston_surface *surface; - int ret; surface = weston_surface_create(ec); if (surface == NULL) goto err; - if (ec->default_dmabuf_feedback) { - ret = create_surface_dmabuf_feedback(ec, surface); - if (ret < 0) - goto err_dmabuf_feedback; - } - surface->resource = wl_resource_create(client, &wl_surface_interface, wl_resource_get_version(resource), id); if (surface->resource == NULL) - goto err_dmabuf_feedback; + goto err_res; wl_resource_set_implementation(surface->resource, &surface_interface, surface, destroy_surface); @@ -4103,7 +4071,7 @@ compositor_create_surface(struct wl_client *client, return; -err_dmabuf_feedback: +err_res: weston_surface_destroy(surface); err: wl_resource_post_no_memory(resource); diff --git a/libweston/linux-dmabuf.c b/libweston/linux-dmabuf.c index 66702a4c..21de498d 100644 --- a/libweston/linux-dmabuf.c +++ b/libweston/linux-dmabuf.c @@ -684,6 +684,7 @@ weston_dmabuf_feedback_destroy(struct weston_dmabuf_feedback *dmabuf_feedback) wl_resource_for_each_safe(res, res_tmp, &dmabuf_feedback->resource_list) { wl_list_remove(wl_resource_get_link(res)); wl_list_init(wl_resource_get_link(res)); + wl_resource_set_user_data(res, NULL); } free(dmabuf_feedback); @@ -786,6 +787,7 @@ weston_dmabuf_feedback_send_all(struct weston_dmabuf_feedback *dmabuf_feedback, { struct wl_resource *res; + assert(!wl_list_empty(&dmabuf_feedback->resource_list)); wl_resource_for_each(res, &dmabuf_feedback->resource_list) weston_dmabuf_feedback_send(dmabuf_feedback, format_table, res, false); @@ -794,7 +796,16 @@ weston_dmabuf_feedback_send_all(struct weston_dmabuf_feedback *dmabuf_feedback, static void dmabuf_feedback_resource_destroy(struct wl_resource *resource) { + struct weston_surface *surface = + wl_resource_get_user_data(resource); + wl_list_remove(wl_resource_get_link(resource)); + + if (surface && + wl_list_empty(&surface->dmabuf_feedback->resource_list)) { + weston_dmabuf_feedback_destroy(surface->dmabuf_feedback); + surface->dmabuf_feedback = NULL; + } } static void @@ -810,7 +821,8 @@ zwp_linux_dmabuf_feedback_implementation = { static struct wl_resource * dmabuf_feedback_resource_create(struct wl_resource *dmabuf_resource, - struct wl_client *client, uint32_t dmabuf_feedback_id) + struct wl_client *client, uint32_t dmabuf_feedback_id, + struct weston_surface *surface) { struct wl_resource *dmabuf_feedback_res; uint32_t version; @@ -826,7 +838,7 @@ dmabuf_feedback_resource_create(struct wl_resource *dmabuf_resource, wl_list_init(wl_resource_get_link(dmabuf_feedback_res)); wl_resource_set_implementation(dmabuf_feedback_res, &zwp_linux_dmabuf_feedback_implementation, - NULL, dmabuf_feedback_resource_destroy); + surface, dmabuf_feedback_resource_destroy); return dmabuf_feedback_res; } @@ -842,7 +854,8 @@ linux_dmabuf_get_default_feedback(struct wl_client *client, dmabuf_feedback_resource = dmabuf_feedback_resource_create(dmabuf_resource, - client, dmabuf_feedback_id); + client, dmabuf_feedback_id, + NULL); if (!dmabuf_feedback_resource) { wl_resource_post_no_memory(dmabuf_resource); return; @@ -853,22 +866,55 @@ linux_dmabuf_get_default_feedback(struct wl_client *client, dmabuf_feedback_resource, true); } +static int +create_surface_dmabuf_feedback(struct weston_compositor *ec, + struct weston_surface *surface) +{ + struct weston_dmabuf_feedback_tranche *tranche; + dev_t main_device = ec->default_dmabuf_feedback->main_device; + uint32_t flags = 0; + + surface->dmabuf_feedback = weston_dmabuf_feedback_create(main_device); + if (!surface->dmabuf_feedback) + return -1; + + tranche = weston_dmabuf_feedback_tranche_create(surface->dmabuf_feedback, + ec->dmabuf_feedback_format_table, + main_device, flags, + RENDERER_PREF); + if (!tranche) { + weston_dmabuf_feedback_destroy(surface->dmabuf_feedback); + surface->dmabuf_feedback = NULL; + return -1; + } + + return 0; +} + static void linux_dmabuf_get_per_surface_feedback(struct wl_client *client, struct wl_resource *dmabuf_resource, uint32_t dmabuf_feedback_id, struct wl_resource *surface_resource) { + struct weston_compositor *compositor = + wl_resource_get_user_data(dmabuf_resource); struct weston_surface *surface = wl_resource_get_user_data(surface_resource); struct wl_resource *dmabuf_feedback_resource; + int ret; dmabuf_feedback_resource = dmabuf_feedback_resource_create(dmabuf_resource, - client, dmabuf_feedback_id); - if (!dmabuf_feedback_resource) { - wl_resource_post_no_memory(dmabuf_resource); - return; + client, dmabuf_feedback_id, + surface); + if (!dmabuf_feedback_resource) + goto err; + + if (!surface->dmabuf_feedback) { + ret = create_surface_dmabuf_feedback(compositor, surface); + if (ret < 0) + goto err_feedback; } /* Surface dma-buf feedback is dynamic and may need to be resent to @@ -879,6 +925,13 @@ linux_dmabuf_get_per_surface_feedback(struct wl_client *client, weston_dmabuf_feedback_send(surface->dmabuf_feedback, surface->compositor->dmabuf_feedback_format_table, dmabuf_feedback_resource, true); + return; + +err_feedback: + wl_resource_set_user_data(dmabuf_feedback_resource, NULL); + wl_resource_destroy(dmabuf_feedback_resource); +err: + wl_resource_post_no_memory(dmabuf_resource); } /** Get the linux_dmabuf_buffer from a wl_buffer resource From f31de214d93b07cc192393175b90f2ec2151c991 Mon Sep 17 00:00:00 2001 From: Pekka Paalanen Date: Thu, 2 Jun 2022 11:24:52 +0300 Subject: [PATCH 29/46] gl-renderer: fix performance regression in frag When color management is disabled, the fragment shader was still first ensuring straight alpha and then immediately just going back to pre-multiplied. This is near-impossible for a shader compiler to optimize out, I guess because of the if-statement to handle division by zero. Having view alpha applied in between certainly didn't make it easier. That causes extra fragment computations that are unnecessary. In the issue report this was found to cause a notable performance regression. Fix the performance regression by introducing special-case paths for when straight alpha is not needed. This skips the unnecessary computations. Fixes: https://gitlab.freedesktop.org/wayland/weston/-/issues/623 Fixes: 9a6a4e7032669be727c965ca19e3e30098c892e7 Signed-off-by: Pekka Paalanen (cherry picked from commit 6234cb98d1b2b3b3cd39b18f69ee8cf4aa7efe32) Dropped SHADER_COLOR_MAPPING_IDENTITY as that is not available in weston 10.0. --- libweston/renderer-gl/fragment.glsl | 43 ++++++++++++++++++++--------- 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/libweston/renderer-gl/fragment.glsl b/libweston/renderer-gl/fragment.glsl index 63a20cd6..cfadb885 100644 --- a/libweston/renderer-gl/fragment.glsl +++ b/libweston/renderer-gl/fragment.glsl @@ -67,6 +67,9 @@ compile_const bool c_input_is_premult = DEF_INPUT_IS_PREMULT; compile_const bool c_green_tint = DEF_GREEN_TINT; compile_const int c_color_pre_curve = DEF_COLOR_PRE_CURVE; +compile_const bool c_need_color_pipeline = + c_color_pre_curve != SHADER_COLOR_CURVE_IDENTITY; + vec4 yuva2rgba(vec4 yuva) { @@ -202,9 +205,6 @@ color_pre_curve(vec3 color) vec4 color_pipeline(vec4 color) { - /* View alpha (opacity) */ - color.a *= alpha; - color.rgb = color_pre_curve(color.rgb); return color; @@ -218,18 +218,35 @@ main() /* Electrical (non-linear) RGBA values, may be premult or not */ color = sample_input_texture(); - /* Ensure straight alpha */ - if (c_input_is_premult) { - if (color.a == 0.0) - color.rgb = vec3(0, 0, 0); - else - color.rgb *= 1.0 / color.a; - } + if (c_need_color_pipeline) { + /* Ensure straight alpha */ + if (c_input_is_premult) { + if (color.a == 0.0) + color.rgb = vec3(0, 0, 0); + else + color.rgb *= 1.0 / color.a; + } - color = color_pipeline(color); + color = color_pipeline(color); - /* pre-multiply for blending */ - color.rgb *= color.a; + /* View alpha (opacity) */ + color.a *= alpha; + + /* pre-multiply for blending */ + color.rgb *= color.a; + } else { + /* Fast path for disabled color management */ + + if (c_input_is_premult) { + /* View alpha (opacity) */ + color *= alpha; + } else { + /* View alpha (opacity) */ + color.a *= alpha; + /* pre-multiply for blending */ + color.rgb *= color.a; + } + } if (c_green_tint) color = vec4(0.0, 0.3, 0.0, 0.2) + color * 0.8; From 00dc1add13b79fbb212b79fb7dd01dc95eecefca Mon Sep 17 00:00:00 2001 From: Derek Foreman Date: Mon, 6 Jun 2022 13:56:19 -0500 Subject: [PATCH 30/46] ci: Fix cobertura syntax This has somehow stopped working. Copied different syntax from a gitlab example. Signed-off-by: Derek Foreman (cherry picked from commit 0f4b411091719becfb43043a9bf035d7d041d354) --- .gitlab-ci.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 02cfaa0d..b6445093 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -296,7 +296,9 @@ x86_64-debian-full-build: - .build-options-full artifacts: reports: - cobertura: $BUILDDIR/meson-logs/coverage.xml + coverage_report: + coverage_format: cobertura + path: $BUILDDIR/meson-logs/coverage.xml aarch64-debian-full-build: extends: From d141f1aeb79ec3e625c45185711e9b7e69990367 Mon Sep 17 00:00:00 2001 From: Simon Ser Date: Thu, 23 Jun 2022 15:52:38 +0200 Subject: [PATCH 31/46] build: bump to version 10.0.1 for the point release --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index d95f82d5..0d53c1b0 100644 --- a/meson.build +++ b/meson.build @@ -1,6 +1,6 @@ project('weston', 'c', - version: '10.0.0', + version: '10.0.1', default_options: [ 'warning_level=3', 'c_std=gnu99', From d8a709993149b9bfac0a62242be4d452ad1d4ac1 Mon Sep 17 00:00:00 2001 From: Daniel Stone Date: Sat, 25 Jun 2022 16:59:38 +0100 Subject: [PATCH 32/46] tests: Use test-desktop-shell for devices-test It doesn't need to be using desktop-shell; trying to use it is not sensible as it will try to bind to all the devices we're repeatedly creating and destroying, sometimes lose the race, and fail the test because desktop-shell has died too early. Signed-off-by: Daniel Stone (cherry picked from commit ed97387a4ed7de217810f7ce708df142a10fc247) --- tests/devices-test.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/devices-test.c b/tests/devices-test.c index 719f4595..e84273cc 100644 --- a/tests/devices-test.c +++ b/tests/devices-test.c @@ -36,6 +36,8 @@ fixture_setup(struct weston_test_harness *harness) compositor_setup_defaults(&setup); + setup.shell = SHELL_TEST_DESKTOP; + return weston_test_harness_execute_as_client(harness, &setup); } DECLARE_FIXTURE_SETUP(fixture_setup); From e945f1e4e7708b7a376d4a29868b5cf2d0b51854 Mon Sep 17 00:00:00 2001 From: Pekka Paalanen Date: Fri, 4 Mar 2022 12:15:27 +0200 Subject: [PATCH 33/46] tests: preserve ivi runner section Everywhere else where use this trick, we also have 'used' in the attributes, except here. Make this consistent. Fixes: https://gitlab.freedesktop.org/wayland/weston/-/issues/517 Signed-off-by: Pekka Paalanen (cherry picked from commit 5ba7ae2937286e53cb5a1e3fde39a3f347a32209) --- tests/ivi-layout-test-plugin.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ivi-layout-test-plugin.c b/tests/ivi-layout-test-plugin.c index 75ce1436..8b8bdc8d 100644 --- a/tests/ivi-layout-test-plugin.c +++ b/tests/ivi-layout-test-plugin.c @@ -53,7 +53,7 @@ struct runner_test { static void runner_func_##name(struct test_context *); \ \ const struct runner_test runner_test_##name \ - __attribute__ ((section ("plugin_test_section"))) = \ + __attribute__ ((used, section ("plugin_test_section"))) = \ { \ #name, runner_func_##name \ }; \ From a569bc2a300416344c51fa5f69e405cae41ce1fb Mon Sep 17 00:00:00 2001 From: Simon Ser Date: Tue, 26 Jul 2022 12:22:25 +0200 Subject: [PATCH 34/46] build: bump to version 10.0.2 for the point release --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index 0d53c1b0..f5aa460f 100644 --- a/meson.build +++ b/meson.build @@ -1,6 +1,6 @@ project('weston', 'c', - version: '10.0.1', + version: '10.0.2', default_options: [ 'warning_level=3', 'c_std=gnu99', From 2f6a824f0479f47ab0f6d8261cfa71e6d8188761 Mon Sep 17 00:00:00 2001 From: Derek Foreman Date: Wed, 10 Aug 2022 11:43:37 -0500 Subject: [PATCH 35/46] Revert "clients/window: Fix animated cursors" This reverts commit f079f43658d9cbffa402a84101b31072775d3619. This only partially fixed a problem introduced in 992ee045f1b59c037165ecdd752c2f2d78f16ee4 I'm reverting that commit in favor of a different fix, so this broken fix needs to go first. Signed-off-by: Derek Foreman (cherry picked from commit 8b0125d601296e59c87a3c7b4392852635adacb4) --- clients/window.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/clients/window.c b/clients/window.c index a0d988f4..d37fc9f6 100644 --- a/clients/window.c +++ b/clients/window.c @@ -354,6 +354,7 @@ struct input { struct wl_surface *pointer_surface; uint32_t modifiers; uint32_t pointer_enter_serial; + uint32_t cursor_serial; float sx, sy; struct wl_list link; @@ -2748,12 +2749,6 @@ input_remove_pointer_focus(struct input *input) input->pointer_focus = NULL; input->current_cursor = CURSOR_UNSET; cancel_pointer_image_update(input); - wl_surface_destroy(input->pointer_surface); - input->pointer_surface = NULL; - if (input->cursor_frame_cb) { - wl_callback_destroy(input->cursor_frame_cb); - input->cursor_frame_cb = NULL; - } } static void @@ -3883,7 +3878,6 @@ schedule_pointer_image_update(struct input *input, wl_callback_add_listener(input->cursor_frame_cb, &pointer_surface_listener, input); - wl_surface_commit(input->pointer_surface); } static void @@ -3967,15 +3961,30 @@ static const struct wl_callback_listener pointer_surface_listener = { void input_set_pointer_image(struct input *input, int pointer) { + int force = 0; + if (!input->pointer) return; - if (pointer == input->current_cursor) + if (input->pointer_enter_serial > input->cursor_serial) + force = 1; + + if (!force && pointer == input->current_cursor) return; input->current_cursor = pointer; + input->cursor_serial = input->pointer_enter_serial; if (!input->cursor_frame_cb) pointer_surface_frame_callback(input, NULL, 0); + else if (force && !input_set_pointer_special(input)) { + /* The current frame callback may be stuck if, for instance, + * the set cursor request was processed by the server after + * this client lost the focus. In this case the cursor surface + * might not be mapped and the frame callback wouldn't ever + * complete. Send a set_cursor and attach to try to map the + * cursor surface again so that the callback will finish */ + input_set_pointer_image_index(input, 0); + } } struct wl_data_device * From c32e3c6bc0a8399d4fa5195a92dafecd2d559310 Mon Sep 17 00:00:00 2001 From: Derek Foreman Date: Wed, 10 Aug 2022 11:44:17 -0500 Subject: [PATCH 36/46] Revert "clients/window: atomically update pointer cursor" This reverts commit 992ee045f1b59c037165ecdd752c2f2d78f16ee4. Recreating the surface for every cursor change causes flickering cursors on some compositors, and is not the best way to achieve atomic cursor updates Signed-off-by: Derek Foreman (cherry picked from commit ebbe30df3c51c843af6a76b6bb10e85b703462df) --- clients/window.c | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/clients/window.c b/clients/window.c index d37fc9f6..dc9f32ca 100644 --- a/clients/window.c +++ b/clients/window.c @@ -3788,8 +3788,6 @@ input_set_pointer_image_index(struct input *input, int index) struct wl_buffer *buffer; struct wl_cursor *cursor; struct wl_cursor_image *image; - struct wl_surface *prev_surface; - struct display *d = input->display; if (!input->pointer) return; @@ -3808,11 +3806,6 @@ input_set_pointer_image_index(struct input *input, int index) if (!buffer) return; - /* Don't re-use the previous surface, otherwise the new buffer and the - * new hotspot aren't applied atomically. */ - prev_surface = input->pointer_surface; - input->pointer_surface = wl_compositor_create_surface(d->compositor); - wl_surface_attach(input->pointer_surface, buffer, 0, 0); wl_surface_damage(input->pointer_surface, 0, 0, image->width, image->height); @@ -3820,9 +3813,6 @@ input_set_pointer_image_index(struct input *input, int index) wl_pointer_set_cursor(input->pointer, input->pointer_enter_serial, input->pointer_surface, image->hotspot_x, image->hotspot_y); - - if (prev_surface) - wl_surface_destroy(prev_surface); } static const struct wl_callback_listener pointer_surface_listener; @@ -3927,11 +3917,11 @@ pointer_surface_frame_callback(void *data, struct wl_callback *callback, time - input->cursor_anim_start, &duration); - input_set_pointer_image_index(input, i); - if (cursor->image_count > 1) schedule_pointer_image_update(input, cursor, duration, force_frame); + + input_set_pointer_image_index(input, i); } static void @@ -5932,6 +5922,8 @@ display_add_input(struct display *d, uint32_t id, int display_seat_version) input); } + input->pointer_surface = wl_compositor_create_surface(d->compositor); + toytimer_init(&input->cursor_timer, CLOCK_MONOTONIC, d, cursor_timer_func); @@ -5999,8 +5991,7 @@ input_destroy(struct input *input) fini_xkb(input); - if (input->pointer_surface) - wl_surface_destroy(input->pointer_surface); + wl_surface_destroy(input->pointer_surface); wl_list_remove(&input->link); wl_seat_destroy(input->seat); From 8f8ff5f2945a5cb6348d7453730beb4c4983a2b7 Mon Sep 17 00:00:00 2001 From: Derek Foreman Date: Wed, 10 Aug 2022 12:59:20 -0500 Subject: [PATCH 37/46] clients: Set the hotspot with attach if we already have a valid cursor We want atomic hotspot updates - this can't happen with wl_pointer_set_cursor. So if we have a surface that already has a cursor role, just update the hotspot when attaching new content. Signed-off-by: Derek Foreman (cherry picked from commit 646cc1b389cc70e742d5cebef3a3b94e9b0a0c9f) --- clients/window.c | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/clients/window.c b/clients/window.c index dc9f32ca..bc9a8a83 100644 --- a/clients/window.c +++ b/clients/window.c @@ -352,6 +352,8 @@ struct input { struct toytimer cursor_timer; bool cursor_timer_running; struct wl_surface *pointer_surface; + bool pointer_surface_has_role; + int hotspot_x, hotspot_y; uint32_t modifiers; uint32_t pointer_enter_serial; uint32_t cursor_serial; @@ -2776,6 +2778,7 @@ pointer_handle_enter(void *data, struct wl_pointer *pointer, input->display->serial = serial; input->pointer_enter_serial = serial; input->pointer_focus = window; + input->pointer_surface_has_role = false; input->sx = sx; input->sy = sy; @@ -3788,6 +3791,7 @@ input_set_pointer_image_index(struct input *input, int index) struct wl_buffer *buffer; struct wl_cursor *cursor; struct wl_cursor_image *image; + int dx = 0, dy = 0; if (!input->pointer) return; @@ -3806,13 +3810,24 @@ input_set_pointer_image_index(struct input *input, int index) if (!buffer) return; - wl_surface_attach(input->pointer_surface, buffer, 0, 0); + if (input->pointer_surface_has_role) { + dx = input->hotspot_x - image->hotspot_x; + dy = input->hotspot_y - image->hotspot_y; + } + wl_surface_attach(input->pointer_surface, buffer, dx, dy); wl_surface_damage(input->pointer_surface, 0, 0, image->width, image->height); wl_surface_commit(input->pointer_surface); - wl_pointer_set_cursor(input->pointer, input->pointer_enter_serial, - input->pointer_surface, - image->hotspot_x, image->hotspot_y); + + if (!input->pointer_surface_has_role) { + wl_pointer_set_cursor(input->pointer, + input->pointer_enter_serial, + input->pointer_surface, + image->hotspot_x, image->hotspot_y); + input->pointer_surface_has_role = true; + } + input->hotspot_x = image->hotspot_x; + input->hotspot_y = image->hotspot_y; } static const struct wl_callback_listener pointer_surface_listener; @@ -3824,11 +3839,14 @@ input_set_pointer_special(struct input *input) wl_pointer_set_cursor(input->pointer, input->pointer_enter_serial, NULL, 0, 0); + input->pointer_surface_has_role = false; return true; } - if (input->current_cursor == CURSOR_UNSET) + if (input->current_cursor == CURSOR_UNSET) { + input->pointer_surface_has_role = false; return true; + } return false; } @@ -5923,6 +5941,7 @@ display_add_input(struct display *d, uint32_t id, int display_seat_version) } input->pointer_surface = wl_compositor_create_surface(d->compositor); + input->pointer_surface_has_role = false; toytimer_init(&input->cursor_timer, CLOCK_MONOTONIC, d, cursor_timer_func); From f88eed4ea8349610ff0b0de774db6a04d457f7aa Mon Sep 17 00:00:00 2001 From: Derek Foreman Date: Mon, 12 Sep 2022 11:52:19 -0500 Subject: [PATCH 38/46] clients: Fix cursors when compositor gives wl_seat before wl_compositor We have no guarantee that we can create a surface for the pointer at the instant we receive a seat that will (probably eventually) need one. Hold off until we receive an enter event before creating this - at that point we know with certainty that wl_compositor is available, since we've used it to create the surface that was entered. Fixes #659 Signed-off-by: Derek Foreman (cherry picked from commit 11ba13d7178d2c2c74373458ab32cb6ab66897b1) --- clients/window.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/clients/window.c b/clients/window.c index bc9a8a83..1d98ee04 100644 --- a/clients/window.c +++ b/clients/window.c @@ -2778,6 +2778,15 @@ pointer_handle_enter(void *data, struct wl_pointer *pointer, input->display->serial = serial; input->pointer_enter_serial = serial; input->pointer_focus = window; + + /* Some compositors advertise wl_seat before wl_compositor. This + * makes it potentially impossible to create the pointer surface + * when we bind the seat, so we need to create our pointer surface + * now instead. + */ + if (!input->pointer_surface) + input->pointer_surface = wl_compositor_create_surface(input->display->compositor); + input->pointer_surface_has_role = false; input->sx = sx; @@ -5940,7 +5949,6 @@ display_add_input(struct display *d, uint32_t id, int display_seat_version) input); } - input->pointer_surface = wl_compositor_create_surface(d->compositor); input->pointer_surface_has_role = false; toytimer_init(&input->cursor_timer, CLOCK_MONOTONIC, d, @@ -6010,7 +6018,8 @@ input_destroy(struct input *input) fini_xkb(input); - wl_surface_destroy(input->pointer_surface); + if (input->pointer_surface) + wl_surface_destroy(input->pointer_surface); wl_list_remove(&input->link); wl_seat_destroy(input->seat); From 6b539a7c78a66df117e233ccda86242e2cd4533d Mon Sep 17 00:00:00 2001 From: Marius Vlad Date: Wed, 14 Dec 2022 10:35:05 +0200 Subject: [PATCH 39/46] build: bump to version 10.0.3 for the point release Signed-off-by: Marius Vlad --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index f5aa460f..7b81c242 100644 --- a/meson.build +++ b/meson.build @@ -1,6 +1,6 @@ project('weston', 'c', - version: '10.0.2', + version: '10.0.3', default_options: [ 'warning_level=3', 'c_std=gnu99', From d3a636dfb8991b194a3ae1df99179032173d3883 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sergio=20G=C3=B3mez?= Date: Wed, 8 Mar 2023 17:36:34 -0500 Subject: [PATCH 40/46] libweston/input: Remove redundant surface destroy listener in constraints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently, the surface destroy listener in pointer constraints is redundant, since surface destruction already handles pointer constraints destruction (see libweston/compositor.c:weston_surface_unref()). Signed-off-by: Sergio Gómez (cherry picked from commit 64da736d37a7df8b3bd6fd43746ac513bae72748) --- include/libweston/libweston.h | 1 - libweston/input.c | 17 ----------------- 2 files changed, 18 deletions(-) diff --git a/include/libweston/libweston.h b/include/libweston/libweston.h index d99dc763..998cf9a7 100644 --- a/include/libweston/libweston.h +++ b/include/libweston/libweston.h @@ -1441,7 +1441,6 @@ struct weston_pointer_constraint { bool hint_is_pending; struct wl_listener pointer_destroy_listener; - struct wl_listener surface_destroy_listener; struct wl_listener surface_commit_listener; struct wl_listener surface_activate_listener; }; diff --git a/libweston/input.c b/libweston/input.c index 6fb4bed3..eb9bea44 100644 --- a/libweston/input.c +++ b/libweston/input.c @@ -3655,8 +3655,6 @@ enable_pointer_constraint(struct weston_pointer_constraint *constraint, constraint->view = view; pointer_constraint_notify_activated(constraint); weston_pointer_start_grab(constraint->pointer, &constraint->grab); - wl_list_remove(&constraint->surface_destroy_listener.link); - wl_list_init(&constraint->surface_destroy_listener.link); } static bool @@ -3680,7 +3678,6 @@ weston_pointer_constraint_destroy(struct weston_pointer_constraint *constraint) weston_pointer_constraint_disable(constraint); wl_list_remove(&constraint->pointer_destroy_listener.link); - wl_list_remove(&constraint->surface_destroy_listener.link); wl_list_remove(&constraint->surface_commit_listener.link); wl_list_remove(&constraint->surface_activate_listener.link); @@ -3876,16 +3873,6 @@ pointer_constraint_pointer_destroyed(struct wl_listener *listener, void *data) weston_pointer_constraint_destroy(constraint); } -static void -pointer_constraint_surface_destroyed(struct wl_listener *listener, void *data) -{ - struct weston_pointer_constraint *constraint = - container_of(listener, struct weston_pointer_constraint, - surface_destroy_listener); - - weston_pointer_constraint_destroy(constraint); -} - static void pointer_constraint_surface_committed(struct wl_listener *listener, void *data) { @@ -3947,8 +3934,6 @@ weston_pointer_constraint_create(struct weston_surface *surface, constraint->surface_activate_listener.notify = pointer_constraint_surface_activate; - constraint->surface_destroy_listener.notify = - pointer_constraint_surface_destroyed; constraint->surface_commit_listener.notify = pointer_constraint_surface_committed; constraint->pointer_destroy_listener.notify = @@ -3958,8 +3943,6 @@ weston_pointer_constraint_create(struct weston_surface *surface, &constraint->surface_activate_listener); wl_signal_add(&pointer->destroy_signal, &constraint->pointer_destroy_listener); - wl_signal_add(&surface->destroy_signal, - &constraint->surface_destroy_listener); wl_signal_add(&surface->commit_signal, &constraint->surface_commit_listener); From 08979a1d798951d79d7640795604fe0be0a3d943 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sergio=20G=C3=B3mez?= Date: Wed, 8 Mar 2023 17:39:49 -0500 Subject: [PATCH 41/46] libweston: Add view unmap listener to pointer constraints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since the logic of pointer constraints assumes a valid view throughout, add a signal to disable constraints when its current view is unmapped by Weston. The assumption that a previously unmapped view is valid already leads to the constraints code crashing. This can happen when attaching a NULL buffer to the surface and commiting, which effectively unmaps the view with the side effect of clearing the surface's input region, which is then assumed valid inside maybe_warp_confined_pointer(). Fixes: #721 Signed-off-by: Sergio Gómez (cherry picked from commit e3079393c400e3dc6498234d1d092f3072fa8b44) --- include/libweston/libweston.h | 2 ++ libweston/compositor.c | 31 ++++++++++++++++--------------- libweston/input.c | 16 ++++++++++++++++ 3 files changed, 34 insertions(+), 15 deletions(-) diff --git a/include/libweston/libweston.h b/include/libweston/libweston.h index 998cf9a7..2174d34d 100644 --- a/include/libweston/libweston.h +++ b/include/libweston/libweston.h @@ -1289,6 +1289,7 @@ struct weston_view { struct weston_surface *surface; struct wl_list surface_link; struct wl_signal destroy_signal; + struct wl_signal unmap_signal; /* struct weston_paint_node::view_link */ struct wl_list paint_node_list; @@ -1441,6 +1442,7 @@ struct weston_pointer_constraint { bool hint_is_pending; struct wl_listener pointer_destroy_listener; + struct wl_listener view_unmap_listener; struct wl_listener surface_commit_listener; struct wl_listener surface_activate_listener; }; diff --git a/libweston/compositor.c b/libweston/compositor.c index f87eb371..b194bc40 100644 --- a/libweston/compositor.c +++ b/libweston/compositor.c @@ -393,6 +393,7 @@ weston_view_create(struct weston_surface *surface) wl_list_insert(&surface->views, &view->surface_link); wl_signal_init(&view->destroy_signal); + wl_signal_init(&view->unmap_signal); wl_list_init(&view->link); wl_list_init(&view->layer_link.link); wl_list_init(&view->paint_node_list); @@ -2225,22 +2226,22 @@ weston_view_unmap(struct weston_view *view) view->output_mask = 0; weston_surface_assign_output(view->surface); - if (weston_surface_is_mapped(view->surface)) - return; - - wl_list_for_each(seat, &view->surface->compositor->seat_list, link) { - struct weston_touch *touch = weston_seat_get_touch(seat); - struct weston_pointer *pointer = weston_seat_get_pointer(seat); - struct weston_keyboard *keyboard = - weston_seat_get_keyboard(seat); - - if (keyboard && keyboard->focus == view->surface) - weston_keyboard_set_focus(keyboard, NULL); - if (pointer && pointer->focus == view) - weston_pointer_clear_focus(pointer); - if (touch && touch->focus == view) - weston_touch_set_focus(touch, NULL); + if (!weston_surface_is_mapped(view->surface)) { + wl_list_for_each(seat, &view->surface->compositor->seat_list, link) { + struct weston_touch *touch = weston_seat_get_touch(seat); + struct weston_pointer *pointer = weston_seat_get_pointer(seat); + struct weston_keyboard *keyboard = + weston_seat_get_keyboard(seat); + + if (keyboard && keyboard->focus == view->surface) + weston_keyboard_set_focus(keyboard, NULL); + if (pointer && pointer->focus == view) + weston_pointer_clear_focus(pointer); + if (touch && touch->focus == view) + weston_touch_set_focus(touch, NULL); + } } + weston_signal_emit_mutable(&view->unmap_signal, view); } WL_EXPORT void diff --git a/libweston/input.c b/libweston/input.c index eb9bea44..dbfe631b 100644 --- a/libweston/input.c +++ b/libweston/input.c @@ -3655,6 +3655,8 @@ enable_pointer_constraint(struct weston_pointer_constraint *constraint, constraint->view = view; pointer_constraint_notify_activated(constraint); weston_pointer_start_grab(constraint->pointer, &constraint->grab); + wl_signal_add(&constraint->view->unmap_signal, + &constraint->view_unmap_listener); } static bool @@ -3669,6 +3671,8 @@ weston_pointer_constraint_disable(struct weston_pointer_constraint *constraint) constraint->view = NULL; pointer_constraint_notify_deactivated(constraint); weston_pointer_end_grab(constraint->grab.pointer); + wl_list_remove(&constraint->view_unmap_listener.link); + wl_list_init(&constraint->view_unmap_listener.link); } void @@ -3873,6 +3877,16 @@ pointer_constraint_pointer_destroyed(struct wl_listener *listener, void *data) weston_pointer_constraint_destroy(constraint); } +static void +pointer_constraint_view_unmapped(struct wl_listener *listener, void *data) +{ + struct weston_pointer_constraint *constraint = + container_of(listener, struct weston_pointer_constraint, + view_unmap_listener); + + disable_pointer_constraint(constraint); +} + static void pointer_constraint_surface_committed(struct wl_listener *listener, void *data) { @@ -3934,6 +3948,8 @@ weston_pointer_constraint_create(struct weston_surface *surface, constraint->surface_activate_listener.notify = pointer_constraint_surface_activate; + constraint->view_unmap_listener.notify = + pointer_constraint_view_unmapped; constraint->surface_commit_listener.notify = pointer_constraint_surface_committed; constraint->pointer_destroy_listener.notify = From 3e334ff6454a9312c619379c92b216baa5b40941 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sergio=20G=C3=B3mez?= Date: Mon, 6 Mar 2023 16:38:11 -0500 Subject: [PATCH 42/46] libweston: Add assert for valid confine region in maybe_warp_confined_pointer() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Sergio Gómez (cherry picked from commit b6423e59d9116d140e33e925d6dd9bf8324188a7) --- libweston/input.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libweston/input.c b/libweston/input.c index dbfe631b..2aa8fba7 100644 --- a/libweston/input.c +++ b/libweston/input.c @@ -4728,6 +4728,7 @@ maybe_warp_confined_pointer(struct weston_pointer_constraint *constraint) pixman_region32_intersect(&confine_region, &constraint->view->surface->input, &constraint->region); + assert(!pixman_region32_selfcheck(&confine_region)); region_to_outline(&confine_region, &borders); pixman_region32_fini(&confine_region); From ae3780c850540a1e384c78fcaf3e68d4dc3e160a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sergio=20G=C3=B3mez?= Date: Thu, 23 Mar 2023 12:38:38 -0500 Subject: [PATCH 43/46] libweston/input: Fix assert for valid confine region MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We need only check that the region is not empty. If either the input region or the constraint region have degenerate extents, the intersection from the previous instruction will set confine_region->data to pixman_region_empty_data. Fixes: b6423e59 Signed-off-by: Sergio Gómez (cherry picked from commit 1ed88f60c0125988cf1d952f0dabf568bfd82a13) --- libweston/input.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libweston/input.c b/libweston/input.c index 2aa8fba7..ec25271a 100644 --- a/libweston/input.c +++ b/libweston/input.c @@ -4728,7 +4728,7 @@ maybe_warp_confined_pointer(struct weston_pointer_constraint *constraint) pixman_region32_intersect(&confine_region, &constraint->view->surface->input, &constraint->region); - assert(!pixman_region32_selfcheck(&confine_region)); + assert(pixman_region32_not_empty(&confine_region)); region_to_outline(&confine_region, &borders); pixman_region32_fini(&confine_region); From 04ab5a17fa7aa716744f73f106cf90683b29aa78 Mon Sep 17 00:00:00 2001 From: Marius Vlad Date: Wed, 17 May 2023 11:39:55 +0300 Subject: [PATCH 44/46] build: bump to version 10.0.4 for the point release Signed-off-by: Marius Vlad --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index 7b81c242..c3b514d0 100644 --- a/meson.build +++ b/meson.build @@ -1,6 +1,6 @@ project('weston', 'c', - version: '10.0.3', + version: '10.0.4', default_options: [ 'warning_level=3', 'c_std=gnu99', From 00624d45b7b46996b12151ccda59ed47e45c6185 Mon Sep 17 00:00:00 2001 From: Marius Vlad Date: Wed, 2 Aug 2023 09:42:04 +0300 Subject: [PATCH 45/46] backend-drm/meson.build: Require at least mesa 21.1.1 We seem to be using at least mesa 21.1.1 since Weston 10, but we never explicitly asked for it. Fixes: #790 Signed-off-by: Marius Vlad (cherry picked from commit 0713ea7ee6216e45ebdb67cef63bcef7961d1d4e) --- libweston/backend-drm/meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libweston/backend-drm/meson.build b/libweston/backend-drm/meson.build index 23db9127..b7af834b 100644 --- a/libweston/backend-drm/meson.build +++ b/libweston/backend-drm/meson.build @@ -42,7 +42,7 @@ deps_drm = [ ] if get_option('renderer-gl') - dep_gbm = dependency('gbm', required: false) + dep_gbm = dependency('gbm', required: false, version: '>= 21.1.1') if not dep_gbm.found() error('drm-backend with GL renderer requires gbm which was not found. Or, you can use \'-Drenderer-gl=false\'.') endif From 2e543aef219b3efc80ce23660fcce64cbcfaed41 Mon Sep 17 00:00:00 2001 From: Marius Vlad Date: Wed, 2 Aug 2023 19:02:12 +0300 Subject: [PATCH 46/46] build: bump to version 10.0.5 for the point release Signed-off-by: Marius Vlad --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index c3b514d0..8b3d46e7 100644 --- a/meson.build +++ b/meson.build @@ -1,6 +1,6 @@ project('weston', 'c', - version: '10.0.4', + version: '10.0.5', default_options: [ 'warning_level=3', 'c_std=gnu99',