[Piglit] [v8 09/13] tests/spec: EXT_image_dma_buf_import sample argb

[Chad Versace]

> +static bool
> +sample_and_destroy_img(unsigned w, unsigned h, EGLImageKHR img)
> +{
> +	GLuint prog, tex;
> +
> +	glGenTextures(1, &tex);
> +	glBindTexture(GL_TEXTURE_EXTERNAL_OES, tex);
> +
> +	/* Set the image as level zero */
> +	glEGLImageTargetTexture2DOES(GL_TEXTURE_EXTERNAL_OES,
> +			(GLeglImageOES)img);
> +	if (!piglit_check_egl_error(EGL_SUCCESS))
> +		return false;

This should check the GL error, not the EGL error. Use piglit_check_gl_error().

Also, glEGLImageTargetTexture2DOES is allowed to fail if it does not support
binding the given image as a texture, in which it emits GL_INVALID_OPERATION.
See the GL_OES_EGL_image spec. Therefore, if glEGLImageTargetTexture2DOES fails
here with GL_INVALID_OPERATION the test should skip rather than fail.



> +static bool
> +sample_buffer(void *buf, int fd, int fourcc, unsigned w, unsigned h,
> +	unsigned stride, unsigned offset)
> +{
> +	EGLImageKHR img;
> +	EGLint attr[] = {
> +		EGL_WIDTH, w,
> +		EGL_HEIGHT, h,
> +		EGL_LINUX_DRM_FOURCC_EXT, fourcc,
> +		EGL_DMA_BUF_PLANE0_FD_EXT, fd,
> +		EGL_DMA_BUF_PLANE0_OFFSET_EXT, offset,
> +		EGL_DMA_BUF_PLANE0_PITCH_EXT, stride,
> +		EGL_NONE
> +	};
> +
> +	img = eglCreateImageKHR(eglGetCurrentDisplay(), EGL_NO_CONTEXT,
> +			EGL_LINUX_DMA_BUF_EXT, (EGLClientBuffer)0, attr);

As in patch 8, if eglCreateImageKHR fails here with EGL_BAD_MATCH,
then the test should skip not fail.

> +
> +	/**
> +	 * Release the creator side of the buffer, EGL should have the
> +	 * ownership now.
> +	 */
> +	piglit_destroy_dma_buf(buf);
> +
> +	/* Close the file descriptor also, EGL does not have ownership */
> +	if (!piglit_check_egl_error(EGL_SUCCESS)) {

Move the above comment into the if-block. Otherwise the comment is confusing.

> +		close(fd);
> +		return false;
> +	}
> +
> +	if (!img) {
> +		fprintf(stderr, "image creation succeed but returned NULL\n");
> +		return false;
> +	}
> +
> +	return sample_and_destroy_img(w, h, img);
> +}




> +enum piglit_result
> +probe_rect_argb(unsigned w, unsigned h, const unsigned char *argb,
> +		bool force_alpha_to_one)
> +{
> +	const unsigned cpp = 4;
> +	unsigned i, j;
> +
> +	for (i = 0; i < h; ++i) {
> +		for (j = 0; j < w; ++j) {
> +			float expected[] = {
> +				argb[i * w * cpp + j * cpp + 2] / 255.0f,
> +				argb[i * w * cpp + j * cpp + 1] / 255.0f,
> +				argb[i * w * cpp + j * cpp + 0] / 255.0f,
> +				argb[i * w * cpp + j * cpp + 3] / 255.0f};
> +
> +			if (force_alpha_to_one)
> +				expected[3] = 1.0f;
> +
> +			if (!piglit_probe_pixel_rgba(j, i, expected))
> +				return PIGLIT_FAIL;
> +		}
> +	}
> +
> +	return PIGLIT_PASS;
> +}

This function looks correct, but has unneeded overhead. The speed of Piglit runs
does matter, especially when doing multiple runs back-to-back or when doing runs
on simulated hardware. So, we do strive to keep Piglit running fast when it
doesn't require too much effort on our part.

One of the slowest GL functions is glReadPixels, and this function calls glReadPixels
for each pixel in the image. Instead, the function should be refactored so that it
makes a single call to piglit_probe_rect_rgba, and hence a single call to glReadPixels.