[Mesa-dev] [PATCH 5/5] meta: Don't use integer handles for shaders or programs.

Pohjolainen, Topi topi.pohjolainen at intel.com
Wed Mar 16 15:36:25 UTC 2016


On Wed, Mar 16, 2016 at 12:13:02AM -0700, Kenneth Graunke wrote:
> Previously, we gave our internal clear/blit shaders actual GL handles
> and stored them in the shader/program hash table.  We used ordinary
> GL API entrypoints to work with them.
> 
> We thought this shouldn't be a problem because GL doesn't allow
> applications to invent their own names for shaders or programs.
> GL allocates all names via glCreateShader and glCreateProgram.
> 
> However, having them in the hash table is a bit risky: if a broken
> application guesses the name of our shaders or programs, it could
> alter them, potentially screwing up future meta operations.
> 
> Also, test cases can observe the programs in the hash table.  Running
> a single dEQP process that executes the following test list:
> 
> dEQP-GLES3.functional.negative_api.buffer.clear
> dEQP-GLES3.functional.negative_api.shader.compile_shader
> dEQP-GLES3.functional.negative_api.shader.delete_shader
> 
> would result in the last two tests breaking.  The compile_shader test
> calls glCompileShader(9) straight away, and since it hasn't even created
> any shaders or programs, it expects to get a GL_INVALID_VALUE error
> because there's no such name.  However, because the clear test ran
> first, it created Meta programs, so an object named "9" did exist.
> 
> This patch reworks Meta to work with gl_shader and gl_shader_program
> pointers directly.  These internal programs have bogus names, and are
> never stored in the hash tables, so they're invisible to applications.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94485
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/mesa/drivers/common/meta.c                    | 163 +++++++++-------------
>  src/mesa/drivers/common/meta.h                    |  24 ++--
>  src/mesa/drivers/common/meta_blit.c               |  16 +--
>  src/mesa/drivers/common/meta_generate_mipmap.c    |   2 +-
>  src/mesa/drivers/dri/i965/brw_context.h           |   2 +-
>  src/mesa/drivers/dri/i965/brw_meta_fast_clear.c   |  11 +-
>  src/mesa/drivers/dri/i965/brw_meta_stencil_blit.c |  59 ++++----
>  7 files changed, 130 insertions(+), 147 deletions(-)
> 
> diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c
> index bdcf316..b673db4 100644
> --- a/src/mesa/drivers/common/meta.c
> +++ b/src/mesa/drivers/common/meta.c
> @@ -121,72 +121,51 @@ _mesa_meta_framebuffer_texture_image(struct gl_context *ctx,
>                               level, layer, false, __func__);
>  }
>  
> -GLuint
> +struct gl_shader *
>  _mesa_meta_compile_shader_with_debug(struct gl_context *ctx, GLenum target,
>                                       const GLcharARB *source)
>  {
> -   GLuint shader;
> -   GLint ok, size;
> -   GLchar *info;
> -
> -   shader = _mesa_CreateShader(target);
> -   _mesa_ShaderSource(shader, 1, &source, NULL);
> -   _mesa_CompileShader(shader);
> -
> -   _mesa_GetShaderiv(shader, GL_COMPILE_STATUS, &ok);
> -   if (ok)
> -      return shader;
> -
> -   _mesa_GetShaderiv(shader, GL_INFO_LOG_LENGTH, &size);
> -   if (size == 0) {
> -      _mesa_DeleteShader(shader);
> -      return 0;
> -   }
> +   const GLuint name = ~0;
> +   struct gl_shader *sh;
> +
> +   sh = ctx->Driver.NewShader(ctx, name, target);
> +   sh->Source = strdup(source);
> +   sh->CompileStatus = false;
> +   _mesa_compile_shader(ctx, sh);
> +
> +   if (!sh->CompileStatus) {
> +      if (sh->InfoLog) {
> +         _mesa_problem(ctx,
> +                       "meta program compile failed:\n%s\nsource:\n%s\n",
> +                       sh->InfoLog, source);
> +      }
>  
> -   info = malloc(size);
> -   if (!info) {
> -      _mesa_DeleteShader(shader);
> -      return 0;
> +      _mesa_reference_shader(ctx, &sh, NULL);
>     }
>  
> -   _mesa_GetShaderInfoLog(shader, size, NULL, info);
> -   _mesa_problem(ctx,
> -		 "meta program compile failed:\n%s\n"
> -		 "source:\n%s\n",
> -		 info, source);
> -
> -   free(info);
> -   _mesa_DeleteShader(shader);
> -
> -   return 0;
> +   return sh;
>  }
>  
> -GLuint
> -_mesa_meta_link_program_with_debug(struct gl_context *ctx, GLuint program)
> +void
> +_mesa_meta_link_program_with_debug(struct gl_context *ctx,
> +                                   struct gl_shader_program *sh_prog)
>  {
> -   GLint ok, size;
> -   GLchar *info;
> -
> -   _mesa_LinkProgram(program);
> -
> -   _mesa_GetProgramiv(program, GL_LINK_STATUS, &ok);
> -   if (ok)
> -      return program;
> +   _mesa_link_program(ctx, sh_prog);
>  
> -   _mesa_GetProgramiv(program, GL_INFO_LOG_LENGTH, &size);
> -   if (size == 0)
> -      return 0;
> -
> -   info = malloc(size);
> -   if (!info)
> -      return 0;
> -
> -   _mesa_GetProgramInfoLog(program, size, NULL, info);
> -   _mesa_problem(ctx, "meta program link failed:\n%s", info);
> +   if (!sh_prog->LinkStatus) {
> +      _mesa_problem(ctx, "meta program link failed:\n%s", sh_prog->InfoLog);
> +   }
> +}
>  
> -   free(info);
> +void
> +_mesa_meta_use_program(struct gl_context *ctx,
> +                       struct gl_shader_program *sh_prog)
> +{
> +   /* Attach shader state to the binding point */
> +   _mesa_reference_pipeline_object(ctx, &ctx->_Shader, &ctx->Shader);
>  
> -   return 0;
> +   /* Update the program */
> +   _mesa_use_program(ctx, sh_prog);
>  }
>  
>  void
> @@ -194,22 +173,25 @@ _mesa_meta_compile_and_link_program(struct gl_context *ctx,
>                                      const char *vs_source,
>                                      const char *fs_source,
>                                      const char *name,
> -                                    GLuint *program)
> +                                    struct gl_shader_program **out_sh_prog)
>  {
> -   GLuint vs = _mesa_meta_compile_shader_with_debug(ctx, GL_VERTEX_SHADER,
> -                                                    vs_source);
> -   GLuint fs = _mesa_meta_compile_shader_with_debug(ctx, GL_FRAGMENT_SHADER,
> -                                                    fs_source);
> -
> -   *program = _mesa_CreateProgram();
> -   _mesa_ObjectLabel(GL_PROGRAM, *program, -1, name);
> -   _mesa_AttachShader(*program, fs);
> -   _mesa_DeleteShader(fs);
> -   _mesa_AttachShader(*program, vs);
> -   _mesa_DeleteShader(vs);
> -   _mesa_meta_link_program_with_debug(ctx, *program);
> -
> -   _mesa_UseProgram(*program);
> +   struct gl_shader_program *sh_prog;
> +   const GLuint id = ~0;
> +
> +   sh_prog = _mesa_new_shader_program(id);
> +   sh_prog->Label = strdup(name);
> +   sh_prog->NumShaders = 2;
> +   sh_prog->Shaders = malloc(2 * sizeof(struct gl_shader *));
> +   sh_prog->Shaders[0] =
> +      _mesa_meta_compile_shader_with_debug(ctx, GL_VERTEX_SHADER, vs_source);
> +   sh_prog->Shaders[1] =
> +      _mesa_meta_compile_shader_with_debug(ctx, GL_FRAGMENT_SHADER, fs_source);
> +
> +   _mesa_meta_link_program_with_debug(ctx, sh_prog);
> +
> +   _mesa_meta_use_program(ctx, sh_prog);
> +
> +   *out_sh_prog = sh_prog;
>  }
>  
>  /**
> @@ -244,8 +226,8 @@ _mesa_meta_setup_blit_shader(struct gl_context *ctx,
>  
>     assert(shader != NULL);
>  
> -   if (shader->shader_prog != 0) {
> -      _mesa_UseProgram(shader->shader_prog);
> +   if (shader->shader_prog != NULL) {
> +      _mesa_meta_use_program(ctx, shader->shader_prog);
>        return;
>     }
>  
> @@ -1528,7 +1510,6 @@ meta_glsl_clear_init(struct gl_context *ctx, struct clear_state *clear)
>        "{\n"
>        "   gl_FragColor = color;\n"
>        "}\n";
> -   GLuint vs, fs;
>     bool has_integer_textures;
>  
>     _mesa_meta_setup_vertex_objects(ctx, &clear->VAO, &clear->buf_obj, true,
> @@ -1592,12 +1573,10 @@ meta_glsl_clear_cleanup(struct gl_context *ctx, struct clear_state *clear)
>     _mesa_DeleteVertexArrays(1, &clear->VAO);
>     clear->VAO = 0;
>     _mesa_reference_buffer_object(ctx, &clear->buf_obj, NULL);
> -   _mesa_DeleteProgram(clear->ShaderProg);
> -   clear->ShaderProg = 0;
> +   _mesa_reference_shader_program(ctx, &clear->ShaderProg, NULL);
>  
>     if (clear->IntegerShaderProg) {
> -      _mesa_DeleteProgram(clear->IntegerShaderProg);
> -      clear->IntegerShaderProg = 0;
> +      _mesa_reference_shader_program(ctx, &clear->IntegerShaderProg, NULL);
>     }
>  }
>  
> @@ -1711,10 +1690,10 @@ meta_clear(struct gl_context *ctx, GLbitfield buffers, bool glsl)
>  
>     if (fb->_IntegerColor) {
>        assert(glsl);
> -      _mesa_UseProgram(clear->IntegerShaderProg);
> +      _mesa_meta_use_program(ctx, clear->IntegerShaderProg);
>        _mesa_Uniform4iv(0, 1, ctx->Color.ClearColor.i);
>     } else if (glsl) {
> -      _mesa_UseProgram(clear->ShaderProg);
> +      _mesa_meta_use_program(ctx, clear->ShaderProg);
>        _mesa_Uniform4fv(0, 1, ctx->Color.ClearColor.f);
>     }
>  
> @@ -2675,25 +2654,17 @@ choose_blit_shader(GLenum target, struct blit_shader_table *table)
>  }
>  
>  void
> -_mesa_meta_blit_shader_table_cleanup(struct blit_shader_table *table)
> +_mesa_meta_blit_shader_table_cleanup(struct gl_context *ctx,
> +                                     struct blit_shader_table *table)
>  {
> -   _mesa_DeleteProgram(table->sampler_1d.shader_prog);
> -   _mesa_DeleteProgram(table->sampler_2d.shader_prog);
> -   _mesa_DeleteProgram(table->sampler_3d.shader_prog);
> -   _mesa_DeleteProgram(table->sampler_rect.shader_prog);
> -   _mesa_DeleteProgram(table->sampler_cubemap.shader_prog);
> -   _mesa_DeleteProgram(table->sampler_1d_array.shader_prog);
> -   _mesa_DeleteProgram(table->sampler_2d_array.shader_prog);
> -   _mesa_DeleteProgram(table->sampler_cubemap_array.shader_prog);
> -
> -   table->sampler_1d.shader_prog = 0;
> -   table->sampler_2d.shader_prog = 0;
> -   table->sampler_3d.shader_prog = 0;
> -   table->sampler_rect.shader_prog = 0;
> -   table->sampler_cubemap.shader_prog = 0;
> -   table->sampler_1d_array.shader_prog = 0;
> -   table->sampler_2d_array.shader_prog = 0;
> -   table->sampler_cubemap_array.shader_prog = 0;
> +   _mesa_reference_shader_program(ctx, &table->sampler_1d.shader_prog, NULL);
> +   _mesa_reference_shader_program(ctx, &table->sampler_2d.shader_prog, NULL);
> +   _mesa_reference_shader_program(ctx, &table->sampler_3d.shader_prog, NULL);
> +   _mesa_reference_shader_program(ctx, &table->sampler_rect.shader_prog, NULL);
> +   _mesa_reference_shader_program(ctx, &table->sampler_cubemap.shader_prog, NULL);
> +   _mesa_reference_shader_program(ctx, &table->sampler_1d_array.shader_prog, NULL);
> +   _mesa_reference_shader_program(ctx, &table->sampler_2d_array.shader_prog, NULL);
> +   _mesa_reference_shader_program(ctx, &table->sampler_cubemap_array.shader_prog, NULL);
>  }
>  
>  /**
> diff --git a/src/mesa/drivers/common/meta.h b/src/mesa/drivers/common/meta.h
> index c2efa50..0a7321c 100644
> --- a/src/mesa/drivers/common/meta.h
> +++ b/src/mesa/drivers/common/meta.h
> @@ -218,7 +218,7 @@ struct blit_shader {
>     const char *type;
>     const char *func;
>     const char *texcoords;
> -   GLuint shader_prog;
> +   struct gl_shader_program *shader_prog;
>  };
>  
>  /**
> @@ -302,7 +302,7 @@ struct blit_state
>     struct gl_buffer_object *buf_obj;
>     struct blit_shader_table shaders_with_depth;
>     struct blit_shader_table shaders_without_depth;
> -   GLuint msaa_shaders[BLIT_MSAA_SHADER_COUNT];
> +   struct gl_shader_program *msaa_shaders[BLIT_MSAA_SHADER_COUNT];
>     struct temp_texture depthTex;
>     bool no_ctsi_fallback;
>  };
> @@ -324,8 +324,8 @@ struct clear_state
>  {
>     GLuint VAO;
>     struct gl_buffer_object *buf_obj;
> -   GLuint ShaderProg;
> -   GLuint IntegerShaderProg;
> +   struct gl_shader_program *ShaderProg;
> +   struct gl_shader_program *IntegerShaderProg;
>  };
>  
>  
> @@ -577,20 +577,25 @@ _mesa_meta_DrawTex(struct gl_context *ctx, GLfloat x, GLfloat y, GLfloat z,
>  void
>  _mesa_meta_drawbuffers_from_bitfield(GLbitfield bits);
>  
> -GLuint
> +struct gl_shader *
>  _mesa_meta_compile_shader_with_debug(struct gl_context *ctx, GLenum target,
>                                       const GLcharARB *source);
>  
>  
> -GLuint
> -_mesa_meta_link_program_with_debug(struct gl_context *ctx, GLuint program);
> +void
> +_mesa_meta_link_program_with_debug(struct gl_context *ctx,
> +                                   struct gl_shader_program *sh_prog);
>  
>  void
>  _mesa_meta_compile_and_link_program(struct gl_context *ctx,
>                                      const char *vs_source,
>                                      const char *fs_source,
>                                      const char *name,
> -                                    GLuint *program);
> +                                    struct gl_shader_program **sh_prog_ptr);
> +
> +extern void
> +_mesa_meta_use_program(struct gl_context *ctx,
> +                       struct gl_shader_program *sh_prog);
>  
>  GLboolean
>  _mesa_meta_alloc_texture(struct temp_texture *tex,
> @@ -655,7 +660,8 @@ void
>  _mesa_meta_glsl_blit_cleanup(struct gl_context *ctx, struct blit_state *blit);
>  
>  void
> -_mesa_meta_blit_shader_table_cleanup(struct blit_shader_table *table);
> +_mesa_meta_blit_shader_table_cleanup(struct gl_context *ctx,
> +                                     struct blit_shader_table *table);
>  
>  void
>  _mesa_meta_glsl_generate_mipmap_cleanup(struct gl_context *ctx,
> diff --git a/src/mesa/drivers/common/meta_blit.c b/src/mesa/drivers/common/meta_blit.c
> index 179dc0d..0066f7f 100644
> --- a/src/mesa/drivers/common/meta_blit.c
> +++ b/src/mesa/drivers/common/meta_blit.c
> @@ -105,12 +105,12 @@ setup_glsl_msaa_blit_scaled_shader(struct gl_context *ctx,
>     }
>  
>     if (blit->msaa_shaders[shader_index]) {
> -      _mesa_UseProgram(blit->msaa_shaders[shader_index]);
> +      _mesa_meta_use_program(ctx, blit->msaa_shaders[shader_index]);
>        /* Update the uniform values. */
>        loc_src_width =
> -         _mesa_GetUniformLocation(blit->msaa_shaders[shader_index], "src_width");
> +         _mesa_program_resource_location(blit->msaa_shaders[shader_index], GL_UNIFORM, "src_width");
>        loc_src_height =
> -         _mesa_GetUniformLocation(blit->msaa_shaders[shader_index], "src_height");
> +         _mesa_program_resource_location(blit->msaa_shaders[shader_index], GL_UNIFORM, "src_height");
>        _mesa_Uniform1f(loc_src_width, src_rb->Width);
>        _mesa_Uniform1f(loc_src_height, src_rb->Height);
>        return;
> @@ -237,9 +237,9 @@ setup_glsl_msaa_blit_scaled_shader(struct gl_context *ctx,
>     _mesa_meta_compile_and_link_program(ctx, vs_source, fs_source, name,
>                                         &blit->msaa_shaders[shader_index]);
>     loc_src_width =
> -      _mesa_GetUniformLocation(blit->msaa_shaders[shader_index], "src_width");
> +      _mesa_program_resource_location(blit->msaa_shaders[shader_index], GL_UNIFORM, "src_width");
>     loc_src_height =
> -      _mesa_GetUniformLocation(blit->msaa_shaders[shader_index], "src_height");
> +      _mesa_program_resource_location(blit->msaa_shaders[shader_index], GL_UNIFORM, "src_height");
>     _mesa_Uniform1f(loc_src_width, src_rb->Width);
>     _mesa_Uniform1f(loc_src_height, src_rb->Height);
>  
> @@ -347,7 +347,7 @@ setup_glsl_msaa_blit_shader(struct gl_context *ctx,
>     }
>  
>     if (blit->msaa_shaders[shader_index]) {
> -      _mesa_UseProgram(blit->msaa_shaders[shader_index]);
> +      _mesa_meta_use_program(ctx, blit->msaa_shaders[shader_index]);
>        return;
>     }
>  
> @@ -1037,8 +1037,8 @@ _mesa_meta_glsl_blit_cleanup(struct gl_context *ctx, struct blit_state *blit)
>        _mesa_reference_buffer_object(ctx, &blit->buf_obj, NULL);
>     }
>  
> -   _mesa_meta_blit_shader_table_cleanup(&blit->shaders_with_depth);
> -   _mesa_meta_blit_shader_table_cleanup(&blit->shaders_without_depth);
> +   _mesa_meta_blit_shader_table_cleanup(ctx, &blit->shaders_with_depth);
> +   _mesa_meta_blit_shader_table_cleanup(ctx, &blit->shaders_without_depth);
>  
>     _mesa_DeleteTextures(1, &blit->depthTex.TexObj);
>     blit->depthTex.TexObj = 0;
> diff --git a/src/mesa/drivers/common/meta_generate_mipmap.c b/src/mesa/drivers/common/meta_generate_mipmap.c
> index 892d8d3..d4b7539 100644
> --- a/src/mesa/drivers/common/meta_generate_mipmap.c
> +++ b/src/mesa/drivers/common/meta_generate_mipmap.c
> @@ -134,7 +134,7 @@ _mesa_meta_glsl_generate_mipmap_cleanup(struct gl_context *ctx,
>     _mesa_reference_sampler_object(ctx, &mipmap->samp_obj, NULL);
>     _mesa_reference_framebuffer(&mipmap->fb, NULL);
>  
> -   _mesa_meta_blit_shader_table_cleanup(&mipmap->shaders);
> +   _mesa_meta_blit_shader_table_cleanup(ctx, &mipmap->shaders);
>  }
>  
>  static GLboolean
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
> index a953745..b45ee5e 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -900,7 +900,7 @@ struct brw_context
>     struct brw_cache cache;
>  
>     /** IDs for meta stencil blit shader programs. */
> -   unsigned meta_stencil_blit_programs[2];
> +   struct gl_shader_program *meta_stencil_blit_programs[2];
>  
>     /* Whether a meta-operation is in progress. */
>     bool meta_in_progress;
> diff --git a/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c b/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c
> index e2882da..1fb5dc8 100644
> --- a/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c
> +++ b/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c
> @@ -26,6 +26,7 @@
>  #include "main/context.h"
>  #include "main/objectlabel.h"
>  #include "main/shaderapi.h"
> +#include "main/shaderobj.h"
>  #include "main/arrayobj.h"
>  #include "main/bufferobj.h"
>  #include "main/buffers.h"
> @@ -61,8 +62,8 @@
>  struct brw_fast_clear_state {
>     struct gl_buffer_object *buf_obj;
>     struct gl_vertex_array_object *array_obj;
> +   struct gl_shader_program *shader_prog;
>     GLuint vao;
> -   GLuint shader_prog;
>     GLint color_location;
>  };
>  
> @@ -131,7 +132,7 @@ brw_bind_rep_write_shader(struct brw_context *brw, float *color)
>     struct gl_context *ctx = &brw->ctx;
>  
>     if (clear->shader_prog) {
> -      _mesa_UseProgram(clear->shader_prog);
> +      _mesa_meta_use_program(ctx, clear->shader_prog);
>        _mesa_Uniform4fv(clear->color_location, 1, color);
>        return;
>     }
> @@ -141,9 +142,9 @@ brw_bind_rep_write_shader(struct brw_context *brw, float *color)
>                                         &clear->shader_prog);
>  
>     clear->color_location =
> -      _mesa_GetUniformLocation(clear->shader_prog, "color");
> +      _mesa_program_resource_location(clear->shader_prog, GL_UNIFORM, "color");
>  
> -   _mesa_UseProgram(clear->shader_prog);
> +   _mesa_meta_use_program(ctx, clear->shader_prog);
>     _mesa_Uniform4fv(clear->color_location, 1, color);
>  }
>  
> @@ -160,7 +161,7 @@ brw_meta_fast_clear_free(struct brw_context *brw)
>  
>     _mesa_DeleteVertexArrays(1, &clear->vao);
>     _mesa_reference_buffer_object(&brw->ctx, &clear->buf_obj, NULL);
> -   _mesa_DeleteProgram(clear->shader_prog);
> +   _mesa_reference_shader_program(&brw->ctx, &clear->shader_prog, NULL);
>     free(clear);
>  
>     if (old_context)
> diff --git a/src/mesa/drivers/dri/i965/brw_meta_stencil_blit.c b/src/mesa/drivers/dri/i965/brw_meta_stencil_blit.c
> index 5b0c2e9..7e04248 100644
> --- a/src/mesa/drivers/dri/i965/brw_meta_stencil_blit.c
> +++ b/src/mesa/drivers/dri/i965/brw_meta_stencil_blit.c
> @@ -193,6 +193,9 @@ static const char *fs_tmpl =
>     "   %s;\n"
>     "}\n";
>  
> +#define get_uniform_loc(sh_prog, name) \
> +   _mesa_program_resource_location(sh_prog, GL_UNIFORM, name)
> +
>  /**
>   * Setup uniforms telling the coordinates of the destination rectangle in the
>   * native w-tiled space. These are needed to ignore pixels that lie outside.
> @@ -201,12 +204,13 @@ static const char *fs_tmpl =
>   * 16x2 y-tiled).
>   */
>  static void
> -setup_bounding_rect(GLuint prog, const struct blit_dims *dims)
> +setup_bounding_rect(struct gl_shader_program *sh_prog,
> +                    const struct blit_dims *dims)
>  {
> -   _mesa_Uniform1i(_mesa_GetUniformLocation(prog, "dst_x0"), dims->dst_x0);
> -   _mesa_Uniform1i(_mesa_GetUniformLocation(prog, "dst_x1"), dims->dst_x1);
> -   _mesa_Uniform1i(_mesa_GetUniformLocation(prog, "dst_y0"), dims->dst_y0);
> -   _mesa_Uniform1i(_mesa_GetUniformLocation(prog, "dst_y1"), dims->dst_y1);
> +   _mesa_Uniform1i(get_uniform_loc(sh_prog, "dst_x0"), dims->dst_x0);
> +   _mesa_Uniform1i(get_uniform_loc(sh_prog, "dst_x1"), dims->dst_x1);
> +   _mesa_Uniform1i(get_uniform_loc(sh_prog, "dst_y0"), dims->dst_y0);
> +   _mesa_Uniform1i(get_uniform_loc(sh_prog, "dst_y1"), dims->dst_y1);
>  }
>  
>  /**
> @@ -215,14 +219,15 @@ setup_bounding_rect(GLuint prog, const struct blit_dims *dims)
>   * between destination and source that may have differing offsets.
>   */
>  static void
> -setup_drawing_rect(GLuint prog, const struct blit_dims *dims)
> +setup_drawing_rect(struct gl_shader_program *sh_prog,
> +                   const struct blit_dims *dims)
>  {
> -   _mesa_Uniform1f(_mesa_GetUniformLocation(prog, "draw_rect_w"),
> +   _mesa_Uniform1f(get_uniform_loc(sh_prog, "draw_rect_w"),
>                     dims->dst_x1 - dims->dst_x0);
> -   _mesa_Uniform1f(_mesa_GetUniformLocation(prog, "draw_rect_h"),
> +   _mesa_Uniform1f(get_uniform_loc(sh_prog, "draw_rect_h"),
>                     dims->dst_y1 - dims->dst_y0);
> -   _mesa_Uniform1f(_mesa_GetUniformLocation(prog, "dst_x_off"), dims->dst_x0);
> -   _mesa_Uniform1f(_mesa_GetUniformLocation(prog, "dst_y_off"), dims->dst_y0);
> +   _mesa_Uniform1f(get_uniform_loc(sh_prog, "dst_x_off"), dims->dst_x0);
> +   _mesa_Uniform1f(get_uniform_loc(sh_prog, "dst_y_off"), dims->dst_y0);
>  }
>  
>  /**
> @@ -241,7 +246,7 @@ setup_drawing_rect(GLuint prog, const struct blit_dims *dims)
>   *   src_x = src_x0 + (dst_x1 -dst_x - 0.5) * scale
>   */
>  static void
> -setup_coord_coeff(GLuint prog, GLuint multiplier, GLuint offset,
> +setup_coord_coeff(GLuint multiplier, GLuint offset,
>                    int src_0, int src_1, int dst_0, int dst_1, bool mirror)
>  {
>     const float scale = ((float)(src_1 - src_0)) / (dst_1 - dst_0);
> @@ -265,22 +270,21 @@ setup_coord_coeff(GLuint prog, GLuint multiplier, GLuint offset,
>   * destination rectangle is adjusted for possible msaa and Y-tiling.
>   */
>  static void
> -setup_coord_transform(GLuint prog, const struct blit_dims *dims)
> +setup_coord_transform(struct gl_shader_program *sh_prog,
> +                      const struct blit_dims *dims)
>  {
> -   setup_coord_coeff(prog,
> -                     _mesa_GetUniformLocation(prog, "src_x_scale"),
> -                     _mesa_GetUniformLocation(prog, "src_x_off"),
> +   setup_coord_coeff(get_uniform_loc(sh_prog, "src_x_scale"),
> +                     get_uniform_loc(sh_prog, "src_x_off"),
>                       dims->src_x0, dims->src_x1, dims->dst_x0, dims->dst_x1,
>                       dims->mirror_x);
>  
> -   setup_coord_coeff(prog,
> -                     _mesa_GetUniformLocation(prog, "src_y_scale"),
> -                     _mesa_GetUniformLocation(prog, "src_y_off"),
> +   setup_coord_coeff(get_uniform_loc(sh_prog, "src_y_scale"),
> +                     get_uniform_loc(sh_prog, "src_y_off"),
>                       dims->src_y0, dims->src_y1, dims->dst_y0, dims->dst_y1,
>                       dims->mirror_y);
>  }
>  
> -static GLuint
> +static struct gl_shader_program *
>  setup_program(struct brw_context *brw, bool msaa_tex)
>  {
>     struct gl_context *ctx = &brw->ctx;
> @@ -291,21 +295,22 @@ setup_program(struct brw_context *brw, bool msaa_tex)
>     _mesa_meta_setup_vertex_objects(&brw->ctx, &blit->VAO, &blit->buf_obj, true,
>                                     2, 2, 0);
>  
> -   GLuint *prog_id = &brw->meta_stencil_blit_programs[msaa_tex];
> +   struct gl_shader_program **sh_prog_p =

I think this could be 'shader_prog' just as you have it in the rest of
patch. Either way:

Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>


More information about the mesa-dev mailing list