[Mesa-dev] Regressions due to commit 1afe3359258

Tue Apr 22 11:36:30 PDT 2014

On Tue, Apr 22, 2014 at 8:54 AM, Ian Romanick <idr at freedesktop.org> wrote:
> 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.

I don't think tests/spec/glsl-1.50/execution/geometry/point-size-out.shader_test
will use draw_rect, right?

There is no VBO with attributeless rendering, right? It's a
draw-arrays call w/o a VBO, right?

-Jordan

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