[Glamor] [PATCH 7/7] Fix the problem because of GLAMOR_CREATE_PIXMAP_FIXUP deleted when pixman createing.

Zhigang Gong zhigang.gong at linux.intel.com
Mon May 14 22:10:51 PDT 2012


On Tue, May 08, 2012 at 08:45:20AM +0800, junyan.he at linux.intel.com wrote:
> From: Junyan He <junyan.he at linux.intel.com>
> 
>  A bug is caused by change the pixmap's creating logic.
>  The flag GLAMOR_CREATE_PIXMAP_FIXUP is deleted when
>  creates the pixman which will be used to contain the
>  gradient picture. Without this flag, the FBO can be
>  re-used and the performance will be improved but the
>  size of FBO may be bigger than the picture's real size.
>  This will cause the caculation for xscale and yscale
>  using pixmap_priv_get_scale get a wrong scale. The
>  result is that the gradient picture seems to have some
>  offset comparing to the right one. Fix this bug.
> 
> 
> Signed-off-by: Junyan He <junyan.he at linux.intel.com>
> ---
>  src/glamor_gradient.c |   19 +++++++++++++------
>  1 files changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/src/glamor_gradient.c b/src/glamor_gradient.c
> index ebedb45..c57260b 100644
> --- a/src/glamor_gradient.c
> +++ b/src/glamor_gradient.c
> @@ -799,12 +799,19 @@ _glamor_gradient_set_pixmap_destination(ScreenPtr screen,
>  
>  	dispatch = glamor_get_dispatch(glamor_priv);
>  
> -	glamor_set_destination_pixmap_priv_nc(pixmap_priv);
> -
> -	pixmap_priv_get_scale(pixmap_priv, xscale, yscale);
> -
> -	glamor_priv->has_source_coords = 1;
> -	glamor_priv->has_mask_coords = 0;
> +	dispatch->glBindFramebuffer(GL_FRAMEBUFFER, pixmap_priv->fbo->fb);
> +#ifndef GLAMOR_GLES2
> +	dispatch->glMatrixMode(GL_PROJECTION);
> +	dispatch->glLoadIdentity();
> +	dispatch->glMatrixMode(GL_MODELVIEW);
> +	dispatch->glLoadIdentity();
> +#endif
> +	dispatch->glViewport(0, 0,
> +	                     dst_picture->pDrawable->width,
> +	                     dst_picture->pDrawable->height);
> +
> +	*xscale = 1.0 / (dst_picture->pDrawable->width);
> +	*yscale = 1.0 / (INT16)(dst_picture->pDrawable->height);

This patch looks incorrect to me. Now the texture may larger than
the pDrawable->width*pDrawable->height. Then if you set a viewport
to pDrawable->width*pDrawable->height, then the texture coords will
not match to the pixel by a 1:1 manner which may be not what you want.

For example, if a pDrawable has a 10x10 pixel array, but it get a 100x100
texture. Then here if you set the viewport to 10x10, then it means 10 pixels
in the texture map to one logical pixel. This may not efficient and sometimes
may cause some unaligned issue.

I wonder does this patch really fix some cairo test failures?

>  
>  	DEBUGF("xscale = %f, yscale = %f,"
>  	       " x_source = %d, y_source = %d, width = %d, height = %d\n",
> -- 
> 1.7.7.6
> 


More information about the Glamor mailing list