[Piglit] [PATCH] ext_render_snorm-render: test for GL_EXT_render_snorm

Tapani Pälli tapani.palli at intel.com
Mon Aug 13 10:31:49 UTC 2018



On 08/11/2018 01:12 AM, Nanley Chery wrote:
> On Thu, Aug 02, 2018 at 02:06:26PM +0300, Tapani Pälli wrote:
>> Test includes:
>>     - texture uploads
>>     - mipmap generation
>>     - framebuffer creation
>>     - rendering to
>>     - reading from
>>     - interaction with GL_EXT_copy_image
> 
> I don't see any interaction with this extension..

Oops yes, this is leftover (as this test was copy-paste from 
ext_texture_norm16 test), will remove this.


>>
>> This test includes only GL_BYTE based formats. R16_SNORM, RG16_SNORM
>> and RGBA16_SNORM are tested in GL_EXT_texture_norm16 tests.
>>
>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>> ---
>>   tests/opengl.py                                  |   5 +
>>   tests/spec/CMakeLists.txt                        |   1 +
>>   tests/spec/ext_render_snorm/CMakeLists.gles2.txt |   7 +
>>   tests/spec/ext_render_snorm/CMakeLists.txt       |   1 +
>>   tests/spec/ext_render_snorm/render.c             | 335 +++++++++++++++++++++++
>>   5 files changed, 349 insertions(+)
>>   create mode 100644 tests/spec/ext_render_snorm/CMakeLists.gles2.txt
>>   create mode 100644 tests/spec/ext_render_snorm/CMakeLists.txt
>>   create mode 100644 tests/spec/ext_render_snorm/render.c
>>
>> diff --git a/tests/opengl.py b/tests/opengl.py
>> index 397676e65..6a8c513b4 100644
>> --- a/tests/opengl.py
>> +++ b/tests/opengl.py
>> @@ -3070,6 +3070,11 @@ with profile.test_list.group_manager(
>>           grouptools.join('spec', 'ext_texture_norm16')) as g:
>>       g(['ext_texture_norm16-render'], 'render')
>>   
>> +with profile.test_list.group_manager(
>> +        PiglitGLTest,
>> +        grouptools.join('spec', 'ext_render_snorm')) as g:
>> +    g(['ext_render_snorm-render'], 'render')
>> +
>>   with profile.test_list.group_manager(
>>           PiglitGLTest,
>>           grouptools.join('spec', 'ext_frag_depth')) as g:
>> diff --git a/tests/spec/CMakeLists.txt b/tests/spec/CMakeLists.txt
>> index 6cf3f76ed..0a2d4bb25 100644
>> --- a/tests/spec/CMakeLists.txt
>> +++ b/tests/spec/CMakeLists.txt
>> @@ -181,3 +181,4 @@ add_subdirectory (ext_occlusion_query_boolean)
>>   add_subdirectory (ext_disjoint_timer_query)
>>   add_subdirectory (intel_blackhole_render)
>>   add_subdirectory (ext_texture_norm16)
>> +add_subdirectory (ext_render_snorm)
>> diff --git a/tests/spec/ext_render_snorm/CMakeLists.gles2.txt b/tests/spec/ext_render_snorm/CMakeLists.gles2.txt
>> new file mode 100644
>> index 000000000..4b90257cc
>> --- /dev/null
>> +++ b/tests/spec/ext_render_snorm/CMakeLists.gles2.txt
>> @@ -0,0 +1,7 @@
>> +link_libraries (
>> +	piglitutil_${piglit_target_api}
>> +)
>> +
>> +piglit_add_executable (ext_render_snorm-render render.c)
>> +
>> +# vim: ft=cmake:
>> diff --git a/tests/spec/ext_render_snorm/CMakeLists.txt b/tests/spec/ext_render_snorm/CMakeLists.txt
>> new file mode 100644
>> index 000000000..144a306f4
>> --- /dev/null
>> +++ b/tests/spec/ext_render_snorm/CMakeLists.txt
>> @@ -0,0 +1 @@
>> +piglit_include_target_api()
>> diff --git a/tests/spec/ext_render_snorm/render.c b/tests/spec/ext_render_snorm/render.c
>> new file mode 100644
>> index 000000000..241812e12
>> --- /dev/null
>> +++ b/tests/spec/ext_render_snorm/render.c
>> @@ -0,0 +1,335 @@
>> +/*
>> + * Copyright © 2018 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
>> + * Basic tests for formats added by GL_EXT_render_snorm extension
>> + *
>> + * https://www.khronos.org/registry/OpenGL/extensions/EXT/EXT_render_snorm.txt
>> + *
>> + * Test includes:
>> + * 	- texture uploads
>> + * 	- mipmap generation
>> + * 	- framebuffer creation
>> + * 	- rendering to
>> + * 	- reading from
>> + *	- interaction with GL_EXT_copy_image
> 
> Same as above.

To be removed.

>> + */
>> +
>> +#include "piglit-util-gl.h"
>> +
>> +PIGLIT_GL_TEST_CONFIG_BEGIN
>> +	config.supports_gl_es_version = 31;
>> +	config.window_visual = PIGLIT_GL_VISUAL_RGBA;
>> +PIGLIT_GL_TEST_CONFIG_END
>> +
>> +#define PIGLIT_RESULT(x) x ? PIGLIT_PASS : PIGLIT_FAIL
>> +
>> +static const char vs_source[] =
>> +	"#version 310 es\n"
>> +	"layout(location = 0) in highp vec4 vertex;\n"
>> +	"layout(location = 1) in highp vec4 uv;\n"
>> +	"out highp vec2 tex_coord;\n"
>> +	"\n"
>> +	"void main()\n"
>> +	"{\n"
>> +	"	gl_Position = vertex;\n"
>> +	"	tex_coord = uv.st;\n"
>> +	"}\n";
>> +
>> +static const char fs_source[] =
>> +	"#version 310 es\n"
>> +	"layout(location = 0) uniform sampler2D texture;\n"
>> +	"in highp vec2 tex_coord;\n"
>> +	"out highp vec4 color;\n"
>> +	"void main()\n"
>> +	"{\n"
>> +	"	color = texture2D(texture, tex_coord);\n"
>> +	"}\n";
>> +
>> +/* trianglestrip, interleaved vertices + texcoords */
>> +static const GLfloat vertex_data[] = {
>> +	-1.0f,  1.0f,
>> +	0.0f,  1.0f,
>> +	1.0f,  1.0f,
>> +	1.0f,  1.0f,
>> +	-1.0f, -1.0f,
>> +	0.0f,  0.0f,
>> +	1.0f, -1.0f,
>> +	1.0f,  0.0f
>> +};
>> +
>> +static struct fmt_test {
> 
> const?

Yes, will change to const.

>> +	GLenum iformat;
>> +	GLenum base_format;
>> +	unsigned bpp;
>> +	GLenum type;
>> +} tests[] = {
>> +	{ GL_R8_SNORM,		GL_RED,		1,	GL_BYTE, },
>> +	{ GL_RG8_SNORM,		GL_RG,		2,	GL_BYTE, },
>> +	{ GL_RGBA8_SNORM,	GL_RGBA,	4,	GL_BYTE, },
>> +};
>> +
>> +static GLuint prog;
>> +
>> +static void
>> +upload(const struct fmt_test *test, void *data)
>> +{
>> +	glTexStorage2D(GL_TEXTURE_2D, 4, test->iformat, piglit_width,
>> +		       piglit_height);
>> +	glTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, piglit_width,
>> +			piglit_height, test->base_format, test->type,
>> +			data);
>> +	glGenerateMipmap(GL_TEXTURE_2D);
>> +	return;
>> +}
>> +
>> +static void
>> +value_for_format(const struct fmt_test *test, void *value)
>> +{
>> +	unsigned short val = SCHAR_MAX;
>> +
>> +	char *v = value;
>> +	/* red */
>> +	v[0] = val;
>> +	/* green */
>> +	if (test->bpp > 1) {
>> +		v[0] = 0;
>> +		v[1] = val;
>> +	}
>> +	/* blue */
>> +	if (test->bpp > 2) {
>> +		v[0] = 0;
>> +		v[1] = 0;
>> +		v[2] = val;
>> +	}
>> +}
>> +
>> +static void
>> +generate_data(const struct fmt_test *test)
>> +{
>> +	unsigned pixels = piglit_width * piglit_height;
>> +	void *data = malloc (pixels * test->bpp);
>> +
>> +	char *p = data;
>> +	for (unsigned i = 0; i < pixels; i++, p += test->bpp)
>> +		value_for_format(test, p);
> 
> How does the GL_RGBA8_SNORM case pass verify_contents()'s assumption
> that v[3] == SCHAR_MAX? The alpha channel is not initialized by
> value_for_format().

Alpha is written in the verify_contents() before comparison.

>> +
>> +	upload(test, data);
>> +	free(data);
>> +}
>> +
>> +static GLuint
>> +create_empty_texture()
>> +{
>> +	GLuint tex;
>> +	glGenTextures(1, &tex);
>> +	glBindTexture(GL_TEXTURE_2D, tex);
>> +
>> +	glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
>> +	glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
>> +	glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
>> +	glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
>> +
>> +	return tex;
>> +}
>> +
>> +static GLuint
>> +create_texture(const struct fmt_test *test)
>> +{
>> +	GLuint tex = create_empty_texture();
>> +	generate_data(test);
>> +	return tex;
>> +}
>> +
>> +static GLuint
>> +create_rbo(const struct fmt_test *test)
>> +{
>> +	GLuint rbo;
>> +	glGenRenderbuffers(1, &rbo);
>> +	glBindRenderbuffer(GL_RENDERBUFFER, rbo);
>> +	glRenderbufferStorage(GL_RENDERBUFFER, test->iformat, piglit_width,
>> +			      piglit_height);
>> +	return rbo;
>> +}
>> +
>> +static GLuint
>> +create_fbo(const struct fmt_test *test, GLuint *tex)
>> +{
>> +	GLuint fbo;
>> +	GLuint fbo_tex = create_texture(test);
> 
> I think you meant to call create_empty_texture() here. We render to the
> fbo without checking its contents beforehand.

Hmm nope, the reason for initializing attachment properly here is to 
check that we can create a complete fbo using that particular format so 
first we create a texture of that format and then call 
glFramebufferTexture2D and check if fbo is complete.

After this there is also render test where we render this same texture 
again to the fbo and this should not result in render corruption.

>> +
>> +	*tex = fbo_tex;
>> +
>> +	glGenFramebuffers(1, &fbo);
>> +	glBindFramebuffer(GL_FRAMEBUFFER, fbo);
>> +	glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
>> +			       GL_TEXTURE_2D, fbo_tex, 0);
>> +	return fbo;
>> +}
>> +
>> +static void
>> +render_texture(GLuint texture, GLenum target, GLuint fbo_target)
>> +{
>> +	glBindTexture(target, texture);
>> +	glBindFramebuffer(GL_FRAMEBUFFER, fbo_target);
>> +
>> +	glViewport(0, 0, piglit_width, piglit_height);
>> +
>> +	glClear(GL_COLOR_BUFFER_BIT);
>> +	glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);
>> +}
>> +
>> +static bool
>> +verify_contents(const struct fmt_test *test)
>> +{
>> +	bool result = true;
>> +	unsigned amount = piglit_width * piglit_height;
>> +	void *pix = malloc (amount * 4);
>> +	glReadPixels(0, 0, piglit_width, piglit_height, GL_RGBA, test->type,
>> +		     pix);
>> +
>> +	char value[4] = { 0 };
>> +	value_for_format(test, value);
>> +	value[3] = SCHAR_MAX;
>> +
>> +	char *p = pix;
>> +	for (unsigned i = 0; i < amount; i++, p += 4) {
>> +		if (memcmp(p, value, sizeof(value)) == 0)
>> +			continue;
>> +
>> +		piglit_report_subtest_result(PIGLIT_FAIL,
>> +					     "format 0x%x read fail",
>> +					     test->iformat);
>> +		result = false;
>> +		break;
>> +	}
>> +
>> +	free(pix);
>> +	return result;
>> +}
>> +
>> +static bool
>> +test_format(const struct fmt_test *test)
>> +{
>> +	bool pass = true;
>> +
>> +	glUseProgram(prog);
>> +	glUniform1i(0 /* explicit loc */, 0);
>> +
>> +	/* Create a texture, upload data */
>> +	const GLuint texture = create_texture(test);
>> +
>> +	glBindTexture(GL_TEXTURE_2D, texture);
> 
> Interestingly, texture was already bound to this target through
> create_texture().

That is true, this is a bit of extra here and can be taken away.

>> +
>> +	/* Test glRenderbufferStorage. */
>> +	GLuint rbo = create_rbo(test);
>> +	if (!rbo || !piglit_check_gl_error(GL_NO_ERROR)) {
>> +		piglit_report_subtest_result(PIGLIT_FAIL,
>> +					     "format 0x%x RBO test",
>> +					     test->iformat);
>> +		pass &= false;
>> +	} else {
>> +		piglit_report_subtest_result(PIGLIT_PASS,
>> +					     "format 0x%x RBO test",
>> +					     test->iformat);
>> +	}
>> +	glDeleteRenderbuffers(1, &rbo);
>> +
>> +	/* Create framebuffer object. */
>> +	GLuint fbo_tex;
>> +	const GLuint fbo = create_fbo(test, &fbo_tex);
>> +
>> +	if (glCheckFramebufferStatus(GL_FRAMEBUFFER) !=
>> +		GL_FRAMEBUFFER_COMPLETE) {
>> +		piglit_report_subtest_result(PIGLIT_FAIL,
>> +					     "format 0x%x fbo fail",
>> +					     test->iformat);
>> +		pass &= false;
>> +	}
>> +
>> +	render_texture(texture, GL_TEXTURE_2D, fbo);
>> +
>> +	/* Test glCopyTexImage2D by copying current fbo content to
>> +	 * a texture, rendering copy back to fbo and verifying fbo contents.
>> +	 */
>> +	GLuint tmp_tex = create_empty_texture();
> 
> It was a little hard to see how tmp_tex was being used at first. I think
> it would be clearer if the "create" functions were instead named
> something like "create_and_bind."

OK, can rename.

>> +	glCopyTexImage2D(GL_TEXTURE_2D, 0, test->iformat, 0, 0, piglit_width,
>> +			 piglit_height, 0);
>> +
>> +	render_texture(tmp_tex, GL_TEXTURE_2D, fbo);
>> +
>> +	/* Verify contents. */
>> +	pass &= verify_contents(test);
>> +
>> +	glDeleteTextures(1, &tmp_tex);
>> +
>> +	/* Render fbo contents to window. */
>> +	render_texture(fbo_tex, GL_TEXTURE_2D, 0);
>> +
>> +	piglit_present_results();
>> +
>> +	glDeleteFramebuffers(1, &fbo);
>> +	glDeleteTextures(1, &texture);
> 
> Maybe delete these resources as soon as they aren't needed?

Sure, I thought it might look 'cleaner' to have all delete happening at 
the very end.

>> +
>> +	return pass;
>> +}
>> +
>> +enum piglit_result
>> +piglit_display(void)
>> +{
>> +	glEnableVertexAttribArray(0);
>> +	glEnableVertexAttribArray(1);
>> +
>> +	glActiveTexture(GL_TEXTURE0);
>> +
>> +	glVertexAttribPointer(0, 2, GL_FLOAT, GL_FALSE, 4 * sizeof(float),
>> +			      vertex_data);
>> +	glVertexAttribPointer(1, 2, GL_FLOAT, GL_FALSE, 4 * sizeof(float),
>> +			      (void*) (vertex_data + (2 * sizeof(float))));
>> +
>> +	bool pass = true;
>> +
>> +	struct fmt_test *test = tests;
> 
> const struct?

Yes!

> -Nanley
> 
>> +
>> +	/* Loop over each format. */
>> +	for (unsigned i = 0; i < ARRAY_SIZE(tests); i++, test++) {
>> +		bool fmt_pass = test_format(test);
>> +		piglit_report_subtest_result(PIGLIT_RESULT(fmt_pass),
>> +					     "format 0x%x",
>> +					     test->iformat);
>> +		pass &= fmt_pass;
>> +	}
>> +
>> +	if (!piglit_check_gl_error(GL_NO_ERROR))
>> +		piglit_report_result(PIGLIT_FAIL);
>> +
>> +	return PIGLIT_RESULT(pass);
>> +}
>> +
>> +void
>> +piglit_init(int argc, char **argv)
>> +{
>> +	piglit_require_extension("GL_EXT_render_snorm");
>> +	prog = piglit_build_simple_program(vs_source, fs_source);
>> +}
>> -- 
>> 2.14.4
>>
>> _______________________________________________
>> Piglit mailing list
>> Piglit at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/piglit


More information about the Piglit mailing list