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

[Brian Paul]

On 06/21/2017 09:35 AM, Thomas Hellstrom wrote:
> 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?

Oh, right, sorry.  I misread the patch.

-Brian

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