[Piglit] Spec justification for test_illegal_begin_mode?

Wed Dec 9 18:19:28 UTC 2020

On 12/9/20 7:55 AM, Brian Paul wrote:
> On 12/08/2020 05:47 PM, Ian Romanick wrote:
>> On 12/1/20 7:43 AM, Brian Paul wrote:
>>> On 11/26/2020 09:22 AM, Brian Paul wrote:
>>>> On Wed, Nov 25, 2020 at 8:19 PM Ian Romanick <idr at freedesktop.org
>>>> <mailto:idr at freedesktop.org>> wrote:
>>>>
>>>>      I was looking at the dlist-beginend subtest
>>>> test_illegal_begin_mode.
>>>>      The test compiles a display list with GL_COMPILE_AND_EXECUTE
>>>> mode.  The
>>>>      glBegin in the test uses an invalid mode.  This should generate a
>>>>      GL_INVALID_OPERATION error immediately.  The test checks this, and
>>>> that
>>>>
>>>>
>>>> GL_INVALID_ENUM, actually, but anyway..
>>>>
>>>>
>>>>      seems correct.
>>>>
>>>>      The test then tries to execute the display list and checks for
>>>>      GL_INVALID_OPERATION again.  I believe this is incorrect.  Section
>>>> 5.4
>>>>      ("Display Lists") of the OpenGL 1.0 spec (this is a gl-1.0 test,
>>>> after
>>>>      all!) says:
>>>>
>>>>           If mode is COMPILE_AND_EXECUTE then commands are executed as
>>>> they
>>>>           are encountered, then placed in the display list.
>>>>
>>>>      Section 2.5 ("GL Errors") of the OpenGL 1.0 spec says:
>>>>
>>>>           In other cases, the command generating the error is ignored
>>>> so that
>>>>           it has no effect on GL state or framebuffer contents.
>>>>
>>>>
>>>> Unfortunately, the spec doesn't clearly define what's meant by
>>>> "ignored" here.  That is, does it mean the command is not compiled
>>>> into the display list?  My intuition is that even if a command has
>>>> invalid args at compile time, the command should still be compiled
>>>> into the list.  If the spec doesn't spell out the answer, I'd opt to
>>>> do whatever other vendors do (hoping they're consistent).
>>>>
>>>>
>>>>      While the section numbers and titles have changed, every later
>>>> version
>>>>      of the GL (compatibility profile) spec says the exact same thing.
>>>>
>>>>      To me, this indicates that the errant glBegin should not be
>>>> included in
>>>>      the display list.  Assuming this test passes on other
>>>> implementations, I
>>>>      think it does so only by luck.  The mis-matched glEnd in the
>>>> display
>>>>      list will generate the same error.  Am I missing some spec
>>>> language to
>>>>      justify this test?
>>>>
>>>>
>>>> I haven't searched, but I suspect the spec doesn't explicitly describe
>>>> this situation.
>>>>
>>>>
>>>>      I noticed this because I was adding a test that does
>>>>
>>>>               glNewList(list, GL_COMPILE_AND_EXECUTE);
>>>>               glColor4fv(green);
>>>>               glBegin(GL_QUADS);
>>>>               glBegin(GL_QUADS);
>>>>               glVertex2f(-1, -1);
>>>>               glVertex2f( 1, -1);
>>>>               glVertex2f( 1, 0);
>>>>               glVertex2f(-1, 0);
>>>>               glEnd();
>>>>               glEndList();
>>>>
>>>>      I expected an error while compiling the list, but I expected
>>>> glCallList
>>>>      to not generate an error.  Errors are generated in both places.
>>>>
>>>>
>>>> I guess my instinct is that errors should be generated both at list
>>>> compile and execute time.  I wonder if there's any API function /
>>>> argument combinations that would be errant at compile time but not at
>>>> execute time, depending on some other state.  If so, that would
>>>> indicate that calls should always be compiled into the list, even if
>>>> they generate an error at compile time.
>>>>
>>>>
>>>>      I think it gets worse.  There's a second part to this new subtest
>>>> that
>>>>      replaces the errant glBegin call with glReadPixels.  glReadPixels
>>>> is not
>>>>      allowed between glBegin and glEnd, so it should generate an
>>>> error in
>>>>      GL_COMPILE_AND_EXECUTE mode.  It does not.
>>>>
>>>>
>>>> Sound like a run of the mill bug.
>>>>
>>>>
>>>>      These changes are in the top commit of:
>>>>
>>>>    
>>>> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fidr%2Fpiglit%2F-%2Fcommits%2Fdisplay-list-hell&data=04%7C01%7Cbrianp%40vmware.com%7C376ec4dc8fd44f30077408d89bdbf4d6%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C1%7C637430716301345623%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=T1KlHoWIe2v%2Fr6ok86UXE7VEUu6fM78ePhJ1PCjqu%2Bs%3D&reserved=0
>>>>
>>>> <https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fidr%2Fpiglit%2F-%2Fcommits%2Fdisplay-list-hell&data=04%7C01%7Cbrianp%40vmware.com%7C376ec4dc8fd44f30077408d89bdbf4d6%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C1%7C637430716301355618%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=9Ud1G%2BEk8CnZm8Yv1YQcCes7dDwZnkDt8wbioWOtb9E%3D&reserved=0>
>>>>
>>>>
>>>>
>>>>      I don't have any systems currently available with closed-source
>>>> drivers,
>>>>      but I would be very interested to see the results of
>>>>      gl-1.0-dlist-beginend from that branch on any closed source
>>>> drivers (on
>>>>      any platform).
>>>>
>>>>
>>>> I can check on Nvidia next week when I'm back.  Maybe someone else can
>>>> test in the mean time.
>>>
>>> Here's the results with Nvidia's 455.38 driver:
>>>
>>> $  build-debug/bin/gl-1.0-dlist-beginend -auto
>>> PIGLIT: {"enumerate subtests": ["glCallList inside glBegin-glEnd",
>>> "nested glCallList inside glBegin-glEnd", "illegal glRect inside
>>> glBegin-glEnd", "illegal glDrawArrays inside glBegin-glEnd", "illegal
>>> glDrawArrays inside glBegin-glEnd (2)", "separate glBegin-glVertex-glEnd
>>> lists", "illegal glBegin mode in display list"]}
>>> PIGLIT: {"subtest": {"glCallList inside glBegin-glEnd" : "pass"}}
>>> PIGLIT: {"subtest": {"nested glCallList inside glBegin-glEnd" : "pass"}}
>>> PIGLIT: {"subtest": {"illegal glRect inside glBegin-glEnd" : "pass"}}
>>> Probe color at (80,80)
>>>    Expected: 0.000000 1.000000 0.000000 1.000000
>>>    Observed: 1.000000 0.000000 0.000000 1.000000
>>> PIGLIT: {"subtest": {"illegal glDrawArrays inside glBegin-glEnd" :
>>> "fail"}}
>>> PIGLIT: {"subtest": {"illegal glDrawArrays inside glBegin-glEnd (2)" :
>>> "pass"}}
>>> PIGLIT: {"subtest": {"separate glBegin-glVertex-glEnd lists" : "pass"}}
>>> PIGLIT: {"subtest": {"illegal glBegin mode in display list" : "pass"}}
>>> PIGLIT: {"result": "fail" }
>>>
>>> Looks like the subtest in question passes.
>>
>>
>> Uh... I don't think that's the right branch.  There should be some new
>> subtests with GL_COMPILE_AND_EXECUTE in the name.  On current Mesa, I
>> get output more like:
>>
>> $ mesa debug bin/gl-1.0-dlist-beginend -auto
>> PIGLIT: {"enumerate subtests": ["glCallList inside glBegin-glEnd",
>> "nested glCallList inside glBegin-glEnd", "illegal glRect inside
>> glBegin-glEnd", "illegal glDrawArrays inside glBegin-glEnd", "illegal
>> glDrawArrays inside glBegin-glEnd (2)", "separate
>> glBegin-glVertex-glEnd lists", "illegal glBegin mode in display list",
>> "glBegin-glEnd inside GL_COMPILE_AND_EXECUTE", "glBegin-glBegin-glEnd
>> inside GL_COMPILE", "glBegin-glBegin-glEnd inside
>> GL_COMPILE_AND_EXECUTE"]}
>> PIGLIT: {"subtest": {"glCallList inside glBegin-glEnd" : "pass"}}
>> PIGLIT: {"subtest": {"nested glCallList inside glBegin-glEnd" : "pass"}}
>> PIGLIT: {"subtest": {"illegal glRect inside glBegin-glEnd" : "pass"}}
>> PIGLIT: {"subtest": {"illegal glDrawArrays inside glBegin-glEnd" :
>> "pass"}}
>> PIGLIT: {"subtest": {"illegal glDrawArrays inside glBegin-glEnd (2)" :
>> "pass"}}
>> PIGLIT: {"subtest": {"separate glBegin-glVertex-glEnd lists" : "pass"}}
>> PIGLIT: {"subtest": {"illegal glBegin mode in display list" : "pass"}}
>> PIGLIT: {"subtest": {"glBegin-glEnd inside GL_COMPILE_AND_EXECUTE" :
>> "pass"}}
>> PIGLIT: {"subtest": {"glBegin-glBegin-glEnd inside GL_COMPILE" : "pass"}}
>> Unexpected GL error: GL_INVALID_OPERATION 0x502
>> (Error at
>> /home/idr/devel/graphics/piglit/tests/spec/gl-1.0/dlist-beginend.c:182)
>> Unexpected GL error: GL_NO_ERROR 0x0
>> (Error at
>> /home/idr/devel/graphics/piglit/tests/spec/gl-1.0/dlist-beginend.c:203)
>> Expected GL error: GL_INVALID_OPERATION 0x502
>> PIGLIT: {"subtest": {"glBegin-glBegin-glEnd inside
>> GL_COMPILE_AND_EXECUTE" : "fail"}}
>> PIGLIT: {"result": "fail" }
> 
> I'm on master branch and the last change to the test is:

Can you try the branch that I linked before? :)

https://gitlab.freedesktop.org/idr/piglit/-/tree/display-list-hell

The top commits should be:

937f4bcd0 dlist-beginend: Test for errors in display lists with GL_COMPILE_AND_EXECUTE
eb2cb99bf dlist-beginend: Test for errors in display lists with GL_COMPILE
2a87084b2 nv_primitive_restart: Test for errors in display lists with GL_COMPILE_AND_EXECUTE
4d667966e dlist-beginend: Try drawing with GL_COMPILE_AND_EXECUTE
dea7f61b3 ext_gpu_shader4: Test gl_PrimitiveID when combined with display lists
829d5c039 nv_primitive_restart: Add basic state set and get tests

> $ git log tests/spec/gl-1.0/dlist-beginend.c
> commit c67e07592991a2455cebbc1bc58147b9e5819c0b
> Author: Dylan Baker <dylan at pnwbakers.com>
> Date:   Thu Jun 28 11:31:49 2018 -0700
> 
>     tests: use subtest frameowrk in gl-1.0-dlist-beginend
> 
> 
> -Brian