Looks fine, with a small suggestion for meta.c :

Reviewed-by: Lionel Landwerlin <lionel.g.landwer...@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,


_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to