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

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


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

-Brian