[Mesa-dev] Regressions due to commit 1afe3359258

[Ilia Mirkin]

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. I haven't checked, but I suspect it would also fix the
piglit failures, and leave the current mesa code in place.

Thoughts?

  -ilia