[Mesa-dev] gallium-bind-sampler-states branch

[Roland Scheidegger]

Am 16.09.2013 18:18, schrieb Brian Paul:
> On 09/16/2013 08:56 AM, Roland Scheidegger wrote:
>> Am 14.09.2013 18:24, schrieb Brian Paul:
>>> On 09/12/2013 09:06 PM, Chia-I Wu wrote:
>>>> Hi Brian,
>>>>
>>>> On Fri, Sep 13, 2013 at 8:46 AM, Brian Paul <brianp at vmware.com> wrote:
>>>>>
>>>>> I just pushed a gallium-bind-sampler-states branch to my git repo at
>>>>> git://people.freedesktop.org/~brianp/mesa
>>>>>
>>>>> It replaces the four
>>>>> pipe_context::bind_fragment/vertex/geometry/compute_sampler_states()
>>>>> functions with a single bind_sampler_states() function:
>>>>>
>>>>>    void (*bind_sampler_states)(struct pipe_context *,
>>>>>                                unsigned shader, unsigned start_slot,
>>>>>                                unsigned num_samplers, void
>>>>> **samplers);
>>>>>
>>>>> At this point start_slot is always zero (at least for non-compute
>>>>> shaders).
>>>>> And as the updated gallium docs explain, at some point calls to
>>>>> bind_sampler_states() will be used to updated sub-ranges, but that
>>>>> never
>>>>> happens currently.
>>>>>
>>>>> I've updated all the drivers, state trackers, utils, etc.
>>>>>
>>>>> I've tested the svga, llvmpipe and softpipe drivers.  'make check'
>>>>> and a
>>>>> texture subset of piglit pass w/out regressions.  I'd appreciate it
>>>>> if other
>>>>> driver developers would test their favorite driver.
>>>> For ilo, the new code does not follow the doc and unbinds samplers not
>>>> in range.
>>>
>>> I think that's OK.  The CSO module (used by the state tracker) currently
>>> always calls pipe_context::bind_sampler_states() with start=0 and count
>>> such that it sets/replaces all samplers, never a sub-range.  That
>>> could/should change in the future.
>>>
>>> See single_sampler_done() in cso_context.c.
>>
>> Not all state trackers use cso though.
> 
> AFAICT, all src/gallium/state_trackers/ use CSO, except for clover (and
> I did miss a function call change there).  I didn't look at any
> out-of-tree state trackers.
Yes, I was thinking about out-of-tree state trackers.

> 
> 
>> While this can be fixed later, I
>> think I'd prefer if the docs would state this isn't really meant to be
>> that way and should be changed in the future in cso and certainly fixed
>> in the drivers.
> 
> As I mentioned earlier, I updated the docs to explain how things current
> work and how it should work in the future:
> 
> """
> * :ref:`Sampler`: Texture sampler states are bound separately for fragment,
>   vertex, geometry and compute shaders with the ``bind_sampler_states``
>   function.  The ``start`` and ``num_samplers`` parameters indicate a range
>   of samplers to change.  NOTE: at this time, start is always zero and
>   the CSO module will always replace all samplers at once (no sub-ranges).
>   This may change in the future.
> """
> 
> Is that what you mean?

Yes, I don't particularly like the wording - it is not obvious that it
is really a bug if a driver can't cope with start != 0, and the "start
is always zero" etc. is just a guarantee from cso (and some other state
trackers) for now but not required already from the interface. No biggie
though.

Roland