[Mesa-dev] [PATCH v3 4/8] intel: Handle firstvertex in an identical way to BaseVertex
Ian Romanick
idr at freedesktop.org
Tue Mar 27 01:06:26 UTC 2018
It looks like there are a couple places in brw_fs_nir.cpp
(emit_system_values_block and fs_visitor::nir_emit_vs_intrinsic) that
handle nir_intrinsic_load_base_instance but not
nir_intrinsic_load_first_vertex. Should those cases be added?
I have pushed a branch to my fd.o repo called gl_VertexID-fixes that
rebases this series on current master. I had to apply some fixes to the
last patch in the series to get it to build. I have tested that whole
series patch-by-patch in the CI with no regressions. I have also
applied my R-b on several patches in the series. I think once we decide
what to do about this patch and the squash! patch in my branch, we can
(finally) push this.
On 01/25/2018 10:15 AM, Antia Puentes wrote:
> Until we set gl_BaseVertex to zero for non-indexed draw calls
> both have an identical value.
>
> The Vertex Elements are kept like that:
> * VE 1: <BaseVertex/firstvertex, BaseInstance, VertexID, InstanceID>
> * VE 2: <Draw ID, 0, 0, 0>
> ---
> src/intel/compiler/brw_nir.c | 3 +++
> src/intel/compiler/brw_vec4.cpp | 1 +
> src/mesa/drivers/dri/i965/brw_context.h | 8 ++++++--
> src/mesa/drivers/dri/i965/brw_draw.c | 14 +++++++++-----
> src/mesa/drivers/dri/i965/brw_draw_upload.c | 7 +++++--
> src/mesa/drivers/dri/i965/genX_state_upload.c | 11 +++++++----
> 6 files changed, 31 insertions(+), 13 deletions(-)
>
> diff --git a/src/intel/compiler/brw_nir.c b/src/intel/compiler/brw_nir.c
> index dbddef0d04d..34b1e44adf0 100644
> --- a/src/intel/compiler/brw_nir.c
> +++ b/src/intel/compiler/brw_nir.c
> @@ -239,6 +239,7 @@ brw_nir_lower_vs_inputs(nir_shader *nir,
> const bool has_sgvs =
> nir->info.system_values_read &
> (BITFIELD64_BIT(SYSTEM_VALUE_BASE_VERTEX) |
> + BITFIELD64_BIT(SYSTEM_VALUE_FIRST_VERTEX) |
> BITFIELD64_BIT(SYSTEM_VALUE_BASE_INSTANCE) |
> BITFIELD64_BIT(SYSTEM_VALUE_VERTEX_ID_ZERO_BASE) |
> BITFIELD64_BIT(SYSTEM_VALUE_INSTANCE_ID));
> @@ -261,6 +262,7 @@ brw_nir_lower_vs_inputs(nir_shader *nir,
>
> switch (intrin->intrinsic) {
> case nir_intrinsic_load_base_vertex:
> + case nir_intrinsic_load_first_vertex:
> case nir_intrinsic_load_base_instance:
> case nir_intrinsic_load_vertex_id_zero_base:
> case nir_intrinsic_load_instance_id:
> @@ -278,6 +280,7 @@ brw_nir_lower_vs_inputs(nir_shader *nir,
> nir_intrinsic_set_base(load, num_inputs);
> switch (intrin->intrinsic) {
> case nir_intrinsic_load_base_vertex:
> + case nir_intrinsic_load_first_vertex:
> nir_intrinsic_set_component(load, 0);
> break;
> case nir_intrinsic_load_base_instance:
> diff --git a/src/intel/compiler/brw_vec4.cpp b/src/intel/compiler/brw_vec4.cpp
> index 36e17d77d47..06c79630119 100644
> --- a/src/intel/compiler/brw_vec4.cpp
> +++ b/src/intel/compiler/brw_vec4.cpp
> @@ -2788,6 +2788,7 @@ brw_compile_vs(const struct brw_compiler *compiler, void *log_data,
> */
> if (shader->info.system_values_read &
> (BITFIELD64_BIT(SYSTEM_VALUE_BASE_VERTEX) |
> + BITFIELD64_BIT(SYSTEM_VALUE_FIRST_VERTEX) |
> BITFIELD64_BIT(SYSTEM_VALUE_BASE_INSTANCE) |
> BITFIELD64_BIT(SYSTEM_VALUE_VERTEX_ID_ZERO_BASE) |
> BITFIELD64_BIT(SYSTEM_VALUE_INSTANCE_ID))) {
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
> index 9046acd175c..0a20706567e 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -881,8 +881,12 @@ struct brw_context
>
> struct {
> struct {
> - /** The value of gl_BaseVertex for the current _mesa_prim. */
> - int gl_basevertex;
> + /**
> + * Either the value of gl_BaseVertex for indexed draw calls or the
> + * value of the argument <first> for non-indexed draw calls for the
> + * current _mesa_prim.
> + */
> + int firstvertex;
>
> /** The value of gl_BaseInstance for the current _mesa_prim. */
> int gl_baseinstance;
> diff --git a/src/mesa/drivers/dri/i965/brw_draw.c b/src/mesa/drivers/dri/i965/brw_draw.c
> index 50cf8b12c74..a1a5161fd35 100644
> --- a/src/mesa/drivers/dri/i965/brw_draw.c
> +++ b/src/mesa/drivers/dri/i965/brw_draw.c
> @@ -816,25 +816,29 @@ brw_draw_single_prim(struct gl_context *ctx,
> * always flag if the shader uses one of the values. For direct draws,
> * we only flag if the values change.
> */
> - const int new_basevertex =
> + const int new_firstvertex =
> prim->indexed ? prim->basevertex : prim->start;
> const int new_baseinstance = prim->base_instance;
> const struct brw_vs_prog_data *vs_prog_data =
> brw_vs_prog_data(brw->vs.base.prog_data);
> if (prim_id > 0) {
> - const bool uses_draw_parameters =
> + const bool uses_firstvertex =
> vs_prog_data->uses_basevertex ||
> + vs_prog_data->uses_firstvertex;
> +
> + const bool uses_draw_parameters =
> + uses_firstvertex ||
> vs_prog_data->uses_baseinstance;
>
> if ((uses_draw_parameters && prim->is_indirect) ||
> - (vs_prog_data->uses_basevertex &&
> - brw->draw.params.gl_basevertex != new_basevertex) ||
> + (uses_firstvertex &&
> + brw->draw.params.firstvertex != new_firstvertex) ||
> (vs_prog_data->uses_baseinstance &&
> brw->draw.params.gl_baseinstance != new_baseinstance))
> brw->ctx.NewDriverState |= BRW_NEW_VERTICES;
> }
>
> - brw->draw.params.gl_basevertex = new_basevertex;
> + brw->draw.params.firstvertex = new_firstvertex;
> brw->draw.params.gl_baseinstance = new_baseinstance;
> brw_bo_unreference(brw->draw.draw_params_bo);
>
> diff --git a/src/mesa/drivers/dri/i965/brw_draw_upload.c b/src/mesa/drivers/dri/i965/brw_draw_upload.c
> index 9b81999ea05..52bf752bff9 100644
> --- a/src/mesa/drivers/dri/i965/brw_draw_upload.c
> +++ b/src/mesa/drivers/dri/i965/brw_draw_upload.c
> @@ -696,8 +696,11 @@ brw_prepare_shader_draw_parameters(struct brw_context *brw)
> const struct brw_vs_prog_data *vs_prog_data =
> brw_vs_prog_data(brw->vs.base.prog_data);
>
> - /* For non-indirect draws, upload gl_BaseVertex. */
> - if ((vs_prog_data->uses_basevertex || vs_prog_data->uses_baseinstance) &&
> + const bool uses_firstvertex =
> + vs_prog_data->uses_basevertex || vs_prog_data->uses_firstvertex;
> +
> + /* For non-indirect draws, upload the shader draw parameters */
> + if ((uses_firstvertex || vs_prog_data->uses_baseinstance) &&
> brw->draw.draw_params_bo == NULL) {
> intel_upload_data(brw, &brw->draw.params, sizeof(brw->draw.params), 4,
> &brw->draw.draw_params_bo,
> diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c b/src/mesa/drivers/dri/i965/genX_state_upload.c
> index d0a980f9730..2f2a573fd05 100644
> --- a/src/mesa/drivers/dri/i965/genX_state_upload.c
> +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c
> @@ -488,7 +488,10 @@ genX(emit_vertices)(struct brw_context *brw)
> }
> #endif
>
> - const bool needs_sgvs_element = (vs_prog_data->uses_basevertex ||
> + const bool uses_firstvertex =
> + vs_prog_data->uses_basevertex || vs_prog_data->uses_firstvertex;
> +
> + const bool needs_sgvs_element = (uses_firstvertex ||
> vs_prog_data->uses_baseinstance ||
> vs_prog_data->uses_instanceid ||
> vs_prog_data->uses_vertexid);
> @@ -533,7 +536,7 @@ genX(emit_vertices)(struct brw_context *brw)
>
> /* Now emit 3DSTATE_VERTEX_BUFFERS and 3DSTATE_VERTEX_ELEMENTS packets. */
> const bool uses_draw_params =
> - vs_prog_data->uses_basevertex ||
> + uses_firstvertex ||
> vs_prog_data->uses_baseinstance;
> const unsigned nr_buffers = brw->vb.nr_buffers +
> uses_draw_params + vs_prog_data->uses_drawid;
> @@ -711,7 +714,7 @@ genX(emit_vertices)(struct brw_context *brw)
> };
>
> #if GEN_GEN >= 8
> - if (vs_prog_data->uses_basevertex ||
> + if (uses_firstvertex ||
> vs_prog_data->uses_baseinstance) {
> elem_state.VertexBufferIndex = brw->vb.nr_buffers;
> elem_state.SourceElementFormat = (enum GENX(SURFACE_FORMAT)) ISL_FORMAT_R32G32_UINT;
> @@ -721,7 +724,7 @@ genX(emit_vertices)(struct brw_context *brw)
> #else
> elem_state.VertexBufferIndex = brw->vb.nr_buffers;
> elem_state.SourceElementFormat = (enum GENX(SURFACE_FORMAT)) ISL_FORMAT_R32G32_UINT;
> - if (vs_prog_data->uses_basevertex)
> + if (uses_firstvertex)
> elem_state.Component0Control = VFCOMP_STORE_SRC;
>
> if (vs_prog_data->uses_baseinstance)
>
More information about the mesa-dev
mailing list