[Mesa-dev] [PATCH v3 4/8] intel: Handle firstvertex in an identical way to BaseVertex
Antia Puentes
apuentes at igalia.com
Tue Mar 27 14:52:40 UTC 2018
Thanks for looking at this.
On 27/03/18 03:06, Ian Romanick wrote:
> 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?
We should definitely include it in the emit_system_values_block to mark
it as unreachable,
as like the others, it is lowered by brw_nir_lower_vs_inputs.
About fs_visitor::nir_emit_vs_intrinsic, it makes sense to add it there
too. As far as I can see,
the vs_inputs intrinsics should be lowered by then, we may want to
replace the code by an
"unreachable (lowered by brw_nir_lower_vs_inputs)".
> 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.
Excellent.
> 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