[Mesa-dev] [PATCH] gallium/docs: clarify set_sampler_views

[Roland Scheidegger]

Am 12.03.19 um 22:48 schrieb Rob Clark:
> On Tue, Mar 12, 2019 at 1:59 PM Roland Scheidegger <sroland at vmware.com> wrote:
>>
>> Am 12.03.19 um 16:16 schrieb Rob Clark:
>>> This previously was not called out clearly, but based on a survey of the
>>> code, it seems the expected behavior is to release the reference to any
>>> sampler views beyond the new range being bound.
>>
>> That isn't really true. This was designed to work like d3d10, where
>> other views are unmodified.
>> The cso code will actually unset all views which previously were set and
>> are above the num_views in the call (this wouldn't be necessary if the
>> pipe function itself would work like this).
>> However, it will only do this for fragment textures, and pass through
>> the parameters unmodified otherwise. Which means behavior might not be
>> very consistent for the different stages...
> 
> hmm, I did notice w/ deqp tests (which aren't so good at
> resetting/clearing state between tests), that I ended up w/ different
> # of sampler views bound (without changing freedreno to match the
> behavior of most of the other drivers).. I didn't really dig in that
> closely but it seemed like mesa/st wasn't clearing the additional
> previously bound textures.  Maybe I overlooked something, but that
> seemed wrong.
> 
> One way or another, I guess we should clarify and change the various
> drivers to have a common behavior because right now there two
> different behaviors and I guess it is at least confusing for new
> gallium driver writers (as it was for me and I've been at it for a
> while)

Yes, I agree with that, the current state there doesn't help anyone.

Roland


> BR,
> -R
> 
>>
>>
>>>
>>> I think radeonsi and freedreno were the only ones not doing this.  Which
>>> could probably temporarily leak a bit of memory by holding on to the
>>> sampler view reference.
>> Not sure about other drivers, but llvmpipe will not do this neither.
>>
>> Roland
>>
>>
>>>
>>> Signed-off-by: Rob Clark <robdclark at gmail.com>
>>> ---
>>>  src/gallium/docs/source/context.rst | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/src/gallium/docs/source/context.rst b/src/gallium/docs/source/context.rst
>>> index f89d9e1005e..199d335f8f4 100644
>>> --- a/src/gallium/docs/source/context.rst
>>> +++ b/src/gallium/docs/source/context.rst
>>> @@ -143,6 +143,9 @@ to the array index which is used for sampling.
>>>    to a respective sampler view and releases a reference to the previous
>>>    sampler view.
>>>
>>> +  Previously bound samplers with index ``>= num_views`` are unbound rather
>>> +  than unmodified.
>>> +
>>>  * ``create_sampler_view`` creates a new sampler view. ``texture`` is associated
>>>    with the sampler view which results in sampler view holding a reference
>>>    to the texture. Format specified in template must be compatible
>>>
>>