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

[Pekka Paalanen]

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?


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);
> >  }
> > 
>