[Glamor] [PATCH 5/7] Fix the problem of vertical and horizontal case error in linear gradient.

He Junyan junyan.he at linux.intel.com
Mon May 14 22:53:17 PDT 2012


于 2012/5/15 13:23, Zhigang Gong 写道:
> On Tue, May 15, 2012 at 01:26:21PM +0800, He Junyan wrote:
>> 于 2012/5/15 12:51, Zhigang Gong 写道:
>>> On Tue, May 08, 2012 at 08:45:03AM +0800, junyan.he at linux.intel.com wrote:
>>>> From: Junyan He<junyan.he at linux.intel.com>
>>>>
>>>>   1. The vertical and horizontal judgement in linear
>>>>   gradient have problem when p1 point and p2 point
>>>>   distance is very small but the gradient pict have a
>>>>   transform matrix which will convert the X Y coordinates
>>>>   to small values. So the judgement is not suitable.
> So here , the "small" should be "large" right?

Should say like this example:
p1: (0, 0.01) p2:(0, 0.02)
Before matix, X and Y: (0, 0,) ---> (100, 100)
So in old code, we judge p1 and p2 in horizontal case.
But in shader, X Y re-caculate matrix, like:
0.000001    0     0
0       0.000001  0
0           0     1
get X and Y: (0, 0) ----> (0.0001, 0.0001)
So P1 and P2 is relatively big, the case is not right any more.



>>>>   Because this judgement's purpose is to assure the
>>>>   divisor not to be zero, so we simply it to enter
>>>>   horizontal judgement when p1 and p2's Y is same.
>>>>   Vertical case is deleted.
>>> If we have non-int transform matrix, both the vertical and
>>> horizontal check are invalid here, right? You may need to
>>> check whether we have a non-int transform matrix, and then
>>> choose different method to check the hor/vert condition?
>> It is not the case. The coordinate of P1 and P2 do not be affected
>> by the matrix.
>> Only X and Y of the texture will be re-caculated by the matrix. So
>> in the old code,
>> when the P1 and P2 is very close, we judge the case is vertical or
>> horizontal, but
>> after X and Y re-caculated by the matix, P1 and P2's distance is
>> enlarged, comparing
>> to the X Y. The re-caculated of X Y happens in the shader and the
>> judgement happens
>> before the shader, no causes this problem.
> Right, P1 and P2 is for the destination pixmap and indeed not affacted by
> transformation. So, you can still simply by checking whether the P1.x == P2.x
> to determine whether it's vertical. Why this patch giving up that checking and
> only check for the horizontal check?
>
>>
>>
>>>>   2. Delete the unused p1 p2 uniform variable.
>>>>
>>>>
>>>> Signed-off-by: Junyan He<junyan.he at linux.intel.com>
>>>> ---
>>>>   src/glamor_gradient.c |   28 ++++------------------------
>>>>   1 files changed, 4 insertions(+), 24 deletions(-)
>>>>
>>>> diff --git a/src/glamor_gradient.c b/src/glamor_gradient.c
>>>> index 7263c2b..12eff03 100644
>>>> --- a/src/glamor_gradient.c
>>>> +++ b/src/glamor_gradient.c
>>>> @@ -491,8 +491,6 @@ _glamor_create_linear_gradient_program(ScreenPtr screen, int stops_count, int dy
>>>>   	    "uniform mat3 transform_mat;\n"
>>>>   	    "uniform int repeat_type;\n"
>>>>   	    "uniform int hor_ver;\n"
>>>> -	    "uniform vec4 pt1;\n"
>>>> -	    "uniform vec4 pt2;\n"
>>>>   	    "uniform float pt_slope;\n"
>>>>   	    "uniform float cos_val;\n"
>>>>   	    "uniform float p1_distance;\n"
>>>> @@ -529,10 +527,6 @@ _glamor_create_linear_gradient_program(ScreenPtr screen, int stops_count, int dy
>>>>   	    "        distance = source_texture_trans.x;\n"
>>>>   	    "        _p1_distance = p1_distance * source_texture_trans.z;\n"
>>>>   	    "        _pt_distance = pt_distance * source_texture_trans.z;\n"
>>>> -	    "    } else if (hor_ver == 2) {\n"//vertical case.
>>>> -	    "        distance = source_texture_trans.y;\n"
>>>> -	    "        _p1_distance = p1_distance * source_texture_trans.z;\n"
>>>> -	    "        _pt_distance = pt_distance * source_texture_trans.z;\n"
>>>>   	    "    } \n"
>>>>   	    "    \n"
>>>>   	    "    distance = distance - _p1_distance; \n"
>>>> @@ -1301,8 +1295,6 @@ glamor_generate_linear_gradient_picture(ScreenPtr screen,
>>>>   	GLfloat n_stops_st[LINEAR_SMALL_STOPS];
>>>>
>>>>   	GLint transform_mat_uniform_location;
>>>> -	GLint pt1_uniform_location;
>>>> -	GLint pt2_uniform_location;
>>>>   	GLint n_stop_uniform_location;
>>>>   	GLint stops_uniform_location;
>>>>   	GLint stop0_uniform_location;
>>>> @@ -1369,10 +1361,6 @@ glamor_generate_linear_gradient_picture(ScreenPtr screen,
>>>>   	}
>>>>
>>>>   	/* Bind all the uniform vars .*/
>>>> -	pt1_uniform_location =
>>>> -	    dispatch->glGetUniformLocation(gradient_prog, "pt1");
>>>> -	pt2_uniform_location =
>>>> -	    dispatch->glGetUniformLocation(gradient_prog, "pt2");
>>>>   	n_stop_uniform_location =
>>>>   	    dispatch->glGetUniformLocation(gradient_prog, "n_stop");
>>>>   	pt_slope_uniform_location =
>>>> @@ -1460,7 +1448,6 @@ glamor_generate_linear_gradient_picture(ScreenPtr screen,
>>>>   	                        y_source,
>>>>   	                        glamor_priv->yInverted,
>>>>   	                        pt1);
>>>> -	dispatch->glUniform4fv(pt1_uniform_location, 1, pt1);
>>>>   	DEBUGF("pt1:(%f %f)\n", pt1[0], pt1[1]);
>>>>
>>>>   	glamor_set_normalize_pt(xscale, yscale,
>>>> @@ -1470,7 +1457,6 @@ glamor_generate_linear_gradient_picture(ScreenPtr screen,
>>>>   	                        y_source,
>>>>   	                        glamor_priv->yInverted,
>>>>   	                        pt2);
>>>> -	dispatch->glUniform4fv(pt2_uniform_location, 1, pt2);
>>>>   	DEBUGF("pt2:(%f %f)\n", pt2[0], pt2[1]);
>>>>
>>>>   	/* Set all the stops and colors to shader. */
>>>> @@ -1545,22 +1531,16 @@ glamor_generate_linear_gradient_picture(ScreenPtr screen,
>>>>   		dispatch->glUniform1i(n_stop_uniform_location, count);
>>>>   	}
>>>>
>>>> -	if (abs((pt2[1] - pt1[1]) / yscale)<   1.0) { // The horizontal case.
>>>> +	if (src_picture->pSourcePict->linear.p2.y ==
>>>> +	              src_picture->pSourcePict->linear.p1.y) { // The horizontal case.
>>>>   		dispatch->glUniform1i(hor_ver_uniform_location, 1);
>>>> -		DEBUGF("p1.x: %f, p2.x: %f, enter the horizontal case\n", pt1[1], pt2[1]);
>>>> +		DEBUGF("p1.y: %f, p2.y: %f, enter the horizontal case\n",
>>>> +		       pt1[1], pt2[1]);
>>>>
>>>>   		p1_distance = pt1[0];
>>>>   		pt_distance = (pt2[0] - p1_distance);
>>>>   		dispatch->glUniform1f(p1_distance_uniform_location, p1_distance);
>>>>   		dispatch->glUniform1f(pt_distance_uniform_location, pt_distance);
>>>> -	} else if (abs((pt2[0] - pt1[0]) / xscale)<   1.0) { //The vertical case.
>>>> -		dispatch->glUniform1i(hor_ver_uniform_location, 2);
>>>> -		DEBUGF("p1.y: %f, p2.y: %f, enter the vertical case\n", pt1[0], pt2[0]);
>>>> -
>>>> -		p1_distance = pt1[1];
>>>> -		pt_distance = (pt2[1] - p1_distance);
>>>> -		dispatch->glUniform1f(p1_distance_uniform_location, p1_distance);
>>>> -		dispatch->glUniform1f(pt_distance_uniform_location, pt_distance);
>>>>   	} else {
>>>>   		/* The slope need to compute here. In shader, the viewport set will change
>>>>   		   the orginal slope and the slope which is vertical to it will not be correct.*/
>>>> --
>>>> 1.7.7.6
>>>>
>>> _______________________________________________
>>> Glamor mailing list
>>> Glamor at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/glamor
>>>
>> _______________________________________________
>> Glamor mailing list
>> Glamor at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/glamor
> _______________________________________________
> Glamor mailing list
> Glamor at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/glamor


More information about the Glamor mailing list