[Mesa-dev] [PATCH 2/6] gallium: implement ARB_seamless_cube_map

[Roland Scheidegger]

Am 02.05.2011 19:13, schrieb Ian Romanick:
> On 05/02/2011 09:19 AM, Roland Scheidegger wrote:
>> Am 02.05.2011 16:40, schrieb Marek Olaák:
>>> On Mon, May 2, 2011 at 3:47 PM, Brian Paul <brianp at vmware.com> wrote:
>>>> On 05/02/2011 07:03 AM, Marek Olaák wrote:
>>>>>
>>>>> ---
>>>>>  src/gallium/include/pipe/p_defines.h        |    1 +
>>>>>  src/gallium/include/pipe/p_state.h          |    1 +
>>>>>  src/mesa/state_tracker/st_atom_rasterizer.c |    6 +++++-
>>>>>  src/mesa/state_tracker/st_extensions.c      |    4 ++++
>>>>>  4 files changed, 11 insertions(+), 1 deletions(-)
>>>>>
>>>>> diff --git a/src/gallium/include/pipe/p_defines.h
>>>>> b/src/gallium/include/pipe/p_defines.h
>>>>> index 431a7fb..176fee9 100644
>>>>> --- a/src/gallium/include/pipe/p_defines.h
>>>>> +++ b/src/gallium/include/pipe/p_defines.h
>>>>> @@ -465,6 +465,7 @@ enum pipe_cap {
>>>>>     PIPE_CAP_VERTEX_ELEMENT_INSTANCE_DIVISOR = 44,
>>>>>     PIPE_CAP_FRAGMENT_COLOR_CLAMP_CONTROL = 45,
>>>>>     PIPE_CAP_MIXED_COLORBUFFER_FORMATS = 46,
>>>>> +   PIPE_CAP_SEAMLESS_CUBE_MAP = 47
>>>>>  };
>>>>>
>>>>>  /* Shader caps not specific to any single stage */
>>>>> diff --git a/src/gallium/include/pipe/p_state.h
>>>>> b/src/gallium/include/pipe/p_state.h
>>>>> index 0c1f509..26e8a8e 100644
>>>>> --- a/src/gallium/include/pipe/p_state.h
>>>>> +++ b/src/gallium/include/pipe/p_state.h
>>>>> @@ -101,6 +101,7 @@ struct pipe_rasterizer_state
>>>>>     unsigned line_smooth:1;
>>>>>     unsigned line_stipple_enable:1;
>>>>>     unsigned line_last_pixel:1;
>>>>> +   unsigned seamless_cube_map:1;
>>>>
>>>> Shouldn't this be sampler state and not rasterizer state?  The AMD extension
>>>> lets this option be set per texture.  Though, I don't know if non-AMD
>>>> hardware supports that mode.
>>>
>>> r600 and r700 hardware has one enable bit that affects the entire
>>> chip. There is no per-sampler enable bit.
>>>
>>> evergreen and later hardware indeed has a per-sampler enable bit.
>>>
>>> I wouldn't like to limit the extension to evergreen, so
>>> pipe_rasterizer_state seemed to be the only choice.
> 
>> You can still do that if it's per-sampler (state tracker ensures it's
>> the same in all sampler state), though it will make things more
>> complicated for r600 and you'll need two caps (for implementing both
>> extensions).
> 
> That seems like the right way to do it.  That's basically how I had
> planned to do it in the i965 driver.  It's a bit tricky, though.  I
> believe it's possible for applications to use both modes.  You have to
> make sure that you get the right state if an app:
> 
>  - turns on the global enable
>  - turns on the per-texture enable for one texture
>  - draw
>  - turn off the global enable
>  - draw

Yes, according to the spec this is possible - seemless filtering for a
sampler is enabled if either the global enable or the per-sampler enable
(or both, of course) is enabled.
The spec even allows for implementations only supporting the per-sampler
enable - though I'm not sure why AMD bothered specifying that instead of
just making the global one mandatory, since obviously all hw which can
support per-sampler enable also can support that globally, so an
implementation only supporting the per-sampler version doesn't really
make a whole lot of sense (especially not since the global one is an ARB
extension).

> 
>> I'm not sure how useful that AMD extension is, but since that extension
>> was done after the ARB one (and it was implemented that way in hw) I
>> guess that there is indeed some use for it. I believe intel chips could
>> also do it per-sampler, not sure about nvidia. Of course, for d3d10 it
>> isn't needed, since it's always enabled (not sure about d3d11 but I
>> guess it's the same) globally anyway, and I don't know if we plan to
>> support the AMD extension.
> 
> Adding the AMD extension is somewhere way, way, way down on my todo list...

Roland