[PATCH weston 6/8] pixman-renderer: move code to draw_view_translated()

[Derek Foreman]

On 09/03/15 05:52 AM, Pekka Paalanen wrote:
> On Fri, 06 Mar 2015 13:46:26 -0600
> Derek Foreman <derekf at osg.samsung.com> wrote:
> 
>> On 06/03/15 05:04 AM, Pekka Paalanen wrote:
>>> From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
>>>
>>> Move code from draw_view() into a new function draw_view_translated().
>>> This new function is correct only if
>>> view_transformation_is_translation().
>>>
>>> The functional change in this is that if view->alpha is not 1.0, we do
>>> not bother computing and painting the non-opaque and opaque regions
>>> separately. Both need to be blended anyway, so just paint it all in one
>>> op=OVER call.
>>
>> Good refactor, good optimization - but I don't like seeing them in the
>> same patch.  Can these be split?
> 
> I'm not sure what I was thinking... I think this commit message is
> wrong.
> 
> Previously, if view->alpha != 1.0, it called 
>>>  		region_global_to_output(output, &repaint);
>>>  		repaint_region(ev, output, &repaint, NULL, PIXMAN_OP_OVER);
> which does paint the whole area as one OVER call.
> 
> And it does the same after this patch too, by not subtracting opaque
> region.
> 
> I think I just need to remove that paragraph from the commit message,
> no?

Ok, that makes sense.

Reviewed-By: Derek Foreman <derekf at osg.samsung.com>

> Thanks,
> pq
> 
> 
>>> Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
>>> ---
>>>  src/pixman-renderer.c | 85 ++++++++++++++++++++++++++++++---------------------
>>>  1 file changed, 51 insertions(+), 34 deletions(-)
>>>
>>> diff --git a/src/pixman-renderer.c b/src/pixman-renderer.c
>>> index 2da6a2b..fc7613c 100644
>>> --- a/src/pixman-renderer.c
>>> +++ b/src/pixman-renderer.c
>>> @@ -402,6 +402,54 @@ repaint_region(struct weston_view *ev, struct weston_output *output,
>>>  }
>>>  
>>>  static void
>>> +draw_view_translated(struct weston_view *view, struct weston_output *output,
>>> +		     pixman_region32_t *repaint_global)
>>> +{
>>> +	struct weston_surface *surface = view->surface;
>>> +	/* non-opaque region in surface coordinates: */
>>> +	pixman_region32_t surface_blend;
>>> +	/* region to be painted in output coordinates: */
>>> +	pixman_region32_t repaint_output;
>>> +
>>> +	pixman_region32_init(&repaint_output);
>>> +
>>> +	/* Blended region is whole surface minus opaque region,
>>> +	 * unless surface alpha forces us to blend all.
>>> +	 */
>>> +	pixman_region32_init_rect(&surface_blend, 0, 0,
>>> +				  surface->width, surface->height);
>>> +
>>> +	if (!(view->alpha < 1.0)) {
>>> +		pixman_region32_subtract(&surface_blend, &surface_blend,
>>> +					 &surface->opaque);
>>> +
>>> +		if (pixman_region32_not_empty(&surface->opaque)) {
>>> +			region_intersect_only_translation(&repaint_output,
>>> +							  repaint_global,
>>> +							  &surface->opaque,
>>> +							  view);
>>> +			region_global_to_output(output, &repaint_output);
>>> +
>>> +			repaint_region(view, output, &repaint_output, NULL,
>>> +				       PIXMAN_OP_SRC);
>>> +		}
>>> +	}
>>> +
>>> +	if (pixman_region32_not_empty(&surface_blend)) {
>>> +		region_intersect_only_translation(&repaint_output,
>>> +						  repaint_global,
>>> +						  &surface_blend, view);
>>> +		region_global_to_output(output, &repaint_output);
>>> +
>>> +		repaint_region(view, output, &repaint_output, NULL,
>>> +			       PIXMAN_OP_OVER);
>>> +	}
>>> +
>>> +	pixman_region32_fini(&surface_blend);
>>> +	pixman_region32_fini(&repaint_output);
>>> +}
>>> +
>>> +static void
>>>  draw_view(struct weston_view *ev, struct weston_output *output,
>>>  	  pixman_region32_t *damage) /* in global coordinates */
>>>  {
>>> @@ -409,9 +457,6 @@ draw_view(struct weston_view *ev, struct weston_output *output,
>>>  	struct pixman_surface_state *ps = get_surface_state(ev->surface);
>>>  	/* repaint bounding region in global coordinates: */
>>>  	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)
>>> @@ -431,41 +476,13 @@ 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)) {
>>> +	if (view_transformation_is_translation(ev)) {
>>> +		draw_view_translated(ev, output, &repaint);
>>> +	} else {
>>>  		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)) {
>>> -			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)) {
>>> -			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);
>>>  	}
>>>  
>>> -
>>>  out:
>>>  	pixman_region32_fini(&repaint);
>>>  }
>>>
>>
>