On Fri, Jun 16, 2017 at 12:29:46PM -0700, Eric Anholt wrote:
> Rafael Antognolli <[email protected]> writes:
>
> > Verify that EGL_NO_SYNC_KHR is returned and EGL_BAD_MATCH error is
> > generated.
> >
> > Signed-off-by: Rafael Antognolli <[email protected]>
> > ---
> > .../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
> > [email protected]
> > https://lists.freedesktop.org/mailman/listinfo/piglit
_______________________________________________
Piglit mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/piglit
