[Mesa-dev] [PATCH V2] mesa: validate sampler type across the whole program

[Nicolai Hähnle]

On 21.04.2017 09:08, Timothy Arceri wrote:
> Currently we were only making sure types were the same within a
> single stage. This looks to have regressed with 953a0af8e3f73.
>
> V2: move the SamplersValidated reset into the common linker
>     code.
>
> https://bugs.freedesktop.org/show_bug.cgi?id=97524
> Cc: Tapani Pälli <tapani.palli at intel.com>
> ---
>  src/mesa/main/uniform_query.cpp |  2 ++
>  src/mesa/main/uniforms.c        | 26 +++++++++++++++++++++-----
>  src/mesa/program/ir_to_mesa.cpp |  5 +++++
>  3 files changed, 28 insertions(+), 5 deletions(-)
>
> diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp
> index c648832..e400d0e 100644
> --- a/src/mesa/main/uniform_query.cpp
> +++ b/src/mesa/main/uniform_query.cpp
> @@ -965,20 +965,22 @@ _mesa_uniform(GLint location, GLsizei count, const GLvoid *values,
>        }
>     }
>
>     _mesa_propagate_uniforms_to_driver_storage(uni, offset, count);
>
>     /* If the uniform is a sampler, do the extra magic necessary to propagate
>      * the changes through.
>      */
>     if (uni->type->is_sampler()) {
>        bool flushed = false;
> +      shProg->SamplersValidated = GL_TRUE;
> +
>        for (int i = 0; i < MESA_SHADER_STAGES; i++) {
>  	 struct gl_linked_shader *const sh = shProg->_LinkedShaders[i];
>
>  	 /* If the shader stage doesn't use the sampler uniform, skip this. */
>  	 if (!uni->opaque[i].active)
>  	    continue;
>
>           bool changed = false;
>           for (int j = 0; j < count; j++) {
>              unsigned unit = uni->opaque[i].index + offset + j;
> diff --git a/src/mesa/main/uniforms.c b/src/mesa/main/uniforms.c
> index 59ae4c5..8869b6e 100644
> --- a/src/mesa/main/uniforms.c
> +++ b/src/mesa/main/uniforms.c
> @@ -60,42 +60,58 @@
>   * So, scan the program->SamplerUnits[] and program->SamplerTargets[]
>   * information to update the prog->TexturesUsed[] values.
>   * Each value of TexturesUsed[unit] is one of zero, TEXTURE_1D_INDEX,
>   * TEXTURE_2D_INDEX, TEXTURE_3D_INDEX, etc.
>   * We'll use that info for state validation before rendering.
>   */
>  void
>  _mesa_update_shader_textures_used(struct gl_shader_program *shProg,
>                                    struct gl_program *prog)
>  {
> -   memset(prog->TexturesUsed, 0, sizeof(prog->TexturesUsed));
> +   GLbitfield mask = prog->SamplersUsed;
> +   gl_shader_stage prog_stage =
> +      _mesa_program_enum_to_shader_stage(prog->Target);
> +   struct gl_linked_shader *shader = shProg->_LinkedShaders[prog_stage];
>
> -   shProg->SamplersValidated = GL_TRUE;
> +   assert(shader);
> +
> +   memset(prog->TexturesUsed, 0, sizeof(prog->TexturesUsed));
>
> -   GLbitfield mask = prog->SamplersUsed;
>     while (mask) {
>        const int s = u_bit_scan(&mask);
>        GLuint unit = prog->SamplerUnits[s];
>        GLuint tgt = prog->sh.SamplerTargets[s];
>        assert(unit < ARRAY_SIZE(prog->TexturesUsed));
>        assert(tgt < NUM_TEXTURE_TARGETS);
>
>        /* The types of the samplers associated with a particular texture
>         * unit must be an exact match.  Page 74 (page 89 of the PDF) of the
>         * OpenGL 3.3 core spec says:
>         *
>         *     "It is not allowed to have variables of different sampler
>         *     types pointing to the same texture image unit within a program
>         *     object."
>         */
> -      if (prog->TexturesUsed[unit] & ~(1 << tgt))
> -         shProg->SamplersValidated = GL_FALSE;
> +      unsigned stages_mask = shProg->data->linked_stages;
> +      while (stages_mask) {
> +         const int stage = u_bit_scan(&stages_mask);
> +
> +         /* Skip validation if we are yet to update textures used in this
> +          * stage.
> +          */
> +         if (prog_stage < stage)
> +            break;
> +
> +         struct gl_program *glprog = shProg->_LinkedShaders[stage]->Program;
> +         if (glprog->TexturesUsed[unit] & ~(1 << tgt))
> +            shProg->SamplersValidated = GL_FALSE;
> +      }
>
>        prog->TexturesUsed[unit] |= (1 << tgt);
>     }
>  }
>
>  /**
>   * Connect a piece of driver storage with a part of a uniform
>   *
>   * \param uni            The uniform with which the storage will be associated
>   * \param element_stride Byte-stride between array elements.
> diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp
> index 6b33266..27507e8 100644
> --- a/src/mesa/program/ir_to_mesa.cpp
> +++ b/src/mesa/program/ir_to_mesa.cpp
> @@ -3110,20 +3110,25 @@ _mesa_glsl_link_shader(struct gl_context *ctx, struct gl_shader_program *prog)
>        if (!prog->Shaders[i]->CompileStatus) {
>  	 linker_error(prog, "linking with uncompiled shader");
>        }
>     }
>
>     if (prog->data->LinkStatus) {
>        link_shaders(ctx, prog);
>     }
>
>     if (prog->data->LinkStatus) {
> +      /* Reset sampler validated to true, validation happends via the

Typo: happends

With the update you mentioned, this is

Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>

> +       * LinkShader call below.
> +       */
> +      shProg->SamplersValidated = GL_TRUE;
> +
>        if (!ctx->Driver.LinkShader(ctx, prog)) {
>           prog->data->LinkStatus = linking_failure;
>        }
>     }
>
>     /* Return early if we are loading the shader from on-disk cache */
>     if (prog->data->LinkStatus == linking_skipped)
>        return;
>
>     if (ctx->_Shader->Flags & GLSL_DUMP) {
>


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.