[Piglit] [PATCH] gl-1.0/swapbuffers-behavior: Try avoid reading from real front

[Brian Paul]

On 06/21/2017 08:34 AM, Thomas Hellstrom wrote:
> Hi Brian,
>
> On 06/21/2017 04:20 PM, Brian Paul wrote:
>> On 06/21/2017 06:24 AM, Thomas Hellstrom wrote:
>>> Window systems (read dri3) that allocate a fake frontbuffer on demand
>>> will
>>> effectively read out from the real front before the first swapbuffers
>>> after
>>> fake front creation, and the real front buffer content is subject to
>>> errors
>>> caused by, among other things, window reparenting. So increase the
>>> likelyhood
>>> of not reading out from the real front by advertizing early that we will
>>> access the front buffer. After all, real front consistency is not the
>>> objective
>>> of this test.
>>>
>>> Fixes sporadic failures due to window reparenting on dri3/vmwgfx.
>>>
>>> Cc: Brian Paul <brianp at vmware.com>
>>> Signed-off-by: Thomas Hellstrom <thellstrom at vmware.com>
>>> ---
>>>   tests/spec/gl-1.0/swapbuffers-behavior.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tests/spec/gl-1.0/swapbuffers-behavior.c
>>> b/tests/spec/gl-1.0/swapbuffers-behavior.c
>>> index d914020..15a07ac 100644
>>> --- a/tests/spec/gl-1.0/swapbuffers-behavior.c
>>> +++ b/tests/spec/gl-1.0/swapbuffers-behavior.c
>>> @@ -62,6 +62,7 @@ piglit_display(void)
>>>
>>>       /* Clear back buffer to green */
>>>       glDrawBuffer(GL_BACK);
>>> +    glReadBuffer(GL_FRONT);
>>
>> The comment no longer agrees with the code.
> I'll fix that.
>
>>
>>>       glClearColor(green[0], green[1], green[2], green[3]);
>>>       glClear(GL_COLOR_BUFFER_BIT);
>>>
>>> @@ -69,7 +70,6 @@ piglit_display(void)
>>>       piglit_swap_buffers();
>>>
>>>           /* Front buffer sanity-check */
>>> -    glReadBuffer(GL_FRONT);
>>
>> Same here.
>
> Actually that comment still makes sense since it's followed by a
> frontbuffer piglit_probe

Oh, right, because of the above change.


>>
>>
>>>       if (!piglit_probe_rect_rgb_silent(0, 0, piglit_width,
>>> piglit_height,
>>>                         green)) {
>>>           printf("SwapBuffers apparently failed!\n");
>>>
>>
>> These changes seem to defeat the purpose of the test.
>>
>> Would a different approach work?  How about after the initial
>> glViewport call we simply clear the front buffer?  That should trigger
>> creation of the fake front buffer, right?
>
> Yes, as long as we access the front buffer before SwapBuffers we should
> be safe. But I don't quite understand how creating the fake front buffer
> using glReadBuffer() would differ form creating it by a write access
> with respect to the test functionality?

glReadBuffer() just sets state and there's no guarantee that it by 
itself will trigger creation of a front buffer.  That may be how Mesa 
does things now, but that could change.  A buffer doesn't really have to 
exist until we try to put something in it.


> All this patch is doing is
> calling glReadBuffer() a little earlier than before?

But we have to clear the back buffer before the swap.  Otherwise, the 
front buffer contents will be undefined after the swap.

-Brian