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

Tue Feb 7 16:31:09 UTC 2017

2017-02-07 16:12 GMT+01:00 Pekka Paalanen <ppaalanen at gmail.com>:
> On Mon,  5 Dec 2016 14:50:37 +0100
> 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>
>> ---
>>
>> v3: removed leftover include, fix wrong name in documentation
>>
>>  Makefile.am               |  2 +-
>>  libweston/compositor.c    | 18 ++++++++++++++++++
>>  libweston/compositor.h    | 39 +++++++++++++++++++++++++++++++++++++++
>>  libweston/launcher-impl.h |  1 +
>>  libweston/launcher-util.c |  6 +++++-
>>  5 files changed, 64 insertions(+), 2 deletions(-)
>>
>> diff --git a/Makefile.am b/Makefile.am
>> index 2219e3d..1a13b85 100644
>> --- a/Makefile.am
>> +++ b/Makefile.am
>> @@ -74,7 +74,7 @@ libweston_ at LIBWESTON_MAJOR@_la_CPPFLAGS = $(AM_CPPFLAGS) -DIN_WESTON
>>  libweston_ at LIBWESTON_MAJOR@_la_CFLAGS = $(AM_CFLAGS) $(COMPOSITOR_CFLAGS) $(LIBUNWIND_CFLAGS)
>>  libweston_ at LIBWESTON_MAJOR@_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
>>  libweston_ at LIBWESTON_MAJOR@_la_LDFLAGS = -version-info $(LT_VERSION_INFO)
>>
>>  libweston_ at LIBWESTON_MAJOR@_la_SOURCES =                     \
>
> Hi,
>
> it is the last moment before the beta release, and while minor features
> would be fine to land, I hesitate mostly because of making libweston.so
> directly depend on libdrm, logind and dbus if support for them is
> enabled on build. OTOH this patch also adds public API, so I think
> those two combined maybe it's better to postpone merging this to the
> next cycle.

Yes, it makes sense.

>
> Another thought is that instead of having
> weston_compositor_activate_vt() and weston_compositor_set_vt_switcher()
> as libweston API it could be an extension API advertized by
> libsession-helper.la through the plugin registry. Then there is no need
> to call libsession-helper from libweston core, avoiding the
> dependencies.
>
> That's what the plugin registry is for: offering e.g. backend-specific
> services. Then it is also obvious to the compositor whether the feature
> is even supposed to work.

Right, that's a good point.

>
> If that sounds good to you, I would be more at ease. If not, then I
> suppose we'll just merge this the next cycle.

That's ok, i'll send a new version sometime in the next six months :P.

Thanks,
Giulio

>
>
> Thanks,
> pq
>
>
>> diff --git a/libweston/compositor.c b/libweston/compositor.c
>> index 6457858..6e44c72 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"
>> @@ -4922,6 +4924,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;
>> diff --git a/libweston/compositor.h b/libweston/compositor.h
>> index ce3d9ab..114d618 100644
>> --- a/libweston/compositor.h
>> +++ b/libweston/compositor.h
>> @@ -1447,6 +1447,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 compositor The compositor instance.
>> + * \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.
>> + *
>> + * \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 8d09025..8ba1673 100644
>> --- a/libweston/launcher-impl.h
>> +++ b/libweston/launcher-impl.h
>> @@ -40,6 +40,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 2b828be..5845cf3 100644
>> --- a/libweston/launcher-util.c
>> +++ b/libweston/launcher-util.c
>> @@ -100,11 +100,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
>