Bug 96410

Currently looking at the code of single program flow namely _mesa_update_shader_textures_used

The current check fails to detect wrongly reused sampler in multiple shader stage. The spec seems to imply that is must be checked in the full program (all stages).

<<
     Errors
  It is not allowed to have variables of different sampler types pointing to
  the same texture image unit within a program object. This situation can only
  be detected at the next rendering command issued which triggers shader invo-
  cations, and an INVALID_OPERATION error will then be generated.
>>

Here a typical example of an invalid program that will wrongly run fine.

***** Vertex Shader
layout(binding = 0) uniform sampler1D sampler_1d;

***** Fragment Shader
layout(binding = 0) uniform sampler2D sampler_2d;

(In reply to gregory.hainaut from comment #1)
> Currently looking at the code of single program flow namely
> _mesa_update_shader_textures_used
> 
> The current check fails to detect wrongly reused sampler in multiple shader
> stage. The spec seems to imply that is must be checked in the full program
> (all stages).
> 
> <<
>      Errors
>   It is not allowed to have variables of different sampler types pointing to
>   the same texture image unit within a program object. This situation can
> only
>   be detected at the next rendering command issued which triggers shader
> invo-
>   cations, and an INVALID_OPERATION error will then be generated.
> >>
> 
> Here a typical example of an invalid program that will wrongly run fine.
> 
> ***** Vertex Shader
> layout(binding = 0) uniform sampler1D sampler_1d;
> 
> ***** Fragment Shader
> layout(binding = 0) uniform sampler2D sampler_2d;

See the FIXME I added when I mostly fixed this a while ago:
https://cgit.freedesktop.org/mesa/mesa/tree/src/mesa/main/uniform_query.cpp#n1095

The problem is the spec requires samplers to default to 0. So unless we manage to eliminate everything that is unused (including early array elements) we will likely trigger errors in programs that run fine elsewhere (since last time I checked Nvidia doesn't do this validation at all)

I confirm that Nvidia doesn't check anything. And we don't even check it for mono program. I don't know if it was intended or a bug.


Potentially we could do something like the code below (I guess I need to add back the unit 0 case). Code is faster because it only check sampler rather than all uniforms.

An alternative will be to check only SamplersValidated for all stages. It will be equivalent to mono program.

I guess the ideal solution would be to support  GL_KHR_no_error to disable all those crazy validations.

--------------------------8<--------------------------
   for (unsigned idx = 0; idx < ARRAY_SIZE(pipeline->CurrentProgram); idx++) {
      if (!shProg[idx])
         continue;

      shader = shProg[idx]->_LinkedShaders[idx];
      if (!shader || !shader->Program)
         continue;

      mask = shader->Program->SamplersUsed;
      while (mask) {
         const int s = u_bit_scan(&mask);
         GLuint unit = shader->SamplerUnits[s];
         GLuint tgt = shader->SamplerTargets[s];

         if (TexturesUsed[unit] & ~(1 << tgt)) {
            pipeline->InfoLog =
               ralloc_asprintf(pipeline,
                     "Program %d: "
                     "Texture unit %d is accessed with 2 different types",
                     shProg[idx]->Name, unit);
            return false;
         }

         TexturesUsed[unit] |= (1 << tgt);
      }

      active_samplers += shader->num_samplers;
   }
--------------------------->8-------------------------

Fixed by commit	6a524c76f502fe15bb3612065a23ece693aed237