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

[Samuel Pitoiset]

On 05/05/2017 10:29 AM, Timothy Arceri wrote:
> On 05/05/17 17:37, Samuel Pitoiset wrote:
>> On 05/05/2017 02:18 AM, Timothy Arceri wrote:
>>> What happened with my suggestion [1]? Was there a problem?
>>>
>>> IMO this patch just hacks around the problem and as mentioned 
>>> previously will result in extra unused uniform storage being allocated.
>>
>> Can we just move on and fix the extra unused uniform storage later?
> 
> Why the rush? If I'm understanding this correctly it should be a 
> straight forward change.

Okay, if I understand you correctly you want to re-introduce 
values_for_type(), put 2 for samplers/images and remove this patch from 
the series? That should work.

> 
>>>
>>> [1] 
>>> https://lists.freedesktop.org/archives/mesa-dev/2017-April/153156.html
>>>
>>> On 03/05/17 06:53, 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.
>>>>
>>>> v3: - update the comments
>>>>
>>>> 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 f1e0885fbd..0a199dbbd6 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;
>>>> +         /* Samplers (bound or bindless) 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;
>>>> +         /* Images (bound or bindless) are counted as two 
>>>> components as
>>>> +          * specified by ARB_bindless_texture. */
>>>> +         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 bf078ad614..89f611d0cb 100644
>>>> --- a/src/compiler/glsl_types.cpp
>>>> +++ b/src/compiler/glsl_types.cpp
>>>> @@ -1293,6 +1293,8 @@ glsl_type::component_slots() const
>>>>      case GLSL_TYPE_SAMPLER:
>>>>      case GLSL_TYPE_IMAGE:
>>>> +      return 2;
>>>> +
>>>>      case GLSL_TYPE_SUBROUTINE:
>>>>         return 1;
>>>>