<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Jul 21, 2017 at 10:21 AM, Quentin Glidic <<a href="mailto:sardemff7+wayland@sardemff7.net" target="_blank">sardemff7+wayland@sardemff7.net</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 7/20/17 4:33 PM, Matt Hoosier wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> It's useful for the shell implementation to know when these change, for example to relay the information on to taskbars or similar. </blockquote> The idea is good, we need something like that. (But maybe not, see below). <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> 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. </blockquote> We don’t care that much about ABI changes, but you’re right that I do care about the struct staying private.<div><div class="h5"> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Signed-off-by: Matt Hoosier <<a href="mailto:matt.hoosier@gmail.com" target="_blank">matt.hoosier@gmail.com</a>> --- 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); </blockquote> </div></div> 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). </blockquote><div> </div><div>I'm fine with using just one signal to announce all this. What sort of name seems appropriate? "name_signal" maybe?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> 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, <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> } 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 </blockquote> -- Quentin “Sardem FF7” Glidic </blockquote><div> </div><div>-Matt </div></div> </div></div>