[Piglit] Spec justification for test_illegal_begin_mode?

[Brian Paul]

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.

-Brian