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

[Samuel Pitoiset]

On 04/26/2017 11:44 AM, Timothy Arceri wrote:
> 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?

Yes, that would be simpler. And we could reduce the extra uniform 
storage for bound samplers later on.

> 
>>
>>>
>>>> +         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