[Piglit] [PATCH 5/5] egl: add eglCopyBuffers test

[Tapani Pälli]

On 13.12.2017 14:22, Emil Velikov wrote:
> Hi Tapani,
> 
> Thanks for having a look!
> 
> On 13 December 2017 at 06:10, Tapani Pälli <tapani.palli at intel.com> wrote:
>> Hi;
>>
>> On 11.12.2017 22:15, Emil Velikov wrote:
> 
>>> +       /* Set the env. variable to force the platform 'detection' to use
>>> +        * different platform (than X11).
>>> +        *
>>> +        * NOTE: This is not perfect, since driver may ignore the
>>> variable, yet
>>> +        * we aim to provide a consistent experience across test runs,
>>> build
>>> +        * permutation and/or driver used.
>>> +        */
>>> +//     setenv("EGL_PLATFORM", "drm", true);
> This must _not_ be commented out - silly mistake while doing some back
> and forth testing.

I was about to ask but was not sure :)

>>> +
>>> +       /* XXX: test should flag regardless of the following call -
>>> testing
>>> +        * has confirmed it.
>>> +        *
>>> +        * NOTE: We cannot do the test twice - with and W/o the call; the
>>> +        * detection result is stored in static variable :-\
>>> +        */
>>> +       piglit_egl_get_default_display(EGL_NONE);
>>> +
>>> +       /* Use X11 since it's the only platform that has EGL pixmap
>>> surfaces */
>>> +       piglit_require_egl_extension(EGL_NO_DISPLAY,
>>> "EGL_EXT_platform_x11");
>>> +}
>>> +
>>> +static enum piglit_result
>>> +draw(struct egl_state *state)
>>> +{
>>> +       EGLNativePixmapType pixmap;
>>> +       enum piglit_result result = PIGLIT_PASS;
>>> +
>>> +       /* Green for a pass */
>>> +       glClearColor(0.0, 1.0, 0.0, 1.0);
>>> +       glClear(GL_COLOR_BUFFER_BIT);
>>> +
>>> +       pixmap = egl_util_create_native_pixmap(state,
>>> egl_default_window_width,
>>> +                                              egl_default_window_height);
>>
>>
>> Should we call XFlush(dpy) here or is it ok to go and use pixmap right away?
>>
> Cannot see any such requirement in the XCreatePixmap manpage. Plus the
> existing piglit code using pixmaps doesn't do it :-\
> Out of curiosity - the extra XFlush does not help with the DRI3 crash
> mentioned earlier.

I was wondering if that would help but you are correct, it is not really 
required, Pixmap handle is valid right away.

>>> +       eglCopyBuffers(state->egl_dpy, state->surf, pixmap);
>>
>>
>> I think we could also read from pixmap and verify that it has green (in
>> every pixel)?
>>
>> There might be need some additional synchronization if doing that, at least
>> dEQP issues eglWaitClient() before reading pixmap contents.
>>
> The test doesn't care how well eglCopyBuffers itself works - aim it to
> illustrate the buggy validation in Mesa.
> Hence the wait + pixmap readback are not really needed.
> 
> Admittedly the test name is quite misleading as-is - I'm short on
> alternatives though :-(

OK, yeah that is fine. It's not far from 'complete test' though but such 
athing can be also added later.

// Tapani