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

[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?
> >>  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