[Mesa-dev] [PATCH 1/3] gallium: add TGSI_SEMANTIC_VERTEXID_ZEROBASE and TGSI_SEMANTIC_BASEVERTEX
Brian Paul
brianp at vmware.com
Wed Dec 10 15:53:10 PST 2014
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.
> 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." ...
>
> .. _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).
Also, should we be explicit and say these values are integers?
> +
> +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?
> 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