Rafael Antognolli <[email protected]> writes:

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

If you're up for it, I'd love to see you give it a shot.

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Piglit mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/piglit

Reply via email to