[Mesa-dev] [PATCH 05/25] mesa/meta: rewrite _mesa_shader_program_use() and _mesa_program_use()
Lionel Landwerlin
lionel.g.landwerlin at intel.com
Wed Jan 11 15:31:36 UTC 2017
Looks fine, with a small suggestion for meta.c :
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
On 09/01/17 05:13, Timothy Arceri wrote:
> These are rewritten to do what the function name suggests, that is
> _mesa_shader_program_use() sets the use of all stage and
> _mesa_program_use() sets the use of a single stage.
>
> This patch is split out to make review easier but will be squashed into
> mesa: use gl_program for CurrentProgram rather than gl_shader_program
> before pushing.
> ---
> src/mesa/drivers/common/meta.c | 19 ++++---------------
> src/mesa/main/pipelineobj.c | 24 ++++++++++++++++++------
> src/mesa/main/shaderapi.c | 34 ++++++++++++++++------------------
> src/mesa/main/shaderapi.h | 9 +++++----
> 4 files changed, 43 insertions(+), 43 deletions(-)
>
> diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c
> index 15d28b2..5b99c6b 100644
> --- a/src/mesa/drivers/common/meta.c
> +++ b/src/mesa/drivers/common/meta.c
> @@ -167,7 +167,7 @@ _mesa_meta_use_program(struct gl_context *ctx,
> _mesa_reference_pipeline_object(ctx, &ctx->_Shader, &ctx->Shader);
>
> /* Update the program */
> - _mesa_use_program(ctx, sh_prog);
> + _mesa_use_shader_program(ctx, sh_prog);
> }
>
> void
> @@ -931,16 +931,6 @@ _mesa_meta_end(struct gl_context *ctx)
> }
>
> if (state & MESA_META_SHADER) {
> - static const GLenum targets[] = {
> - GL_VERTEX_SHADER,
> - GL_TESS_CONTROL_SHADER,
> - GL_TESS_EVALUATION_SHADER,
> - GL_GEOMETRY_SHADER,
> - GL_FRAGMENT_SHADER,
> - GL_COMPUTE_SHADER,
> - };
> - STATIC_ASSERT(MESA_SHADER_STAGES == ARRAY_SIZE(targets));
> -
> bool any_shader;
>
> if (ctx->Extensions.ARB_vertex_program) {
> @@ -966,14 +956,13 @@ _mesa_meta_end(struct gl_context *ctx)
>
> any_shader = false;
> for (i = 0; i < MESA_SHADER_STAGES; i++) {
> - /* It is safe to call _mesa_use_shader_program even if the extension
> + /* It is safe to call _mesa_use_program even if the extension
> * necessary for that program state is not supported. In that case,
> * the saved program object must be NULL and the currently bound
> - * program object must be NULL. _mesa_use_shader_program is a no-op
> + * program object must be NULL. _mesa_use_program is a no-op
> * in that case.
> */
> - _mesa_use_shader_program(ctx, targets[i], save->Program[i],
> - &ctx->Shader);
> + _mesa_use_program(ctx, i, save->Program[i], &ctx->Shader);
>
> /* Do this *before* killing the reference. :)
> */
> diff --git a/src/mesa/main/pipelineobj.c b/src/mesa/main/pipelineobj.c
> index d3f9cec..ec5df89 100644
> --- a/src/mesa/main/pipelineobj.c
> +++ b/src/mesa/main/pipelineobj.c
> @@ -218,6 +218,18 @@ _mesa_reference_pipeline_object_(struct gl_context *ctx,
> }
> }
>
> +static void
> +use_program_stage(struct gl_context *ctx, GLenum type,
I think you can replace the "GLenum type" argument by "gl_shader_stage
stage" here and below and save yourself an indirection.
> + struct gl_shader_program *shProg,
> + struct gl_pipeline_object *pipe) {
> + gl_shader_stage stage = _mesa_shader_enum_to_shader_stage(type);
> + struct gl_program *prog = NULL;
> + if (shProg && shProg->_LinkedShaders[stage])
> + prog = shProg->_LinkedShaders[stage]->Program;
> +
> + _mesa_use_program(ctx, stage, prog, pipe);
> +}
> +
> /**
> * Bound program to severals stages of the pipeline
> */
> @@ -325,22 +337,22 @@ _mesa_UseProgramStages(GLuint pipeline, GLbitfield stages, GLuint program)
> * configured for the indicated shader stages."
> */
> if ((stages & GL_VERTEX_SHADER_BIT) != 0)
> - _mesa_use_shader_program(ctx, GL_VERTEX_SHADER, shProg, pipe);
> + use_program_stage(ctx, GL_VERTEX_SHADER, shProg, pipe);
>
> if ((stages & GL_FRAGMENT_SHADER_BIT) != 0)
> - _mesa_use_shader_program(ctx, GL_FRAGMENT_SHADER, shProg, pipe);
> + use_program_stage(ctx, GL_FRAGMENT_SHADER, shProg, pipe);
>
> if ((stages & GL_GEOMETRY_SHADER_BIT) != 0)
> - _mesa_use_shader_program(ctx, GL_GEOMETRY_SHADER, shProg, pipe);
> + use_program_stage(ctx, GL_GEOMETRY_SHADER, shProg, pipe);
>
> if ((stages & GL_TESS_CONTROL_SHADER_BIT) != 0)
> - _mesa_use_shader_program(ctx, GL_TESS_CONTROL_SHADER, shProg, pipe);
> + use_program_stage(ctx, GL_TESS_CONTROL_SHADER, shProg, pipe);
>
> if ((stages & GL_TESS_EVALUATION_SHADER_BIT) != 0)
> - _mesa_use_shader_program(ctx, GL_TESS_EVALUATION_SHADER, shProg, pipe);
> + use_program_stage(ctx, GL_TESS_EVALUATION_SHADER, shProg, pipe);
>
> if ((stages & GL_COMPUTE_SHADER_BIT) != 0)
> - _mesa_use_shader_program(ctx, GL_COMPUTE_SHADER, shProg, pipe);
> + use_program_stage(ctx, GL_COMPUTE_SHADER, shProg, pipe);
>
> pipe->Validated = false;
> }
> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
> index d92b013..9fa8ce1 100644
> --- a/src/mesa/main/shaderapi.c
> +++ b/src/mesa/main/shaderapi.c
> @@ -1230,17 +1230,12 @@ _mesa_active_program(struct gl_context *ctx, struct gl_shader_program *shProg,
>
>
> static void
> -use_shader_program(struct gl_context *ctx, gl_shader_stage stage,
> - struct gl_shader_program *shProg,
> - struct gl_pipeline_object *shTarget)
> +use_program(struct gl_context *ctx, gl_shader_stage stage,
> + struct gl_program *new_prog, struct gl_pipeline_object *shTarget)
> {
> struct gl_program **target;
> - struct gl_program *new_prog = NULL;
>
> target = &shTarget->CurrentProgram[stage];
> - if ((shProg != NULL) && (shProg->_LinkedShaders[stage] != NULL))
> - new_prog = shProg->_LinkedShaders[stage]->Program;
> -
> if (new_prog) {
> _mesa_program_init_subroutine_defaults(ctx, new_prog);
> }
> @@ -1282,11 +1277,15 @@ use_shader_program(struct gl_context *ctx, gl_shader_stage stage,
> * Use the named shader program for subsequent rendering.
> */
> void
> -_mesa_use_program(struct gl_context *ctx, struct gl_shader_program *shProg)
> +_mesa_use_shader_program(struct gl_context *ctx,
> + struct gl_shader_program *shProg)
> {
> - int i;
> - for (i = 0; i < MESA_SHADER_STAGES; i++)
> - use_shader_program(ctx, i, shProg, &ctx->Shader);
> + for (int i = 0; i < MESA_SHADER_STAGES; i++) {
> + struct gl_program *new_prog = NULL;
> + if (shProg && shProg->_LinkedShaders[i])
> + new_prog = shProg->_LinkedShaders[i]->Program;
> + use_program(ctx, i, new_prog, &ctx->Shader);
> + }
> _mesa_active_program(ctx, shProg, "glUseProgram");
> }
>
> @@ -1884,10 +1883,10 @@ _mesa_UseProgram(GLuint program)
> /* Attach shader state to the binding point */
> _mesa_reference_pipeline_object(ctx, &ctx->_Shader, &ctx->Shader);
> /* Update the program */
> - _mesa_use_program(ctx, shProg);
> + _mesa_use_shader_program(ctx, shProg);
> } else {
> /* Must be done first: detach the progam */
> - _mesa_use_program(ctx, shProg);
> + _mesa_use_shader_program(ctx, shProg);
> /* Unattach shader_state binding point */
> _mesa_reference_pipeline_object(ctx, &ctx->_Shader, ctx->Pipeline.Default);
> /* If a pipeline was bound, rebind it */
> @@ -2169,12 +2168,11 @@ invalid_value:
>
>
> void
> -_mesa_use_shader_program(struct gl_context *ctx, GLenum type,
> - struct gl_shader_program *shProg,
> - struct gl_pipeline_object *shTarget)
> +_mesa_use_program(struct gl_context *ctx, gl_shader_stage stage,
> + struct gl_program *prog,
> + struct gl_pipeline_object *shTarget)
> {
> - gl_shader_stage stage = _mesa_shader_enum_to_shader_stage(type);
> - use_shader_program(ctx, stage, shProg, shTarget);
> + use_program(ctx, stage, prog, shTarget);
> }
>
>
> diff --git a/src/mesa/main/shaderapi.h b/src/mesa/main/shaderapi.h
> index 06de11f..a89dbfb 100644
> --- a/src/mesa/main/shaderapi.h
> +++ b/src/mesa/main/shaderapi.h
> @@ -51,7 +51,8 @@ _mesa_copy_string(GLchar *dst, GLsizei maxLength,
> GLsizei *length, const GLchar *src);
>
> extern void
> -_mesa_use_program(struct gl_context *ctx, struct gl_shader_program *shProg);
> +_mesa_use_shader_program(struct gl_context *ctx,
> + struct gl_shader_program *shProg);
>
> extern void
> _mesa_active_program(struct gl_context *ctx, struct gl_shader_program *shProg,
> @@ -213,9 +214,9 @@ extern void GLAPIENTRY
> _mesa_ProgramParameteri(GLuint program, GLenum pname, GLint value);
>
> void
> -_mesa_use_shader_program(struct gl_context *ctx, GLenum type,
> - struct gl_shader_program *shProg,
> - struct gl_pipeline_object *shTarget);
> +_mesa_use_program(struct gl_context *ctx, gl_shader_stage stage,
> + struct gl_program *prog,
> + struct gl_pipeline_object *shTarget);
>
> extern void
> _mesa_copy_linked_program_data(const struct gl_shader_program *src,
More information about the mesa-dev
mailing list