[PATCH weston 5/8] pixman-renderer: change repaint_region() arguments

[Derek Foreman]

On 06/03/15 05:04 AM, Pekka Paalanen wrote:
> From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> 
> Change the region argument types in repaint_region(), moving the
> final_region computation to the caller. The caller is in a better
> position deciding if source_clip is needed or if it can be intersected
> into the final_region via a simple translation. This avoids
> surf_region/source_clip implying that the transformation is only a
> translation.
> 
> The region_global_to_output() call is also moved into the callers so
> that repaint_region() would not modify caller-provided data. Modifying
> caller provided data could be surprising.
> 
> repaint_region() still cannot handle source_clip, it just ignores it.
> The situation is the same as before.
> 
> Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> ---
>  src/pixman-renderer.c | 59 ++++++++++++++++++++++++++++-----------------------
>  1 file changed, 33 insertions(+), 26 deletions(-)
> 
> diff --git a/src/pixman-renderer.c b/src/pixman-renderer.c
> index ecb4109..2da6a2b 100644
> --- a/src/pixman-renderer.c
> +++ b/src/pixman-renderer.c
> @@ -326,40 +326,32 @@ region_intersect_only_translation(pixman_region32_t *result_global,
>  	pixman_region32_intersect(result_global, result_global, global);
>  }
>  
> +/** Paint an intersected region
> + *
> + * \param ev The view to be painted.
> + * \param output The output being painted.
> + * \param repaint_output The region to be painted in output coordinates.
> + * \param source_clip The region of the source image to use, in source image
> + *                    coordinates. If NULL, use the whole source image.
> + *                    XXX: UNIMPLEMENTED
> + * \param pixman_op Compositing operator, either SRC or OVER.
> + */
>  static void
>  repaint_region(struct weston_view *ev, struct weston_output *output,
> -	       pixman_region32_t *region, pixman_region32_t *surf_region,
> -	       pixman_op_t pixman_op)
> +	       pixman_region32_t *repaint_output,
> +	       pixman_region32_t *source_clip, pixman_op_t pixman_op)

Why add source clip now and not in the other patch (7/8) that actually
implements it?

>  {
>  	struct pixman_renderer *pr =
>  		(struct pixman_renderer *) output->compositor->renderer;
>  	struct pixman_surface_state *ps = get_surface_state(ev->surface);
>  	struct pixman_output_state *po = get_output_state(output);
>  	struct weston_buffer_viewport *vp = &ev->surface->buffer_viewport;
> -	pixman_region32_t final_region;
>  	pixman_transform_t transform;
>  	pixman_image_t *mask_image;
>  	pixman_color_t mask = { 0, };
>  
> -	/* The final region to be painted is the intersection of
> -	 * 'region' and 'surf_region'. However, 'region' is in the global
> -	 * coordinates, and 'surf_region' is in the surface-local
> -	 * coordinates
> -	 */
> -	pixman_region32_init(&final_region);
> -	if (surf_region) {
> -		region_intersect_only_translation(&final_region, region,
> -						  surf_region, ev);
> -	} else {
> -		/* If there is no surface region, just use the global region */
> -		pixman_region32_copy(&final_region, region);
> -	}
> -
> -	/* Convert from global to output coord */
> -	region_global_to_output(output, &final_region);
> -
> -	/* And clip to it */
> -	pixman_image_set_clip_region32 (po->shadow_image, &final_region);
> +	/* Clip rendering to the damaged output region */
> +	pixman_image_set_clip_region32(po->shadow_image, repaint_output);
>  
>  	pixman_renderer_compute_transform(&transform, ev, output);
>  	pixman_image_set_transform(ps->image, &transform);
> @@ -407,8 +399,6 @@ repaint_region(struct weston_view *ev, struct weston_output *output,
>  					 pixman_image_get_height (po->shadow_image) /* height */);
>  
>  	pixman_image_set_clip_region32 (po->shadow_image, NULL);
> -
> -	pixman_region32_fini(&final_region);
>  }
>  
>  static void
> @@ -421,6 +411,7 @@ draw_view(struct weston_view *ev, struct weston_output *output,
>  	pixman_region32_t repaint;
>  	/* non-opaque region in surface coordinates: */
>  	pixman_region32_t surface_blend;
> +	pixman_region32_t repaint_output;
>  
>  	/* No buffer attached */
>  	if (!ps->image)
> @@ -441,21 +432,37 @@ draw_view(struct weston_view *ev, struct weston_output *output,
>  
>  	/* TODO: Implement repaint_region_complex() using pixman_composite_trapezoids() */
>  	if (ev->alpha != 1.0 || !view_transformation_is_translation(ev)) {
> +		region_global_to_output(output, &repaint);
>  		repaint_region(ev, output, &repaint, NULL, PIXMAN_OP_OVER);
>  	} else {
> +		pixman_region32_init(&repaint_output);
> +
>  		/* blended region is whole surface minus opaque region: */
>  		pixman_region32_init_rect(&surface_blend, 0, 0,
>  					  ev->surface->width, ev->surface->height);
>  		pixman_region32_subtract(&surface_blend, &surface_blend, &ev->surface->opaque);
>  
>  		if (pixman_region32_not_empty(&ev->surface->opaque)) {
> -			repaint_region(ev, output, &repaint, &ev->surface->opaque, PIXMAN_OP_SRC);
> +			region_intersect_only_translation(&repaint_output,
> +							  &repaint,
> +							  &ev->surface->opaque,
> +							  ev);
> +			region_global_to_output(output, &repaint_output);
> +			repaint_region(ev, output, &repaint_output, NULL,
> +				       PIXMAN_OP_SRC);
>  		}
>  
>  		if (pixman_region32_not_empty(&surface_blend)) {
> -			repaint_region(ev, output, &repaint, &surface_blend, PIXMAN_OP_OVER);
> +			region_intersect_only_translation(&repaint_output,
> +							  &repaint,
> +							  &surface_blend,
> +							  ev);
> +			region_global_to_output(output, &repaint_output);
> +			repaint_region(ev, output, &repaint_output, NULL,
> +				       PIXMAN_OP_OVER);
>  		}
>  		pixman_region32_fini(&surface_blend);
> +		pixman_region32_fini(&repaint_output);
>  	}
>  
>  
>