[PATCH weston v2 2/2] compositor: allow to control the vt switching

Pekka Paalanen ppaalanen at gmail.com
Fri Nov 25 14:10:41 UTC 2016


On Wed, 29 Jun 2016 11:57:12 +0200
Giulio Camuffo <giuliocamuffo at gmail.com> wrote:

> A compositor may want to control the vt switching, for example to
> ensure to have a lock screen before it. To enable that add a vfunc
> that will be called when CTRL+ALT+FN is pressed. The default behavior
> is to do the switching, but the user can change it by using the new
> weston_compositor_set_vt_switcher() function, so that it can delay the
> switching to later, by calling weston_compositor_activate_vt().
> 
> Signed-off-by: Giulio Camuffo <giuliocamuffo at gmail.com>
> ---
> 
> v2: split the get_vt addition to its own commit
> 
>  Makefile.am               |  2 +-
>  compositor/main.c         |  1 +
>  libweston/compositor.c    | 18 ++++++++++++++++++
>  libweston/compositor.h    | 39 +++++++++++++++++++++++++++++++++++++++
>  libweston/launcher-impl.h |  1 +
>  libweston/launcher-util.c |  6 +++++-
>  6 files changed, 65 insertions(+), 2 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 16433b8..a906dd3 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -66,7 +66,7 @@ libweston_la_CPPFLAGS = $(AM_CPPFLAGS) -DIN_WESTON
>  libweston_la_CFLAGS = $(AM_CFLAGS) $(COMPOSITOR_CFLAGS) $(LIBUNWIND_CFLAGS)
>  libweston_la_LIBADD = $(COMPOSITOR_LIBS) $(LIBUNWIND_LIBS) \
>  	$(DLOPEN_LIBS) -lm $(CLOCK_GETTIME_LIBS) \
> -	$(LIBINPUT_BACKEND_LIBS) libshared.la
> +	$(LIBINPUT_BACKEND_LIBS) libshared.la libsession-helper.la

Hmm... why is this change here?

>  libweston_la_LDFLAGS = -release ${LIBWESTON_ABI_VERSION}
>  
>  libweston_la_SOURCES =					\
> diff --git a/compositor/main.c b/compositor/main.c
> index 6cf9194..1285f44 100644
> --- a/compositor/main.c
> +++ b/compositor/main.c
> @@ -54,6 +54,7 @@
>  #include "git-version.h"
>  #include "version.h"
>  #include "weston.h"
> +#include "launcher-util.h"

I don't see launcher-util.h being installed, so compositor should not
be accessing it either. And it doesn't - a left-over?

>  
>  #include "compositor-drm.h"
>  #include "compositor-headless.h"
> diff --git a/libweston/compositor.c b/libweston/compositor.c
> index 0c405ac..7bf9c69 100644
> --- a/libweston/compositor.c
> +++ b/libweston/compositor.c
> @@ -55,6 +55,8 @@
>  
>  #include "compositor.h"
>  #include "viewporter-server-protocol.h"
> +#include "launcher-impl.h"
> +#include "launcher-util.h"
>  #include "presentation-time-server-protocol.h"
>  #include "shared/helpers.h"
>  #include "shared/os-compatibility.h"
> @@ -4599,6 +4601,22 @@ compositor_bind(struct wl_client *client,
>  }
>  
>  WL_EXPORT int
> +weston_compositor_activate_vt(struct weston_compositor *compositor, int vt)
> +{
> +	if (compositor->launcher)
> +		return weston_launcher_activate_vt(compositor->launcher, vt);
> +	return -1;
> +}
> +
> +WL_EXPORT void
> +weston_compositor_set_vt_switcher(struct weston_compositor *compositor,
> +				  weston_compositor_vt_switcher_func_t switcher)
> +{
> +	if (compositor->launcher)
> +		compositor->launcher->vt_switcher = switcher;
> +}
> +
> +WL_EXPORT int
>  weston_environment_get_fd(const char *env)
>  {
>  	char *e, *end;
> diff --git a/libweston/compositor.h b/libweston/compositor.h
> index 099d4bf..cda6c97 100644
> --- a/libweston/compositor.h
> +++ b/libweston/compositor.h
> @@ -1371,6 +1371,45 @@ void
>  weston_compositor_set_default_pointer_grab(struct weston_compositor *compositor,
>  			const struct weston_pointer_grab_interface *interface);
>  
> +/**
> + * Request a vt switch for the compositor.
> + *
> + * This will only work if the compositor is running as the owner of
> + * the session.
> + *
> + * \param launcher The compositor instance.

launcher?

> + * \param vt The vt to switch to.
> + *
> + * Returns 0 on success, -1 otherwise.
> + *
> + * \sa weston_compositor_set_vt_switcher
> + */
> +int
> +weston_compositor_activate_vt(struct weston_compositor *compositor, int vt);
> +
> +typedef void (*weston_compositor_vt_switcher_func_t)(
> +		struct weston_compositor *compositor, int vt);
> +/**
> + * Set the vt switcher for the compositor.
> + *
> + * If the compositor is the owner of the session, the CTRL+ALT+FN key
> + * combinations will trigger a vt switch. The default behavior is to do
> + * the switching immediately, but some compositors may want to make sure to
> + * e.g. draw a lock screen before doing the switch.
> + * This function allows to register a custom vt switcher so that the actual
> + * vt switch can be controlled by calling \a weston_compositor_activate_vt.

The rationale is good.

> + *
> + * \param compositor The compositor instance.
> + * \param switcher The vt switcher function, which will be called when a
> + *                 CTRL+ALT+FN key combination is pressed, carrying the
> + *                 requested vt.
> + *
> + * \sa weston_compositor_activate_vt
> + */
> +void
> +weston_compositor_set_vt_switcher(struct weston_compositor *compositor,
> +				  weston_compositor_vt_switcher_func_t switcher);
> +
>  int
>  weston_environment_get_fd(const char *env);
>  
> diff --git a/libweston/launcher-impl.h b/libweston/launcher-impl.h
> index 0bc3245..8b0531d 100644
> --- a/libweston/launcher-impl.h
> +++ b/libweston/launcher-impl.h
> @@ -39,6 +39,7 @@ struct launcher_interface {
>  
>  struct weston_launcher {
>  	struct launcher_interface *iface;
> +	weston_compositor_vt_switcher_func_t vt_switcher;
>  };
>  
>  extern struct launcher_interface launcher_logind_iface;
> diff --git a/libweston/launcher-util.c b/libweston/launcher-util.c
> index 5fbfdcf..7bd91ae 100644
> --- a/libweston/launcher-util.c
> +++ b/libweston/launcher-util.c
> @@ -99,11 +99,15 @@ switch_vt_binding(struct weston_keyboard *keyboard,
>  	struct weston_compositor *compositor = data;
>  	struct weston_launcher *launcher = compositor->launcher;
>  	int vt = key - KEY_F1 + 1;
> +	weston_compositor_vt_switcher_func_t switcher = launcher->vt_switcher;
>  
>  	if (vt == launcher->iface->get_vt(launcher))
>  		return;
>  
> -	weston_launcher_activate_vt(launcher, vt);
> +	if (switcher)
> +		launcher->vt_switcher(compositor, vt);
> +	else
> +		weston_launcher_activate_vt(launcher, vt);
>  }
>  
>  WL_EXPORT void

However, I think the architecture here is unnecessarily complicated.
Now the call chain looks like:

launcher-util sets key bindings
user smashes keys -> launcher-util -> switcher callback ->
	weston_compositor_activate_vt -> launcher-util ->
	launcher-backend

If the compositor (or shell) was resposible of setting the key bindings
itself, it would look like this:

shell sets key bindings
user smashes keys -> shell -> weston_compositor_activate_vt -> ....

Here in the shell step the compositor could retrieve Weston's VT, check
if it's a no-op, do all the prep e.g. locking and call the VT switching
function. This would be much easier to understand from API point of view.

What do you think?


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 801 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20161125/bb22e801/attachment-0001.sig>


More information about the wayland-devel mailing list