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

[Samuel Pitoiset]

On 05/24/2017 12:53 PM, Nicolai Hähnle wrote:
> 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;
> 
> changed also needs to be to true when sampler->bound was previously false.

Good catch.

> 
> As a side note, I'm a bit surprised at how eagerly this code calls 
> _mesa_update_shader_textures_used, which is a pretty heavy loop. We 
> should be able to eliminate that and only update before a draw call, 
> since it's quite common to switch multiple textures between draws.
> 
> Also, the update_bound_bindless_{sampler,image}_flag could be done 
> lazily once before the draw, since at least for samplers we have various 
> update loops already anyway. But these optimizations could be done 
> separately.

Right, these improvements can be done later on.

> 
> Cheers,
> Nicolai
> 
> 
>> +               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
>>
> 
>