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

[Rafael Antognolli]

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?

Anyway, thank you for the reviews!

> > +		}
> > +	}
> > +
> > +	return result;
> > +}
> > +
> > +/**
> > + * Verify that eglCreateSyncKHR() emits correct error when given a display that
> > + * does not match the display of the bound context.
> > + *
> > + * From the EGL_KHR_fence_sync spec:
> > + *
> > + *	* If <type> is EGL_SYNC_FENCE_KHR or EGL_SYNC_NATIVE_FENCE_ANDROID and
> > + *	  no context is current for the bound API (i.e., eglGetCurrentContext
> > + *	  returns EGL_NO_CONTEXT), EGL_NO_SYNC_KHR is returned and an
> > + *	  EGL_BAD_MATCH error is generated.
> 
> This citation seems to be in EGL_ANDROID_native_fence_sync, not KHR.
> Also, I think this is the wrong block of spec text -- you want the one
> about "does not match the EGLDisplay of the currently bound context". I
> was going to suggest gutting almost all of the test, given the original
> text :)

Yeah, sorry, I copied the wrong one...

> > +static enum piglit_result
> > +test_eglCreateSyncKHR_native_wrong_display_same_thread(void *test_data)
> > +{
> > +	enum piglit_result result = PIGLIT_PASS;
> > +	EGLDisplay wrong_dpy = 0;
> > +	EGLSyncKHR sync = 0;
> > +
> > +	result = test_setup();
> > +	if (result != PIGLIT_PASS) {
> > +		return result;
> > +	}
> > +
> > +	piglit_logi("create second EGLDisplay");
> > +	result = init_other_display(&wrong_dpy, g_dpy);
> > +	if (result != PIGLIT_PASS) {
> > +		goto cleanup;
> > +	}
> > +
> > +	piglit_require_egl_extension(wrong_dpy, "EGL_KHR_fence_sync");
> > +
> > +	piglit_logi("try to create sync with second display");
> > +	sync = peglCreateSyncKHR(wrong_dpy, EGL_SYNC_NATIVE_FENCE_ANDROID, NULL);
> > +	if (sync != EGL_NO_SYNC_KHR) {
> > +		piglit_loge("eglCreateSyncKHR() incorrectly succeeded");
> > +		result = PIGLIT_FAIL;
> > +		goto cleanup;
> > +	}
> > +	if (!piglit_check_egl_error(EGL_BAD_MATCH)) {
> > +		piglit_loge("eglCreateSyncKHR emitted wrong error");
> > +		result = PIGLIT_FAIL;
> > +		goto cleanup;
> > +	}
> > +
> > +cleanup:
> > +	if (wrong_dpy) {
> > +		eglTerminate(wrong_dpy);
> > +	}
> > +	test_cleanup(EGL_NO_SYNC_KHR, &result);
> > +	return result;
> > +}
> > +
> >  static const struct piglit_subtest fence_sync_subtests[] = {
> >  	{
> >  		"eglCreateSyncKHR_native_no_fence",
> > @@ -567,6 +658,11 @@ static const struct piglit_subtest fence_sync_subtests[] = {
> >  		"eglCreateSyncKHR_invalid_display",
> >  		test_eglCreateSyncKHR_native_invalid_display,
> >  	},
> > +	{
> > +		"eglCreateSyncKHR_wrong_display_same_thread",
> > +		"eglCreateSyncKHR_wrong_display_same_thread",
> > +		test_eglCreateSyncKHR_native_wrong_display_same_thread,
> > +	},
> >  	{0},
> >  };
> >  
> > -- 
> > 2.7.4
> >
> > _______________________________________________
> > Piglit mailing list
> > Piglit at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/piglit