[Mesa-dev] [PATCH 1/3] gallium: add TGSI_SEMANTIC_VERTEXID_ZEROBASE and TGSI_SEMANTIC_BASEVERTEX
Brian Paul
brianp at vmware.com
Wed Dec 10 16:23:31 PST 2014
On 12/10/2014 05:09 PM, Roland Scheidegger wrote:
> Am 11.12.2014 um 00:53 schrieb Brian Paul:
>> On 12/10/2014 02:22 PM, sroland at vmware.com wrote:
>>> From: Roland Scheidegger <sroland at vmware.com>
>>>
>>> Plus a new PIPE_CAP_VERTEXID_NOOFFSET query. The idea is that drivers not
>>> supporting vertex ids with base vertex offset applied (so, only support
>>> d3d10-style vertex ids) will get such a d3d10-style vertex id instead -
>>> with the caveat they'll also need to handle the basevertex system value
>>> too (this follows what core mesa already does).
>>> Additionally, this is also useful for other state trackers (for instance
>>> llvmpipe / draw right now implement the d3d10 behavior on purpose, but
>>> with different semantics it can just do both).
>>> Doesn't do anything yet.
>>> And fix up the docs wrt similar values.
>>> ---
>>> src/gallium/auxiliary/tgsi/tgsi_scan.c | 6 ++++
>>> src/gallium/auxiliary/tgsi/tgsi_scan.h | 2 ++
>>> src/gallium/auxiliary/tgsi/tgsi_strings.c | 2 ++
>>> src/gallium/docs/source/screen.rst | 6 ++++
>>> src/gallium/docs/source/tgsi.rst | 36
>>> ++++++++++++++++++++++++
>>> src/gallium/drivers/freedreno/freedreno_screen.c | 1 +
>>> src/gallium/drivers/i915/i915_screen.c | 1 +
>>> src/gallium/drivers/ilo/ilo_screen.c | 2 ++
>>> src/gallium/drivers/llvmpipe/lp_screen.c | 2 ++
>>> src/gallium/drivers/nouveau/nv30/nv30_screen.c | 1 +
>>> src/gallium/drivers/nouveau/nv50/nv50_screen.c | 1 +
>>> src/gallium/drivers/r300/r300_screen.c | 1 +
>>> src/gallium/drivers/r600/r600_pipe.c | 1 +
>>> src/gallium/drivers/radeonsi/si_pipe.c | 1 +
>>> src/gallium/drivers/softpipe/sp_screen.c | 2 ++
>>> src/gallium/drivers/svga/svga_screen.c | 1 +
>>> src/gallium/drivers/vc4/vc4_screen.c | 1 +
>>> src/gallium/include/pipe/p_defines.h | 1 +
>>> src/gallium/include/pipe/p_shader_tokens.h | 4 ++-
>>> 19 files changed, 71 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_scan.c
>>> b/src/gallium/auxiliary/tgsi/tgsi_scan.c
>>> index 649c327..954eb10 100644
>>> --- a/src/gallium/auxiliary/tgsi/tgsi_scan.c
>>> +++ b/src/gallium/auxiliary/tgsi/tgsi_scan.c
>>> @@ -213,6 +213,12 @@ tgsi_scan_shader(const struct tgsi_token *tokens,
>>> else if (semName == TGSI_SEMANTIC_VERTEXID) {
>>> info->uses_vertexid = TRUE;
>>> }
>>> + else if (semName == TGSI_SEMANTIC_VERTEXID_ZEROBASE) {
>>> + info->uses_vertexid_zerobase = TRUE;
>>> + }
>>> + else if (semName == TGSI_SEMANTIC_BASEVERTEX) {
>>> + info->uses_basevertex = TRUE;
>>> + }
>>> else if (semName == TGSI_SEMANTIC_PRIMID) {
>>> info->uses_primid = TRUE;
>>> }
>>
>> Tangential, but I wonder if we could someday replace the instances of:
>>
>> if (semName == TGSI_SEMANTIC_FOO) {
>> info->uses_foo = TRUE;
>> }
>>
>> with something like:
>>
>> info->semantics_used |= (1 << TGSI_SEMANTIC_FOO);
>>
>> Maybe separate bitfields for read vs. written registers.
> Yes that sounds good to me.
>
>>
>>
>>
>>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_scan.h
>>> b/src/gallium/auxiliary/tgsi/tgsi_scan.h
>>> index 61ce813..b353097 100644
>>> --- a/src/gallium/auxiliary/tgsi/tgsi_scan.h
>>> +++ b/src/gallium/auxiliary/tgsi/tgsi_scan.h
>>> @@ -74,6 +74,8 @@ struct tgsi_shader_info
>>> boolean uses_kill; /**< KILL or KILL_IF instruction used? */
>>> boolean uses_instanceid;
>>> boolean uses_vertexid;
>>> + boolean uses_vertexid_zerobase;
>>> + boolean uses_basevertex;
>>> boolean uses_primid;
>>> boolean uses_frontface;
>>> boolean writes_psize;
>>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_strings.c
>>> b/src/gallium/auxiliary/tgsi/tgsi_strings.c
>>> index 01fa5a9..7a45e2d 100644
>>> --- a/src/gallium/auxiliary/tgsi/tgsi_strings.c
>>> +++ b/src/gallium/auxiliary/tgsi/tgsi_strings.c
>>> @@ -86,6 +86,8 @@ const char *tgsi_semantic_names[TGSI_SEMANTIC_COUNT] =
>>> "SAMPLEPOS",
>>> "SAMPLEMASK",
>>> "INVOCATIONID",
>>> + "VERTEXID_ZEROBASE",
>>> + "BASEVERTEX",
>>> };
>>>
>>> const char *tgsi_texture_names[TGSI_TEXTURE_COUNT] =
>>> diff --git a/src/gallium/docs/source/screen.rst
>>> b/src/gallium/docs/source/screen.rst
>>> index e711ad4..6ccbe5b 100644
>>> --- a/src/gallium/docs/source/screen.rst
>>> +++ b/src/gallium/docs/source/screen.rst
>>> @@ -233,6 +233,12 @@ The integer capabilities:
>>> * ``PIPE_CAP_CLIP_HALFZ``: Whether the driver supports the
>>> pipe_rasterizer_state::clip_halfz being set to true. This is required
>>> for enabling ARB_clip_control.
>>> +* ``PIPE_CAP_VERTEXID_NOOFFSET``: Whether the driver needs lowering
>>> of the
>>> + vertex id to not include the basevertex offset. If so vertex id will
>>> + use TGSI_SEMANTIC_VERTEXID_ZEROBASE and TGSI_SEMANTIC_BASEVERTEX
>>> instead
>>> + of TGSI_SEMANTIC_VERTEXID. Only relevant if geometry shaders are
>>> supported.
>>> + (Currently not possible to query availability of these two
>>> semantics outside
>>> + of this).
>>
>> The first sentence is a little hard to parse (at least to me). How about:
>>
>> "If true, the driver only supports TGSI_SEMANTIC_VERTEXID_ZEROBASE (and
>> not TGSI_SEMANTIC_VERTEXID). This means state trackers which need base
>> offsetting will need to lower TGSI_SEMANTIC_VERTEXID to
>> TGSI_SEMANTIC_VERTEXID_ZEROBASE and TGSI_SEMANTIC_BASEVERTEX." ...
> Sounds better indeed.
>
>>
>>>
>>> .. _pipe_capf:
>>> diff --git a/src/gallium/docs/source/tgsi.rst
>>> b/src/gallium/docs/source/tgsi.rst
>>> index cbb8f74..c12858d 100644
>>> --- a/src/gallium/docs/source/tgsi.rst
>>> +++ b/src/gallium/docs/source/tgsi.rst
>>> @@ -2723,6 +2723,42 @@ For geometry shaders, this semantic label
>>> indicates that a system value
>>> contains the current invocation id (i.e. gl_InvocationID). Only the
>>> X value is
>>> used.
>>>
>>> +TGSI_SEMANTIC_INSTANCEID
>>> +""""""""""""""""""""""""
>>> +
>>> +For vertex shaders, this semantic label indicates that a system value
>>> contains
>>> +the current instance id (i.e. gl_InstanceID). It does not include the
>>> base
>>> +instance. Only the X value is used.
>>> +
>>> +TGSI_SEMANTIC_VERTEXID
>>> +""""""""""""""""""""""
>>> +
>>> +For vertex shaders, this semantic label indicates that a system value
>>> contains
>>> +the current vertex id (i.e. gl_VertexID). It does (unlike in d3d10)
>>> include the
>>> +base vertex. Only the X value is used.
>>> +
>>> +TGSI_SEMANTIC_VERTEXID_ZEROBASE
>>> +"""""""""""""""""""""""""""""""
>>> +
>>> +For vertex shaders, this semantic label indicates that a system value
>>> contains
>>> +the current vertex id without including the base vertex (this
>>> corresponds to
>>> +d3d10 vertex id, so TGSI_SEMANTIC_VERTEXID_ZEROBASE +
>>> TGSI_SEMANTIC_BASEVERTEX
>>> +== TGSI_SEMANTIC_VERTEXID). Only the X value is used.
>>
>> I wonder if those two semantics should be renamed:
>>
>> TGSI_SEMANTIC_VERTEXID -> TGSI_SEMANTIC_VERTEXID_WITH_BASE
>> TGSI_SEMANTIC_VERTEXID_ZEROBASE -> TGSI_SEMANTIC_VERTEXID
>>
>> This would make TGSI_SEMANTIC_VERTEXID consistent with
>> TGSI_SEMANTIC_INSTANCEID so they're both zero-based (do not include the
>> base value).
> I'm not sure, I was just going by the names similar to what core mesa is
> using - SYSTEM_VALUE_VERTEX_ID / SYSTEM_VALUE_VERTEX_ID_ZERO_BASE.
OK, I didn't realize that core mesa already has these terms. In that
case, I don't have a strong opinion.
> I don't mind renaming the existing VERTEXID one though imho if renaming
> we should avoid calling the one without base vertex just VERTEXID.
> Maybe VERTEXID_WITH_BASE and VERTEXID_NO_BASE?
Well, that would certainly make things clear to the reader. Your call,
as far as I'm concerned.
>
>>
>> Also, should we be explicit and say these values are integers?
> Yes.
>
>>
>>
>>> +
>>> +TGSI_SEMANTIC_BASEVERTEX
>>> +""""""""""""""""""""""""
>>> +
>>> +For vertex shaders, this semantic label indicates that a system value
>>> contains
>>> +the base vertex (i.e. gl_BaseVertex). Only the X value is used.
>>> +
>>> +TGSI_SEMANTIC_PRIMID
>>> +""""""""""""""""""""
>>> +
>>> +For geometry and fragment shaders, this semantic label indicates the
>>> value
>>> +contains the primitive id (i.e. gl_PrimitiveID). Only the X value is
>>> used.
>>> +FIXME: This right now can be either a ordinary input or a system
>>> value...
>>> +
>>> +
>>> Declaration Interpolate
>>> ^^^^^^^^^^^^^^^^^^^^^^^
>>>
>>> diff --git a/src/gallium/drivers/freedreno/freedreno_screen.c
>>> b/src/gallium/drivers/freedreno/freedreno_screen.c
>>> index ce105b8..43f5366 100644
>>> --- a/src/gallium/drivers/freedreno/freedreno_screen.c
>>> +++ b/src/gallium/drivers/freedreno/freedreno_screen.c
>>> @@ -228,6 +228,7 @@ fd_screen_get_param(struct pipe_screen *pscreen,
>>> enum pipe_cap param)
>>> case PIPE_CAP_CONDITIONAL_RENDER_INVERTED:
>>> case PIPE_CAP_SAMPLER_VIEW_TARGET:
>>> case PIPE_CAP_CLIP_HALFZ:
>>> + case PIPE_CAP_VERTEXID_NOOFSET:
>>
>> How about NO_OFFSET instead of NOOFFSET?
> Ahh your eyes are as sharp as Marek's ;-). Apparently this doesn't get
> compiled in my setup...
>
>>
>>
>>> return 0;
>>>
>>> case PIPE_CAP_MAX_VIEWPORTS:
>>> diff --git a/src/gallium/drivers/i915/i915_screen.c
>>> b/src/gallium/drivers/i915/i915_screen.c
>>> index 1c60499..dedc7a8 100644
>>> --- a/src/gallium/drivers/i915/i915_screen.c
>>> +++ b/src/gallium/drivers/i915/i915_screen.c
>>> @@ -226,6 +226,7 @@ i915_get_param(struct pipe_screen *screen, enum
>>> pipe_cap cap)
>>> case PIPE_CAP_TGSI_VS_WINDOW_SPACE_POSITION:
>>> case PIPE_CAP_CONDITIONAL_RENDER_INVERTED:
>>> case PIPE_CAP_CLIP_HALFZ:
>>> + case PIPE_CAP_VERTEXID_NOOFFSET:
>>> return 0;
>>>
>>> case PIPE_CAP_MAX_DUAL_SOURCE_RENDER_TARGETS:
>>> diff --git a/src/gallium/drivers/ilo/ilo_screen.c
>>> b/src/gallium/drivers/ilo/ilo_screen.c
>>> index 06aa973..cbf1083 100644
>>> --- a/src/gallium/drivers/ilo/ilo_screen.c
>>> +++ b/src/gallium/drivers/ilo/ilo_screen.c
>>> @@ -495,6 +495,8 @@ ilo_get_param(struct pipe_screen *screen, enum
>>> pipe_cap param)
>>> return true;
>>> case PIPE_CAP_CLIP_HALFZ:
>>> return true;
>>> + case PIPE_CAP_VERTEXID_NOOFFSET:
>>> + return false;
>>>
>>> default:
>>> return 0;
>>> diff --git a/src/gallium/drivers/llvmpipe/lp_screen.c
>>> b/src/gallium/drivers/llvmpipe/lp_screen.c
>>> index 6af43cc..a394fa7 100644
>>> --- a/src/gallium/drivers/llvmpipe/lp_screen.c
>>> +++ b/src/gallium/drivers/llvmpipe/lp_screen.c
>>> @@ -282,6 +282,8 @@ llvmpipe_get_param(struct pipe_screen *screen,
>>> enum pipe_cap param)
>>> return 0;
>>> case PIPE_CAP_CLIP_HALFZ:
>>> return 1;
>>> + case PIPE_CAP_VERTEXID_NOOFFSET:
>>> + return 0;
>>> }
>>> /* should only get here on unhandled cases */
>>> debug_printf("Unexpected PIPE_CAP %d query\n", param);
>>> diff --git a/src/gallium/drivers/nouveau/nv30/nv30_screen.c
>>> b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
>>> index 2b65f8c..6ab5203 100644
>>> --- a/src/gallium/drivers/nouveau/nv30/nv30_screen.c
>>> +++ b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
>>> @@ -157,6 +157,7 @@ nv30_screen_get_param(struct pipe_screen *pscreen,
>>> enum pipe_cap param)
>>> case PIPE_CAP_CONDITIONAL_RENDER_INVERTED:
>>> case PIPE_CAP_SAMPLER_VIEW_TARGET:
>>> case PIPE_CAP_CLIP_HALFZ:
>>> + case PIPE_CAP_VERTEXID_NOOFFSET:
>>> return 0;
>>>
>>> case PIPE_CAP_VENDOR_ID:
>>> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_screen.c
>>> b/src/gallium/drivers/nouveau/nv50/nv50_screen.c
>>> index fcf0098..16cedc9 100644
>>> --- a/src/gallium/drivers/nouveau/nv50/nv50_screen.c
>>> +++ b/src/gallium/drivers/nouveau/nv50/nv50_screen.c
>>> @@ -205,6 +205,7 @@ nv50_screen_get_param(struct pipe_screen *pscreen,
>>> enum pipe_cap param)
>>> case PIPE_CAP_TGSI_VS_WINDOW_SPACE_POSITION:
>>> case PIPE_CAP_COMPUTE:
>>> case PIPE_CAP_DRAW_INDIRECT:
>>> + case PIPE_CAP_VERTEXID_NOOFFSET:
>>> return 0;
>>>
>>> case PIPE_CAP_VENDOR_ID:
>>> diff --git a/src/gallium/drivers/r300/r300_screen.c
>>> b/src/gallium/drivers/r300/r300_screen.c
>>> index e653eab..bb6ba32 100644
>>> --- a/src/gallium/drivers/r300/r300_screen.c
>>> +++ b/src/gallium/drivers/r300/r300_screen.c
>>> @@ -181,6 +181,7 @@ static int r300_get_param(struct pipe_screen*
>>> pscreen, enum pipe_cap param)
>>> case PIPE_CAP_TGSI_FS_FINE_DERIVATIVE:
>>> case PIPE_CAP_CONDITIONAL_RENDER_INVERTED:
>>> case PIPE_CAP_SAMPLER_VIEW_TARGET:
>>> + case PIPE_CAP_VERTEXID_NOOFFSET:
>>> return 0;
>>>
>>> /* SWTCL-only features. */
>>> diff --git a/src/gallium/drivers/r600/r600_pipe.c
>>> b/src/gallium/drivers/r600/r600_pipe.c
>>> index 0b571e4..1be037f 100644
>>> --- a/src/gallium/drivers/r600/r600_pipe.c
>>> +++ b/src/gallium/drivers/r600/r600_pipe.c
>>> @@ -325,6 +325,7 @@ static int r600_get_param(struct pipe_screen*
>>> pscreen, enum pipe_cap param)
>>> case PIPE_CAP_DRAW_INDIRECT:
>>> case PIPE_CAP_CONDITIONAL_RENDER_INVERTED:
>>> case PIPE_CAP_SAMPLER_VIEW_TARGET:
>>> + case PIPE_CAP_VERTEXID_NOOFFSET:
>>> return 0;
>>>
>>> /* Stream output. */
>>> diff --git a/src/gallium/drivers/radeonsi/si_pipe.c
>>> b/src/gallium/drivers/radeonsi/si_pipe.c
>>> index 8fc5c19..e86a593 100644
>>> --- a/src/gallium/drivers/radeonsi/si_pipe.c
>>> +++ b/src/gallium/drivers/radeonsi/si_pipe.c
>>> @@ -253,6 +253,7 @@ static int si_get_param(struct pipe_screen*
>>> pscreen, enum pipe_cap param)
>>> case PIPE_CAP_TGSI_FS_FINE_DERIVATIVE:
>>> case PIPE_CAP_CONDITIONAL_RENDER_INVERTED:
>>> case PIPE_CAP_SAMPLER_VIEW_TARGET:
>>> + case PIPE_CAP_VERTEXID_NOOFFSET:
>>> return 0;
>>>
>>> case PIPE_CAP_TEXTURE_BORDER_COLOR_QUIRK:
>>> diff --git a/src/gallium/drivers/softpipe/sp_screen.c
>>> b/src/gallium/drivers/softpipe/sp_screen.c
>>> index 57cd9b6..296c94a 100644
>>> --- a/src/gallium/drivers/softpipe/sp_screen.c
>>> +++ b/src/gallium/drivers/softpipe/sp_screen.c
>>> @@ -231,6 +231,8 @@ softpipe_get_param(struct pipe_screen *screen,
>>> enum pipe_cap param)
>>> return 1;
>>> case PIPE_CAP_CLIP_HALFZ:
>>> return 1;
>>> + case PIPE_CAP_VERTEXID_NOOFFSET:
>>> + return 0;
>>> }
>>> /* should only get here on unhandled cases */
>>> debug_printf("Unexpected PIPE_CAP %d query\n", param);
>>> diff --git a/src/gallium/drivers/svga/svga_screen.c
>>> b/src/gallium/drivers/svga/svga_screen.c
>>> index 691d9df..9bcfb3c 100644
>>> --- a/src/gallium/drivers/svga/svga_screen.c
>>> +++ b/src/gallium/drivers/svga/svga_screen.c
>>> @@ -282,6 +282,7 @@ svga_get_param(struct pipe_screen *screen, enum
>>> pipe_cap param)
>>> case PIPE_CAP_CONDITIONAL_RENDER_INVERTED:
>>> case PIPE_CAP_SAMPLER_VIEW_TARGET:
>>> case PIPE_CAP_CLIP_HALFZ:
>>> + case PIPE_CAP_VERTEXID_NOOFFSET:
>>> return 0;
>>> case PIPE_CAP_MIN_MAP_BUFFER_ALIGNMENT:
>>> return 64;
>>> diff --git a/src/gallium/drivers/vc4/vc4_screen.c
>>> b/src/gallium/drivers/vc4/vc4_screen.c
>>> index 18451bd..0ead51e 100644
>>> --- a/src/gallium/drivers/vc4/vc4_screen.c
>>> +++ b/src/gallium/drivers/vc4/vc4_screen.c
>>> @@ -167,6 +167,7 @@ vc4_screen_get_param(struct pipe_screen *pscreen,
>>> enum pipe_cap param)
>>> case PIPE_CAP_CONDITIONAL_RENDER_INVERTED:
>>> case PIPE_CAP_SAMPLER_VIEW_TARGET:
>>> case PIPE_CAP_CLIP_HALFZ:
>>> + case PIPE_CAP_VERTEXID_NOOFFSET:
>>> return 0;
>>>
>>> /* Stream output. */
>>> diff --git a/src/gallium/include/pipe/p_defines.h
>>> b/src/gallium/include/pipe/p_defines.h
>>> index 8c4e415..c5ee2e8 100644
>>> --- a/src/gallium/include/pipe/p_defines.h
>>> +++ b/src/gallium/include/pipe/p_defines.h
>>> @@ -572,6 +572,7 @@ enum pipe_cap {
>>> PIPE_CAP_MAX_VERTEX_ATTRIB_STRIDE = 109,
>>> PIPE_CAP_SAMPLER_VIEW_TARGET = 110,
>>> PIPE_CAP_CLIP_HALFZ = 111,
>>> + PIPE_CAP_VERTEXID_NOOFFSET,
>>> };
>>>
>>> #define PIPE_QUIRK_TEXTURE_BORDER_COLOR_SWIZZLE_NV50 (1 << 0)
>>> diff --git a/src/gallium/include/pipe/p_shader_tokens.h
>>> b/src/gallium/include/pipe/p_shader_tokens.h
>>> index 98f0670..c8bf448 100644
>>> --- a/src/gallium/include/pipe/p_shader_tokens.h
>>> +++ b/src/gallium/include/pipe/p_shader_tokens.h
>>> @@ -176,7 +176,9 @@ struct tgsi_declaration_interp
>>> #define TGSI_SEMANTIC_SAMPLEPOS 25
>>> #define TGSI_SEMANTIC_SAMPLEMASK 26
>>> #define TGSI_SEMANTIC_INVOCATIONID 27
>>> -#define TGSI_SEMANTIC_COUNT 28 /**< number of semantic values */
>>> +#define TGSI_SEMANTIC_VERTEXID_ZEROBASE 28
>>> +#define TGSI_SEMANTIC_BASEVERTEX 29
>>> +#define TGSI_SEMANTIC_COUNT 30 /**< number of semantic values */
>>>
>>> struct tgsi_declaration_semantic
>>> {
>>>
>>
>> Otherwise, everything else looks OK to me.
>>
>> Reviewed-by: Brian Paul <brianp at vmware.com>
>>
>
More information about the mesa-dev
mailing list