Am 14.03.19 um 14:13 schrieb Rob Clark: > On Tue, Mar 12, 2019 at 1:59 PM Roland Scheidegger <srol...@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.
I think this should be illegal. As you've noted some drivers can't handle it, and I don't see a particularly good reason to allow it. Well I guess it trades some complexity in state trackers with some complexity in drivers... 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 <robdcl...@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 >>> >> _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
