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

Zhigang Gong zhigang.gong at linux.intel.com
Fri Mar 2 01:48:37 PST 2012


On Wed, Feb 29, 2012 at 07:23:57AM +0800, junyan.he at linux.intel.com wrote:
> From: Junyan He <junyan.he at linux.intel.com>
> 
>  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.
> 

In general, LGTM, thanks for the contribution.
Some comments as below, please check.

> 
> Signed-off-by: Junyan He <junyan.he at linux.intel.com>
> ---
>  src/glamor.c             |    2 +
>  src/glamor_gl_dispatch.c |    4 +-
>  src/glamor_gl_dispatch.h |   15 +-
>  src/glamor_priv.h        |   13 ++
>  src/glamor_render.c      |  525 ++++++++++++++++++++++++++++++++++++++++++++++
>  src/glamor_utils.h       |  201 ++++++++++++++++++
>  6 files changed, 753 insertions(+), 7 deletions(-)
>  mode change 100644 => 100755 src/glamor.c
>  mode change 100644 => 100755 src/glamor_gl_dispatch.c
>  mode change 100644 => 100755 src/glamor_gl_dispatch.h
>  mode change 100644 => 100755 src/glamor_priv.h
>  mode change 100644 => 100755 src/glamor_render.c
>  mode change 100644 => 100755 src/glamor_utils.h

Please don't change source file's mode.

> 
> diff --git a/src/glamor.c b/src/glamor.c
> old mode 100644
> new mode 100755
> index 0d9ba28..532b9ef
> --- a/src/glamor.c
> +++ b/src/glamor.c
> @@ -389,6 +389,7 @@ glamor_init(ScreenPtr screen, unsigned int flags)
>  	glamor_init_tile_shader(screen);
>  	glamor_init_putimage_shaders(screen);
>  	glamor_init_finish_access_shaders(screen);
> +	glamor_init_gradient_shader(screen);
>  	glamor_pixmap_init(screen);
>  
>  	glamor_priv->flags = flags;
> @@ -416,6 +417,7 @@ glamor_release_screen_priv(ScreenPtr screen)
>  	glamor_fini_tile_shader(screen);
>  	glamor_fini_putimage_shaders(screen);
>  	glamor_fini_finish_access_shaders(screen);
> +	glamor_fini_gradient_shader(screen);
>  	glamor_pixmap_fini(screen);
>  	free(glamor_priv);
>  
> diff --git a/src/glamor_gl_dispatch.c b/src/glamor_gl_dispatch.c
> old mode 100644
> new mode 100755
> index ef0ac43..fc3c5c0
> --- a/src/glamor_gl_dispatch.c
> +++ b/src/glamor_gl_dispatch.c
> @@ -65,10 +65,12 @@ glamor_gl_dispatch_init_impl(struct glamor_gl_dispatch *dispatch,
>  
>  	INIT_FUNC(dispatch, glUseProgram, get_proc_address);
>  	INIT_FUNC(dispatch, glUniform1i, get_proc_address);
> +	INIT_FUNC(dispatch, glUniform1f, get_proc_address);
>  	INIT_FUNC(dispatch, glUniform4f, get_proc_address);
> +	INIT_FUNC(dispatch, glUniform4fv, get_proc_address);
>  	INIT_FUNC(dispatch, glUniform1fv, get_proc_address);
>  	INIT_FUNC(dispatch, glUniform2fv, get_proc_address);
> -	INIT_FUNC(dispatch, glUniform4fv, get_proc_address);
> +	INIT_FUNC(dispatch, glUniformMatrix3fv, get_proc_address);
>  	INIT_FUNC(dispatch, glCreateProgram, get_proc_address);
>  	INIT_FUNC(dispatch, glDeleteProgram, get_proc_address);
>  	INIT_FUNC(dispatch, glCreateShader, get_proc_address);
> diff --git a/src/glamor_gl_dispatch.h b/src/glamor_gl_dispatch.h
> old mode 100644
> new mode 100755
> index bd33bcc..6adbde9
> --- a/src/glamor_gl_dispatch.h
> +++ b/src/glamor_gl_dispatch.h
> @@ -60,7 +60,7 @@ typedef struct glamor_gl_dispatch {
>  			      const GLvoid * data, GLenum usage);
>  	GLvoid *(*glMapBuffer) (GLenum target, GLenum access);
>  	GLvoid *(*glMapBufferRange) (GLenum target, GLintptr offset, GLsizeiptr length, GLbitfield access);
> -	GLboolean(*glUnmapBuffer) (GLenum target);
> +	GLboolean (*glUnmapBuffer) (GLenum target);
>  	void (*glBindBuffer) (GLenum target, GLuint buffer);
>  	void (*glDeleteBuffers) (GLsizei n, const GLuint * buffers);
>  
> @@ -71,7 +71,7 @@ typedef struct glamor_gl_dispatch {
>  	void (*glDeleteFramebuffers) (GLsizei n,
>  				      const GLuint * framebuffers);
>  	void (*glGenFramebuffers) (GLsizei n, GLuint * framebuffers);
> -	 GLenum(*glCheckFramebufferStatus) (GLenum target);
> +	GLenum (*glCheckFramebufferStatus) (GLenum target);
>  	void (*glBlitFramebuffer) (GLint srcX0, GLint srcY0, GLint srcX1,
>  				   GLint srcY1, GLint dstX0, GLint dstY0,
>  				   GLint dstX1, GLint dstY1,
> @@ -92,6 +92,7 @@ typedef struct glamor_gl_dispatch {
>  				const GLint * length);
>  	void (*glUseProgram) (GLuint program);
>  	void (*glUniform1i) (GLint location, GLint v0);
> +	void (*glUniform1f) (GLint location, GLfloat v0);
>  	void (*glUniform4f) (GLint location, GLfloat v0, GLfloat v1,
>  			     GLfloat v2, GLfloat v3);
>  	void (*glUniform1fv) (GLint location, GLsizei count,
> @@ -100,9 +101,11 @@ typedef struct glamor_gl_dispatch {
>  			      const GLfloat * value);
>  	void (*glUniform4fv) (GLint location, GLsizei count,
>  			      const GLfloat * value);
> -	 GLuint(*glCreateProgram) (void);
> -	 GLuint(*glDeleteProgram) (GLuint);
> -	 GLuint(*glCreateShader) (GLenum type);
> +	void (*glUniformMatrix3fv) (GLint location, GLsizei count,
> +		           GLboolean transpose, const GLfloat* value);
> +	GLuint (*glCreateProgram) (void);
> +	GLuint (*glDeleteProgram) (GLuint);
> +	GLuint (*glCreateShader) (GLenum type);
>  	void (*glCompileShader) (GLuint shader);
>  	void (*glAttachShader) (GLuint program, GLuint shader);
>  	void (*glGetShaderiv) (GLuint shader, GLenum pname,
> @@ -113,7 +116,7 @@ typedef struct glamor_gl_dispatch {
>  				GLint * params);
>  	void (*glGetProgramInfoLog) (GLuint program, GLsizei bufSize,
>  				     GLsizei * length, GLchar * infoLog);
> -	 GLint(*glGetUniformLocation) (GLuint program,
> +	GLint (*glGetUniformLocation) (GLuint program,
>  				       const GLchar * name);
>  
>  } glamor_gl_dispatch;
> diff --git a/src/glamor_priv.h b/src/glamor_priv.h
> old mode 100644
> new mode 100755
> index 84aef97..a2fdf21
> --- a/src/glamor_priv.h
> +++ b/src/glamor_priv.h
> @@ -109,6 +109,13 @@ enum shader_in {
>  	SHADER_IN_COUNT,
>  };
>  
> +enum gradient_shader_type {
> +	GRADIENT_SHADER_LINEAR,
> +	GRADIENT_SHADER_RADIAL,
> +	GRADIENT_SHADER_CONICAL,
> +	GRADIENT_SHADER_COUNT,
> +};
> +
>  struct glamor_screen_private;
>  struct glamor_pixmap_private;
>  typedef void (*glamor_pixmap_validate_function_t) (struct
> @@ -207,6 +214,9 @@ typedef struct glamor_screen_private {
>  	GLint tile_prog;
>  	GLint tile_wh;
>  
> +	/* glamor gradient */
> +	GLint gradient_prog[GRADIENT_SHADER_COUNT];
> +
>  	/* glamor_putimage */
>  	GLint put_image_xybitmap_prog;
>  	GLint put_image_xybitmap_fg_uniform_location;
> @@ -548,6 +558,9 @@ Bool glamor_tile(PixmapPtr pixmap, PixmapPtr tile,
>  void glamor_init_tile_shader(ScreenPtr screen);
>  void glamor_fini_tile_shader(ScreenPtr screen);
>  
> +void glamor_init_gradient_shader(ScreenPtr screen);
> +void glamor_fini_gradient_shader(ScreenPtr screen);
> +
>  /* glamor_triangles.c */
>  void
>  
> diff --git a/src/glamor_render.c b/src/glamor_render.c
> old mode 100644
> new mode 100755
> index 9f0b034..c64fe59
> --- a/src/glamor_render.c
> +++ b/src/glamor_render.c
> @@ -1275,6 +1275,522 @@ done:
>  	return ret;
>  }
>  
> +static GLint
> +_glamor_create_linear_gradient_program(ScreenPtr screen, int stops_count)
> +{
> +	glamor_screen_private *glamor_priv;
> +	glamor_gl_dispatch *dispatch;
> +
> +	GLint gradient_prog = 0;
> +	char *gradient_fs = NULL;
> +	GLint fs_prog, vs_prog;
> +
> +	const char *gradient_vs =
> +	    GLAMOR_DEFAULT_PRECISION
> +	    "attribute vec4 v_position;\n"
> +	    "attribute vec4 v_texcoord;\n"
> +	    "varying vec2 source_texture;\n"
> +	    "\n"
> +	    "void main()\n"
> +	    "{\n"
> +	    "    gl_Position = v_position;\n"
> +	    "    source_texture = v_texcoord.xy;\n"
> +	    "}\n";
> +
> +	const char *gradient_fs_template =
> +	    GLAMOR_DEFAULT_PRECISION
> +	    "uniform mat3 transform_mat;\n"
> +	    "uniform int repeat_type;\n"
> +	    "uniform int hor_ver;\n"
> +	    "uniform int n_stop;\n"
> +	    "uniform float stops[%d];\n"
> +	    "uniform vec4 stop_colors[%d];\n"
> +	    "uniform vec4 pt1;\n"
> +	    "uniform vec4 pt2;\n"
> +	    "uniform float pt_slope;\n"
> +	    "varying vec2 source_texture;\n"
> +	    "\n"
> +	    "vec4 get_color()\n"
> +	    "{\n"
> +	    "    vec3 tmp = vec3(source_texture.x, source_texture.y, 1.0);\n"
> +	    "    float len_percentage;\n"
> +	    "    float distance;\n"
> +	    "    float p1_distance;\n"
> +	    "    float p2_distance;\n"
> +	    "    float y_dist;\n"
> +	    "    float slope = pt_slope; \n"
> +	    "    float new_alpha; \n"
> +	    "    float cos_val;\n"
> +	    "    int i = 0;\n"
> +	    "    int revserse = 0;\n"
> +	    "    vec4 gradient_color;\n"
> +	    "    vec3 source_texture_trans = transform_mat * tmp;\n"
> +	    "    \n"
> +	    "    if(hor_ver == 0) { \n" //Normal case.
> +	    "        cos_val = sqrt(1.0/(slope*slope + 1.0));\n"
> +	    "        y_dist = source_texture_trans.y - source_texture_trans.x*slope;\n"
> +	    "        distance = y_dist * cos_val;\n"
> +	    "        \n"
> +	    "        y_dist = pt1.y - pt1.x*slope;\n"
> +	    "        p1_distance = y_dist * cos_val * source_texture_trans.z;\n"
> +	    "        \n"
> +	    "        y_dist = pt2.y - pt2.x*slope;\n"
> +	    "        p2_distance = y_dist * cos_val * source_texture_trans.z;\n"
> +	    "        \n"
> +	    "    } else if (hor_ver == 1) {\n"//horizontal case.
> +	    "        distance = source_texture_trans.x;\n"
> +	    "        p1_distance = pt1.x * source_texture_trans.z;\n"
> +	    "        p2_distance = pt2.x * source_texture_trans.z;\n"
> +	    "    } else if (hor_ver == 2) {\n"//vertical case.
> +	    "        distance = source_texture_trans.y;\n"
> +	    "        p1_distance = pt1.y * source_texture_trans.z;\n"
> +	    "        p2_distance = pt2.y * source_texture_trans.z;\n"
> +	    "    } \n"
> +	    "    \n"
> +	    "    distance = distance - p1_distance; \n"
> +	    "    \n"
> +	    "    if(repeat_type == %d){\n" // repeat normal
> +	    "        while(distance > p2_distance - p1_distance) \n"
> +	    "            distance = distance - (p2_distance - p1_distance); \n"
> +	    "        while(distance < 0.0) \n"
> +	    "            distance = distance + (p2_distance - p1_distance); \n"
> +	    "    }\n"
> +	    "    \n"
> +	    "    if(repeat_type == %d) {\n" // repeat reflect
> +	    "        while(distance > p2_distance - p1_distance) {\n"
> +	    "            distance = distance - (p2_distance - p1_distance); \n"
> +	    "            if(revserse == 0)\n"
> +	    "                revserse = 1;\n"
> +	    "            else\n"
> +	    "                revserse = 0;\n"
> +	    "        }\n"
> +	    "        while(distance < 0.0) {\n"
> +	    "            distance = distance + (p2_distance - p1_distance); \n"
> +	    "            if(revserse == 0)\n"
> +	    "                revserse = 1;\n"
> +	    "            else\n"
> +	    "	             revserse = 0;\n"
> +	    "        }\n"
> +	    "        if(revserse == 1) {\n"
> +	    "            distance = (p2_distance - p1_distance) - distance; \n"
> +	    "        }\n"
> +	    "    }\n"
> +	    "    \n"
> +	    "    len_percentage = distance/(p2_distance - p1_distance);\n"
> +	    "    for(i = 0; i < n_stop - 1; i++) {\n"
> +	    "        if(len_percentage < stops[i])\n"
> +	    "            break; \n"
> +	    "    }\n"
> +	    "    \n"
> +	    "    float percentage = (len_percentage - stops[i-1])/(stops[i] - stops[i-1]);\n"
> +	    "    if(stops[i] - stops[i-1] > 2.0)\n"
> +	    "        percentage = 0.0;\n" //For comply with pixman, walker->stepper overflow.
> +	    "    new_alpha = percentage * stop_colors[i].a + \n"
> +	    "                       (1.0-percentage) * stop_colors[i-1].a; \n"
> +	    "    gradient_color = vec4((percentage * stop_colors[i].r \n"
> +	    "                          + (1.0-percentage) * stop_colors[i-1].r)*new_alpha, \n"
> +	    "                          (percentage * stop_colors[i].g \n"
> +	    "                          + (1.0-percentage) * stop_colors[i-1].g)*new_alpha, \n"
> +	    "                          (percentage * stop_colors[i].b \n"
> +	    "                          + (1.0-percentage) * stop_colors[i-1].b)*new_alpha, \n"
> +	    "                          new_alpha);\n"
Seems can be optimized to,
      gradient_color = new_alpha * vec4(percentage * stop_colors[i].rgb 
      		       		        + (1.0 - percentage) * stop_colors[i - 1].rgb,
					1.0);

Although I believe glsl compiler will do the optimization, I still think the above style is
clearer ;).

> +	    "    \n"
> +	    "    return gradient_color;\n"
> +	    "}\n"
> +	    "\n"
> +	    "void main()\n"
> +	    "{\n"
> +	    "    gl_FragColor = get_color();\n"
> +	    "}\n";

You are using the normalize coords directly here, I assume you want to use eaxct texture size
to store the gradient pixmap. This is acceptable currently, but it may reduce the fbo cache
utility which may bring some overhead especially on PVR platform.

Actually, use a larger texture is not impossible, what you need to do is to adjust the coords
which is outside the actual box. You can refer the same case at the tile shader or the compsiting
shaders. The adjusting algorithm is at fragment shader function rel_tex_coord(), you can reuse
that function here. It is trival.

> +
> +	glamor_priv = glamor_get_screen_private(screen);
> +	dispatch = glamor_get_dispatch(glamor_priv);
> +
> +	gradient_prog = dispatch->glCreateProgram();
> +
> +	vs_prog = glamor_compile_glsl_prog(dispatch,
> +	          GL_VERTEX_SHADER, gradient_vs);
> +
> +	XNFasprintf(&gradient_fs,
> +	            gradient_fs_template, stops_count, stops_count,
> +	            PIXMAN_REPEAT_NORMAL, PIXMAN_REPEAT_REFLECT);
> +
> +	fs_prog = glamor_compile_glsl_prog(dispatch,
> +	          GL_FRAGMENT_SHADER, gradient_fs);
> +
> +	free(gradient_fs);
> +
> +	dispatch->glAttachShader(gradient_prog, vs_prog);
> +	dispatch->glAttachShader(gradient_prog, fs_prog);
> +
> +	dispatch->glBindAttribLocation(gradient_prog, GLAMOR_VERTEX_POS, "v_position");
> +	dispatch->glBindAttribLocation(gradient_prog, GLAMOR_VERTEX_SOURCE, "v_texcoord");
> +
> +	glamor_link_glsl_prog(dispatch, gradient_prog);
> +
> +	dispatch->glUseProgram(0);
> +
> +	glamor_put_dispatch(glamor_priv);
> +	return gradient_prog;
> +}
> +
> +#define LINEAR_DEFAULT_STOPS 32 + 2

Is 32+2 enough? If the stops are larger than it we need to pay for the compiling/linking
the shader on the fly. This default value seems too small to me.

> +void
> +glamor_init_gradient_shader(ScreenPtr screen)
> +{
> +	glamor_screen_private *glamor_priv;
> +
> +	glamor_priv = glamor_get_screen_private(screen);
> +
> +	glamor_priv->gradient_prog[GRADIENT_SHADER_LINEAR] =
> +	    _glamor_create_linear_gradient_program(screen,
> +	            LINEAR_DEFAULT_STOPS);
> +}
> +
> +void
> +glamor_fini_gradient_shader(ScreenPtr screen)
> +{
> +	glamor_screen_private *glamor_priv;
> +	glamor_gl_dispatch *dispatch;
> +
> +	glamor_priv = glamor_get_screen_private(screen);
> +	dispatch = glamor_get_dispatch(glamor_priv);
> +
> +	dispatch->glDeleteProgram(
> +	    glamor_priv->gradient_prog[GRADIENT_SHADER_LINEAR]);
> +
> +	glamor_put_dispatch(glamor_priv);
> +}
> +
> +static PicturePtr
> +_glamor_generate_linear_gradient_picture(ScreenPtr screen,
> +        PicturePtr src_picture,
> +        int x_source, int y_source,
> +        int width, int height,
> +        PictFormatShort format)
> +{
> +	glamor_screen_private *glamor_priv;
> +	glamor_gl_dispatch *dispatch;
> +	PicturePtr dst_picure;
> +	PixmapPtr pixmap;
> +	glamor_pixmap_private *pixmap_priv;
> +	GLint gradient_prog = 0;
> +	int error;
> +	float pt_distance;
> +	float tex_vertices[8];
> +	int stops_count;
> +	GLint transform_mat_uniform_location;
> +	GLint pt1_uniform_location;
> +	GLint pt2_uniform_location;
> +	GLint n_stop_uniform_location;
> +	GLint stops_uniform_location;
> +	GLint stop_colors_uniform_location;
> +	GLint pt_slope_uniform_location;
> +	GLint repeat_type_uniform_location;
> +	GLint hor_ver_uniform_location;
> +	GLfloat *stop_colors = NULL;
> +	GLfloat *n_stops = NULL;
> +	int i = 0;
> +	int count = 0;
> +	float slope;
> +	GLfloat xscale, yscale;
> +	GLfloat pt1[4], pt2[4];
> +	float vertices[8];
> +	float transform_mat[3][3];
> +
> +	/* Create a pixmap with VBO. */
> +	pixmap = glamor_create_pixmap(screen,
> +	         width, height,
> +	         PIXMAN_FORMAT_DEPTH(format),
> +	         GLAMOR_CREATE_PIXMAP_FIXUP);
> +
> +	if (!pixmap)
> +		goto GRADIENT_FAIL;
> +
> +	dst_picure = CreatePicture(0, &pixmap->drawable,
> +	             PictureMatchFormat(screen,
> +	                                PIXMAN_FORMAT_DEPTH(format), format),
> +	             0, 0, serverClient, &error);
> +
> +	/* Release the reference, picture will hold the last one. */
> +	glamor_destroy_pixmap(pixmap);
> +
> +	if (!dst_picure)
> +		goto GRADIENT_FAIL;
> +
> +	ValidatePicture(dst_picure);
> +
> +	glamor_priv = glamor_get_screen_private(screen);
> +	dispatch =  glamor_get_dispatch(glamor_priv);
> +
> +	stops_count = src_picture->pSourcePict->linear.nstops + 2 > LINEAR_DEFAULT_STOPS ?
> +	              src_picture->pSourcePict->linear.nstops + 2 : LINEAR_DEFAULT_STOPS;
> +
> +	/* Because the max value of nstops is unkown, so create a program
> +	   when nstops > default.*/
> +	if (src_picture->pSourcePict->linear.nstops <= LINEAR_DEFAULT_STOPS) {
> +		gradient_prog = glamor_priv->gradient_prog[GRADIENT_SHADER_LINEAR];
> +	} else {
> +		gradient_prog = _glamor_create_linear_gradient_program(screen,
> +		                src_picture->pSourcePict->linear.nstops);
> +	}
> +
> +	/* 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");
> +	stops_uniform_location =
> +	    dispatch->glGetUniformLocation(gradient_prog, "stops");
> +	stop_colors_uniform_location =
> +	    dispatch->glGetUniformLocation(gradient_prog, "stop_colors");
> +	pt_slope_uniform_location =
> +	    dispatch->glGetUniformLocation(gradient_prog, "pt_slope");
> +	repeat_type_uniform_location =
> +	    dispatch->glGetUniformLocation(gradient_prog, "repeat_type");
> +	hor_ver_uniform_location =
> +	    dispatch->glGetUniformLocation(gradient_prog, "hor_ver");
> +	transform_mat_uniform_location =
> +	    dispatch->glGetUniformLocation(gradient_prog, "transform_mat");
> +
> +	dispatch->glUseProgram(gradient_prog);
> +
> +	dispatch->glUniform1i(repeat_type_uniform_location, src_picture->repeatType);
> +
> +	if (src_picture->transform) {
> +		int r, c;
> +
> +		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;
> +					}
> +				}
> +				transform_mat[r][c] = (float)pixman_fixed_to_double(
> +				            src_picture->transform->matrix[r][c]);
> +			}
> +		}
> +	} else {
> +		int r, c;
> +
> +		for (r = 0; r < 3; r++)
> +			for (c = 0; c < 3; c++)
> +				transform_mat[r][c] = (c == r) ? 1.0 : 0.0;
  we can just use a static identity matrix here rather than create one every time.

> +	}
> +	dispatch->glUniformMatrix3fv(transform_mat_uniform_location, 1, 1, &transform_mat[0][0]);
> +
> +	pixmap = glamor_get_drawable_pixmap(dst_picure->pDrawable);
> +	pixmap_priv = glamor_get_pixmap_private(pixmap);
> +
> +	if (!GLAMOR_PIXMAP_PRIV_HAS_FBO(pixmap_priv)) { /* should always have here. */
> +		goto GRADIENT_FAIL;
> +	}
> +
> +	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;
> +	glamor_setup_composite_vbo(screen, 4*2);
> +
> +	DEBUGF("xscale = %f, yscale = %f,"
> +	       " x_source = %d, y_source = %d, width = %d, height = %d\n",
> +	       xscale, yscale, x_source, y_source, width, height);
> +
> +	glamor_set_normalize_vcoords(xscale, yscale,
> +	        0, 0,
> +	        (INT16)(width), (INT16)(height),
> +	        1, vertices);
> +
> +	glamor_set_normalize_tcoords(xscale, yscale,
> +	        0, 0,
> +	        (INT16)(width), (INT16)(height),
> +	        1, tex_vertices);
> +
> +	DEBUGF("vertices --> leftup : %f X %f, rightup: %f X %f,"
> +	       "rightbottom: %f X %f, leftbottom : %f X %f\n",
> +	       vertices[0], vertices[1], vertices[2], vertices[3],
> +	       vertices[4], vertices[5], vertices[6], vertices[7]);
> +	DEBUGF("tex_vertices --> leftup : %f X %f, rightup: %f X %f,"
> +	       "rightbottom: %f X %f, leftbottom : %f X %f\n",
> +	       tex_vertices[0], tex_vertices[1], tex_vertices[2], tex_vertices[3],
> +	       tex_vertices[4], tex_vertices[5], tex_vertices[6], tex_vertices[7]);
> +
> +	/* Normalize the PTs. */
> +	pt1[0] = t_from_x_coord_x(xscale,
> +	         pixman_fixed_to_int(src_picture->pSourcePict->linear.p1.x) - x_source);
> +	pt1[1] = t_from_x_coord_y_inverted(yscale,
> +	         pixman_fixed_to_int(src_picture->pSourcePict->linear.p1.y) - y_source);
You need to write a new macro or inline function to wrap the normalization of PTs just
as glamor_set_normalize_vcoords does, as the glamor->yInverted is not always 1, you need to
check and handle it respectively. Although, I haven't test that code path for a relatively
long time, I still don't want to break it too obviously.

> +	pt1[3] = pt1[4] = 0.0;
> +	dispatch->glUniform4fv(pt1_uniform_location, 1, pt1);
> +	DEBUGF("pt1:(%f %f)\n", pt1[0], pt1[1]);
> +
> +	pt2[0] = t_from_x_coord_x(xscale,
> +	         pixman_fixed_to_int(src_picture->pSourcePict->linear.p2.x) - x_source);
> +	pt2[1] = t_from_x_coord_y_inverted(yscale,
> +	         pixman_fixed_to_int(src_picture->pSourcePict->linear.p2.y) - y_source);
> +	pt2[3] = pt2[4] = 0.0;
> +	dispatch->glUniform4fv(pt2_uniform_location, 1, pt2);
> +	DEBUGF("pt2:(%f %f)\n", pt2[0], pt2[1]);
> +
> +	pt_distance = sqrt((pt2[1] - pt1[1]) * (pt2[1] - pt1[1]) +
> +	                   (pt2[0] - pt1[0]) * (pt2[0] - pt1[0]));
> +
> +	/* Set all the stops and colors to shader. */
> +	stop_colors = malloc(4 * stops_count * sizeof(float));
> +	if (stop_colors == NULL) {
> +		ErrorF("Failed to allocate stop_colors memory.\n");
> +		exit(1);
> +	}
> +
> +	n_stops = malloc(stops_count * sizeof(float));
> +	if (n_stops == NULL) {
> +		ErrorF("Failed to allocate n_stops memory.\n");
> +		exit(1);
> +	}
can you allocate one stop_colors and n_stops for all the stops less or equal to DEFAULT_STOPS, and
then dynamic allocate buffers for large stops?

> +
> +	for (i = 1; i < src_picture->pSourcePict->linear.nstops + 1; i++) {
> +		stop_colors[i*4] = pixman_fixed_to_double(
> +		                       src_picture->pSourcePict->linear.stops[i-1].color.red);
> +		stop_colors[i*4+1] = pixman_fixed_to_double(
> +		            src_picture->pSourcePict->linear.stops[i-1].color.green);
> +		stop_colors[i*4+2] = pixman_fixed_to_double(
> +		            src_picture->pSourcePict->linear.stops[i-1].color.blue);
> +		stop_colors[i*4+3] = pixman_fixed_to_double(
> +		            src_picture->pSourcePict->linear.stops[i-1].color.alpha);
> +
> +		n_stops[i] = (GLfloat)pixman_fixed_to_double(
> +		                 src_picture->pSourcePict->linear.stops[i-1].x);
> +	}
> +
> +	count = src_picture->pSourcePict->linear.nstops + 2;
> +
> +	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.
> +			break;
> +		case PIXMAN_REPEAT_NORMAL:
> +			REPEAT_FILL_STOPS(0, count - 2);
> +			n_stops[0] = n_stops[count-2] - 1.0;
> +
> +			REPEAT_FILL_STOPS(count - 1, 1);
> +			n_stops[count-1] = n_stops[1] + 1.0;
> +			break;
> +		case PIXMAN_REPEAT_REFLECT:
> +			REPEAT_FILL_STOPS(0, 1);
> +			n_stops[0] = -n_stops[1];
> +
> +			REPEAT_FILL_STOPS(count - 1, count - 2);
> +			n_stops[count-1] = 1.0 + 1.0 - n_stops[count-2];
> +			break;
> +		case PIXMAN_REPEAT_PAD:
> +			REPEAT_FILL_STOPS(0, 1);
> +			n_stops[0] = -(float)INT_MAX;
> +
> +			REPEAT_FILL_STOPS(count - 1, count - 2);
> +			n_stops[count-1] = (float)INT_MAX;
> +			break;
> +#undef REPEAT_FILL_STOPS
> +	}
> +
> +	/*
> +	for (i = 0; i < count; i++) {
> +		DEBUGF("n_stops[%d] = %f, color = r:%f g:%f b:%f a:%f\n",
> +		        i, n_stops[i],
> +		        stop_colors[i*4], stop_colors[i*4+1],
> +		        stop_colors[i*4+2], stop_colors[i*4+3]);
> +	}*/
> +
Please just remove the above code if you think it is useless now, or you can
just uncomment it. It will not bring any overhead when you define the DEBUGF
as a NULL operation.

> +	dispatch->glUniform4fv(stop_colors_uniform_location, count, stop_colors);
> +	dispatch->glUniform1fv(stops_uniform_location, count, n_stops);
> +	dispatch->glUniform1i(n_stop_uniform_location, count);
> +
> +	free(n_stops);
> +	free(stop_colors);
> +
> +	if ((pt2[1] - pt1[1]) / yscale < 1.0) { // 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]);
> +	} else if ((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]);
> +	} 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.*/
> +		slope = - (float)(src_picture->pSourcePict->linear.p2.x - src_picture->pSourcePict->linear.p1.x) /
> +		        (float)(src_picture->pSourcePict->linear.p2.y - src_picture->pSourcePict->linear.p1.y);
> +		slope = slope * yscale / xscale;
> +		dispatch->glUniform1f(pt_slope_uniform_location, slope);
> +		dispatch->glUniform1i(hor_ver_uniform_location, 0);
> +	}
> +
> +	/* set the transform matrix. */	/* Now rendering. */
> +	glamor_emit_composite_rect(screen, tex_vertices, NULL, vertices);
> +
> +	if (glamor_priv->render_nr_verts) {
> +		if (glamor_priv->gl_flavor == GLAMOR_GL_DESKTOP)
> +			dispatch->glUnmapBuffer(GL_ARRAY_BUFFER);
> +		else {
> +
> +			dispatch->glBindBuffer(GL_ARRAY_BUFFER, glamor_priv->vbo);
> +			dispatch->glBufferData(GL_ARRAY_BUFFER,
> +			        glamor_priv->vbo_offset,
> +			        glamor_priv->vb, GL_DYNAMIC_DRAW);
> +		}
> +
> +		dispatch->glDrawElements(GL_TRIANGLES, 6, GL_UNSIGNED_SHORT, NULL);
> +	}
> +
> +	dispatch->glBindBuffer(GL_ARRAY_BUFFER, 0);
> +	dispatch->glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0);
> +
> +	dispatch->glDisableVertexAttribArray(GLAMOR_VERTEX_POS);
> +	dispatch->glDisableVertexAttribArray(GLAMOR_VERTEX_SOURCE);
> +	dispatch->glUseProgram(0);
> +	if (src_picture->pSourcePict->linear.nstops > LINEAR_DEFAULT_STOPS)
> +		dispatch->glDeleteProgram(gradient_prog);
> +
> +	glamor_put_dispatch(glamor_priv);
> +	return dst_picure;
> +
> +GRADIENT_FAIL:
> +	dispatch->glBindBuffer(GL_ARRAY_BUFFER, 0);
> +	dispatch->glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0);
> +
> +	dispatch->glDisableVertexAttribArray(GLAMOR_VERTEX_POS);
> +	dispatch->glDisableVertexAttribArray(GLAMOR_VERTEX_SOURCE);
> +	dispatch->glUseProgram(0);
> +	if (src_picture->pSourcePict->linear.nstops > LINEAR_DEFAULT_STOPS)
> +		dispatch->glDeleteProgram(gradient_prog);
> +	glamor_put_dispatch(glamor_priv);
> +	return NULL;
> +}
> +#undef LINEAR_DEFAULT_STOPS
> +
>  static PicturePtr
>  glamor_convert_gradient_picture(ScreenPtr screen,
>  				PicturePtr source,
> @@ -1290,6 +1806,15 @@ 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);
> +
> +		//glamor_compare_picture_with_pixman(screen, source,
> +		//         dst, x_source, y_source, 0, 3);
Remove the above code if you don't need it or use a macro to comment it out.

> +                return dst;
> +	}
> +
>  	pixmap = glamor_create_pixmap(screen,
>  				      width,
>  				      height,
> diff --git a/src/glamor_utils.h b/src/glamor_utils.h
> old mode 100644
> new mode 100755
> index 4a51ba5..b50a39f
> --- a/src/glamor_utils.h
> +++ b/src/glamor_utils.h
> @@ -664,6 +664,207 @@ static inline void glamor_dump_pixmap(PixmapPtr pixmap, int x, int y, int w, int
>  	glamor_finish_access(&pixmap->drawable, GLAMOR_ACCESS_RO);
>  }
>  
> +static inline void _glamor_compare_pixmaps(PixmapPtr pixmap1, PixmapPtr pixmap2,
> +                                           int x, int y, int w, int h,
> +                                           PictFormatShort short_format,
> +                                           int all, int diffs)
> +{
> +	int i, j;
> +	unsigned char * p1 = pixmap1->devPrivate.ptr;
> +	unsigned char * p2 = pixmap2->devPrivate.ptr;
> +	int line_need_printed = 0;
> +	int test_code = 0xAABBCCDD;
> +	int little_endian = 0;
> +	unsigned char *p_test;
> +	int bpp = pixmap1->drawable.depth == 8? 1 : 4;
> +
> +	assert(pixmap1->devKind == pixmap2->devKind);
> +	int stride = pixmap1->devKind;
> +
> +	ErrorF("stride:%d, width:%d, height:%d\n", stride, w, h);
> +
> +	p1 = p1 + y * stride + x;
> +	p2 = p2 + y * stride + x;
> +
> +	if(all) {
> +		for (i = 0; i < h; i++) {
> +			ErrorF("line %3d: ", i);
> +
> +			for (j = 0; j < stride; j++) {
> +				if(j % bpp == 0)
> +					ErrorF("[%d]%2x:%2x ", j/bpp, p1[j], p2[j]);
> +				else
> +					ErrorF("%2x:%2x ", p1[j], p2[j]);
> +			}
> +
> +			p1 += stride;
> +			p2 += stride;
> +			ErrorF("\n");
> +		}
> +	} else {
> +		if (short_format == PICT_a8r8g8b8) {
> +			p_test = (unsigned char *)&test_code;
> +			little_endian = (*p_test == 0xDD);
> +			bpp = 4;
> +
> +			for (i = 0; i < h; i++) {
> +				line_need_printed = 0;
> +
> +				for (j = 0; j < stride; j++) {
> +					if (p1[j] != p2[j] && (p1[j] - p2[j] > diffs || p2[j] - p1[j] > diffs)) {
> +						if (line_need_printed) {
> +							if(little_endian) {
> +								switch (j % 4) {
> +									case 2:
> +									ErrorF("[%d]RED:%2x:%2x ", j/bpp, p1[j], p2[j]);
> +									break;
> +									case 1:
> +									ErrorF("[%d]GREEN:%2x:%2x ", j/bpp, p1[j], p2[j]);
> +									break;
> +									case 0:
> +									ErrorF("[%d]BLUE:%2x:%2x ", j/bpp, p1[j], p2[j]);
> +									break;
> +									case 3:
> +									ErrorF("[%d]Alpha:%2x:%2x ", j/bpp, p1[j], p2[j]);
> +									break;
> +								}
> +							}else {
> +								switch (j % 4) {
> +									case 1:
> +									ErrorF("[%d]RED:%2x:%2x ", j/bpp, p1[j], p2[j]);
> +									break;
> +									case 2:
> +									ErrorF("[%d]GREEN:%2x:%2x ", j/bpp, p1[j], p2[j]);
> +									break;
> +									case 3:
> +									ErrorF("[%d]BLUE:%2x:%2x ", j/bpp, p1[j], p2[j]);
> +									break;
> +									case 0:
> +									ErrorF("[%d]Alpha:%2x:%2x ", j/bpp, p1[j], p2[j]);
> +									break;
> +								}
> +							}
> +						} else {
> +							line_need_printed = 1;
> +							j = -1;
> +							ErrorF("line %3d: ", i);
> +							continue;
> +						}
> +					}
> +				}
> +
> +				p1 += stride;
> +				p2 += stride;
> +				ErrorF("\n");
> +			}
> +		} //more format can be added here.
> +		else { // the default format, just print.
> +			for (i = 0; i < h; i++) {
> +				line_need_printed = 0;
> +
> +				for (j = 0; j < stride; j++) {
> +					if (p1[j] != p2[j]) {
> +						if (line_need_printed) {
> +							ErrorF("[%d]%2x:%2x ", j/bpp, p1[j], p2[j]);
> +						} else {
> +							line_need_printed = 1;
> +							j = -1;
> +							ErrorF("line %3d: ", i);
> +							continue;
> +						}
> +					}
> +				}
> +
> +				p1 += stride;
> +				p2 += stride;
> +				ErrorF("\n");
> +			}
> +		}
> +	}
> +}
> +
> +static inline void glamor_compare_pixmaps(PixmapPtr pixmap1, PixmapPtr pixmap2,
> +                                          int x, int y, int w, int h, int all, int diffs)
> +{
> +	assert(pixmap1->drawable.depth == pixmap2->drawable.depth);
> +
> +	glamor_prepare_access(&pixmap1->drawable, GLAMOR_ACCESS_RO);
> +	glamor_prepare_access(&pixmap2->drawable, GLAMOR_ACCESS_RO);
> +
> +	_glamor_compare_pixmaps(pixmap1, pixmap2, x, y, w, h, -1, all, diffs);
> +
> +	glamor_finish_access(&pixmap1->drawable, GLAMOR_ACCESS_RO);
> +	glamor_finish_access(&pixmap2->drawable, GLAMOR_ACCESS_RO);
> +}
> +
> +static inline void glamor_compare_pictures( PicturePtr fst_picture,
> +                                            PicturePtr snd_picture,
> +                                            int width, int height,
> +                                            int all, int diffs)
> +{
> +	PixmapPtr fst_pixmap;
> +	PixmapPtr snd_pixmap;
> +	glamor_pixmap_private *fst_pixmap_priv;
> +	glamor_pixmap_private *snd_pixmap_priv;
> +
> +	if (!fst_picture->pDrawable || !snd_picture->pDrawable)
> +		return;
> +
> +	if (fst_picture->format != snd_picture->format)
> +		return;
> +
> +	fst_pixmap = glamor_get_drawable_pixmap(fst_picture->pDrawable);
> +	snd_pixmap = glamor_get_drawable_pixmap(snd_picture->pDrawable);
> +
> +	if (fst_pixmap->drawable.depth != snd_pixmap->drawable.depth)
> +		return;
> +
> +	glamor_prepare_access(&fst_pixmap->drawable, GLAMOR_ACCESS_RO);
> +	glamor_prepare_access(&snd_pixmap->drawable, GLAMOR_ACCESS_RO);
> +
> +	_glamor_compare_pixmaps(fst_pixmap, snd_pixmap, 0, 0, width, height,
> +		                   fst_picture->format, all, diffs);
> +
> +	glamor_finish_access(&fst_pixmap->drawable, GLAMOR_ACCESS_RO);
> +	glamor_finish_access(&snd_pixmap->drawable, GLAMOR_ACCESS_RO);
> +}
> +
> +static inline void glamor_compare_picture_with_pixman( ScreenPtr screen,
> +	                                               PicturePtr src_pic, PicturePtr picture,
> +	                                               int x_source, int y_source,
> +	                                               int all, int diffs)
> +{
maybe you can consider consolidate this function into the glamor_compare_pictures.
You can check the picture type at glamor_compare_picture, and if the first or second
picture doesn't have a valid drawable, then call this function to convert it to a
normal picture.

Then you can call one function to compare all pictures rather than call two different
functions and need to check the type at the caller side.

> +	PixmapPtr pixmap;
> +	PicturePtr pixman_pic;
> +	PictFormatShort format;
> +	int width;
> +	int height;
> +	int error;
> +
> +	format = picture->format;
> +	width = picture->pDrawable->width;
> +	height = picture->pDrawable->height;
> +
> +	pixmap = glamor_create_pixmap(screen,
> +		width, height,
> +		PIXMAN_FORMAT_DEPTH(format),
> +		GLAMOR_CREATE_PIXMAP_CPU);
> +
> +	pixman_pic = CreatePicture(0,
> +		&pixmap->drawable,
> +		PictureMatchFormat(screen, PIXMAN_FORMAT_DEPTH(format), format),
> +		0, 0, serverClient, &error);
> +
> +	fbComposite(PictOpSrc, src_pic, NULL, pixman_pic, x_source, y_source,
> +		0, 0, 0, 0, width, height);
> +
> +	glamor_destroy_pixmap(pixmap);
> +
> +	glamor_compare_pictures(picture, pixman_pic, width, height, all, diffs);
> +
> +	glamor_destroy_picture(pixman_pic);
> +}
> +
>  static inline void glamor_make_current(ScreenPtr screen)
>  {
>  	glamor_egl_make_current(screen);
> -- 
> 1.7.7.6
> 
> _______________________________________________
> Glamor mailing list
> Glamor at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/glamor


More information about the Glamor mailing list