[Piglit] [PATCH] gles-3.0: NV_read_depth extension test

Iago Toral itoral at igalia.com
Fri Oct 9 00:26:16 PDT 2015


On Thu, 2015-10-08 at 15:13 +0300, Tapani Pälli wrote:
> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
> ---
>  tests/all.py                             |   5 +
>  tests/spec/gles-3.0/CMakeLists.gles3.txt |   1 +
>  tests/spec/gles-3.0/read-depth.c         | 176 +++++++++++++++++++++++++++++++
>  3 files changed, 182 insertions(+)
>  create mode 100644 tests/spec/gles-3.0/read-depth.c
> 
> diff --git a/tests/all.py b/tests/all.py
> index 5bc36d0..79d6314 100644
> --- a/tests/all.py
> +++ b/tests/all.py
> @@ -4179,6 +4179,11 @@ with profile.group_manager(
>      g(['khr_compressed_astc-miptree_gles2'], 'miptree-gles')
>  
>  with profile.group_manager(
> +         PiglitGLTest,
> +         grouptools.join('spec', 'nv_read_depth')) as g:
> +    g(['read_depth_gles3'])
> +
> +with profile.group_manager(
>          PiglitGLTest,
>          grouptools.join('spec', 'oes_compressed_paletted_texture')) as g:
>      g(['oes_compressed_paletted_texture-api'], 'basic API')
> diff --git a/tests/spec/gles-3.0/CMakeLists.gles3.txt b/tests/spec/gles-3.0/CMakeLists.gles3.txt
> index d56a43e..864c862 100644
> --- a/tests/spec/gles-3.0/CMakeLists.gles3.txt
> +++ b/tests/spec/gles-3.0/CMakeLists.gles3.txt
> @@ -6,5 +6,6 @@ piglit_add_executable (gles-3.0-drawarrays-vertexid drawarrays-vertexid.c)
>  piglit_add_executable(minmax_${piglit_target_api} minmax.c)
>  piglit_add_executable(oes_compressed_etc2_texture-miptree_gles3 oes_compressed_etc2_texture-miptree.c)
>  piglit_add_executable(texture-immutable-levels_gles3 texture-immutable-levels.c)
> +piglit_add_executable(read_depth_gles3 read-depth.c)
>  
>  # vim: ft=cmake:
> diff --git a/tests/spec/gles-3.0/read-depth.c b/tests/spec/gles-3.0/read-depth.c
> new file mode 100644
> index 0000000..456c7e0
> --- /dev/null
> +++ b/tests/spec/gles-3.0/read-depth.c
> @@ -0,0 +1,176 @@
> +/*
> + * Copyright © 2015 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.
> + */
> +
> +/** @file read-depth.c
> + *
> + * Tests NV_read_depth implementation
> + *
> + * Test iterates over table of depth buffer formats and expected types to
> + * read values back from each format. For each format it renders a rectangle at
> + * different depth levels, reads back a pixel and verifies expected depth value.
> + */
> +
> +#include "piglit-util-gl.h"
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +
> +	config.supports_gl_es_version = 30;
> +	config.window_visual = PIGLIT_GL_VISUAL_DEPTH;
> +
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +static GLint prog;
> +
> +const char *vs_source =
> +        "attribute vec4 vertex;\n"
> +	"uniform float depth;\n"

Indentation

> +        "void main()\n"
> +        "{\n"
> +        "       gl_Position = vec4(vertex.xy, depth, 1.0);\n"
> +        "}\n";
> +
> +const char *fs_source =
> +        "void main()\n"
> +        "{\n"
> +        "       gl_FragColor = vec4(1.0, 1.0, 1.0, 1.0);\n"
> +        "}\n";
> +
> +const GLenum tests[] = {
> +	GL_DEPTH_COMPONENT16, GL_UNSIGNED_INT_24_8_OES,
> +	GL_DEPTH_COMPONENT24, GL_UNSIGNED_INT_24_8_OES,
> +	GL_DEPTH_COMPONENT32F, GL_FLOAT,
> +};
> +#define TESTS_SIZE (sizeof(tests)/sizeof(tests[0]))
> +
> +
> +static bool
> +equals(float a, float b)
> +{
> +   if (fabs(a - b) < 0.00001)
> +      return true;

Indentation (piglit is 8 spaces like you do in other places)

> +   return false;
> +}
> +
> +static GLuint
> +create_depth_fbo(GLenum depth_type)
> +{
> +	GLuint fbo, buffer;
> +	GLenum status;
> +
> +	glGenRenderbuffers(1, &buffer);
> +	glBindRenderbuffer(GL_RENDERBUFFER, buffer);
> +	glRenderbufferStorage(GL_RENDERBUFFER,
> +		depth_type, piglit_width, piglit_height);

The secons line should be aligned with GL_RENDERBUFFER, you have this
same issue in every other place where you break a function call in two
lines.

> +	glGenFramebuffers(1, &fbo);
> +	glBindFramebuffer(GL_FRAMEBUFFER, fbo);
> +
> +	glFramebufferRenderbuffer(GL_FRAMEBUFFER,
> +		GL_DEPTH_ATTACHMENT, GL_RENDERBUFFER, buffer);
> +
> +	status = glCheckFramebufferStatus(GL_FRAMEBUFFER);
> +	if (status != GL_FRAMEBUFFER_COMPLETE) {
> +		fprintf(stderr, "error creating framebuffer, status 0x%x\n",
> +			status);
> +		return 0;
> +	}
> +	return fbo;
> +}
> +
> +static bool
> +read_depth(GLenum type, float expect)
> +{
> +	GLfloat data;
> +	GLuint uint_pixel;
> +	if (type == GL_FLOAT) {
> +		glReadPixels(0, 0, 1, 1, GL_DEPTH_COMPONENT, type,
> +			(void *) &data);
> +	} else {
> +		glReadPixels(0, 0, 1, 1, GL_DEPTH_COMPONENT, type,
> +			(void *) &uint_pixel);
> +		data = (1.0 * ((float) uint_pixel)) / 4294967295.0;

Is this correct? For GL_UNSIGNED_INT_24_8_OES the maximum value we
should see in uint_pixel is that in which the 24 most significant bits
are 1 and the 8 stencil bits are undefined (actually they should be zero
according to your patch in Mesa). The number you divide by, however, is
2^32-1...., so the result is always going to be a stretch lower than
1.0. Shouldn't we do this instead?:

uint_pixel = uint_pixel >> 8;
data = (1.0 * ((float) uint_pixel)) / 16777215.0;

> +	}
> +	piglit_check_gl_error(GL_NO_ERROR);
> +
> +	if (!equals(data, expect)) {
> +		fprintf(stderr, "expected %f, got %f\n", expect, data);
> +		return false;
> +	}
> +	return true;
> +}
> +
> +enum piglit_result
> +piglit_display(void)
> +{
> +	GLboolean pass = true;
> +	const GLfloat rect[] = {
> +		-1.0f,  1.0f,
> +		1.0f,  1.0f,
> +		-1.0f, -1.0f,
> +		1.0f, -1.0f,
> +	};
> +
> +	glEnable(GL_DEPTH_TEST);
> +	glVertexAttribPointer(PIGLIT_ATTRIB_POS, 2, GL_FLOAT, GL_FALSE, 0,
> +			rect);
> +	glEnableVertexAttribArray(PIGLIT_ATTRIB_POS);
> +
> +	/* Loop through formats listed in 'tests'. */
> +	for (unsigned j = 0; j < TESTS_SIZE; j+=2) {
> +
> +		GLuint fbo = create_depth_fbo(tests[j]);
> +		if (!fbo)
> +			return PIGLIT_FAIL;
> +
> +		float i = -1.0;
> +		float step = 0.1;
> +		float expect = 0.0;
> +
> +		/* Step from -1.0 to 1.0, linear depth. Render a
> +                 * rectangle at depth i, read pixel and verify
> +                 * expected depth value.
> +                 */
> +		for (i = -1.0; !equals(i, 1.0 + step); i += step) {

Since this is the only place where you call equals I think it is just
easier to do:

for (i = -1.0; i < 1.00001 + step; i += step) {

> +			glClear(GL_DEPTH_BUFFER_BIT);
> +			glUniform1f(glGetUniformLocation(prog, "depth"), i);
> +
> +			glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);
> +
> +			if (!(read_depth(tests[j + 1], expect)))
> +				return PIGLIT_FAIL;
> +
> +			expect += step / 2.0;
> +		}
> +		glDeleteFramebuffers(1, &fbo);
> +	}
> +	return pass ? PIGLIT_PASS : PIGLIT_FAIL;
> +}
> +
> +void
> +piglit_init(int argc, char **argv)
> +{
> +	piglit_require_extension("GL_NV_read_depth");
> +	prog = piglit_build_simple_program(vs_source, fs_source);
> +	glUseProgram(prog);
> +}




More information about the Piglit mailing list