[Mesa-dev] [PATCH 1/4] mesa: replace UsesClipDistance with ClipDistanceArraySize

Brian Paul brianp at vmware.com
Mon Oct 19 09:20:14 PDT 2015


On 10/18/2015 11:14 AM, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak at amd.com>
>
> This is more practical and needed by gallium.
> ---
>   src/glsl/linker.cpp                | 33 +++++++++++++++------------------
>   src/glsl/nir/glsl_to_nir.cpp       |  3 ++-
>   src/mesa/drivers/dri/i965/brw_vs.c |  2 +-
>   src/mesa/main/mtypes.h             |  5 +----
>   src/mesa/main/shaderapi.c          |  6 +++---
>   5 files changed, 22 insertions(+), 27 deletions(-)
>
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index 25ca928..9dd7a49 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -651,7 +651,7 @@ link_invalidate_variable_locations(exec_list *ir)
>
>
>   /**
> - * Set UsesClipDistance and ClipDistanceArraySize based on the given shader.
> + * Set ClipDistanceArraySize based on the given shader.
>    *
>    * Also check for errors based on incorrect usage of gl_ClipVertex and
>    * gl_ClipDistance.
> @@ -660,7 +660,7 @@ link_invalidate_variable_locations(exec_list *ir)
>    */
>   static void
>   analyze_clip_usage(struct gl_shader_program *prog,
> -                   struct gl_shader *shader, GLboolean *UsesClipDistance,
> +                   struct gl_shader *shader,
>                      GLuint *ClipDistanceArraySize)

We usually don't capitalize locals or parameter names.

Can you use clipDistanceArraySize or clip_distance_array_size?

The series looks good to me otherwise.
Reviewed-by: Brian Paul <brianp at vmware.com>


>   {
>      *ClipDistanceArraySize = 0;
> @@ -686,13 +686,14 @@ analyze_clip_usage(struct gl_shader_program *prog,
>                         _mesa_shader_stage_to_string(shader->Stage));
>            return;
>         }
> -      *UsesClipDistance = clip_distance.variable_found();
> -      ir_variable *clip_distance_var =
> -         shader->symbols->get_variable("gl_ClipDistance");
> -      if (clip_distance_var)
> +
> +      if (clip_distance.variable_found()) {
> +         ir_variable *clip_distance_var =
> +               shader->symbols->get_variable("gl_ClipDistance");
> +
> +         assert(clip_distance_var);
>            *ClipDistanceArraySize = clip_distance_var->type->length;
> -   } else {
> -      *UsesClipDistance = false;
> +      }
>      }
>   }
>
> @@ -700,8 +701,7 @@ analyze_clip_usage(struct gl_shader_program *prog,
>   /**
>    * Verify that a vertex shader executable meets all semantic requirements.
>    *
> - * Also sets prog->Vert.UsesClipDistance and prog->Vert.ClipDistanceArraySize
> - * as a side effect.
> + * Also sets prog->Vert.ClipDistanceArraySize as a side effect.
>    *
>    * \param shader  Vertex shader executable to be verified
>    */
> @@ -754,8 +754,7 @@ validate_vertex_shader_executable(struct gl_shader_program *prog,
>         }
>      }
>
> -   analyze_clip_usage(prog, shader, &prog->Vert.UsesClipDistance,
> -                      &prog->Vert.ClipDistanceArraySize);
> +   analyze_clip_usage(prog, shader, &prog->Vert.ClipDistanceArraySize);
>   }
>
>   void
> @@ -765,8 +764,7 @@ validate_tess_eval_shader_executable(struct gl_shader_program *prog,
>      if (shader == NULL)
>         return;
>
> -   analyze_clip_usage(prog, shader, &prog->TessEval.UsesClipDistance,
> -                      &prog->TessEval.ClipDistanceArraySize);
> +   analyze_clip_usage(prog, shader, &prog->TessEval.ClipDistanceArraySize);
>   }
>
>
> @@ -797,8 +795,8 @@ validate_fragment_shader_executable(struct gl_shader_program *prog,
>   /**
>    * Verify that a geometry shader executable meets all semantic requirements
>    *
> - * Also sets prog->Geom.VerticesIn, prog->Geom.UsesClipDistance, and
> - * prog->Geom.ClipDistanceArraySize as a side effect.
> + * Also sets prog->Geom.VerticesIn, and prog->Geom.ClipDistanceArraySize as
> + * a side effect.
>    *
>    * \param shader Geometry shader executable to be verified
>    */
> @@ -812,8 +810,7 @@ validate_geometry_shader_executable(struct gl_shader_program *prog,
>      unsigned num_vertices = vertices_per_prim(prog->Geom.InputType);
>      prog->Geom.VerticesIn = num_vertices;
>
> -   analyze_clip_usage(prog, shader, &prog->Geom.UsesClipDistance,
> -                      &prog->Geom.ClipDistanceArraySize);
> +   analyze_clip_usage(prog, shader, &prog->Geom.ClipDistanceArraySize);
>   }
>
>   /**
> diff --git a/src/glsl/nir/glsl_to_nir.cpp b/src/glsl/nir/glsl_to_nir.cpp
> index cf5bb93..3f7cc83 100644
> --- a/src/glsl/nir/glsl_to_nir.cpp
> +++ b/src/glsl/nir/glsl_to_nir.cpp
> @@ -160,7 +160,8 @@ glsl_to_nir(const struct gl_shader_program *shader_prog,
>      shader->info.outputs_written = sh->Program->OutputsWritten;
>      shader->info.system_values_read = sh->Program->SystemValuesRead;
>      shader->info.uses_texture_gather = sh->Program->UsesGather;
> -   shader->info.uses_clip_distance_out = sh->Program->UsesClipDistanceOut;
> +   shader->info.uses_clip_distance_out =
> +      sh->Program->ClipDistanceArraySize != 0;
>      shader->info.separate_shader = shader_prog->SeparateShader;
>      shader->info.gs.vertices_out = sh->Geom.VerticesOut;
>      shader->info.gs.invocations = sh->Geom.Invocations;
> diff --git a/src/mesa/drivers/dri/i965/brw_vs.c b/src/mesa/drivers/dri/i965/brw_vs.c
> index de9a867..f1df2df 100644
> --- a/src/mesa/drivers/dri/i965/brw_vs.c
> +++ b/src/mesa/drivers/dri/i965/brw_vs.c
> @@ -346,7 +346,7 @@ brw_vs_populate_key(struct brw_context *brw,
>
>      if (ctx->Transform.ClipPlanesEnabled != 0 &&
>          ctx->API == API_OPENGL_COMPAT &&
> -       !vp->program.Base.UsesClipDistanceOut) {
> +       vp->program.Base.ClipDistanceArraySize == 0) {
>         key->nr_userclip_plane_consts =
>            _mesa_logbase2(ctx->Transform.ClipPlanesEnabled) + 1;
>      }
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index e9d8ea4..610c25d 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -1909,7 +1909,7 @@ struct gl_program
>       * For vertex and geometry shaders, true if the program uses the
>       * gl_ClipDistance output.  Ignored for fragment shaders.
>       */
> -   GLboolean UsesClipDistanceOut;
> +   unsigned ClipDistanceArraySize;
>
>
>      /** Named parameters, constants, etc. from program text */
> @@ -2632,7 +2632,6 @@ struct gl_shader_program
>          * True if gl_ClipDistance is written to.  Copied into
>          * gl_tess_eval_program by _mesa_copy_linked_program_data().
>          */
> -      GLboolean UsesClipDistance;
>         GLuint ClipDistanceArraySize; /**< Size of the gl_ClipDistance array, or
>                                            0 if not present. */
>      } TessEval;
> @@ -2655,7 +2654,6 @@ struct gl_shader_program
>          * True if gl_ClipDistance is written to.  Copied into
>          * gl_geometry_program by _mesa_copy_linked_program_data().
>          */
> -      GLboolean UsesClipDistance;
>         GLuint ClipDistanceArraySize; /**< Size of the gl_ClipDistance array, or
>                                            0 if not present. */
>         bool UsesEndPrimitive;
> @@ -2668,7 +2666,6 @@ struct gl_shader_program
>          * True if gl_ClipDistance is written to.  Copied into gl_vertex_program
>          * by _mesa_copy_linked_program_data().
>          */
> -      GLboolean UsesClipDistance;
>         GLuint ClipDistanceArraySize; /**< Size of the gl_ClipDistance array, or
>                                            0 if not present. */
>      } Vert;
> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
> index 6a2f60d..96d68af 100644
> --- a/src/mesa/main/shaderapi.c
> +++ b/src/mesa/main/shaderapi.c
> @@ -2072,7 +2072,7 @@ _mesa_copy_linked_program_data(gl_shader_stage type,
>   {
>      switch (type) {
>      case MESA_SHADER_VERTEX:
> -      dst->UsesClipDistanceOut = src->Vert.UsesClipDistance;
> +      dst->ClipDistanceArraySize = src->Vert.ClipDistanceArraySize;
>         break;
>      case MESA_SHADER_TESS_CTRL: {
>         struct gl_tess_ctrl_program *dst_tcp =
> @@ -2087,7 +2087,7 @@ _mesa_copy_linked_program_data(gl_shader_stage type,
>         dst_tep->Spacing = src->TessEval.Spacing;
>         dst_tep->VertexOrder = src->TessEval.VertexOrder;
>         dst_tep->PointMode = src->TessEval.PointMode;
> -      dst->UsesClipDistanceOut = src->TessEval.UsesClipDistance;
> +      dst->ClipDistanceArraySize = src->TessEval.ClipDistanceArraySize;
>         break;
>      }
>      case MESA_SHADER_GEOMETRY: {
> @@ -2097,7 +2097,7 @@ _mesa_copy_linked_program_data(gl_shader_stage type,
>         dst_gp->Invocations = src->Geom.Invocations;
>         dst_gp->InputType = src->Geom.InputType;
>         dst_gp->OutputType = src->Geom.OutputType;
> -      dst->UsesClipDistanceOut = src->Geom.UsesClipDistance;
> +      dst->ClipDistanceArraySize = src->Geom.ClipDistanceArraySize;
>         dst_gp->UsesEndPrimitive = src->Geom.UsesEndPrimitive;
>         dst_gp->UsesStreams = src->Geom.UsesStreams;
>         break;
>



More information about the mesa-dev mailing list