[Mesa-dev] mistake in "mesa/vbo: move some Draw checks out of validation"?

[Timothy Arceri]

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 <tarceri at itsqueeze.com>
> 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 <nicolai.haehnle at amd.com>
> 
> 
> 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