[Mesa-dev] Regressions due to commit 1afe3359258

[Jordan Justen]

On Sat, Apr 19, 2014 at 10:03 AM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> On Fri, Apr 18, 2014 at 1:35 PM, Jordan Justen
> <jordan.l.justen at intel.com> wrote:
>> On 2014-04-18 08:46:00, Kenneth Graunke wrote:
>>> On 04/18/2014 12:09 AM, Ilia Mirkin wrote:
>>> > Hi Ken,
>>> >
>>> > I just did a bisect looking for the failure that's causing a few
>>> > gs-related piglits to fail on nv50, and it came up with the below. Any
>>> > ideas? Here are the tests that are failing:
>>> >
>>> > tests/spec/glsl-1.50/execution/geometry/clip-distance-out-values.shader_test
>>> > tests/spec/glsl-1.50/execution/geometry/max-input-components.shader_test
>>> > tests/spec/glsl-1.50/execution/geometry/point-size-out.shader_test
>>> > tests/spec/glsl-1.50/execution/geometry/primitive-id-out.shader_test
>>> > tests/spec/glsl-1.50/execution/gs-redeclares-both-pervertex-blocks.shader_test
>>> > tests/spec/glsl-1.50/execution/interface-vs-named-to-gs-array.shader_test
>>> > tests/spec/glsl-1.50/execution/redeclare-pervertex-out-subset-gs.shader_test
>>> > tests/spec/glsl-1.50/execution/redeclare-pervertex-subset-vs-to-gs.shader_test
>>> >
>>> > 1afe3359258a9e89b62c8638761f52d78f6d1cbc is the first bad commit
>>> > commit 1afe3359258a9e89b62c8638761f52d78f6d1cbc
>>> > Author: Kenneth Graunke <kenneth at whitecape.org>
>>> > Date:   Thu Mar 20 11:53:16 2014 -0700
>>> >
>>> >     mesa: In core profile, refuse to draw unless a VAO is bound.
>>> >
>>> >     Core profile requires a non-default VAO to be bound.  Currently, calls
>>> >     to glVertexAttribPointer raise INVALID_OPERATION unless a VAO is bound,
>>> >     and we never actually get any vertex data set.  Trying to draw without
>>> >     any vertex data can only cause problems.  In i965, it causes a crash.
>>> >
>>> >     Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76400
>>> >     Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
>>> >     Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>
>>> >     Cc: mesa-stable at lists.freedesktop.org
>>> >
>>> > Reverting this commit on top of master makes it work again. I have no
>>> > idea whether it's the tests/piglit infra that are wrong, or if it's
>>> > the commit that's wrong, but wanted to raise the issue. This still
>>> > happens with the (almost-)latest piglit (latest doesn't compile due to
>>> > EGL stuff...)
>>> >
>>> > Let me know if you'd like any additional info.
>>> >
>>> > Thanks,
>>> >
>>> >   -ilia
>>>
>>> Yeah, we're seeing those too.  I believe the commit is wrong: with
>>> geometry shaders, you can just generate vertices using a GS program and
>>> not actually ever upload any vertex data.  At which point, you probably
>>> don't need a VAO.  I haven't double checked the rules, though.
>>
>> I'm not sure spec-wise, but I found that nvidia requires a VAO, even
>> without vertex data. (See piglit b28cdc88, but note that this commit
>> caused some other issues and was reverted in c20596b8).
>
> So this is what the spec says (GL4.4, page 322/323):
>
> An INVALID_OPERATION error is generated by any commands which
> modify, draw from, or query vertex array state when no vertex array is bound.
> This occurs in the initial GL state, and may occur as a result of
> BindVertexArray
> or a side effect of DeleteVertexArrays.
>
> And this is what one of the piglits in question does:
>
> [test]
> clear color 0.0 0.0 0.0 0.0
> clear
> draw arrays GL_TRIANGLES 0 3
> probe all rgba 0.0 1.0 0.0 1.0
>
> and this is what shader_runner does:
>
> if (link_ok && vertex_data_start != NULL) {
>   program_must_be_in_use();
>   if (gl_version.num >= 31) {
>     GLuint vao;
>
>     glGenVertexArrays(1, &vao);
>     glBindVertexArray(vao);
>   }
>
>   num_vbo_rows = setup_vbo_from_text(prog, vertex_data_start,
>     vertex_data_end);
>   vbo_present = true;
> }
>
> So.... what's the right move here? I think it might actually be to
> change shader_runner to always generate a VAO even if there is no
> vertex data.

Right, and that is what I did in piglit b28cdc88, but:
http://lists.freedesktop.org/archives/piglit/2014-April/010179.html
caused me to revert it (c20596b8).

Brian pointed out tests/shaders/glsl-fs-any.shader_test as an example
that failed on nvidia with b28cdc88.

Yet, b28cdc88 fixes
tests/spec/glsl-1.50/execution/geometry/point-size-out.shader_test on
nvidia, so hopefully there is a way to make nvidia happy on both
tests. :)

-Jordan