[Piglit] [PATCH v2 06/12] egl_android_native_fence_sync: Create fence from other display.

[Eric Anholt]

Rafael Antognolli <rafael.antognolli at intel.com> writes:

> On Fri, Jun 16, 2017 at 12:29:46PM -0700, Eric Anholt wrote:
>> Rafael Antognolli <rafael.antognolli at intel.com> writes:
>> 
>> > Verify that EGL_NO_SYNC_KHR is returned and EGL_BAD_MATCH error is
>> > generated.
>> >
>> > Signed-off-by: Rafael Antognolli <rafael.antognolli at intel.com>
>> > ---
>> >  .../egl_android_native_fence_sync.c                | 98 +++++++++++++++++++++-
>> >  1 file changed, 97 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/tests/egl/spec/egl_android_native_fence_sync/egl_android_native_fence_sync.c b/tests/egl/spec/egl_android_native_fence_sync/egl_android_native_fence_sync.c
>> > index eb01014..721bc44 100644
>> > --- a/tests/egl/spec/egl_android_native_fence_sync/egl_android_native_fence_sync.c
>> > +++ b/tests/egl/spec/egl_android_native_fence_sync/egl_android_native_fence_sync.c
>> > @@ -515,7 +515,7 @@ cleanup:
>> >   * Verify that eglCreateSyncKHR emits correct error when given an invalid
>> >   * display.
>> >   *
>> > - * From the EGL_KHR_fence_sync spec:
>> > + * From the EGL_ANDROID_native_fence_sync spec:
>> 
>> The citation appears in both specs.  Let's pick one and use it in the
>> commit adding the citation, rather than changing it here.
>> 
>> >   *
>> >   *     If <dpy> is not the name of a valid, initialized EGLDisplay,
>> >   *     EGL_NO_SYNC_KHR is returned and an EGL_BAD_DISPLAY error is
>> > @@ -546,6 +546,97 @@ test_eglCreateSyncKHR_native_invalid_display(void *test_data)
>> >  	return result;
>> >  }
>> >  
>> > +static enum piglit_result
>> > +init_other_display(EGLDisplay *out_other_dpy, EGLDisplay orig_dpy)
>> > +{
>> > +	enum piglit_result result = PIGLIT_PASS;
>> > +	EGLDisplay other_dpy = 0;
>> > +	int i;
>> > +
>> > +	static const EGLint platforms[] = {
>> > +		EGL_PLATFORM_X11_EXT,
>> > +		EGL_PLATFORM_WAYLAND_EXT,
>> > +		EGL_PLATFORM_GBM_MESA,
>> > +		0,
>> > +	};
>> > +
>> > +	for (i = 0; platforms[i] != 0; ++i) {
>> > +		result = init_display(platforms[i], &other_dpy);
>> > +		switch (result) {
>> > +			case PIGLIT_SKIP:
>> > +				break;
>> > +			case PIGLIT_PASS:
>> > +				if (other_dpy && other_dpy != orig_dpy) {
>> > +					*out_other_dpy = other_dpy;
>> > +					return PIGLIT_PASS;
>> > +				} else {
>> > +					result = PIGLIT_SKIP;
>> > +					break;
>> > +				}
>> > +			default:
>> > +				break;
>> 
>> Switch cases should be at the same indentation level as the switch
>> itself.
>> 
>> Actually, since this code looks like it's from
>> tests/egl/spec/egl_khr_fence_sync/egl_khr_fence_sync.c, could you just
>> move its init_other_display() function to a helper in
>> tests/util/piglit-util-egl.c?
>
> If I move it to piglit-util-egl.c, then I'll need to move init_display() as
> well. However, that one is checking for the sync extension, which is quite
> specific to these two tests, so it's probably not a good idea to move it
> there. And if we did, it would probably make sense to also move
> init_context(). But I'm not sure these functions belong there...
>
>> I do wonder if it would be possible to share more of our test code
>> between these two tests.  Seems messy, though.
>
> I think one idea would be to put the native fence tests inside
> egl_khr_fence_sync.c, so we could reuse a lot of these functions, and test
> functionality both for native and non-native fences. What do you think?

If you're up for it, I'd love to see you give it a shot.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/piglit/attachments/20170616/12852b07/attachment.sig>