[Piglit] [PATCH] Test that sRGB-related blits are allowed and do not do encoding/decoding.

Ian Romanick idr at freedesktop.org
Wed Oct 10 10:03:42 PDT 2012


On 09/24/2012 04:35 PM, Paul Berry wrote:
> The behaviour specified by OpenGL for blits involving sRGB is
> self-contradictory--it is unclear whether blits should perform sRGB
> encoding/decoding, and if so, whether this encoding/decoding should be
> dependent upon the setting of the GL_FRAMEBUFFER_SRGB enable flag.
> Experiments with nVidia and ATI drivers indicate that they never do
> any sRGB encoding or decoding during a blit, regardless of the buffer
> formats and regardless of the setting of GL_FRAMEBUFFER_SRGB.
>
> Furthermore, OpenGL specifies that multisampled blits between formats
> that are not "identical" should fail, however nVidia and ATI drivers
> allow these blits.
>
> Existing games, such as Valve's "Left For Dead 2", appear to rely on
> the behaviour exhibited by the the nVidia and ATI drivers, so it seems
> sensible for Piglit to test for this behaviour too.

The scaled blit check is a bit hinkey.  The scaled and 1:1 blits produce 
the same results because the source image is, basically, vertical 
stripes.  This opens the possibility for a buggy implementation that 
doesn't do the scaled blit... it just does the 1:1 blit.

I have a couple minor comments below...

> ---
>   tests/all.tests                                   |  17 +
>   tests/spec/arb_framebuffer_srgb/CMakeLists.gl.txt |   1 +
>   tests/spec/arb_framebuffer_srgb/blit.c            | 358 ++++++++++++++++++++++
>   3 files changed, 376 insertions(+)
>   create mode 100644 tests/spec/arb_framebuffer_srgb/blit.c
>
> diff --git a/tests/all.tests b/tests/all.tests
> index 177a9bb..053857d 100644
> --- a/tests/all.tests
> +++ b/tests/all.tests
> @@ -1172,6 +1172,23 @@ for format in ('rgba', 'depth'):
>           arb_framebuffer_object[test_name] = PlainExecTest(test_name + ' -auto')
>
>
> +# Group ARB_framebuffer_sRGB
> +arb_framebuffer_srgb = Group()
> +spec['ARB_framebuffer_sRGB'] = arb_framebuffer_srgb
> +for backing_type in ('texture', 'renderbuffer'):
> +        for srgb_types in ('linear', 'srgb', 'linear_to_srgb',
> +                           'srgb_to_linear'):
> +                for blit_type in ('single_sampled', 'upsample', 'downsample',
> +                                  'msaa', 'scaled'):
> +                        for framebuffer_srgb_setting in ('enabled',
> +                                                         'disabled'):
> +                                test_name = ' '.join(
> +                                        ['blit', backing_type, srgb_types,
> +                                         blit_type, framebuffer_srgb_setting])
> +                                arb_framebuffer_srgb[test_name] = concurrent_test(
> +                                        'arb_framebuffer_srgb-' + test_name)
> +
> +
>   # Group ARB_sampler_objects
>   arb_sampler_objects = Group()
>   spec['ARB_sampler_objects'] = arb_sampler_objects
> diff --git a/tests/spec/arb_framebuffer_srgb/CMakeLists.gl.txt b/tests/spec/arb_framebuffer_srgb/CMakeLists.gl.txt
> index f4ef120..5aa4b63 100644
> --- a/tests/spec/arb_framebuffer_srgb/CMakeLists.gl.txt
> +++ b/tests/spec/arb_framebuffer_srgb/CMakeLists.gl.txt
> @@ -10,3 +10,4 @@ link_libraries (
>   )
>
>   piglit_add_executable (arb_framebuffer_srgb-pushpop pushpop.c)
> +piglit_add_executable (arb_framebuffer_srgb-blit blit.c)
> diff --git a/tests/spec/arb_framebuffer_srgb/blit.c b/tests/spec/arb_framebuffer_srgb/blit.c
> new file mode 100644
> index 0000000..bbd69b5
> --- /dev/null
> +++ b/tests/spec/arb_framebuffer_srgb/blit.c
> @@ -0,0 +1,358 @@
> +/*
> + * Copyright © 2012 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 blit.c
> + *
> + * Test the sRGB behaviour of blits.
> + *
> + * The GL 4.3 spec is contradictory about how blits should be handled
> + * when the source or destination buffer is sRGB.  From section 18.3.1
> + * Blitting Pixel Rectangles:
> + *
> + * (1) When values are taken from the read buffer, if the value of
> + *     FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING for the framebuffer
> + *     attachment corresponding to the read buffer is SRGB (see
> + *     section 9.2.3), the red, green, and blue components are
> + *     converted from the non-linear sRGB color space according to
> + *     equation 8.14.
> + *
> + * (2) When values are taken from the read buffer, no linearization is
> + *     performed even if the format of the buffer is SRGB.
> + *
> + * (3) When values are written to the draw buffers, blit operations
> + *     bypass most of the fragment pipeline. The only fragment
> + *     operations which affect a blit are the pixel ownership test,
> + *     the scissor test, and sRGB conversion (see section
> + *     17.3.9). Color, depth, and stencil masks (see section 17.4.2)
> + *     are ignored.
> + *
> + * (4) If SAMPLE_BUFFERS for either the read framebuffer or draw
> + *     framebuffer is greater than zero, no copy is performed and an
> + *     INVALID_OPERATION error is generated if the dimensions of the
> + *     source and destination rectangles provided to BlitFramebuffer
> + *     are not identical, or if the formats of the read and draw
> + *     framebuffers are not identical.
> + *
> + * And from section 17.3.9 sRGB Conversion:
> + *
> + * (5) If FRAMEBUFFER_SRGB is enabled and the value of
> + *     FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING for the framebuffer
> + *     attachment corresponding to the destination buffer is SRGB1
> + *     (see section 9.2.3), the R, G, and B values after blending are
> + *     converted into the non-linear sRGB color space by computing
> + *     ... [formula follows] ... If FRAMEBUFFER_SRGB is disabled or
> + *     the value of FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING is not SRGB,
> + *     then ... [no conversion is applied].
> + *
> + * Paragraphs (1) and (2) seem irreconcilable: the first says that
> + * linearization should happen when reading from SRGB buffers, the
> + * second says that it shouldn't.
> + *
> + * The remaining paragraphs are self-consistent, however they aren't
> + * consistent with the observed behaviour of existing drivers (notably
> + * nVidia and ATI drivers).  Existing drivers seem to follow the much
> + * simpler rule that blits preserve the underlying binary
> + * representation of the pixels, regardless of whether the format is
> + * sRGB and regardless of the setting of FRAMEBUFFER_SRGB.
> + * Furthermore, sRGB and non-sRGB formats are considered "identical"
> + * for the purposes of paragraph (4).  Existing games seem to rely on
> + * this behaviour.
> + *
> + * This test verifies that blitting is permitted, and preserves the
> + * underlying binary representation of the pixels, under any specified
> + * combination of the following circumstances:
> + *
> + * - Using framebuffers backed by textures vs renderbuffers.
> + * - Blitting from sRGB vs linear, and to sRGB vs linear.
> + * - Doing a 1:1 blit from a single-sampled vs MSAA buffer, and to a
> + *   single-sampled vs MSAA buffer, or doing a scaled blit between
> + *   two single-sampled buffers.
> + * - With FRAMEBUFFER_SRGB enabled vs disabled.
> + *
> + * The combination to test is selected using command-line parameters.
> + *
> + * The test operates by rendering an image to a source framebuffer
> + * where each pixel's 8-bit color value is equal to its X coordinate.
> + * Then it blits this image to a destination framebuffer, and checks
> + * (using glReadPixels) that each pixel's 8-bit color value is still
> + * equal to its X coordinate.
> + *
> + * Since glReadPixels cannot be used directly on MSAA buffers, an
> + * additional resolve blit is added when necessary, to convert the
> + * image to single-sampled before reading the pixel values.
> + */
> +
> +#include "piglit-util-gl-common.h"
> +
> +const int PATTERN_WIDTH = 256;
> +const int PATTERN_HEIGHT = 64;
> +
> +PIGLIT_GL_TEST_MAIN(PATTERN_WIDTH, PATTERN_HEIGHT,
> +		    GLUT_DOUBLE | GLUT_RGB | GLUT_ALPHA);
> +
> +/* Test parameters */
> +static bool use_textures;
> +static GLenum src_format;
> +static GLenum dst_format;
> +static GLsizei src_samples;
> +static GLsizei dst_samples;
> +static bool scaled_blit;
> +static bool enable_srgb_framebuffer;
> +
> +/* GL objects */
> +static GLuint src_fbo;
> +static GLuint dst_fbo;
> +static GLuint resolve_fbo;
> +static GLint prog;
> +
> +static char *vs_text = \
> +	"#version 120\n"
> +	"void main()\n"
> +	"{\n"
> +	"  gl_Position = gl_Vertex;\n"
> +	"}\n";
> +
> +static char *fs_text = \
> +	"#version 120\n"
> +	"void main()\n"
> +	"{\n"
> +	"  float x = gl_FragCoord.x;\n"
> +	"  gl_FragColor = vec4((x - 0.5) / 255.0);\n"
> +	"}\n";
> +
> +static GLuint
> +setup_fbo(GLenum internalformat, GLsizei num_samples)
> +{
> +	GLuint fbo;
> +	glGenFramebuffers(1, &fbo);
> +	glBindFramebuffer(GL_DRAW_FRAMEBUFFER, fbo);
> +	if (use_textures && num_samples == 0) {
> +		GLuint tex;
> +		const GLint level = 0;
> +		const GLint border = 0;

Why not just pass literals to the TexImage2D call?

> +		glGenTextures(1, &tex);
> +		glBindTexture(GL_TEXTURE_2D, tex);
> +		glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER,
> +				GL_NEAREST);
> +		glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER,
> +				GL_NEAREST);
> +		glTexImage2D(GL_TEXTURE_2D, level,
> +			     internalformat, PATTERN_WIDTH, PATTERN_HEIGHT,
> +			     border, GL_RGBA, GL_BYTE, NULL);
> +		glFramebufferTexture2D(GL_DRAW_FRAMEBUFFER,
> +				       GL_COLOR_ATTACHMENT0,
> +				       GL_TEXTURE_2D, tex, level);
> +	} else {
> +		GLuint rb;
> +		glGenRenderbuffers(1, &rb);
> +		glBindRenderbuffer(GL_RENDERBUFFER, rb);
> +		glRenderbufferStorageMultisample(GL_RENDERBUFFER, num_samples,
> +						 internalformat, PATTERN_WIDTH,
> +						 PATTERN_HEIGHT);
> +		glFramebufferRenderbuffer(GL_DRAW_FRAMEBUFFER,
> +					  GL_COLOR_ATTACHMENT0,
> +					  GL_RENDERBUFFER, rb);
> +	}
> +	return fbo;
> +}
> +
> +static void
> +print_usage_and_exit(char *prog_name)
> +{
> +	printf("Usage: %s <backing_type> <sRGB_types> <blit_type>\n"
> +	       "          <framebuffer_srgb_setting>\n"
> +	       "  where <backing_type> is one of:\n"
> +	       "    texture (ignored for multisampled framebuffers)\n"
> +	       "    renderbuffer\n"
> +	       "  where <sRGB_types> is one of:\n"
> +	       "    linear (both buffers linear)\n"
> +	       "    srgb (both buffers sRGB)\n"
> +	       "    linear_to_srgb\n"
> +	       "    srgb_to_linear\n"
> +	       "  where <blit_type> is one of:\n"
> +	       "    single_sampled\n"
> +	       "    upsample\n"
> +	       "    downsample\n"
> +	       "    msaa\n"
> +	       "    scaled\n"
> +	       "  where framebuffer_srgb_setting is one of:\n"
> +	       "    enabled\n"
> +	       "    disabled\n",
> +	       prog_name);
> +	piglit_report_result(PIGLIT_FAIL);
> +}
> +
> +void
> +piglit_init(int argc, char **argv)
> +{
> +	GLint vs, fs;
> +
> +	if (argc != 5) {
> +		print_usage_and_exit(argv[0]);
> +	}
> +
> +	if (strcmp(argv[1], "texture") == 0) {
> +		use_textures = true;
> +	} else if (strcmp(argv[1], "renderbuffer") == 0) {
> +		use_textures = false;
> +	} else {
> +		print_usage_and_exit(argv[0]);
> +	}
> +
> +	if (strcmp(argv[2], "linear") == 0) {
> +		src_format = GL_RGBA;
> +		dst_format = GL_RGBA;
> +	} else if (strcmp(argv[2], "srgb") == 0) {
> +		src_format = GL_SRGB8_ALPHA8;
> +		dst_format = GL_SRGB8_ALPHA8;
> +	} else if (strcmp(argv[2], "linear_to_srgb") == 0) {
> +		src_format = GL_RGBA;
> +		dst_format = GL_SRGB8_ALPHA8;
> +	} else if (strcmp(argv[2], "srgb_to_linear") == 0) {
> +		src_format = GL_SRGB8_ALPHA8;
> +		dst_format = GL_RGBA;
> +	} else {
> +		print_usage_and_exit(argv[0]);
> +	}
> +
> +	if (strcmp(argv[3], "single_sampled") == 0) {
> +		src_samples = 0;
> +		dst_samples = 0;
> +		scaled_blit = false;
> +	} else if (strcmp(argv[3], "upsample") == 0) {
> +		src_samples = 0;
> +		dst_samples = 1; /* selects minimum available sample count */
> +		scaled_blit = false;
> +	} else if (strcmp(argv[3], "downsample") == 0) {
> +		src_samples = 1;
> +		dst_samples = 0;
> +		scaled_blit = false;
> +	} else if (strcmp(argv[3], "msaa") == 0) {
> +		src_samples = 1;
> +		dst_samples = 1;
> +		scaled_blit = false;
> +	} else if (strcmp(argv[3], "scaled") == 0) {
> +		src_samples = 0;
> +		dst_samples = 0;
> +		scaled_blit = true;
> +	} else {
> +		print_usage_and_exit(argv[0]);
> +	}
> +
> +	if (strcmp(argv[4], "enabled") == 0) {
> +		enable_srgb_framebuffer = true;
> +	} else if (strcmp(argv[4], "disabled") == 0) {
> +		enable_srgb_framebuffer = true;
> +	} else {
> +		print_usage_and_exit(argv[0]);
> +	}
> +
> +	piglit_require_gl_version(21);
> +	piglit_require_extension("ARB_framebuffer_object");
> +	piglit_require_extension("ARB_framebuffer_sRGB");
> +
> +	vs = piglit_compile_shader_text(GL_VERTEX_SHADER, vs_text);
> +	fs = piglit_compile_shader_text(GL_FRAGMENT_SHADER, fs_text);
> +	prog = piglit_link_simple_program(vs, fs);
> +	glDeleteShader(vs);
> +	glDeleteShader(fs);
> +
> +	src_fbo = setup_fbo(src_format, src_samples);
> +	dst_fbo = setup_fbo(dst_format, dst_samples);
> +	if (dst_samples != 0)
> +		resolve_fbo = setup_fbo(dst_format, 0);
> +	else
> +		resolve_fbo = 0;
> +}
> +
> +static bool
> +analyze_image(GLuint fbo)
> +{
> +	GLfloat *expected_data = malloc(PATTERN_WIDTH * PATTERN_HEIGHT * 4 *
> +					sizeof(GLfloat));
> +	unsigned x, y, component;
> +	bool pass;
> +
> +	for (y = 0; y < PATTERN_HEIGHT; ++y) {
> +		for (x = 0; x < PATTERN_WIDTH; ++x) {
> +			for (component = 0; component < 4; ++component) {
> +				expected_data[(y * PATTERN_WIDTH + x)
> +					      * 4 + component] = x / 255.0;
> +			}
> +		}
> +	}
> +
> +	glBindFramebuffer(GL_READ_FRAMEBUFFER, fbo);
> +	pass = piglit_probe_image_rgba(0, 0, PATTERN_WIDTH, PATTERN_HEIGHT,
> +				       expected_data);
> +	free(expected_data);
> +	return pass;
> +}
> +
> +enum piglit_result
> +piglit_display()
> +{
> +	bool pass;
> +
> +	glUseProgram(prog);
> +	glDisable(GL_FRAMEBUFFER_SRGB);
> +
> +	/* Clear buffers */
> +	if (resolve_fbo != 0) {
> +		glBindFramebuffer(GL_DRAW_FRAMEBUFFER, resolve_fbo);
> +		glClear(GL_COLOR_BUFFER_BIT);
> +	}
> +	glBindFramebuffer(GL_DRAW_FRAMEBUFFER, dst_fbo);
> +	glClear(GL_COLOR_BUFFER_BIT);
> +	glBindFramebuffer(GL_DRAW_FRAMEBUFFER, src_fbo);
> +	glClear(GL_COLOR_BUFFER_BIT);
> +
> +	/* Draw the source image */
> +	glViewport(0, 0, PATTERN_WIDTH, PATTERN_HEIGHT);
> +	piglit_draw_rect(-1, -1, 2, 2);
> +
> +	/* Do the blit */
> +	glBindFramebuffer(GL_READ_FRAMEBUFFER, src_fbo);
> +	glBindFramebuffer(GL_DRAW_FRAMEBUFFER, dst_fbo);
> +	if (enable_srgb_framebuffer)
> +		glEnable(GL_FRAMEBUFFER_SRGB);
> +	glBlitFramebuffer(0, 0, PATTERN_WIDTH,
> +			  scaled_blit ? 1 : PATTERN_HEIGHT,
> +			  0, 0, PATTERN_WIDTH, PATTERN_HEIGHT,
> +			  GL_COLOR_BUFFER_BIT, GL_NEAREST);
> +	glDisable(GL_FRAMEBUFFER_SRGB);
> +
> +	/* If necessary, do a resolve blit */
> +	if (resolve_fbo != 0) {

This confused me at first because resolve (verb) is used as a noun.  I 
expected resolve_fbo to be a bool.  I completely understand why it's 
named this, and I have no suggestion to improve it.  I'm just 
complaining. :(

> +		glBindFramebuffer(GL_READ_FRAMEBUFFER, dst_fbo);
> +		glBindFramebuffer(GL_DRAW_FRAMEBUFFER, resolve_fbo);
> +		glBlitFramebuffer(0, 0, PATTERN_WIDTH, PATTERN_HEIGHT,
> +				  0, 0, PATTERN_WIDTH, PATTERN_HEIGHT,
> +				  GL_COLOR_BUFFER_BIT, GL_NEAREST);
> +		pass = analyze_image(resolve_fbo);
> +	} else {
> +		pass = analyze_image(dst_fbo);
> +	}
> +
> +	return pass ? PIGLIT_PASS : PIGLIT_FAIL;
> +}
>


More information about the Piglit mailing list