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