On 19/08/17 08:02, Brian Paul wrote:
Hi Timothy,

I happened to be looking at the VBO vertex array code and I think I spotted a mistake in this commit:

commit 4df2931a87fe082f90871564a89a09c826641f5b
Author: Timothy Arceri <[email protected]>
Date:   Mon Apr 3 16:38:18 2017 +1000

     mesa/vbo: move some Draw checks out of validation

     These checks do not generate any errors. Move them so we can add
     KHR_no_error support and still make sure we do these checks.

     Reviewed-by: Nicolai Hähnle <[email protected]>


Specifically, the old code in check_valid_to_render() to check whether vertex attribute array 0 is enabled for API_OPENGLES was:

        /* For OpenGL ES, only draw if we have vertex positions
         */
        if (!ctx->Array.VAO->VertexAttrib[VERT_ATTRIB_POS].Enabled)
           return false;  //BP: don't draw

your patch moved this into skip_validated_draw() where the return value is basically inverted from the old code.

       /* For OpenGL ES, only draw if we have vertex positions
        */
       if (ctx->Array.VAO->VertexAttrib[VERT_ATTRIB_POS].Enabled)
          return false; >        break;

if we don't return false there, we'll unconditionally return false at the end of the function (i.e. we can never return true).

I think the above condition should read:

       if (!ctx->Array.VAO->VertexAttrib[VERT_ATTRIB_POS].Enabled)
          return true;  //BP: skip draw

The //BP: comments are added by me.

I guess we don't have any piglit tests for ES that check that drawing is skipped when the vertex position array is disabled.

-Brian

Hi Brian,

Yes your assessment does seem correct to me. I can write a patch but I won't get to it until later today or tomorrow.

Thanks,
Tim
_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to