Topi Pohjolainen <[email protected]> writes: > diff --git a/tests/spec/ext_image_dma_buf_import/invalid_attributes.c > b/tests/spec/ext_image_dma_buf_import/invalid_attributes.c > new file mode 100644 > index 0000000..a6a6954 > --- /dev/null > +++ b/tests/spec/ext_image_dma_buf_import/invalid_attributes.c > @@ -0,0 +1,286 @@ > +/* > + * Copyright © 2013 Intel Corporation > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the next > + * paragraph) shall be included in all copies or substantial portions of the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER > DEALINGS > + * IN THE SOFTWARE. > + */ > + > +#include "piglit-util-egl.h" > +#define EGL_EGLEXT_PROTOTYPES 1 > +#include <EGL/eglext.h> > +#include <unistd.h> > +#include "ext_image_dma_buf_fourcc.h" > + > +/** > + * @file invalid_attributes.c > + * > + * From the EXT_image_dma_buf_import spec: > + * > + * "* If <target> is EGL_LINUX_DMA_BUF_EXT and <buffer> is not NULL, the > + * error EGL_BAD_PARAMETER is generated. > + * > + * * If <target> is EGL_LINUX_DMA_BUF_EXT, and the EGL_LINUX_DRM_FOURCC_EXT > + * attribute is set to a format not supported by the EGL, EGL_BAD_MATCH > + * is generated. > + * > + * * If <target> is EGL_LINUX_DMA_BUF_EXT, and the EGL_LINUX_DRM_FOURCC_EXT > + * attribute indicates a single-plane format, EGL_BAD_ATTRIBUTE is > + * generated if any of the EGL_DMA_BUF_PLANE1_* or EGL_DMA_BUF_PLANE2_* > + * attributes are specified. > + * > + * * If <target> is EGL_LINUX_DMA_BUF_EXT and one or more of the values > + * specified for a plane's pitch or offset isn't supported by EGL, > + * EGL_BAD_ACCESS is generated." > + */ > + > +PIGLIT_GL_TEST_CONFIG_BEGIN > + > + config.supports_gl_es_version = 10; > + > +PIGLIT_GL_TEST_CONFIG_END > + > +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 }, > + };
I think this particular subtest could be better done like the
test_invalid_hints, where a single attribute as a function parameter is
plugged into the stock array. It would clarify what's going on in the
test, and then you could easily do PLANE2_* checks, too.
> + for (i = 0; i < ARRAY_SIZE(excess_attributes); ++i) {
> + /**
> + * 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."
> + */
Please remove this copypasta from the eglCreateImageKHR callsites that
aren't testing this particular spec text.
> + 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_invalid_context(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(), eglGetCurrentContext(),
> + EGL_LINUX_DMA_BUF_EXT, (EGLClientBuffer)NULL, attr);
> +
> + if (!piglit_check_egl_error(EGL_BAD_CONTEXT)) {
> + if (img)
> + eglDestroyImageKHR(eglGetCurrentDisplay(), img);
> + return false;
> + }
It's not explicitly called out in the spec, but I think this error case
would be EGL_BAD_PARAMETER like the other bad parameter cases for this
call.
> +
> +/**
> + * One re-uses the buffer for all the tests. Each test is expected to fail
> + * meaning that the ownership is not transferred to the EGL in any point.
> + */
"One re-uses" reads awkwardly. I think "We reuse" would be better.
> +enum piglit_result
> +piglit_display(void)
> +{
> + const unsigned char pixels[2 * 2 * 4];
> + struct piglit_dma_buf *buf;
> + unsigned stride;
> + unsigned offset;
> + int fd;
> + enum piglit_result res;
> + bool pass;
> +
> + res = piglit_create_dma_buf(2, 2, 4, pixels, 2 * 4, &buf, &fd, &stride,
> + &offset);
> + if (res != PIGLIT_PASS)
> + return res;
> +
> + pass = test_excess_attributes(fd, 2, 2, 2 * 4, 0);
> + pass = test_buffer_not_null(fd, 2, 2, 2 * 4, 0) && pass;
> + pass = test_invalid_context(fd, 2, 2, 2 * 4, 0) && pass;
> + pass = test_invalid_format(fd, 2, 2, 2 * 4, 0) && pass;
> + pass = test_pitch_zero(fd, 2, 2, 2 * 4, 0) && pass;
You should pass in the allocated stride from the piglit_create_dma_buf,
or for an implementation that aligns stride, you'll get BAD_ACCESS
errors by ignoring it. Also, please make variables "w" and "h" and use
them so I don't have to see this wall of 2s and 4s all over. The "4"
parameter in create_dma_buf was not obvious what it was doing (cpp, not
size)
pgpMQEFlLC4vk.pgp
Description: PGP signature
_______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
