[Piglit] Spec justification for test_illegal_begin_mode?

Wed Dec 9 00:47:01 UTC 2020

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://gitlab.freedesktop.org/idr/piglit/-/commits/display-list-hell
>> <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%7C1bece8f6bb15456a704108d892278cef%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637420045848819859%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=CGW3oxTFndTrF4fVU8EADRhSr1ORAODexsSgl4pYsjc%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" }

> -Brian