[Mesa-dev] Regressions due to commit 1afe3359258

[Ian Romanick]

On 04/19/2014 11:44 AM, Jordan Justen wrote:
> 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.

The thing that isn't clear to me from the previous discussion... on
up-to-date NVIDIA drivers, do these tests pass or fail?  AMD?  If the
tests run, unmodified, on other vendors and we think the spec is overly
restrictive, we (I) should file a spec bug.  If the test fails (due to
GL_INVALID_OPERATION being generated), we should fix shader_runner.

> 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).

That bug sounds like shader_runner was using a VAO without a VBO.
Presumably the use of piglit_draw_rect* is to blame.

> 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
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev