[Mesa-dev] [PATCH] i965: Don't bail on vertex element processing if we need draw params.

[Jason Ekstrand]

Bah... Forgot this:

Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>

On Dec 20, 2016 13:12, "Jason Ekstrand" <jason at jlekstrand.net> wrote:

> On Dec 19, 2016 13:27, "Kenneth Graunke" <kenneth at whitecape.org> wrote:
>
> BaseVertex, BaseInstance, DrawID, and some edge flag conditions need
> vertex buffer and elements structs.  We can't bail early in this case.
>
> Gen4-7 already do this properly.  Gen8+ did not.
>
> Thanks to Ilia Mirkin for helping track this down.
>
> Cc: mesa-stable at lists.freedesktop.org
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99144
> Reported-by
> <https://bugs.freedesktop.org/show_bug.cgi?id=99144Reported-by>:
> Pierre-Eric Pelloux-Prayer <pelloux at gmail.com>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/mesa/drivers/dri/i965/gen8_draw_upload.c | 34
> ++++++++++++++--------------
>  1 file changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/gen8_draw_upload.c
> b/src/mesa/drivers/dri/i965/gen8_draw_upload.c
> index 69ba8e9..3177f9a 100644
> --- a/src/mesa/drivers/dri/i965/gen8_draw_upload.c
> +++ b/src/mesa/drivers/dri/i965/gen8_draw_upload.c
> @@ -110,6 +110,22 @@ gen8_emit_vertices(struct brw_context *brw)
>        ADVANCE_BATCH();
>     }
>
> +   /* Normally we don't need an element for the SGVS attribute because the
> +    * 3DSTATE_VF_SGVS instruction lets you store the generated attribute
> in an
> +    * element that is past the list in 3DSTATE_VERTEX_ELEMENTS. However if
> +    * we're using draw parameters then we need an element for the those
> +    * values.  Additionally if there is an edge flag element then the SGVS
> +    * can't be inserted past that so we need a dummy element to ensure
> that
> +    * the edge flag is the last one.
> +    */
> +   const bool needs_sgvs_element = (vs_prog_data->uses_basevertex ||
> +                                    vs_prog_data->uses_baseinstance ||
> +                                    ((vs_prog_data->uses_instanceid ||
> +                                      vs_prog_data->uses_vertexid) &&
> +                                     uses_edge_flag));
>
>
> Out of curiosity, why are we trying so hard to avoid an extra element?
>
> +   const unsigned nr_elements =
> +      brw->vb.nr_enabled + needs_sgvs_element + vs_prog_data->uses_drawid;
> +
>     /* If the VS doesn't read any inputs (calculating vertex position from
>      * a state variable for some reason, for example), emit a single pad
>      * VERTEX_ELEMENT struct and bail.
> @@ -117,7 +133,7 @@ gen8_emit_vertices(struct brw_context *brw)
>      * The stale VB state stays in place, but they don't do anything unless
>      * a VE loads from them.
>      */
> -   if (brw->vb.nr_enabled == 0) {
> +   if (nr_elements == 0) {
>
>
> Seems reasonable.
>
>        BEGIN_BATCH(3);
>        OUT_BATCH((_3DSTATE_VERTEX_ELEMENTS << 16) | (3 - 2));
>        OUT_BATCH((0 << GEN6_VE0_INDEX_SHIFT) |
> @@ -172,22 +188,6 @@ gen8_emit_vertices(struct brw_context *brw)
>        ADVANCE_BATCH();
>     }
>
> -   /* Normally we don't need an element for the SGVS attribute because the
> -    * 3DSTATE_VF_SGVS instruction lets you store the generated attribute
> in an
> -    * element that is past the list in 3DSTATE_VERTEX_ELEMENTS. However if
> -    * we're using draw parameters then we need an element for the those
> -    * values.  Additionally if there is an edge flag element then the SGVS
> -    * can't be inserted past that so we need a dummy element to ensure
> that
> -    * the edge flag is the last one.
> -    */
> -   const bool needs_sgvs_element = (vs_prog_data->uses_basevertex ||
> -                                    vs_prog_data->uses_baseinstance ||
> -                                    ((vs_prog_data->uses_instanceid ||
> -                                      vs_prog_data->uses_vertexid) &&
> -                                     uses_edge_flag));
> -   const unsigned nr_elements =
> -      brw->vb.nr_enabled + needs_sgvs_element + vs_prog_data->uses_drawid;
> -
>     /* The hardware allows one more VERTEX_ELEMENTS than VERTEX_BUFFERS,
>      * presumably for VertexID/InstanceID.
>      */
> --
> 2.10.2
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161220/45457635/attachment.html>