[Mesa-dev] [PATCH] vbo: fix glVertexAttrib(index=0)

[Marek Olšák]

Reviewed-by: Marek Olšák <marek.olsak at amd.com>

Marek

On Tue, Aug 22, 2017 at 10:21 PM, Brian Paul <brianp at vmware.com> wrote:
> Depending on which extension or GL spec you read the behavior of
> glVertexAttrib(index=0) either sets the current value for generic
> attribute 0, or it emits a vertex just like glVertex().  I believe
> it should do either, depending on context (see below).
>
> The piglit gl-2.0-vertex-const-attr test declares two vertex attributes:
>   attribute vec2 vertex;
>   attribute vec4 attr;
> and the GLSL linker assigns "vertex" to location 0 and "attr" to location 1.
> The test passes.
>
> But if the declarations were reversed such that "attr" was location 0 and
> "vertex" was location 1, the test would fail to draw properly.
>
> The problem is the call to glVertexAttrib(index=0) to set attr's value
> was interpreted as glVertex() and did not set generic attribute[0]'s value.
> Interesting, calling glVertex() outside glBegin/End (which is effectively
> what the piglit test does) does not generate a GL error.
>
> I believe the behavior of glVertexAttrib(index=0) should depend on
> whether it's called inside or outside of glBegin/glEnd().  If inside
> glBegin/End(), it should act like glVertex().  Else, it should behave
> like glVertexAttrib(index > 0).  This seems to be what NVIDIA does.
>
> This patch makes two changes:
>
> 1. Check if we're inside glBegin/End for glVertexAttrib()
> 2. Fix the vertex array binding for recalculate_input_bindings().  As it was,
>    we were using &vbo->currval[VBO_ATTRIB_POS], but that's interpreted
>    as a zero-stride attribute and doesn't make sense for array drawing.
>
> No Piglit regressions.  Fixes updated gl-2.0-vertex-const-attr test and
> passes new gl-2.0-vertex-attrib-0 test.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101941
> ---
>  src/mesa/vbo/vbo_attrib_tmp.h | 7 +++++--
>  src/mesa/vbo/vbo_exec_array.c | 2 +-
>  2 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/src/mesa/vbo/vbo_attrib_tmp.h b/src/mesa/vbo/vbo_attrib_tmp.h
> index 5718ac5..126e4ef 100644
> --- a/src/mesa/vbo/vbo_attrib_tmp.h
> +++ b/src/mesa/vbo/vbo_attrib_tmp.h
> @@ -524,15 +524,18 @@ TAG(MultiTexCoord4fv)(GLenum target, const GLfloat * v)
>
>  /**
>   * If index=0, does glVertexAttrib*() alias glVertex() to emit a vertex?
> + * It depends on a few things, including whether we're inside or outside
> + * of glBegin/glEnd.
>   */
>  static inline bool
>  is_vertex_position(const struct gl_context *ctx, GLuint index)
>  {
> -   return index == 0 && _mesa_attr_zero_aliases_vertex(ctx);
> +   return (index == 0 &&
> +           _mesa_attr_zero_aliases_vertex(ctx) &&
> +           _mesa_inside_begin_end(ctx));
>  }
>
>
> -
>  static void GLAPIENTRY
>  TAG(VertexAttrib1fARB)(GLuint index, GLfloat x)
>  {
> diff --git a/src/mesa/vbo/vbo_exec_array.c b/src/mesa/vbo/vbo_exec_array.c
> index edd55ce..e3421fa 100644
> --- a/src/mesa/vbo/vbo_exec_array.c
> +++ b/src/mesa/vbo/vbo_exec_array.c
> @@ -356,7 +356,7 @@ recalculate_input_bindings(struct gl_context *ctx)
>           else if (array[VERT_ATTRIB_POS].Enabled)
>              inputs[0] = &vertexAttrib[VERT_ATTRIB_POS];
>           else {
> -            inputs[0] = &vbo->currval[VBO_ATTRIB_POS];
> +            inputs[0] = &vbo->currval[VBO_ATTRIB_GENERIC0];
>              const_inputs |= VERT_BIT_POS;
>           }
>
> --
> 1.9.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev