On 04/16/2013 12:45 PM, Topi Pohjolainen wrote:
These are rather simple. Without the capability of creating
textures out of the images (glEGLImageTargetTexture2DOES()), one
cannot do much with the images as such.

Extra attention should be paid to the buffer lifecycle:

    "3. Does ownership of the file descriptor pass to the EGL library?

     ANSWER: If eglCreateImageKHR is successful, EGL assumes ownership
     of the file descriptors and is responsible for closing them."

In the tests I've chosen the approach where the creator releases
its own access to the buffers as at least in Intel's case I cannot
see how the EGL driver could do it on the creators behalf.

Signed-off-by: Topi Pohjolainen <[email protected]>
---
  tests/spec/CMakeLists.txt                          |   1 +
  .../ext_image_dma_buf_import/CMakeLists.gles1.txt  |  19 ++
  tests/spec/ext_image_dma_buf_import/CMakeLists.txt |   1 +
  tests/spec/ext_image_dma_buf_import/close_buffer.c | 116 ++++++++++
  .../spec/ext_image_dma_buf_import/create_yuv420.c  | 145 ++++++++++++
  .../ext_image_dma_buf_import/invalid_attributes.c  | 255 +++++++++++++++++++++
  .../spec/ext_image_dma_buf_import/invalid_hints.c  | 144 ++++++++++++
  .../ext_image_dma_buf_import/missing_attributes.c  | 182 +++++++++++++++
  8 files changed, 863 insertions(+)
  create mode 100644 tests/spec/ext_image_dma_buf_import/CMakeLists.gles1.txt
  create mode 100644 tests/spec/ext_image_dma_buf_import/CMakeLists.txt
  create mode 100644 tests/spec/ext_image_dma_buf_import/close_buffer.c
  create mode 100644 tests/spec/ext_image_dma_buf_import/create_yuv420.c
  create mode 100644 tests/spec/ext_image_dma_buf_import/invalid_attributes.c
  create mode 100644 tests/spec/ext_image_dma_buf_import/invalid_hints.c
  create mode 100644 tests/spec/ext_image_dma_buf_import/missing_attributes.c



diff --git a/tests/spec/ext_image_dma_buf_import/close_buffer.c 
b/tests/spec/ext_image_dma_buf_import/close_buffer.c


+/**
+ * @file close_buffer.c
+ *
+ * From the EXT_image_dma_buf_import spec:
+ *
+ * "3. Does ownership of the file descriptor pass to the EGL library?
+ *
+ *   ANSWER: If eglCreateImageKHR is successful, EGL assumes ownership of the
+ *   file descriptors and is responsible for closing them."

In don't understand why this issue is quoted here, so it feels like noise to me.
A little more explanation about its context would help. Are you somehow testing
this text?


diff --git a/tests/spec/ext_image_dma_buf_import/create_yuv420.c 
b/tests/spec/ext_image_dma_buf_import/create_yuv420.c

create_yuv420.c looks good to me.


diff --git a/tests/spec/ext_image_dma_buf_import/invalid_attributes.c 
b/tests/spec/ext_image_dma_buf_import/invalid_attributes.c


+static bool
+test_excess_attributes(int fd, unsigned w, unsigned h, unsigned stride,
+               unsigned offset)
+{
+       unsigned i;
+       const EGLint excess_attributes[][2 * 7 + 1] = {
+               { EGL_HEIGHT, w,
+                 EGL_WIDTH, h,
+                 EGL_LINUX_DRM_FOURCC_EXT, DRM_FORMAT_ARGB8888,
+                 EGL_DMA_BUF_PLANE0_FD_EXT, fd,
+                 EGL_DMA_BUF_PLANE0_OFFSET_EXT, offset,
+                 EGL_DMA_BUF_PLANE0_PITCH_EXT, stride,
+                 EGL_DMA_BUF_PLANE1_FD_EXT, fd,
+                 EGL_NONE },
+               { EGL_HEIGHT, w,
+                 EGL_WIDTH, h,
+                 EGL_LINUX_DRM_FOURCC_EXT, DRM_FORMAT_ARGB8888,
+                 EGL_DMA_BUF_PLANE0_FD_EXT, fd,
+                 EGL_DMA_BUF_PLANE0_OFFSET_EXT, offset,
+                 EGL_DMA_BUF_PLANE0_PITCH_EXT, stride,
+                 EGL_DMA_BUF_PLANE1_OFFSET_EXT, 0,
+                 EGL_NONE },
+               { EGL_HEIGHT, w,
+                 EGL_WIDTH, h,
+                 EGL_LINUX_DRM_FOURCC_EXT, DRM_FORMAT_ARGB8888,
+                 EGL_DMA_BUF_PLANE0_FD_EXT, fd,
+                 EGL_DMA_BUF_PLANE0_OFFSET_EXT, offset,
+                 EGL_DMA_BUF_PLANE0_PITCH_EXT, stride,
+                 EGL_DMA_BUF_PLANE1_PITCH_EXT, stride,
+                 EGL_NONE },
+       };
+
+       for (i = 0; i < sizeof(excess_attributes) /
+                       sizeof(excess_attributes[0]); ++i) {

This should use the ARRAY_SIZE defined in piglit-util.h.

+               /**
+                * The spec says:
+                *
+                *  "If <target> is EGL_LINUX_DMA_BUF_EXT, <dpy> must be a valid
+                *   display, <ctx> must be EGL_NO_CONTEXT, and <buffer> must be
+                *   NULL, cast into the type EGLClientBuffer."
+                */
+               EGLImageKHR img = eglCreateImageKHR(eglGetCurrentDisplay(),
+                               EGL_NO_CONTEXT, EGL_LINUX_DMA_BUF_EXT,
+                               (EGLClientBuffer)NULL, excess_attributes[i]);
+
+               if (!piglit_check_egl_error(EGL_BAD_ATTRIBUTE)) {
+                       if (img)
+                               eglDestroyImageKHR(eglGetCurrentDisplay(), img);
+                       return false;
+               }
+       }
+
+       return true;
+}
+
+static bool
+test_buffer_not_null(int fd, unsigned w, unsigned h, unsigned stride,
+               unsigned offset)
+{
+       EGLImageKHR img;
+       EGLint attr[] = {
+               EGL_WIDTH, w,
+               EGL_HEIGHT, h,
+               EGL_LINUX_DRM_FOURCC_EXT, DRM_FORMAT_ARGB8888,
+               EGL_DMA_BUF_PLANE0_FD_EXT, fd,
+               EGL_DMA_BUF_PLANE0_OFFSET_EXT, offset,
+               EGL_DMA_BUF_PLANE0_PITCH_EXT, stride,
+               EGL_NONE
+       };
+
+       /**
+        * The spec says:
+        *
+        *     "If <target> is EGL_LINUX_DMA_BUF_EXT, <dpy> must be a valid
+        *      display, <ctx> must be EGL_NO_CONTEXT, and <buffer> must be
+        *      NULL, cast into the type EGLClientBuffer."
+        */
+       img = eglCreateImageKHR(eglGetCurrentDisplay(), EGL_NO_CONTEXT,
+                       EGL_LINUX_DMA_BUF_EXT, (EGLClientBuffer)1, attr);
+
+       if (!piglit_check_egl_error(EGL_BAD_PARAMETER)) {
+               if (img)
+                       eglDestroyImageKHR(eglGetCurrentDisplay(), img);
+               return false;
+       }

In addition to verifying that EGL_BAD_PARAMETER is raised when `buffer != NULL`,
we also need a test that verifies EGL_BAD_CONTEXT is raised when `ctx != 
EGL_NO_CONTEXT`.

+
+       return true;
+}


+
+/**
+ * On re-uses the buffer for all the tests. The spec says:

I'm not sure what you mean here. Did you mean "The same buffer is re-used for all 
tests"?

+ *
+ * "If <target> is EGL_LINUX_DMA_BUF_EXT and eglCreateImageKHR fails,
+ *  EGL does not retain ownership of the file descriptor and it is the
+ *  responsibility of the application to close it."

Again, I don't understand why the fd comment is here, so it feels like noise
to me. Please explain.

+ */
+enum piglit_result
+piglit_display(void)


diff --git a/tests/spec/ext_image_dma_buf_import/invalid_hints.c 
b/tests/spec/ext_image_dma_buf_import/invalid_hints.c


+/**
+ * On re-uses the buffer for all the tests. The spec says:
+ *
+ * "If <target> is EGL_LINUX_DMA_BUF_EXT and eglCreateImageKHR fails,
+ *  EGL does not retain ownership of the file descriptor and it is the
+ *  responsibility of the application to close it."

Same questions as above.

+ */
+enum piglit_result
+piglit_display(void)


diff --git a/tests/spec/ext_image_dma_buf_import/missing_attributes.c 
b/tests/spec/ext_image_dma_buf_import/missing_attributes.c


+static bool
+test_all(int fd, unsigned w, unsigned h, unsigned stride, unsigned offset)
+{
+       /* there are six mandatory attributes */

It took me a minute to see that this array did. A comment here would be nice
that quickly explains that each array has exactly one mandatory attribute
removed.

+       const EGLint missing_attributes[][2 * 5 + 1] = {
+               { EGL_HEIGHT, h,
+                 EGL_LINUX_DRM_FOURCC_EXT, DRM_FORMAT_ARGB8888,
+                 EGL_DMA_BUF_PLANE0_FD_EXT, fd,
+                 EGL_DMA_BUF_PLANE0_OFFSET_EXT, offset,
+                 EGL_DMA_BUF_PLANE0_PITCH_EXT, stride,
+                 EGL_NONE },
+               { EGL_WIDTH, w,
+                 EGL_LINUX_DRM_FOURCC_EXT, DRM_FORMAT_ARGB8888,
+                 EGL_DMA_BUF_PLANE0_FD_EXT, fd,
+                 EGL_DMA_BUF_PLANE0_OFFSET_EXT, offset,
+                 EGL_DMA_BUF_PLANE0_PITCH_EXT, stride,
+                 EGL_NONE },
+               { EGL_WIDTH, w,
+                 EGL_HEIGHT, h,
+                 EGL_DMA_BUF_PLANE0_FD_EXT, fd,
+                 EGL_DMA_BUF_PLANE0_OFFSET_EXT, offset,
+                 EGL_DMA_BUF_PLANE0_PITCH_EXT, stride,
+                 EGL_NONE },
+               { EGL_WIDTH, w,
+                 EGL_HEIGHT, h,
+                 EGL_LINUX_DRM_FOURCC_EXT, DRM_FORMAT_ARGB8888,
+                 EGL_DMA_BUF_PLANE0_OFFSET_EXT, offset,
+                 EGL_DMA_BUF_PLANE0_PITCH_EXT, stride,
+                 EGL_NONE },
+               { EGL_WIDTH, w,
+                 EGL_HEIGHT, h,
+                 EGL_LINUX_DRM_FOURCC_EXT, DRM_FORMAT_ARGB8888,
+                 EGL_DMA_BUF_PLANE0_FD_EXT, fd,
+                 EGL_DMA_BUF_PLANE0_PITCH_EXT, stride,
+                 EGL_NONE },
+               { EGL_WIDTH, w,
+                 EGL_HEIGHT, h,
+                 EGL_LINUX_DRM_FOURCC_EXT, DRM_FORMAT_ARGB8888,
+                 EGL_DMA_BUF_PLANE0_FD_EXT, fd,
+                 EGL_DMA_BUF_PLANE0_OFFSET_EXT, offset,
+                 EGL_NONE },
+       };
+       bool pass = true;
+       unsigned i;
+
+       for (i = 0; i < sizeof(missing_attributes) /
+                       sizeof(missing_attributes[0]); ++i) {

ARRAY_SIZE

+               pass &= test_missing(fd, missing_attributes[i]);
+       }
+
+       return pass;
+}
+
+/**
+ * On re-uses the buffer for all the tests. The spec says:
+ *
+ * "If <target> is EGL_LINUX_DMA_BUF_EXT and eglCreateImageKHR fails,
+ *  EGL does not retain ownership of the file descriptor and it is the
+ *  responsibility of the application to close it."

Again, same questions as above.

+ */
+enum piglit_result
+piglit_display(void)
_______________________________________________
Piglit mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/piglit

Reply via email to