[Mesa-dev] [PATCH 1/3] gallium: add TGSI_SEMANTIC_VERTEXID_ZEROBASE and TGSI_SEMANTIC_BASEVERTEX
Roland Scheidegger
sroland at vmware.com
Wed Dec 10 16:09:55 PST 2014
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.
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?
>
> 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