[PATCH] libweston-desktop: add signals for title, app_id
Matt Hoosier
matt.hoosier at gmail.com
Tue Jul 25 14:07:01 UTC 2017
On Tue, Jul 25, 2017 at 7:26 AM, Jonas Ådahl <jadahl at gmail.com> wrote:
> On Tue, Jul 25, 2017 at 06:42:34AM -0600, Matt Hoosier wrote:
> > 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.
>
> There is no need to attach a new buffer to commit a new state. One could
> for example commit a state only changing the title and nothing else, and
> it'd be completely correct. The benefit of synchronizing it is for
> example that a shell can synchronously change some overlay with the
> content.
>
Yes, agreed. But that's not the way that the Wayland backends of big
programming frameworks like Qt, Gtk+, etc work. In practice, you need to
trigger rendering if you want a commit to happen.
>
> Regarding the app_id, the app_id should normally only ever be set once,
> as part of the initial state that doesn't have a buffer attached yet.
>
That's a fair point. The title can still change at will though, can't it?
Isn't the regular old window title what gets used to show things like the
working directly in an xterm or the name of the currently-edited document
in a word processor?
>
>
> Jonas
>
> >
> > 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/1c8df68c/attachment.html>
More information about the wayland-devel
mailing list