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

[Timothy Arceri]

On 05/05/17 18:41, Samuel Pitoiset wrote:
> 
> 
> 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.

As discussed offline. I was misunderstanding the how we handled doubles. 
I'm fine with this patch.

Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com>

> 
>>
>>>>
>>>> [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;
>>>>>