[PATCH weston 7/7] compositor: damage pending subsurfaces when committing them

[Micah Fedke]

On 01/27/2017 04:30 AM, Emilio Pozuelo Monfort wrote:
> When a client changes the subsurfaces state, we need to damage
> them so the result is visible. We do that by flagging the surfaces
> when the state changes and causing damage when committing the
> state. This prevents normal repaints from considering these changes
> until a commit has happened, and allows the client to atomically
> schedule several changes.
>
> This fixes the subsurface_z_order test, which is now marked as expected
> to succeed.
>
> Signed-off-by: Emilio Pozuelo Monfort <emilio.pozuelo at collabora.co.uk>
> Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> ---
>  libweston/compositor.c       | 31 +++++++++++++++++++++++++++++++
>  libweston/compositor.h       |  3 +++
>  tests/subsurface-shot-test.c |  2 +-
>  3 files changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/libweston/compositor.c b/libweston/compositor.c
> index 5e232778..81392063 100644
> --- a/libweston/compositor.c
> +++ b/libweston/compositor.c
> @@ -2179,6 +2179,12 @@ view_list_add_subsurface_view(struct weston_compositor *compositor,
>  	}
>  }
>
> +/* This recursively adds the sub-surfaces for a view, relying on the
> + * sub-surface order. Thus, if a client restacks the sub-surfaces, that
> + * change first happens to the sub-surface list, and then automatically
> + * propagates here. See weston_surface_damage_subsurfaces() for how the
> + * sub-surfaces receive damage when the client changes the state.
> + */
>  static void
>  view_list_add(struct weston_compositor *compositor,
>  	      struct weston_view *view)
> @@ -2667,6 +2673,24 @@ surface_set_input_region(struct wl_client *client,
>  	}
>  }
>
> +/* Cause damage to this sub-surface and all its children.
> + *
> + * This is useful when there are state changes that need an implicit
> + * damage, e.g. a z-order change.
> + */
> +static void
> +weston_surface_damage_subsurfaces(struct weston_subsurface *sub)
> +{
> +	struct weston_subsurface *child;
> +
> +	weston_surface_damage(sub->surface);
> +	sub->reordered = false;
> +
> +	wl_list_for_each(child, &sub->surface->subsurface_list, parent_link)
> +		if (child != sub)
> +			weston_surface_damage_subsurfaces(child);
> +}
> +
>  static void
>  weston_surface_commit_subsurface_order(struct weston_surface *surface)
>  {
> @@ -2676,6 +2700,9 @@ weston_surface_commit_subsurface_order(struct weston_surface *surface)
>  				 parent_link_pending) {
>  		wl_list_remove(&sub->parent_link);
>  		wl_list_insert(&surface->subsurface_list, &sub->parent_link);
> +
> +		if (sub->reordered)
> +			weston_surface_damage_subsurfaces(sub);
>  	}
>  }
>
> @@ -3625,6 +3652,8 @@ subsurface_place_above(struct wl_client *client,
>  	wl_list_remove(&sub->parent_link_pending);
>  	wl_list_insert(sibling->parent_link_pending.prev,
>  		       &sub->parent_link_pending);
> +
> +	sub->reordered = true;
>  }
>
>  static void
> @@ -3647,6 +3676,8 @@ subsurface_place_below(struct wl_client *client,
>  	wl_list_remove(&sub->parent_link_pending);
>  	wl_list_insert(&sibling->parent_link_pending,
>  		       &sub->parent_link_pending);
> +
> +	sub->reordered = true;
>  }
>
>  static void
> diff --git a/libweston/compositor.h b/libweston/compositor.h
> index b049c980..08e728a9 100644
> --- a/libweston/compositor.h
> +++ b/libweston/compositor.h
> @@ -1235,6 +1235,9 @@ struct weston_subsurface {
>  	struct weston_surface_state cached;
>  	struct weston_buffer_reference cached_buffer_ref;
>
> +	/* Sub-surface has been reordered; need to apply damage. */
> +	bool reordered;
> +
>  	int synchronized;
>
>  	/* Used for constructing the view tree */
> diff --git a/tests/subsurface-shot-test.c b/tests/subsurface-shot-test.c
> index 29b03c41..df72ec28 100644
> --- a/tests/subsurface-shot-test.c
> +++ b/tests/subsurface-shot-test.c
> @@ -173,7 +173,7 @@ surface_commit_color(struct client *client, struct wl_surface *surface,
>  	return buf;
>  }
>
> -FAIL_TEST(subsurface_z_order)
> +TEST(subsurface_z_order)
>  {
>  	const char *test_name = get_test_name();
>  	struct client *client;

This series looks good to me, but note that there may be some room for 
future optimization in the subsurface damage code above.
This, and the rest of the series,
Reviewed-by: Micah Fedke <micah.fedke at collabora.co.uk>


-- 

Micah Fedke
Collabora Ltd.
+44 1223 362967
https://www.collabora.com/
https://twitter.com/collaboraltd