[Piglit] [PATCH] GL_MESA_pack_invert: Add a test for a few glReadPixels() cases.
Brian Paul
brianp at vmware.com
Thu Jul 9 08:11:49 PDT 2015
Minor nits below.
On 07/08/2015 11:31 PM, Eric Anholt wrote:
> We had no tests of this extension in the tree, and sure enough i965
> was broken (causing GPU hangs).
> ---
>
> A year and a half later, here's a resend (with style fixes)
>
> tests/all.py | 5 +
> tests/spec/CMakeLists.txt | 1 +
> tests/spec/mesa_pack_invert/CMakeLists.gl.txt | 12 ++
> tests/spec/mesa_pack_invert/CMakeLists.txt | 1 +
> tests/spec/mesa_pack_invert/readpixels.c | 200 ++++++++++++++++++++++++++
> 5 files changed, 219 insertions(+)
> create mode 100644 tests/spec/mesa_pack_invert/CMakeLists.gl.txt
> create mode 100644 tests/spec/mesa_pack_invert/CMakeLists.txt
> create mode 100644 tests/spec/mesa_pack_invert/readpixels.c
>
> diff --git a/tests/all.py b/tests/all.py
> index f2e1765..b85d2a2 100644
> --- a/tests/all.py
> +++ b/tests/all.py
> @@ -3928,6 +3928,11 @@ with profile.group_manager(
>
> with profile.group_manager(
> PiglitGLTest,
> + grouptools.join('spec', 'mesa_pack_invert')) as g:
> + g(['mesa_pack_invert-readpixels'])
> +
> +with profile.group_manager(
> + PiglitGLTest,
> grouptools.join('spec', 'oes_read_format')) as g:
> g(['oes-read-format'], run_concurrent=False)
>
> diff --git a/tests/spec/CMakeLists.txt b/tests/spec/CMakeLists.txt
> index b0973ac..65cab37 100644
> --- a/tests/spec/CMakeLists.txt
> +++ b/tests/spec/CMakeLists.txt
> @@ -129,3 +129,4 @@ add_subdirectory (arb_pipeline_statistics_query)
> add_subdirectory (arb_texture_stencil8)
> add_subdirectory (arb_vertex_attrib_64bit)
> add_subdirectory (ext_framebuffer_blit)
> +add_subdirectory (mesa_pack_invert)
> diff --git a/tests/spec/mesa_pack_invert/CMakeLists.gl.txt b/tests/spec/mesa_pack_invert/CMakeLists.gl.txt
> new file mode 100644
> index 0000000..8768d83
> --- /dev/null
> +++ b/tests/spec/mesa_pack_invert/CMakeLists.gl.txt
> @@ -0,0 +1,12 @@
> +include_directories(
> + ${GLEXT_INCLUDE_DIR}
> + ${OPENGL_INCLUDE_PATH}
> +)
> +
> +link_libraries (
> + piglitutil_${piglit_target_api}
> + ${OPENGL_gl_LIBRARY}
> + ${OPENGL_glu_LIBRARY}
> +)
> +
> +piglit_add_executable (mesa_pack_invert-readpixels readpixels.c)
> diff --git a/tests/spec/mesa_pack_invert/CMakeLists.txt b/tests/spec/mesa_pack_invert/CMakeLists.txt
> new file mode 100644
> index 0000000..144a306
> --- /dev/null
> +++ b/tests/spec/mesa_pack_invert/CMakeLists.txt
> @@ -0,0 +1 @@
> +piglit_include_target_api()
> diff --git a/tests/spec/mesa_pack_invert/readpixels.c b/tests/spec/mesa_pack_invert/readpixels.c
> new file mode 100644
> index 0000000..6f54dcb
> --- /dev/null
> +++ b/tests/spec/mesa_pack_invert/readpixels.c
> @@ -0,0 +1,200 @@
> +/*
> + * 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.
> + */
> +
> +
> +/** @file readpixels.c
> + *
> + * Simple touch test of glReadPixels() using GL_PACK_INVERT_MESA, to a
> + * PBO or user memory, with format conversions or (hopefully) not.
> + */
> +
> +#include "piglit-util-gl.h"
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +
> + config.supports_gl_compat_version = 10;
> +
> + config.window_visual = PIGLIT_GL_VISUAL_DOUBLE | PIGLIT_GL_VISUAL_RGBA;
> +
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +/* Size of the RGBW rect on the screen: at 6 pixels, the unorm failure
> + * result fits in an 80-column terminal.
> + */
> +#define W 6
> +#define H 6
> +
> +static bool
> +check_unorm(uint32_t *data, const char *name)
Could const-qualify data.
> +{
> + int x, y;
> +
> + for (y = 0; y < H; y++) {
> + for (x = 0; x < W; x++) {
> + uint32_t colors[] = {
> + 0x000000ff,
> + 0xffffffff,
> + 0x00ff0000,
> + 0x0000ff00
> + };
Could move this out of the loops and declare it static const.
> + int i = (y >= H / 2) * 2 + (x >= W / 2);
> + uint32_t expected = colors[i];
> +
> + if (data[y * W + x] != expected) {
> + fprintf(stderr,
> + "%s pixel value read at (%d, %d) was "
> + "0x%08x, expected 0x%08x\n",
> + name, x, y, data[y * W + x], expected);
> + fprintf(stderr, "\n");
> +
> + for (y = 0; y < H; y++) {
> + for (x = 0; x < W; x++) {
> + fprintf(stderr, "0x%08x ",
> + data[y * W + x]);
> + }
> + fprintf(stderr, "\n");
> + }
> +
> + piglit_report_subtest_result(PIGLIT_FAIL, name);
> + return false;
> + }
> + }
> + }
> +
> + piglit_report_subtest_result(PIGLIT_PASS, name);
> + return true;
> +}
> +
> +static bool
> +check_float(float *data, const char *name)
> +{
> + int x, y;
> +
> + for (y = 0; y < H; y++) {
> + for (x = 0; x < W; x++) {
> + float colors[4][4] = {
> + {0, 0, 1, 0},
> + {1, 1, 1, 1},
> + {1, 0, 0, 0},
> + {0, 1, 0, 0},
> + };
Same comment as above.
> + int i = (y >= H / 2) * 2 + (x >= W / 2);
> + float *expected = colors[i];
> + float *result = &data[(y * W + x) * 4];
> +
> + if (memcmp(result, expected, sizeof(float[4])) != 0) {
> + fprintf(stderr,
> + "%s pixel value read at (%d, %d):\n"
> + " was %f, %f, %f, %f\n"
> + " expected %f, %f, %f, %f\n",
> + name, x, y,
> + result[0],
> + result[1],
> + result[2],
> + result[3],
> + expected[0],
> + expected[1],
> + expected[2],
> + expected[3]);
> + fprintf(stderr, "\n");
> +
> + piglit_report_subtest_result(PIGLIT_FAIL, name);
> + return false;
> + }
> + }
> + }
> +
> + piglit_report_subtest_result(PIGLIT_PASS, name);
> + return true;
> +}
> +
> +enum piglit_result
> +piglit_display(void)
> +{
> + bool pass = true;
> + GLuint pbo;
> + uint32_t bgra_unorm[W * H];
> + float rgba_float[W * H * 4];
> + void *map;
> +
> + glGenBuffers(1, &pbo);
> + glBindBuffer(GL_PIXEL_PACK_BUFFER, pbo);
> + glBufferData(GL_PIXEL_PACK_BUFFER, W * H * sizeof(float[4]),
> + NULL, GL_STREAM_READ_ARB);
> + glPixelStorei(GL_PACK_ALIGNMENT, 1);
> +
> + glClearColor(0.5, 0.5, 0.5, 0.5);
> + glClear(GL_COLOR_BUFFER_BIT);
> +
> + piglit_ortho_projection(piglit_width, piglit_height, false);
> +
> + glColor4f(1, 0, 0, 0);
> + piglit_draw_rect(5, 5,
> + W / 2, H / 2);
> + glColor4f(0, 1, 0, 0);
> + piglit_draw_rect(5 + W / 2, 5,
> + W / 2, H / 2);
> + glColor4f(0, 0, 1, 0);
> + piglit_draw_rect(5, 5 + H / 2,
> + W / 2, H / 2);
> + glColor4f(1, 1, 1, 1);
> + piglit_draw_rect(5 + W / 2, 5 + H / 2,
> + W / 2, H / 2);
> +
> + glPixelStorei(GL_PACK_INVERT_MESA, 1);
> +
> + glBindBuffer(GL_PIXEL_PACK_BUFFER, 0);
> + glReadPixels(5, 5, W, H, GL_BGRA, GL_UNSIGNED_BYTE, bgra_unorm);
> + if (!check_unorm(bgra_unorm, "non-PBO unorm BGRA"))
> + pass = false;
> +
> + glBindBuffer(GL_PIXEL_PACK_BUFFER, pbo);
> + glReadPixels(5, 5, W, H, GL_BGRA, GL_UNSIGNED_BYTE, NULL);
> + map = glMapBuffer(GL_PIXEL_PACK_BUFFER, GL_READ_ONLY);
> + if (!check_unorm(map, "PBO unorm BGRA"))
> + pass = false;
> + glUnmapBuffer(GL_PIXEL_PACK_BUFFER);
> +
> + glBindBuffer(GL_PIXEL_PACK_BUFFER, 0);
> + glReadPixels(5, 5, W, H, GL_RGBA, GL_FLOAT, rgba_float);
> + if (!check_float(rgba_float, "non-PBO float RGBA"))
> + pass = false;
> +
> + glBindBuffer(GL_PIXEL_PACK_BUFFER, pbo);
> + glReadPixels(5, 5, W, H, GL_RGBA, GL_FLOAT, NULL);
> + map = glMapBuffer(GL_PIXEL_PACK_BUFFER, GL_READ_ONLY);
> + if (!check_float(map, "PBO float RGBA"))
> + pass = false;
> + glUnmapBuffer(GL_PIXEL_PACK_BUFFER);
> +
> + piglit_present_results();
> +
> + return pass ? PIGLIT_PASS : PIGLIT_FAIL;
> +}
> +
> +void piglit_init(int argc, char **argv)
> +{
> + piglit_require_extension("GL_ARB_pixel_buffer_object");
> + piglit_require_extension("GL_MESA_pack_invert");
> + piglit_require_extension("GL_EXT_bgra");
> +}
>
I see these compiler warnings here:
/home/brianp/piglit/tests/spec/mesa_pack_invert/readpixels.c: In
function ‘check_unorm’:
/home/brianp/piglit/tests/spec/mesa_pack_invert/readpixels.c:78:5:
warning: format not a string literal and no format arguments
[-Wformat-security]
piglit_report_subtest_result(PIGLIT_FAIL, name);
^
/home/brianp/piglit/tests/spec/mesa_pack_invert/readpixels.c:84:2:
warning: format not a string literal and no format arguments
[-Wformat-security]
piglit_report_subtest_result(PIGLIT_PASS, name);
^
/home/brianp/piglit/tests/spec/mesa_pack_invert/readpixels.c: In
function ‘check_float’:
/home/brianp/piglit/tests/spec/mesa_pack_invert/readpixels.c:121:5:
warning: format not a string literal and no format arguments
[-Wformat-security]
piglit_report_subtest_result(PIGLIT_FAIL, name);
^
/home/brianp/piglit/tests/spec/mesa_pack_invert/readpixels.c:127:2:
warning: format not a string literal and no format arguments
[-Wformat-security]
piglit_report_subtest_result(PIGLIT_PASS, name);
^
Probably have to insert a "%s" format string.
Tested and passes with llvmpipe and swrast.
Reviewed-by: Brian Paul <brianp at vmware.com>
Tested-by: Brian Paul <brianp at vmware.com>
More information about the Piglit
mailing list