[Mesa-dev] [PATCH v2 24/64] mesa: fix setting uniform variables for bindless samplers/images

[Nicolai Hähnle]

On 30.05.2017 22:35, Samuel Pitoiset wrote:
> This fixes a 64-bit vs 32-bit mismatch when setting an array
> of bindless samplers. Also, we need to unconditionally set
> size_mul to 2 when the underlying uniform is bindless.
> 
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
> ---
>   src/mesa/main/uniform_query.cpp | 29 +++++++++++++++++++++++------
>   1 file changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp
> index 7bd04e1935..42ca93a6f5 100644
> --- a/src/mesa/main/uniform_query.cpp
> +++ b/src/mesa/main/uniform_query.cpp
> @@ -322,12 +322,20 @@ _mesa_get_uniform(struct gl_context *ctx, GLuint program, GLint location,
>   
>      {
>         unsigned elements = uni->type->components();
> -      /* XXX: Remove the sampler/image check workarounds when bindless is fully
> -       * implemented.
> -       */
> -      const int dmul =
> -         (uni->type->is_64bit() && !uni->type->is_sampler() && !uni->type->is_image()) ? 2 : 1;
>         const int rmul = glsl_base_type_is_64bit(returnType) ? 2 : 1;
> +      int dmul = (uni->type->is_64bit()) ? 2 : 1;
> +
> +      if ((uni->type->is_sampler() || uni->type->is_image()) &&
> +          !uni->is_bindless) {
> +         /* The ARB_bindless_texture spec says:
> +          *
> +          * "When a sampler or image uniform's value is queried via any of the
> +          *  GetUniform* commands, the returned value will reflect the most
> +          *  recently set value through either UniformHandle* or Uniform1i*,
> +          *  converted to the requested type."
> +          */

This spec quote feels a bit off for explaining why dmul = 1 in this 
case. Some explanation is missing of how 32 bits are stored for 
non-bindless samplers.

Apart from that, patches 23 & 24:

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

... though it would be nice to have a test for the mixed array cases. At 
least I don't remember seeing one.


> +         dmul = 1;
> +      }
>   
>         /* Calculate the source base address *BEFORE* modifying elements to
>          * account for the size of the user's buffer.
> @@ -955,9 +963,18 @@ _mesa_uniform(GLint location, GLsizei count, const GLvoid *values,
>   
>      /* Store the data in the "actual type" backing storage for the uniform.
>       */
> -   if (!uni->type->is_boolean()) {
> +   if (!uni->type->is_boolean() && !uni->is_bindless) {
>         memcpy(&uni->storage[size_mul * components * offset], values,
>                sizeof(uni->storage[0]) * components * count * size_mul);
> +   } else if (uni->is_bindless) {
> +      const union gl_constant_value *src =
> +         (const union gl_constant_value *) values;
> +      GLuint64 *dst = (GLuint64 *)&uni->storage[components * offset].i;
> +      const unsigned elems = components * count;
> +
> +      for (unsigned i = 0; i < elems; i++) {
> +         dst[i] = src[i].i;
> +      }
>      } else {
>         const union gl_constant_value *src =
>            (const union gl_constant_value *) values;
> 


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