[PATCH weston 1/2] compositor: Generalize output previous damage into per buffer damage

Kristian Høgsberg hoegsberg at gmail.com
Fri Sep 14 10:40:01 PDT 2012


On Fri, Sep 14, 2012 at 04:12:03PM +0300, Ander Conselvan de Oliveira wrote:
> From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira at intel.com>
> 
> This is a more generic fix for the issue solved in 4f521731 where
> damage obscured by overlays could be lost in one of the output buffers
> due to rapid move of a surface in an overlay plane.
> 
> This changes the renderer so it keeps track of the damage in each
> buffer. Every time a new frame is drawn, the damage of the frame is
> added to all the buffers and the rendered regions are cleared from
> the current buffer's damage.

Very nice, it wasn't too much work after all, and I think the logic is
actually easier to follow now.

Committed as it, but one small comment/thought below.

Kristian

> ---
>  src/compositor.c     |   30 ++++++++++++++----------------
>  src/compositor.h     |    3 ++-
>  src/gles2-renderer.c |   17 ++++++++++++++---
>  src/screenshooter.c  |   15 ++++++++++++---
>  4 files changed, 42 insertions(+), 23 deletions(-)
> 
> diff --git a/src/compositor.c b/src/compositor.c
> index 8cb8a3f..bda1381 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -863,7 +863,7 @@ weston_output_repaint(struct weston_output *output, uint32_t msecs)
>  	struct weston_animation *animation, *next;
>  	struct weston_frame_callback *cb, *cnext;
>  	struct wl_list frame_callback_list;
> -	pixman_region32_t opaque, output_damage, new_damage;
> +	pixman_region32_t opaque, output_damage;
>  
>  	weston_compositor_update_drag_surfaces(ec);
>  
> @@ -896,21 +896,10 @@ weston_output_repaint(struct weston_output *output, uint32_t msecs)
>  	pixman_region32_fini(&opaque);
>  
>  	pixman_region32_init(&output_damage);
> -
> -	pixman_region32_init(&new_damage);
> -	pixman_region32_copy(&new_damage, &ec->primary_plane.damage);
> -
> -	pixman_region32_union(&ec->primary_plane.damage,
> -			      &ec->primary_plane.damage,
> -			      &output->previous_damage);
> -
> -	pixman_region32_intersect(&output->previous_damage,
> -				  &new_damage, &output->region);
> -
>  	pixman_region32_intersect(&output_damage,
>  				  &ec->primary_plane.damage, &output->region);
> -
> -	pixman_region32_fini(&new_damage);
> +	pixman_region32_subtract(&ec->primary_plane.damage,
> +				 &ec->primary_plane.damage, &output->region);

This effectively removes the double buffer swap assumption from the
core repaint loop and damage tracking, which is very nice.  We just
pass the region to repaint into the renderer.

>  	if (output->dirty)
>  		weston_output_update_matrix(output);
> @@ -2521,9 +2510,13 @@ WL_EXPORT void
>  weston_output_destroy(struct weston_output *output)
>  {
>  	struct weston_compositor *c = output->compositor;
> +	int i;
>  
>  	pixman_region32_fini(&output->region);
> -	pixman_region32_fini(&output->previous_damage);
> +
> +	for (i = 0; i < 2; i++)
> +		pixman_region32_fini(&output->buffer_damage[i]);
> +
>  	output->compositor->output_id_pool &= ~(1 << output->id);
>  
>  	wl_display_remove_global(c->wl_display, output->global);
> @@ -2647,10 +2640,15 @@ weston_output_transform_init(struct weston_output *output, uint32_t transform)
>  WL_EXPORT void
>  weston_output_move(struct weston_output *output, int x, int y)
>  {
> +	int i;
> +
>  	output->x = x;
>  	output->y = y;
>  
> -	pixman_region32_init(&output->previous_damage);
> +	output->current_buffer = 0;
> +	for (i = 0; i < 2; i++)
> +		pixman_region32_init(&output->buffer_damage[i]);
> +
>  	pixman_region32_init_rect(&output->region, x, y,
>  				  output->width,
>  				  output->height);
> diff --git a/src/compositor.h b/src/compositor.h
> index 6716bdc..4760993 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -160,7 +160,8 @@ struct weston_output {
>  	int32_t mm_width, mm_height;
>  	struct weston_border border;
>  	pixman_region32_t region;
> -	pixman_region32_t previous_damage;
> +	int current_buffer;
> +	pixman_region32_t buffer_damage[2];
>  	int repaint_needed;
>  	int repaint_scheduled;
>  	struct weston_output_zoom zoom;
> diff --git a/src/gles2-renderer.c b/src/gles2-renderer.c
> index 0e8b8ce..761f4fe 100644
> --- a/src/gles2-renderer.c
> +++ b/src/gles2-renderer.c
> @@ -612,6 +612,7 @@ draw_surface(struct weston_surface *es, struct weston_output *output,
>  	pixman_region32_t repaint;
>  	/* non-opaque region in surface coordinates: */
>  	pixman_region32_t surface_blend;
> +	pixman_region32_t *buffer_damage;
>  	GLint filter;
>  	int i;
>  
> @@ -623,8 +624,8 @@ draw_surface(struct weston_surface *es, struct weston_output *output,
>  	if (!pixman_region32_not_empty(&repaint))
>  		goto out;
>  
> -	pixman_region32_subtract(&ec->primary_plane.damage,
> -				 &ec->primary_plane.damage, &repaint);
> +	buffer_damage = &output->buffer_damage[output->current_buffer];
> +	pixman_region32_subtract(buffer_damage, buffer_damage, &repaint);
>  
>  	glBlendFunc(GL_ONE, GL_ONE_MINUS_SRC_ALPHA);
>  
> @@ -702,7 +703,7 @@ gles2_renderer_repaint_output(struct weston_output *output,
>  	struct weston_compositor *compositor = output->compositor;
>  	EGLBoolean ret;
>  	static int errored;
> -	int32_t width, height;
> +	int32_t width, height, i;
>  
>  	width = output->current->width +
>  		output->border.left + output->border.right;
> @@ -736,6 +737,14 @@ gles2_renderer_repaint_output(struct weston_output *output,
>  		pixman_region32_fini(&undamaged);
>  	}
>  
> +	for (i = 0; i < 2; i++)
> +		pixman_region32_union(&output->buffer_damage[i],
> +				      &output->buffer_damage[i],
> +				      output_damage);
> +
> +	pixman_region32_union(output_damage, output_damage,
> +			      &output->buffer_damage[output->current_buffer]);
> +

output->buffer_damage[output->current_buffer] is already the union of
output_damage and output->buffer_damage[output->current_buffer] here.
I was going to suggest that we just pass
output->buffer_damage[output->current_buffer] to repaint_surfaces, but
that won't work.  We can't change the output_damage region during
repaint since then some surfaces won't get repainted.  But we could
just copy output->buffer_damage[output->current_buffer] to output
damage instead of doing another union.

>  	repaint_surfaces(output, output_damage);
>  
>  	wl_signal_emit(&output->frame_signal, output);
> @@ -747,6 +756,8 @@ gles2_renderer_repaint_output(struct weston_output *output,
>  		print_egl_error_state();
>  	}
>  
> +	output->current_buffer ^= 1;
> +
>  }
>  
>  static void
> diff --git a/src/screenshooter.c b/src/screenshooter.c
> index ba80ce5..ffcc970 100644
> --- a/src/screenshooter.c
> +++ b/src/screenshooter.c
> @@ -261,7 +261,7 @@ weston_recorder_frame_notify(struct wl_listener *listener, void *data)
>  	struct weston_output *output = data;
>  	uint32_t msecs = output->frame_time;
>  	pixman_box32_t *r;
> -	pixman_region32_t damage;
> +	pixman_region32_t damage, *previous_damage;
>  	int i, j, k, n, width, height, run, stride;
>  	uint32_t delta, prev, *d, *s, *p, next;
>  	struct {
> @@ -270,9 +270,18 @@ weston_recorder_frame_notify(struct wl_listener *listener, void *data)
>  	} header;
>  	struct iovec v[2];
>  
> +	/* When recording, this will be exactly the region that was repainted
> +	 * in this frame. Since overlays are disabled, the whole primary plane
> +	 * damage is rendered. For the first frame, the whole output will be
> +	 * damaged and that damage will be added to both buffers causing the
> +	 * non-current buffer damage to be while output. Rendering will clear
> +	 * all the damage in the current buffer so in the next frame (when
> +	 * that is non-current) the only damage left will be the one added
> +	 * from the primary plane. */
> +	previous_damage = &output->buffer_damage[output->current_buffer ^ 1];
> +
>  	pixman_region32_init(&damage);
> -	pixman_region32_intersect(&damage, &output->region,
> -				  &output->previous_damage);
> +	pixman_region32_intersect(&damage, &output->region, previous_damage);
>  
>  	r = pixman_region32_rectangles(&damage, &n);
>  	if (n == 0)
> -- 
> 1.7.9.5
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel


More information about the wayland-devel mailing list