[Mesa-dev] [RFC PATCH 22/65] mesa: handle bindless uniforms bound to texture/image units

[Nicolai Hähnle]

On 25.05.2017 12:51, Samuel Pitoiset wrote:
> On 05/24/2017 01:01 PM, Nicolai Hähnle wrote:
>> Something else just occurred to me. Do you have a test case where an 
>> array of bindless sampler uniforms is set at once with glUniform1iv?
> 
> https://hastebin.com/jiduzevage
> 
> Works fine.

I really think this is pure chance (and I think I see why it works in 
the code), but in part it's because you didn't test rendering with a 
uniform array of samplers.

On the second thought, the whole uni->is_bindless_handle design can't 
possibly work. Consider an array

   layout(bindless_texture) uniform sampler2D tex[2];

which is bound with

   glUniform1i(loc, tex_unit);
   glUniformHandleui64ARB(loc + 1, handle);

We don't need uni->is_bindless_handle; what we do need is that each of 
_mesa_uniform, _mesa_uniform_handle, and _mesa_get_uniform use size_mul 
== 2 unconditionally when the uniform is bindless.

Cheers,
Nicolai


>> That should have a 32- vs. 64-bit mismatch when the user-supplied 
>> array is directly memcpy()'d to uni->storage. I think you need to fix 
>> that mismatch. On the plus side, that probably makes patch #23 
>> unnecessary.
> 
> I don't see where a mismatch could happen actually.
> 
>>
>> Cheers,
>> Nicolai
>>
>>
>> On 19.05.2017 18:52, Samuel Pitoiset wrote:
>>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>>> ---
>>>   src/mesa/main/uniform_query.cpp | 122 
>>> ++++++++++++++++++++++++++++++++++++++--
>>>   1 file changed, 116 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/src/mesa/main/uniform_query.cpp 
>>> b/src/mesa/main/uniform_query.cpp
>>> index cc145f29e9..be04e48d53 100644
>>> --- a/src/mesa/main/uniform_query.cpp
>>> +++ b/src/mesa/main/uniform_query.cpp
>>> @@ -993,9 +993,25 @@ _mesa_uniform(GLint location, GLsizei count, 
>>> const GLvoid *values,
>>>            bool changed = false;
>>>            for (int j = 0; j < count; j++) {
>>>               unsigned unit = uni->opaque[i].index + offset + j;
>>> -            if (sh->Program->SamplerUnits[unit] != ((unsigned *) 
>>> values)[j]) {
>>> -               sh->Program->SamplerUnits[unit] = ((unsigned *) 
>>> values)[j];
>>> -               changed = true;
>>> +            unsigned value = ((unsigned *)values)[j];
>>> +
>>> +            if (uni->is_bindless) {
>>> +               struct gl_bindless_sampler *sampler =
>>> +                  &sh->Program->sh.BindlessSamplers[unit];
>>> +
>>> +               /* Mark this bindless sampler as bound to a texture 
>>> unit.
>>> +                */
>>> +               if (sampler->unit != value) {
>>> +                  sampler->unit = value;
>>> +                  changed = true;
>>> +               }
>>> +               sampler->bound = true;
>>> +               sh->Program->sh.HasBoundBindlessSampler = true;
>>> +            } else {
>>> +               if (sh->Program->SamplerUnits[unit] != value) {
>>> +                  sh->Program->SamplerUnits[unit] = value;
>>> +                  changed = true;
>>> +               }
>>>               }
>>>            }
>>> @@ -1024,9 +1040,23 @@ _mesa_uniform(GLint location, GLsizei count, 
>>> const GLvoid *values,
>>>            if (!uni->opaque[i].active)
>>>               continue;
>>> -         for (int j = 0; j < count; j++)
>>> -            sh->Program->sh.ImageUnits[uni->opaque[i].index + offset 
>>> + j] =
>>> -               ((GLint *) values)[j];
>>> +         for (int j = 0; j < count; j++) {
>>> +            unsigned unit = uni->opaque[i].index + offset + j;
>>> +            unsigned value = ((unsigned *)values)[j];
>>> +
>>> +            if (uni->is_bindless) {
>>> +               struct gl_bindless_image *image =
>>> +                  &sh->Program->sh.BindlessImages[unit];
>>> +
>>> +               /* Mark this bindless image as bound to an image unit.
>>> +                */
>>> +               image->unit = value;
>>> +               image->bound = true;
>>> +               sh->Program->sh.HasBoundBindlessImage = true;
>>> +            } else {
>>> +               sh->Program->sh.ImageUnits[unit] = value;
>>> +            }
>>> +         }
>>>         }
>>>         ctx->NewDriverState |= ctx->DriverFlags.NewImageUnits;
>>> @@ -1173,6 +1203,40 @@ _mesa_uniform_matrix(GLint location, GLsizei 
>>> count,
>>>      _mesa_propagate_uniforms_to_driver_storage(uni, offset, count);
>>>   }
>>> +static void
>>> +update_bound_bindless_sampler_flag(struct gl_program *prog)
>>> +{
>>> +   unsigned i;
>>> +
>>> +   if (likely(!prog->sh.HasBoundBindlessSampler))
>>> +      return;
>>> +
>>> +   for (i = 0; i < prog->sh.NumBindlessSamplers; i++) {
>>> +      struct gl_bindless_sampler *sampler = 
>>> &prog->sh.BindlessSamplers[i];
>>> +
>>> +      if (sampler->bound)
>>> +         return;
>>> +   }
>>> +   prog->sh.HasBoundBindlessSampler = false;
>>> +}
>>> +
>>> +static void
>>> +update_bound_bindless_image_flag(struct gl_program *prog)
>>> +{
>>> +   unsigned i;
>>> +
>>> +   if (likely(!prog->sh.HasBoundBindlessImage))
>>> +      return;
>>> +
>>> +   for (i = 0; i < prog->sh.NumBindlessImages; i++) {
>>> +      struct gl_bindless_image *image = &prog->sh.BindlessImages[i];
>>> +
>>> +      if (image->bound)
>>> +         return;
>>> +   }
>>> +   prog->sh.HasBoundBindlessImage = false;
>>> +}
>>> +
>>>   /**
>>>    * Called via glUniformHandleui64*ARB() functions.
>>>    */
>>> @@ -1236,6 +1300,52 @@ _mesa_uniform_handle(GLint location, GLsizei 
>>> count, const GLvoid *values,
>>>             sizeof(uni->storage[0]) * components * count * size_mul);
>>>      _mesa_propagate_uniforms_to_driver_storage(uni, offset, count);
>>> +
>>> +   if (uni->type->is_sampler()) {
>>> +      /* Mark this bindless sampler as not bound to a texture unit 
>>> because
>>> +       * it refers to a texture handle.
>>> +       */
>>> +      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;
>>> +
>>> +         for (int j = 0; j < count; j++) {
>>> +            unsigned unit = uni->opaque[i].index + offset + j;
>>> +            struct gl_bindless_sampler *sampler =
>>> +               &sh->Program->sh.BindlessSamplers[unit];
>>> +
>>> +            sampler->bound = false;
>>> +         }
>>> +
>>> +         update_bound_bindless_sampler_flag(sh->Program);
>>> +      }
>>> +   }
>>> +
>>> +   if (uni->type->is_image()) {
>>> +      /* Mark this bindless image as not bound to an image unit 
>>> because it
>>> +       * refers to a texture handle.
>>> +       */
>>> +      for (int i = 0; i < MESA_SHADER_STAGES; i++) {
>>> +         struct gl_linked_shader *sh = shProg->_LinkedShaders[i];
>>> +
>>> +         /* If the shader stage doesn't use the sampler uniform, 
>>> skip this. */
>>> +         if (!uni->opaque[i].active)
>>> +            continue;
>>> +
>>> +         for (int j = 0; j < count; j++) {
>>> +            unsigned unit = uni->opaque[i].index + offset + j;
>>> +            struct gl_bindless_image *image =
>>> +               &sh->Program->sh.BindlessImages[unit];
>>> +
>>> +            image->bound = false;
>>> +         }
>>> +
>>> +         update_bound_bindless_image_flag(sh->Program);
>>> +      }
>>> +   }
>>>   }
>>>   extern "C" bool
>>>
>>
>>


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