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 <[email protected]> > > --- > > 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 _______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
