diff mbox series

weston: Backport patches to always activate the top-level surface

Message ID 20220105222141.25234-1-marex@denx.de
State New
Headers show
Series weston: Backport patches to always activate the top-level surface | expand

Commit Message

Marek Vasut Jan. 5, 2022, 10:21 p.m. UTC 
In case the device has only touchscreen input device and no keyboard or mouse,
the top level surface is never activated. The behavior differs from a device
which has a keyboard (or gpio-keys, or even uinput-emulated keyboard), where
callchain activate()->weston_view_activate()->weston_seat_set_keyboard_focus()->
weston_keyboard_set_focus()->wl_signal_emit(&keyboard->focus_signal, keyboard)->
handle_keyboard_focus()->weston_desktop_surface_set_activated(..., true); sets
the top level surface as activated. On device with touchscreen, the above is
never called, hence the top level surface is never activated. Add explicit
weston_desktop_surface_set_activated(shsurf->desktop_surface, true); into
activate() to always active the top level surface.

This fixes at least two known issues on such devices:
- Wayland terminal cursor is an empty bar (full bar with keyboard present)
- Chromium dropdown menus are randomly placed (they are placed correctly
  when keyboard is present, because then chromium can find the activated
  top level surface)

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Steve Sakoman <steve@sakoman.com>
---
 ...move-no-op-de-activation-of-the-xdg-.patch | 32 ++++++
 ...name-gain-lose-keyboard-focus-to-act.patch | 57 +++++++++++
 ...bed-keyboard-focus-handle-code-when-.patch | 99 +++++++++++++++++++
 meta/recipes-graphics/wayland/weston_8.0.0.bb |  3 +
 4 files changed, 191 insertions(+)
 create mode 100644 meta/recipes-graphics/wayland/weston/0002-desktop-shell-Remove-no-op-de-activation-of-the-xdg-.patch
 create mode 100644 meta/recipes-graphics/wayland/weston/0003-desktop-shell-Rename-gain-lose-keyboard-focus-to-act.patch
 create mode 100644 meta/recipes-graphics/wayland/weston/0004-desktop-shell-Embed-keyboard-focus-handle-code-when-.patch

Comments

Marek Vasut Jan. 5, 2022, 10:25 p.m. UTC | #1 
On 1/5/22 23:21, Marek Vasut wrote:
> In case the device has only touchscreen input device and no keyboard or mouse,
> the top level surface is never activated. The behavior differs from a device
> which has a keyboard (or gpio-keys, or even uinput-emulated keyboard), where
> callchain activate()->weston_view_activate()->weston_seat_set_keyboard_focus()->
> weston_keyboard_set_focus()->wl_signal_emit(&keyboard->focus_signal, keyboard)->
> handle_keyboard_focus()->weston_desktop_surface_set_activated(..., true); sets
> the top level surface as activated. On device with touchscreen, the above is
> never called, hence the top level surface is never activated. Add explicit
> weston_desktop_surface_set_activated(shsurf->desktop_surface, true); into
> activate() to always active the top level surface.
> 
> This fixes at least two known issues on such devices:
> - Wayland terminal cursor is an empty bar (full bar with keyboard present)
> - Chromium dropdown menus are randomly placed (they are placed correctly
>    when keyboard is present, because then chromium can find the activated
>    top level surface)
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Steve Sakoman <steve@sakoman.com>

And that Subject should've had [dunfell] tag, sorry.
Do you need a resend ?
Steve Sakoman Jan. 5, 2022, 10:40 p.m. UTC | #2 
On Wed, Jan 5, 2022 at 12:25 PM Marek Vasut <marex@denx.de> wrote:
>
> On 1/5/22 23:21, Marek Vasut wrote:
> > In case the device has only touchscreen input device and no keyboard or mouse,
> > the top level surface is never activated. The behavior differs from a device
> > which has a keyboard (or gpio-keys, or even uinput-emulated keyboard), where
> > callchain activate()->weston_view_activate()->weston_seat_set_keyboard_focus()->
> > weston_keyboard_set_focus()->wl_signal_emit(&keyboard->focus_signal, keyboard)->
> > handle_keyboard_focus()->weston_desktop_surface_set_activated(..., true); sets
> > the top level surface as activated. On device with touchscreen, the above is
> > never called, hence the top level surface is never activated. Add explicit
> > weston_desktop_surface_set_activated(shsurf->desktop_surface, true); into
> > activate() to always active the top level surface.
> >
> > This fixes at least two known issues on such devices:
> > - Wayland terminal cursor is an empty bar (full bar with keyboard present)
> > - Chromium dropdown menus are randomly placed (they are placed correctly
> >    when keyboard is present, because then chromium can find the activated
> >    top level surface)
> >
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Steve Sakoman <steve@sakoman.com>
>
> And that Subject should've had [dunfell] tag, sorry.
> Do you need a resend ?

No, I've got it!

Thanks,

Steve
Marek Vasut Jan. 5, 2022, 11:07 p.m. UTC | #3 
On 1/5/22 23:40, Steve Sakoman wrote:
> On Wed, Jan 5, 2022 at 12:25 PM Marek Vasut <marex@denx.de> wrote:
>>
>> On 1/5/22 23:21, Marek Vasut wrote:
>>> In case the device has only touchscreen input device and no keyboard or mouse,
>>> the top level surface is never activated. The behavior differs from a device
>>> which has a keyboard (or gpio-keys, or even uinput-emulated keyboard), where
>>> callchain activate()->weston_view_activate()->weston_seat_set_keyboard_focus()->
>>> weston_keyboard_set_focus()->wl_signal_emit(&keyboard->focus_signal, keyboard)->
>>> handle_keyboard_focus()->weston_desktop_surface_set_activated(..., true); sets
>>> the top level surface as activated. On device with touchscreen, the above is
>>> never called, hence the top level surface is never activated. Add explicit
>>> weston_desktop_surface_set_activated(shsurf->desktop_surface, true); into
>>> activate() to always active the top level surface.
>>>
>>> This fixes at least two known issues on such devices:
>>> - Wayland terminal cursor is an empty bar (full bar with keyboard present)
>>> - Chromium dropdown menus are randomly placed (they are placed correctly
>>>     when keyboard is present, because then chromium can find the activated
>>>     top level surface)
>>>
>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>> Cc: Steve Sakoman <steve@sakoman.com>
>>
>> And that Subject should've had [dunfell] tag, sorry.
>> Do you need a resend ?
> 
> No, I've got it!
> 
> Thanks,

Thanks
diff mbox series

Patch

diff --git a/meta/recipes-graphics/wayland/weston/0002-desktop-shell-Remove-no-op-de-activation-of-the-xdg-.patch b/meta/recipes-graphics/wayland/weston/0002-desktop-shell-Remove-no-op-de-activation-of-the-xdg-.patch
new file mode 100644
index 0000000000..fb36d3817a
--- /dev/null
+++ b/meta/recipes-graphics/wayland/weston/0002-desktop-shell-Remove-no-op-de-activation-of-the-xdg-.patch
@@ -0,0 +1,32 @@ 
+From 5c74a0640e873694bf60a88eceb21f664cb4b8f7 Mon Sep 17 00:00:00 2001
+From: Marius Vlad <marius.vlad@collabora.com>
+Date: Fri, 5 Mar 2021 20:03:49 +0200
+Subject: [PATCH 2/5] desktop-shell: Remove no-op de-activation of the xdg
+ top-level surface
+
+The shsurf is calloc'ed so the surface count is always 0.  Not only
+that but the surface is not set as active by default, so there's no
+need to de-activate it.
+
+Upstream-Status: Backport [05bef4c18a3e82376a46a4a28d978389c4c0fd0f]
+Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
+---
+ desktop-shell/shell.c | 2 --
+ 1 file changed, 2 deletions(-)
+
+diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
+index 442a625f..3791be25 100644
+--- a/desktop-shell/shell.c
++++ b/desktop-shell/shell.c
+@@ -2427,8 +2427,6 @@ desktop_surface_added(struct weston_desktop_surface *desktop_surface,
+ 	wl_list_init(&shsurf->children_link);
+ 
+ 	weston_desktop_surface_set_user_data(desktop_surface, shsurf);
+-	weston_desktop_surface_set_activated(desktop_surface,
+-					     shsurf->focus_count > 0);
+ }
+ 
+ static void
+-- 
+2.34.1
+
diff --git a/meta/recipes-graphics/wayland/weston/0003-desktop-shell-Rename-gain-lose-keyboard-focus-to-act.patch b/meta/recipes-graphics/wayland/weston/0003-desktop-shell-Rename-gain-lose-keyboard-focus-to-act.patch
new file mode 100644
index 0000000000..dcd0700fca
--- /dev/null
+++ b/meta/recipes-graphics/wayland/weston/0003-desktop-shell-Rename-gain-lose-keyboard-focus-to-act.patch
@@ -0,0 +1,57 @@ 
+From edb31c456ae3da7ffffefb668a37ab88075c4b67 Mon Sep 17 00:00:00 2001
+From: Marius Vlad <marius.vlad@collabora.com>
+Date: Fri, 5 Mar 2021 21:40:22 +0200
+Subject: [PATCH 3/5] desktop-shell: Rename gain/lose keyboard focus to
+ activate/de-activate
+
+This way it better reflects that it handles activation rather that input
+focus.
+
+Upstream-Status: Backport [ab39e1d76d4f6715cb300bc37f5c2a0e2d426208]
+Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
+---
+ desktop-shell/shell.c | 8 ++++----
+ 1 file changed, 4 insertions(+), 4 deletions(-)
+
+diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
+index 3791be25..c4669f11 100644
+--- a/desktop-shell/shell.c
++++ b/desktop-shell/shell.c
+@@ -1869,14 +1869,14 @@ handle_pointer_focus(struct wl_listener *listener, void *data)
+ }
+ 
+ static void
+-shell_surface_lose_keyboard_focus(struct shell_surface *shsurf)
++shell_surface_deactivate(struct shell_surface *shsurf)
+ {
+ 	if (--shsurf->focus_count == 0)
+ 		weston_desktop_surface_set_activated(shsurf->desktop_surface, false);
+ }
+ 
+ static void
+-shell_surface_gain_keyboard_focus(struct shell_surface *shsurf)
++shell_surface_activate(struct shell_surface *shsurf)
+ {
+ 	if (shsurf->focus_count++ == 0)
+ 		weston_desktop_surface_set_activated(shsurf->desktop_surface, true);
+@@ -1891,7 +1891,7 @@ handle_keyboard_focus(struct wl_listener *listener, void *data)
+ 	if (seat->focused_surface) {
+ 		struct shell_surface *shsurf = get_shell_surface(seat->focused_surface);
+ 		if (shsurf)
+-			shell_surface_lose_keyboard_focus(shsurf);
++			shell_surface_deactivate(shsurf);
+ 	}
+ 
+ 	seat->focused_surface = weston_surface_get_main_surface(keyboard->focus);
+@@ -1899,7 +1899,7 @@ handle_keyboard_focus(struct wl_listener *listener, void *data)
+ 	if (seat->focused_surface) {
+ 		struct shell_surface *shsurf = get_shell_surface(seat->focused_surface);
+ 		if (shsurf)
+-			shell_surface_gain_keyboard_focus(shsurf);
++			shell_surface_activate(shsurf);
+ 	}
+ }
+ 
+-- 
+2.34.1
+
diff --git a/meta/recipes-graphics/wayland/weston/0004-desktop-shell-Embed-keyboard-focus-handle-code-when-.patch b/meta/recipes-graphics/wayland/weston/0004-desktop-shell-Embed-keyboard-focus-handle-code-when-.patch
new file mode 100644
index 0000000000..7ca72f8494
--- /dev/null
+++ b/meta/recipes-graphics/wayland/weston/0004-desktop-shell-Embed-keyboard-focus-handle-code-when-.patch
@@ -0,0 +1,99 @@ 
+From 899ad5a6a8a92f2c10e0694a45c982b7d878aed6 Mon Sep 17 00:00:00 2001
+From: Marius Vlad <marius.vlad@collabora.com>
+Date: Fri, 5 Mar 2021 21:44:26 +0200
+Subject: [PATCH 4/5] desktop-shell: Embed keyboard focus handle code when
+ activating
+
+We shouldn't be constrained by having a keyboard plugged-in, so avoid
+activating/de-activating the window/surface in the keyboard focus
+handler and embed it straight into the window activation part.
+
+Upstream-Status: Backport [f12697bb3e4c6eb85437ed905e7de44ae2a0ba69]
+Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
+---
+ desktop-shell/shell.c | 41 +++++++++++++++++++++++++----------------
+ 1 file changed, 25 insertions(+), 16 deletions(-)
+
+diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
+index c4669f11..c6a4fe91 100644
+--- a/desktop-shell/shell.c
++++ b/desktop-shell/shell.c
+@@ -1885,22 +1885,7 @@ shell_surface_activate(struct shell_surface *shsurf)
+ static void
+ handle_keyboard_focus(struct wl_listener *listener, void *data)
+ {
+-	struct weston_keyboard *keyboard = data;
+-	struct shell_seat *seat = get_shell_seat(keyboard->seat);
+-
+-	if (seat->focused_surface) {
+-		struct shell_surface *shsurf = get_shell_surface(seat->focused_surface);
+-		if (shsurf)
+-			shell_surface_deactivate(shsurf);
+-	}
+-
+-	seat->focused_surface = weston_surface_get_main_surface(keyboard->focus);
+-
+-	if (seat->focused_surface) {
+-		struct shell_surface *shsurf = get_shell_surface(seat->focused_surface);
+-		if (shsurf)
+-			shell_surface_activate(shsurf);
+-	}
++	/* FIXME: To be removed later. */
+ }
+ 
+ /* The surface will be inserted into the list immediately after the link
+@@ -2438,6 +2423,7 @@ desktop_surface_removed(struct weston_desktop_surface *desktop_surface,
+ 	struct shell_surface *shsurf_child, *tmp;
+ 	struct weston_surface *surface =
+ 		weston_desktop_surface_get_surface(desktop_surface);
++	struct weston_seat *seat;
+ 
+ 	if (!shsurf)
+ 		return;
+@@ -2448,6 +2434,18 @@ desktop_surface_removed(struct weston_desktop_surface *desktop_surface,
+ 	}
+ 	wl_list_remove(&shsurf->children_link);
+ 
++	wl_list_for_each(seat, &shsurf->shell->compositor->seat_list, link) {
++		struct shell_seat *shseat = get_shell_seat(seat);
++		/* activate() controls the focused surface activation and
++		 * removal of a surface requires invalidating the
++		 * focused_surface to avoid activate() use a stale (and just
++		 * removed) surface when attempting to de-activate it. It will
++		 * also update the focused_surface once it has a chance to run.
++		 */
++		if (surface == shseat->focused_surface)
++			shseat->focused_surface = NULL;
++	}
++
+ 	wl_signal_emit(&shsurf->destroy_signal, shsurf);
+ 
+ 	if (shsurf->fullscreen.black_view)
+@@ -3836,6 +3834,7 @@ activate(struct desktop_shell *shell, struct weston_view *view,
+ 	struct workspace *ws;
+ 	struct weston_surface *old_es;
+ 	struct shell_surface *shsurf, *shsurf_child;
++	struct shell_seat *shseat = get_shell_seat(seat);
+ 
+ 	main_surface = weston_surface_get_main_surface(es);
+ 	shsurf = get_shell_surface(main_surface);
+@@ -3855,6 +3854,16 @@ activate(struct desktop_shell *shell, struct weston_view *view,
+ 
+ 	weston_view_activate(view, seat, flags);
+ 
++	if (shseat->focused_surface) {
++		struct shell_surface *current_focus =
++			get_shell_surface(shseat->focused_surface);
++		assert(current_focus);
++		shell_surface_deactivate(current_focus);
++	}
++
++	shseat->focused_surface = main_surface;
++	shell_surface_activate(shsurf);
++
+ 	state = ensure_focus_state(shell, seat);
+ 	if (state == NULL)
+ 		return;
+-- 
+2.34.1
+
diff --git a/meta/recipes-graphics/wayland/weston_8.0.0.bb b/meta/recipes-graphics/wayland/weston_8.0.0.bb
index 2b120d7404..e647fbc686 100644
--- a/meta/recipes-graphics/wayland/weston_8.0.0.bb
+++ b/meta/recipes-graphics/wayland/weston_8.0.0.bb
@@ -12,6 +12,9 @@  SRC_URI = "https://wayland.freedesktop.org/releases/${BPN}-${PV}.tar.xz \
            file://systemd-notify.weston-start \
            file://xwayland.weston-start \
            file://0001-weston-launch-Provide-a-default-version-that-doesn-t.patch \
+           file://0002-desktop-shell-Remove-no-op-de-activation-of-the-xdg-.patch \
+           file://0003-desktop-shell-Rename-gain-lose-keyboard-focus-to-act.patch \
+           file://0004-desktop-shell-Embed-keyboard-focus-handle-code-when-.patch \
 "
 SRC_URI[md5sum] = "53e4810d852df0601d01fd986a5b22b3"
 SRC_URI[sha256sum] = "7518b49b2eaa1c3091f24671bdcc124fd49fc8f1af51161927afa4329c027848"