[Mesa-dev] [PATCH v2 04/31] glsl: make component_slots() returns 2 for samplers/images

[Timothy Arceri]

On 26/04/17 17:47, Samuel Pitoiset wrote:
> 
> 
> On 04/26/2017 03:46 AM, Timothy Arceri wrote:
>>
>>
>> On 24/04/17 20:35, Samuel Pitoiset wrote:
>>> Bindless samplers/images are 64-bit unsigned integers, which
>>> means they consume two components as specified by
>>> ARB_bindless_texture.
>>>
>>> It looks like we are not wasting uniform storage by changing
>>> this because default-block uniforms are not packed. So, if
>>> we use N uint uniforms, they occupy N * 16 bytes in the
>>> constant buffer. This is something that could be improved.
>>>
>>> Though, count_uniform_size needs to be adjusted to not count
>>> a sampler (or image) twice.
>>>
>>> As a side effect, this will probably break the cache if you
>>> have one because it will consider sampler/image types as
>>> two components.
>>>
>>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>>> ---
>>>   src/compiler/glsl/link_uniforms.cpp | 8 ++++++--
>>>   src/compiler/glsl_types.cpp         | 2 ++
>>>   2 files changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/compiler/glsl/link_uniforms.cpp 
>>> b/src/compiler/glsl/link_uniforms.cpp
>>> index b462cb9d59..3331c85af4 100644
>>> --- a/src/compiler/glsl/link_uniforms.cpp
>>> +++ b/src/compiler/glsl/link_uniforms.cpp
>>> @@ -340,9 +340,13 @@ private:
>>>         if (type->contains_subroutine()) {
>>>            this->num_shader_subroutines += values;
>>>         } else if (type->contains_sampler()) {
>>> -         this->num_shader_samplers += values;
>>> +         /* Old-style (or bound) samplers are counted as two 
>>> components as
>>> +          * specified by ARB_bindless_texture. */
>>> +         this->num_shader_samplers += values / 2;
>>>         } else if (type->contains_image()) {
>>> -         this->num_shader_images += values;
>>> +         /* Old-style (or bound) images are counted as two 
>>> components as
>>> +          * specified by ARB_bindless_texture. */
>>
>> */ should go on the following line.
>>
>> I think I would rather see 862361c4f547efb reverted and image support 
>> added. You are missing some of the uses with this change, and it just 
>> seems cleaner to keep the counting separate to begin with.
> 
> Which uses are you talking about?

Everywhere you changed values_for_type() to component_slots() in 
862361c4f547efb.

Or to we want to allocate two uniform slots for each sampler/image?

> 
>>
>>> +         this->num_shader_images += values / 2;
>>>            /* As drivers are likely to represent image uniforms as
>>>             * scalar indices, count them against the limit of uniform
>>> diff --git a/src/compiler/glsl_types.cpp b/src/compiler/glsl_types.cpp
>>> index 0480bef80e..df148cfd21 100644
>>> --- a/src/compiler/glsl_types.cpp
>>> +++ b/src/compiler/glsl_types.cpp
>>> @@ -1298,6 +1298,8 @@ glsl_type::component_slots() const
>>>      case GLSL_TYPE_SAMPLER:
>>>      case GLSL_TYPE_IMAGE:
>>> +      return 2;
>>> +
>>>      case GLSL_TYPE_SUBROUTINE:
>>>         return 1;
>>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev