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

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

Any opinion about whether views==NULL should be allowed?  Currently I
have something like:

----
diff --git a/src/gallium/docs/source/context.rst
b/src/gallium/docs/source/context.rst
index f89d9e1005e..06d30bfb38b 100644
--- a/src/gallium/docs/source/context.rst
+++ b/src/gallium/docs/source/context.rst
@@ -143,6 +143,11 @@ to the array index which is used for sampling.
   to a respective sampler view and releases a reference to the previous
   sampler view.

+  Sampler views outside of ``[start_slot, start_slot + num_views)`` are
+  unmodified.  If ``views`` is NULL, the behavior is the same as if
+  ``views[n]`` was NULL for the entire range, ie. releasing the reference
+  for all the sampler views in the specified range.
+
 * ``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
----

But going thru the other drivers, a lot of them also don't handle the
views==NULL case.  This case doesn't seem to come up with mesa/st, but
does with XA and nine, and some of the test code.

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