[PATCH] libweston-desktop: add signals for title, app_id

[Matt Hoosier]

I'd find it a little bit inconvenient for the title, class, and app-id to
be treated as buffered state. They make a great poor-man's way of
identifying surfaces for embedded systems that implement simple layer
management policies without wholesale adopting IVI shell (whose lack of
wl_shell makes it difficult to integrate off-the-rack Wayland libraries and
apps). Since the surface title doesn't have anything to do with correct
graphical presentation, it'd be nicer if that data is just available
immediately to the rest of the shell rather than being something that only
trickles in whenever the app happens to post a buffer next.

On Tue, Jul 25, 2017 at 12:17 AM, Jonas Ådahl <jadahl at gmail.com> wrote:

> On Fri, Jul 21, 2017 at 05:21:38PM +0200, Quentin Glidic wrote:
> > On 7/20/17 4:33 PM, Matt Hoosier wrote:
> > > It's useful for the shell implementation to know when these change,
> > > for example to relay the information on to taskbars or similar.
> >
> > The idea is good, we need something like that. (But maybe not, see
> below).
> >
> >
> > > To avoid ABI changes or the need to make the weston_desktop_surface
> > > definition public, new functions are introduced for attaching
> > > listeners to these signals.
> >
> > We don’t care that much about ABI changes, but you’re right that I do
> care
> > about the struct staying private.
> >
> >
> > > Signed-off-by: Matt Hoosier <matt.hoosier at gmail.com>
> > > ---
> > >   libweston-desktop/libweston-desktop.h |  6 ++++++
> > >   libweston-desktop/surface.c           | 21 +++++++++++++++++++++
> > >   2 files changed, 27 insertions(+)
> > >
> > > diff --git a/libweston-desktop/libweston-desktop.h
> b/libweston-desktop/libweston-desktop.h
> > > index 03b04c7..e38257e 100644
> > > --- a/libweston-desktop/libweston-desktop.h
> > > +++ b/libweston-desktop/libweston-desktop.h
> > > @@ -164,6 +164,12 @@ weston_desktop_surface_set_size(struct
> weston_desktop_surface *surface,
> > >                             int32_t width, int32_t height);
> > >   void
> > >   weston_desktop_surface_close(struct weston_desktop_surface
> *surface);
> > > +void
> > > +weston_desktop_surface_add_title_listener(struct
> weston_desktop_surface *surface,
> > > +                                     struct wl_listener *listener);
> > > +void
> > > +weston_desktop_surface_add_app_id_listener(struct
> weston_desktop_surface *surface,
> > > +                                      struct wl_listener *listener);
> > >   void *
> > >   weston_desktop_surface_get_user_data(struct weston_desktop_surface
> *surface);
> > > diff --git a/libweston-desktop/surface.c b/libweston-desktop/surface.c
> > > index d3be936..97a455c 100644
> > > --- a/libweston-desktop/surface.c
> > > +++ b/libweston-desktop/surface.c
> > > @@ -64,6 +64,8 @@ struct weston_desktop_surface {
> > >             char *title;
> > >             char *app_id;
> > >             pid_t pid;
> > > +           struct wl_signal title_signal;
> > > +           struct wl_signal app_id_signal;
> > >     };
> > >     struct {
> > >             struct weston_desktop_surface *parent;
> > > @@ -287,6 +289,9 @@ weston_desktop_surface_create(struct
> weston_desktop *desktop,
> > >     wl_list_init(&surface->view_list);
> > >     wl_list_init(&surface->grab_link);
> > > +   wl_signal_init(&surface->title_signal);
> > > +   wl_signal_init(&surface->app_id_signal);
> > > +
> > >     return surface;
> > >   }
> > > @@ -511,6 +516,20 @@ weston_desktop_surface_close(struct
> weston_desktop_surface *surface)
> > >
> surface->implementation_data);
> > >   }
> > > +WL_EXPORT void
> > > +weston_desktop_surface_add_title_listener(struct
> weston_desktop_surface *surface,
> > > +                                     struct wl_listener *listener)
> > > +{
> > > +   wl_signal_add(&surface->title_signal, listener);
> > > +}
> > > +
> > > +WL_EXPORT void
> > > +weston_desktop_surface_add_app_id_listener(struct
> weston_desktop_surface *surface,
> > > +                                      struct wl_listener *listener)
> > > +{
> > > +   wl_signal_add(&surface->app_id_signal, listener);
> > > +}
> > > +
> > >   struct weston_desktop_surface *
> > >   weston_desktop_surface_from_client_link(struct wl_list *link)
> > >   {
> > > @@ -687,6 +706,7 @@ weston_desktop_surface_set_title(struct
> weston_desktop_surface *surface,
> > >     free(surface->title);
> > >     surface->title = tmp;
> > > +   wl_signal_emit(&surface->title_signal, surface->title);
> >
> > I would rather pass the surface as the signal data. Just in case we have
> in
> > the future a value that doesn’t fit in a pointer. And calling a getter is
> > not that bad.
> >
> > Also, I prefer to have one signal only, it would reduce the API size (and
> > avoid it growing too much in the future, since it could work in this
> case).
> > We can move the free() to keep both pointer around until after the signal
> > fired, and a simple pointer comparison will work (if you stored the
> "const
> > char *" directly, which you should as with the signal, we guarantee it
> will
> > exist until destroy or signal).
> >
> >
> > But it raised a question : Jonas, are set_title() and set_app_id()
> supposed
> > to be active without a commit()?
>
> No. As per specification they are not tied to wl_surface.commit(). I
> suppose it is something we can change though, while we can, if it makes
> any sense. Opinions?
>
>
> Jonas
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20170725/eb0942c8/attachment-0001.html>