[Mesa-dev] [PATCH 7/8] gallium: create TGSI_PROPERTY to disable viewport and clipping

Roland Scheidegger sroland at vmware.com
Wed May 21 04:11:55 PDT 2014


Yes but I'm saying you need to translate it away in the state tracker, 
cause you can't really require all drivers to implement it, because this 
is a feature where I really don't think you can say it's the drivers 
fault if it's not implemented.

Roland

On 05/21/2014 11:54 AM, Marek Olšák wrote:
> We don't translate it away. The property does 2 things not available
> in the pipe interface:
> - It disables viewport clipping, so that window coordinates can pass
> through untouched.
> - It disables viewport transformation.
>
> Our hardware is capable of both, however, some APIs are not.
>
> Marek
>
> On Wed, May 21, 2014 at 12:42 PM, Roland Scheidegger <sroland at vmware.com> wrote:
>> On 05/21/2014 12:49 AM, Marek Olšák wrote:
>>>
>>> Hi,
>>>
>>> Could somebody from VMWare please review this patch? It's for st/nine
>>> (open d3d9 state tracker).
>>>
>>> Thanks,
>>>
>>> Marek
>>>
>>> On Sat, May 17, 2014 at 1:20 AM, Automated rebase
>>> <david.heidelberger at ixit.cz> wrote:
>>>>
>>>> From: Christoph Bumiller <e0425955 at student.tuwien.ac.at>
>>>>
>>>> ---
>>>>    src/gallium/auxiliary/tgsi/tgsi_strings.c  |  3 ++-
>>>>    src/gallium/auxiliary/tgsi/tgsi_ureg.c     | 16 ++++++++++++++++
>>>>    src/gallium/auxiliary/tgsi/tgsi_ureg.h     |  4 ++++
>>>>    src/gallium/docs/source/tgsi.rst           |  9 +++++++++
>>>>    src/gallium/include/pipe/p_shader_tokens.h |  3 ++-
>>>>    5 files changed, 33 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_strings.c
>>>> b/src/gallium/auxiliary/tgsi/tgsi_strings.c
>>>> index 5b6e47f..c3e7118 100644
>>>> --- a/src/gallium/auxiliary/tgsi/tgsi_strings.c
>>>> +++ b/src/gallium/auxiliary/tgsi/tgsi_strings.c
>>>> @@ -120,7 +120,8 @@ const char *tgsi_property_names[TGSI_PROPERTY_COUNT]
>>>> =
>>>>       "FS_COORD_PIXEL_CENTER",
>>>>       "FS_COLOR0_WRITES_ALL_CBUFS",
>>>>       "FS_DEPTH_LAYOUT",
>>>> -   "VS_PROHIBIT_UCPS"
>>>> +   "VS_PROHIBIT_UCPS",
>>>> +   "VS_POSITION_WINDOW_SPACE"
>>>>    };
>>>>
>>>>    const char *tgsi_type_names[5] =
>>>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_ureg.c
>>>> b/src/gallium/auxiliary/tgsi/tgsi_ureg.c
>>>> index 2bf93ee..bd0a3f7 100644
>>>> --- a/src/gallium/auxiliary/tgsi/tgsi_ureg.c
>>>> +++ b/src/gallium/auxiliary/tgsi/tgsi_ureg.c
>>>> @@ -173,6 +173,7 @@ struct ureg_program
>>>>       unsigned char property_fs_coord_pixel_center; /* =
>>>> TGSI_FS_COORD_PIXEL_CENTER_* */
>>>>       unsigned char property_fs_color0_writes_all_cbufs; /* =
>>>> TGSI_FS_COLOR0_WRITES_ALL_CBUFS * */
>>>>       unsigned char property_fs_depth_layout; /* TGSI_FS_DEPTH_LAYOUT */
>>>> +   boolean property_vs_window_space_position; /*
>>>> TGSI_VS_WINDOW_SPACE_POSITION */
>>>>
>>>>       unsigned nr_addrs;
>>>>       unsigned nr_preds;
>>>> @@ -331,6 +332,13 @@ ureg_property_fs_depth_layout(struct ureg_program
>>>> *ureg,
>>>>       ureg->property_fs_depth_layout = fs_depth_layout;
>>>>    }
>>>>
>>>> +void
>>>> +ureg_property_vs_window_space_position(struct ureg_program *ureg,
>>>> +                                       boolean vs_window_space_position)
>>>> +{
>>>> +   ureg->property_vs_window_space_position = vs_window_space_position;
>>>> +}
>>>> +
>>>>    struct ureg_src
>>>>    ureg_DECL_fs_input_cyl_centroid(struct ureg_program *ureg,
>>>>                           unsigned semantic_name,
>>>> @@ -1508,6 +1516,14 @@ static void emit_decls( struct ureg_program *ureg
>>>> )
>>>>                        ureg->property_fs_depth_layout);
>>>>       }
>>>>
>>>> +   if (ureg->property_vs_window_space_position) {
>>>> +      assert(ureg->processor == TGSI_PROCESSOR_VERTEX);
>>>> +
>>>> +      emit_property(ureg,
>>>> +                    TGSI_PROPERTY_VS_WINDOW_SPACE_POSITION,
>>>> +                    ureg->property_vs_window_space_position);
>>>> +   }
>>>> +
>>>>       if (ureg->processor == TGSI_PROCESSOR_VERTEX) {
>>>>          for (i = 0; i < UREG_MAX_INPUT; i++) {
>>>>             if (ureg->vs_inputs[i/32] & (1 << (i%32))) {
>>>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_ureg.h
>>>> b/src/gallium/auxiliary/tgsi/tgsi_ureg.h
>>>> index a0a50b7..28edea6 100644
>>>> --- a/src/gallium/auxiliary/tgsi/tgsi_ureg.h
>>>> +++ b/src/gallium/auxiliary/tgsi/tgsi_ureg.h
>>>> @@ -184,6 +184,10 @@ void
>>>>    ureg_property_fs_depth_layout(struct ureg_program *ureg,
>>>>                                  unsigned fs_depth_layout);
>>>>
>>>> +void
>>>> +ureg_property_vs_window_space_position(struct ureg_program *ureg,
>>>> +                                       boolean
>>>> vs_window_space_position);
>>>> +
>>>>
>>>>
>>>> /***********************************************************************
>>>>     * Build shader declarations:
>>>> diff --git a/src/gallium/docs/source/tgsi.rst
>>>> b/src/gallium/docs/source/tgsi.rst
>>>> index 9500b9d..2ca3c3b 100644
>>>> --- a/src/gallium/docs/source/tgsi.rst
>>>> +++ b/src/gallium/docs/source/tgsi.rst
>>>> @@ -2848,6 +2848,15 @@ input primitive. Each invocation will have a
>>>> different
>>>>    TGSI_SEMANTIC_INVOCATIONID system value set. If not specified, assumed
>>>> to
>>>>    be 1.
>>>>
>>>> +VS_WINDOW_SPACE_POSITION
>>>> +""""""""""""""""""""""""""
>>>> +If this property is set on the vertex shader, the TGSI_SEMANTIC_POSITION
>>>> output
>>>> +is assumed to contain window space coordinates.
>>>> +Division of X,Y,Z by W and the viewport transformation are disabled, and
>>>> 1/W is
>>>> +directly taken from the 4-th component of the shader output.
>>>> +Naturally, clipping is not performed on window coordinates either.
>>>> +The effect of this property is undefined if a geometry or tessellation
>>>> shader
>>>> +are in use.
>>>>
>>>>    Texture Sampling and Texture Formats
>>>>    ------------------------------------
>>>> diff --git a/src/gallium/include/pipe/p_shader_tokens.h
>>>> b/src/gallium/include/pipe/p_shader_tokens.h
>>>> index d095bd3..9261b79 100644
>>>> --- a/src/gallium/include/pipe/p_shader_tokens.h
>>>> +++ b/src/gallium/include/pipe/p_shader_tokens.h
>>>> @@ -237,7 +237,8 @@ union tgsi_immediate_data
>>>>    #define TGSI_PROPERTY_FS_DEPTH_LAYOUT        6
>>>>    #define TGSI_PROPERTY_VS_PROHIBIT_UCPS       7
>>>>    #define TGSI_PROPERTY_GS_INVOCATIONS         8
>>>> -#define TGSI_PROPERTY_COUNT                  9
>>>> +#define TGSI_PROPERTY_VS_WINDOW_SPACE_POSITION 9
>>>> +#define TGSI_PROPERTY_COUNT                  10
>>>>
>>>>    struct tgsi_property {
>>>>       unsigned Type         : 4;  /**< TGSI_TOKEN_TYPE_PROPERTY */
>>>> --
>>
>>
>> I can't say I'm a big fan of it (I guess alternatively you could use a
>> TGSI_SEMANTIC_POSITIONT output but that doesn't really make a difference
>> neither).
>> This was quite some d3d hack which doesn't really fit into a modern shader
>> based world (of course, it doesn't work with d3d10 neither). So, the usual
>> way to deal with this is to translate this away, since this was only used
>> for old or simple stuff afaik (if it was complex you tend to not already
>> know the screen space coords) noone is going to care if it runs at 1500 or
>> 1520 fps anyway.
>> But if you really think this is a must have and are going to use it in a
>> public state tracker you probably have to implement a cap bit for it too,
>> and be able to deal with drivers which can't handle it - it seems
>> unreasonable to me to assume all drivers will handle it (hence you don't
>> really save anything in the state tracker, you still need all the code
>> translating it away anyway).
>>
>> Roland


More information about the mesa-dev mailing list