On Sat, Apr 7, 2018 at 10:49 AM, Ian Romanick <i...@freedesktop.org> wrote:
> On 04/06/2018 11:00 PM, Jason Ekstrand wrote: > > On Fri, Apr 6, 2018 at 2:53 PM, Ian Romanick <i...@freedesktop.org > > <mailto:i...@freedesktop.org>> wrote: > > > > From: Neil Roberts <nrobe...@igalia.com <mailto:nrobe...@igalia.com > >> > > > > The base vertex in Vulkan is different from GL in that for > non-indexed > > primitives the value is taken from the firstVertex parameter instead > > of being set to zero. This coincides with the new > > SYSTEM_VALUE_FIRST_VERTEX > > instead of BASE_VERTEX. > > > > Reviewed-by: Ian Romanick <ian.d.roman...@intel.com > > <mailto:ian.d.roman...@intel.com>> > > --- > > src/compiler/spirv/vtn_variables.c | 2 +- > > src/intel/vulkan/genX_cmd_buffer.c | 16 ++++++++++++---- > > src/intel/vulkan/genX_pipeline.c | 2 ++ > > 3 files changed, 15 insertions(+), 5 deletions(-) > > > > diff --git a/src/compiler/spirv/vtn_variables.c > > b/src/compiler/spirv/vtn_variables.c > > index b2897407fb1..9bb7d5a575e 100644 > > --- a/src/compiler/spirv/vtn_variables.c > > +++ b/src/compiler/spirv/vtn_variables.c > > @@ -1296,7 +1296,7 @@ vtn_get_builtin_location(struct vtn_builder > *b, > > set_mode_system_value(b, mode); > > break; > > case SpvBuiltInBaseVertex: > > - *location = SYSTEM_VALUE_BASE_VERTEX; > > + *location = SYSTEM_VALUE_FIRST_VERTEX; > > > > > > Given that we are taking something called BaseVertex and using the > > FIRST_VERTEX location when there is a location called BASE_VERTEX, I > > think a comment is in order. > > That is fair, but I struggled a bit to come up with something > non-circular. The best I could get is: > > /* OpenGL gl_BaseVertex (SYSTEM_VALUE_BASE_VERTEX) is not the > * same semantic as SPIR-V BaseVertex > (SYSTEM_VALUE_FIRST_VERTEX). > */ > That's fine with me. > > set_mode_system_value(b, mode); > > break; > > case SpvBuiltInBaseInstance: > > diff --git a/src/intel/vulkan/genX_cmd_buffer.c > > b/src/intel/vulkan/genX_cmd_buffer.c > > index 3c703f6be44..7d190a4d5cf 100644 > > --- a/src/intel/vulkan/genX_cmd_buffer.c > > +++ b/src/intel/vulkan/genX_cmd_buffer.c > > @@ -2673,7 +2673,9 @@ void genX(CmdDraw)( > > > > genX(cmd_buffer_flush_state)(cmd_buffer); > > > > - if (vs_prog_data->uses_basevertex || > > vs_prog_data->uses_baseinstance) > > + if (vs_prog_data->uses_firstvertex || > > + vs_prog_data->uses_basevertex || > > + vs_prog_data->uses_baseinstance) > > > > > > I was going to ask if this was needed. Then I saw patch 5. > > > > Reviewed-by: Jason Ekstrand <ja...@jlekstrand.net > > <mailto:ja...@jlekstrand.net>> > > > > > > emit_base_vertex_instance(cmd_buffer, firstVertex, > > firstInstance); > > if (vs_prog_data->uses_drawid) > > emit_draw_index(cmd_buffer, 0); > > @@ -2711,7 +2713,9 @@ void genX(CmdDrawIndexed)( > > > > genX(cmd_buffer_flush_state)(cmd_buffer); > > > > - if (vs_prog_data->uses_basevertex || > > vs_prog_data->uses_baseinstance) > > + if (vs_prog_data->uses_firstvertex || > > + vs_prog_data->uses_basevertex || > > + vs_prog_data->uses_baseinstance) > > emit_base_vertex_instance(cmd_buffer, vertexOffset, > > firstInstance); > > if (vs_prog_data->uses_drawid) > > emit_draw_index(cmd_buffer, 0); > > @@ -2850,7 +2854,9 @@ void genX(CmdDrawIndirect)( > > struct anv_bo *bo = buffer->bo; > > uint32_t bo_offset = buffer->offset + offset; > > > > - if (vs_prog_data->uses_basevertex || > > vs_prog_data->uses_baseinstance) > > + if (vs_prog_data->uses_firstvertex || > > + vs_prog_data->uses_basevertex || > > + vs_prog_data->uses_baseinstance) > > emit_base_vertex_instance_bo(cmd_buffer, bo, bo_offset + > 8); > > if (vs_prog_data->uses_drawid) > > emit_draw_index(cmd_buffer, i); > > @@ -2889,7 +2895,9 @@ void genX(CmdDrawIndexedIndirect)( > > uint32_t bo_offset = buffer->offset + offset; > > > > /* TODO: We need to stomp base vertex to 0 somehow */ > > - if (vs_prog_data->uses_basevertex || > > vs_prog_data->uses_baseinstance) > > + if (vs_prog_data->uses_firstvertex || > > + vs_prog_data->uses_basevertex || > > + vs_prog_data->uses_baseinstance) > > emit_base_vertex_instance_bo(cmd_buffer, bo, bo_offset + > 12); > > if (vs_prog_data->uses_drawid) > > emit_draw_index(cmd_buffer, i); > > diff --git a/src/intel/vulkan/genX_pipeline.c > > b/src/intel/vulkan/genX_pipeline.c > > index eb2d4147357..a473f42c7e1 100644 > > --- a/src/intel/vulkan/genX_pipeline.c > > +++ b/src/intel/vulkan/genX_pipeline.c > > @@ -98,6 +98,7 @@ emit_vertex_input(struct anv_pipeline *pipeline, > > const bool needs_svgs_elem = vs_prog_data->uses_vertexid || > > vs_prog_data->uses_instanceid || > > vs_prog_data->uses_basevertex || > > + vs_prog_data->uses_firstvertex || > > vs_prog_data->uses_baseinstance; > > > > uint32_t elem_count = __builtin_popcount(elements) - > > @@ -178,6 +179,7 @@ emit_vertex_input(struct anv_pipeline *pipeline, > > * well. Just do all or nothing. > > */ > > uint32_t base_ctrl = (vs_prog_data->uses_basevertex || > > + vs_prog_data->uses_firstvertex || > > vs_prog_data->uses_baseinstance) ? > > VFCOMP_STORE_SRC : VFCOMP_STORE_0; > > > > -- > > 2.14.3 > > > > _______________________________________________ > > mesa-dev mailing list > > mesa-dev@lists.freedesktop.org <mailto:mesa-dev@lists. > freedesktop.org> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > > <https://lists.freedesktop.org/mailman/listinfo/mesa-dev> >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
