On Fri, Jul 21, 2017 at 10:21 AM, Quentin Glidic < sardemff7+wayl...@sardemff7.net> 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.hoos...@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). > I'm fine with using just one signal to announce all this. What sort of name seems appropriate? "name_signal" maybe? > > > But it raised a question : Jonas, are set_title() and set_app_id() > supposed to be active without a commit()? > > If so, maybe should we test equality in libweston-desktop, to avoid > sending useless signals? (Just in case a client would be lazy and send the > title on every frame.) > And I would like to know if one signal sounds good to you? > > If *not*, then the committed callback is already the place to check for > changes, and we need to adjust the code to match that. (And to keep old > pointers around for safe comparison.) > > > Cheers, > > } >> void >> @@ -701,6 +721,7 @@ weston_desktop_surface_set_app_id(struct >> weston_desktop_surface *surface, >> free(surface->app_id); >> surface->app_id = tmp; >> + wl_signal_emit(&surface->app_id_signal, surface->app_id); >> } >> void >> >> > > -- > > Quentin “Sardem FF7” Glidic > -Matt
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel