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

[Thomas Hellstrom]

On 06/21/2017 05:22 PM, Brian Paul wrote:
> 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.
>
>

True. That will be a bit more generic, but otoh the on-demand creation 
of a fake front in the first place is probably a bit dri3-specific.

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

But we're still clearing the back buffer before the swap, right? The 
draw render buffer hasn't changed?

> -Brian
>
In any case, I'll modify the patch to clear the frontbuffer before the 
first swapbuffer. It will make it more clear what we're doing and why.

/Thomas