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

[He Junyan]

于 2012/5/15 13:10, Zhigang Gong 写道:
> 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?
>
In my consideration, the bugs cause by the fbo's height and width is 
bigger than
the gradient's Drawable need. So it causes the xscale and yscale be a 
wrong value.
I think set glViewport to (0, 0) ---> (dst_picture->pDrawable->width, 
dst_picture->pDrawable->height)
will make the  X Y coordinate from (0,0) --> (width, height) to (-1, 
-1), (1, 1),
that is what I need? Is my understanding right?


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