Pekka Paalanen wrote: > Instead of all this, could you not have made Weston simply use > wl_resource_post_event() instead of wl_resource_queue_event()?
Yes, posting the event rather than queuing it when there are no frame callbacks (is that what you meant?) is a lot simpler than having the queue disabling mechanism. I'm not sure why I didn't think of that. Otherwise if you meant using post all of the time, then that was discussed here and it doesn't seem like such a good idea: http://lists.freedesktop.org/archives/wayland-devel/2013-September/010987.html > I can't say if that is a good heuristic or not, to assume that > surfaces where a client is not interested in frame callbacks need > buffer releases ASAP. Apart from an explicit request, I have no > better suggestions. As far as I understand, the queuing mechanism is specifically an optimisation aimed at the frame callback mechanism to avoid waking up the client twice. Therefore disabling it when there are no frame callbacks seems like quite a natural thing to do in my mind. It's not that the clients want the event ‘ASAP’ but rather that without the patch the compositor will never send the events no matter how long the client waits. Here is a second version of the patch to do the posting. It makes the patch for doing the queue disabling redundant. Regards, - Neil -- >8 -- Weston now keeps track of the number of frame callbacks that are installed for a particular client using some data attached in the destroy listener of the wl_client. Whenever a buffer release is sent this number is checked and if it is zero then the event will be posted instead of queued. That way if the client isn't using frame callbacks it can get buffer release events immediately. Without this the client may never get the buffer release events because nothing else will post an event to the client. --- src/compositor.c | 113 +++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 105 insertions(+), 8 deletions(-) diff --git a/src/compositor.c b/src/compositor.c index 8c9e0fe..766c2cc 100644 --- a/src/compositor.c +++ b/src/compositor.c @@ -998,6 +998,7 @@ weston_surface_unmap(struct weston_surface *surface) struct weston_frame_callback { struct wl_resource *resource; struct wl_list link; + int committed; }; @@ -1106,6 +1107,71 @@ weston_buffer_reference_handle_destroy(struct wl_listener *listener, ref->buffer = NULL; } +struct weston_client_data { + struct wl_listener listener; + int n_pending_frame_callbacks; +}; + +static void +destroy_client_data(struct wl_listener *listener, void *data) +{ + struct weston_client_data *client_data = + container_of(listener, + struct weston_client_data, + listener); + + free(client_data); +} + +static struct weston_client_data * +weston_get_client_data(struct wl_client *client, + int create) +{ + struct weston_client_data *data; + struct wl_listener *listener; + + listener = wl_client_get_destroy_listener(client, destroy_client_data); + if (listener) { + return container_of(listener, + struct weston_client_data, + listener); + } + + if (!create) + return NULL; + + data = malloc(sizeof(struct weston_client_data)); + if (!data) + return NULL; + + data->n_pending_frame_callbacks = 0; + data->listener.notify = destroy_client_data; + wl_client_add_destroy_listener(client, &data->listener); + + return data; +} + +static void +send_buffer_release(struct wl_resource *resource) +{ + struct wl_client *client = wl_resource_get_client(resource); + struct weston_client_data *client_data; + + assert(client); + + client_data = weston_get_client_data(client, 0 /* no create */); + + /* If the client has frame callbacks installed then we'll + * queue the event rather than posting it immediately so that + * it will be sent along with the frame callback to avoid + * waking up the client an extra time */ + if (client_data && + client_data->n_pending_frame_callbacks > 0) + wl_resource_queue_event(resource, WL_BUFFER_RELEASE); + else + wl_resource_post_event(resource, WL_BUFFER_RELEASE); +} + WL_EXPORT void weston_buffer_reference(struct weston_buffer_reference *ref, struct weston_buffer *buffer) @@ -1113,9 +1179,7 @@ weston_buffer_reference(struct weston_buffer_reference *ref, if (ref->buffer && buffer != ref->buffer) { ref->buffer->busy_count--; if (ref->buffer->busy_count == 0) { - assert(wl_resource_get_client(ref->buffer->resource)); - wl_resource_queue_event(ref->buffer->resource, - WL_BUFFER_RELEASE); + send_buffer_release(ref->buffer->resource); } wl_list_remove(&ref->destroy_listener.link); } @@ -1483,6 +1547,15 @@ destroy_frame_callback(struct wl_resource *resource) { struct weston_frame_callback *cb = wl_resource_get_user_data(resource); + if (cb->committed) { + struct wl_client *client = wl_resource_get_client(resource); + struct weston_client_data *client_data = + weston_get_client_data(client, 0 /* no create */); + + if (client_data) + --client_data->n_pending_frame_callbacks; + } + wl_list_remove(&cb->link); free(cb); } @@ -1562,12 +1635,41 @@ weston_surface_commit_subsurface_order(struct weston_surface *surface) } static void +add_frame_callbacks(struct weston_surface *surface, + struct wl_list *callbacks) +{ + struct weston_frame_callback *cb; + int count = 0; + + wl_list_for_each(cb, callbacks, link) { + cb->committed = 1; + count++; + } + + if (count > 0) { + struct wl_client *client = + wl_resource_get_client(surface->resource); + struct weston_client_data *client_data = + weston_get_client_data(client, 1 /* create */); + + if (client_data) + client_data->n_pending_frame_callbacks += count; + + wl_list_insert_list(&surface->frame_callback_list, callbacks); + } +} + +static void weston_surface_commit(struct weston_surface *surface) { pixman_region32_t opaque; int surface_width = 0; int surface_height = 0; + /* wl_surface.frame */ + add_frame_callbacks(surface, &surface->pending.frame_callback_list); + wl_list_init(&surface->pending.frame_callback_list); + /* wl_surface.set_buffer_transform */ surface->buffer_transform = surface->pending.buffer_transform; @@ -1626,11 +1728,6 @@ weston_surface_commit(struct weston_surface *surface) pixman_region32_intersect(&surface->input, &surface->input, &surface->pending.input); - /* wl_surface.frame */ - wl_list_insert_list(&surface->frame_callback_list, - &surface->pending.frame_callback_list); - wl_list_init(&surface->pending.frame_callback_list); - weston_surface_commit_subsurface_order(surface); weston_surface_schedule_repaint(surface); -- 1.8.3.1 _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
