[Mesa-dev] [PATCH 05/10] glsl: Add all system variables to the input resource list.

[Ilia Mirkin]

On Thu, Mar 31, 2016 at 2:53 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> System values are just built-in input variables that we've opted to
> special-case out of convenience.  We need to consider all inputs,
> regardless of how we've classified them.
>
> Unfortunately, there's one exception: we shouldn't add gl_BaseVertex
> unless ARB_shader_draw_parameters is enabled, because it doesn't
> actually exist in the language, and shouldn't be counted in the
> GL_ACTIVE_RESOURCES query.
>
> Fixes dEQP-GLES31.functional.program_interface_query.program_input.
> resource_list.compute.empty, which expects gl_NumWorkGroups to appear
> in the resource list.
>
> v2: Delete more code
> v3: Add basevertex hack back in to avoid regressing CTS tests.
>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/compiler/glsl/linker.cpp   | 15 +++++++--------
>  src/mesa/main/shader_query.cpp |  9 +--------
>  2 files changed, 8 insertions(+), 16 deletions(-)
>
> diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
> index 29c63ee..48aa395 100644
> --- a/src/compiler/glsl/linker.cpp
> +++ b/src/compiler/glsl/linker.cpp
> @@ -3531,15 +3531,14 @@ add_interface_variables(struct gl_shader_program *shProg,
>           continue;
>
>        switch (var->data.mode) {
> -      /* From GL 4.3 core spec, section 11.1.1 (Vertex Attributes):
> -       * "For GetActiveAttrib, all active vertex shader input variables
> -       * are enumerated, including the special built-in inputs gl_VertexID
> -       * and gl_InstanceID."
> -       */
>        case ir_var_system_value:
> -         if (var->data.location != SYSTEM_VALUE_VERTEX_ID &&
> -             var->data.location != SYSTEM_VALUE_VERTEX_ID_ZERO_BASE &&
> -             var->data.location != SYSTEM_VALUE_INSTANCE_ID)
> +         /* We use gl_BaseVertex internally for gl_VertexID lowering.
> +          * We don't want it to leak into the program resource list.
> +          *
> +          * TODO: We do need to add it when ARB_shader_draw_parameters
> +          *       is in use.  We've lost that information by now...
> +          */
> +         if (var->data.location == SYSTEM_VALUE_BASE_VERTEX)

Perhaps only do this if the lowering is enabled? That way this will
only be an issue for i965. i.e.

if (ctx->Const.VertexID_is_zero_based && loc == BASE_VERTEX)

I guess it's a little annoying since you don't get the context in this
function :( Perhaps there's some clever way of getting at it? If not,
then never mind =/

>              continue;
>           /* FALLTHROUGH */
>        case ir_var_shader_in:
> diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
> index 993dc86..e85e81d 100644
> --- a/src/mesa/main/shader_query.cpp
> +++ b/src/mesa/main/shader_query.cpp
> @@ -112,14 +112,7 @@ is_active_attrib(const gl_shader_variable *var)
>        return var->location != -1;
>
>     case ir_var_system_value:
> -      /* From GL 4.3 core spec, section 11.1.1 (Vertex Attributes):
> -       * "For GetActiveAttrib, all active vertex shader input variables
> -       * are enumerated, including the special built-in inputs gl_VertexID
> -       * and gl_InstanceID."
> -       */
> -      return var->location == SYSTEM_VALUE_VERTEX_ID ||
> -             var->location == SYSTEM_VALUE_VERTEX_ID_ZERO_BASE ||
> -             var->location == SYSTEM_VALUE_INSTANCE_ID;
> +      return true;
>
>     default:
>        return false;
> --
> 2.7.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev