[Glamor] [PATCH 2/2] Add the feature of generating linear gradient picture using shader

[Chris Wilson]

On Wed,  7 Mar 2012 02:44:08 +0800, junyan.he at linux.intel.com wrote:
>  Add the feature of generating linear gradient picture by
>  using shader. This logic will replace the original
>  gradient picture generating manner in glamor which
>  firstly use pixman and then upload to GPU. Compare it
>  to the result generated by pixman, the difference of
>  each color of each pixel is normally 0, sometimes
>  1/255, and 2/255 at most. The pixman use fix int but
>  shader use float, so may have difference. There is just
>  one case that we will fallback. When the picture has
>  transform matrix and the matrix is a projective
>  matrix(which has non 0 at the third column and row.),
>  the shader's coordinate has been normalized and so it
>  is hard to work as what the pixman do. Some logic of
>  pixmap compare has been added to for easing the debug
>  of the pixmap generated by shader.

One thing to mention here is the benefit of using a linear shader rather
pixman, some evidence to justify the work. Do you have any performance
figures that you can share?

> @@ -1290,21 +1827,34 @@ glamor_convert_gradient_picture(ScreenPtr screen,
>  	else
>  		format = source->format;
>  
> +	if (source->pSourcePict->type == SourcePictTypeLinear) {
> +		dst = _glamor_generate_linear_gradient_picture(screen,
> +		        source, x_source, y_source, width, height, format);
> +

source->pSourcePict may be NULL as glamor_convert_gradient_picture() may
also be called for an unattached picture->pDrawable. (wtf!)

> +		for (r = 0; r < 3; r++) {
> +			for (c = 0; c < 3; c++) {
> +				if ((r >= 2 || c >= 2) && r != c) {
> +					if (transform_mat[r][c]) {
> +						ErrorF("Project matrix change for"
> +						       "linear gradient, FallBack\n");
> +						goto GRADIENT_FAIL;
> +					}
> +				}

This test is wrong, it misclassifies a translation matrix as a
projection matrix.

> +	switch (src_picture->repeatType) {
> +#define REPEAT_FILL_STOPS(m, n) \
> +		stop_colors[(m)*4 + 0] = stop_colors[(n)*4 + 0]; \
> +		stop_colors[(m)*4 + 1] = stop_colors[(n)*4 + 1]; \
> +		stop_colors[(m)*4 + 2] = stop_colors[(n)*4 + 2]; \
> +		stop_colors[(m)*4 + 3] = stop_colors[(n)*4 + 3];
> +
> +		default:
> +		case PIXMAN_REPEAT_NONE:
> +			stop_colors[0] = 0.0;      //R
> +			stop_colors[1] = 0.0;      //G
> +			stop_colors[2] = 0.0;      //B
> +			stop_colors[3] = 0.0;      //Alpha
> +			n_stops[0] = -(float)INT_MAX;  //should be small enough.
> +
> +			stop_colors[0 + (count-1)*4] = 0.0;      //R
> +			stop_colors[1 + (count-1)*4] = 0.0;      //G
> +			stop_colors[2 + (count-1)*4] = 0.0;      //B
> +			stop_colors[3 + (count-1)*4] = 0.0;      //Alpha
> +			n_stops[count-1] = (float)INT_MAX;  //should be large enough.
This falls foul of the "pixman overflow emulation" in the shader. And
it needs to handle a subrange where the linear gradient is only defined
between, for example, [0.4, 0.6]. The expected behaviour is that the end
stops are then extrapolated out to 0.0 and 1.0 before the repeat mode is
then applied. The other repeat modes also appear not to handle subranges
either.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre