[Piglit] [PATCH v2] readpixels-oob: Add a glReadPixels out-of-bounds test

Brian Paul brianp at vmware.com
Wed Feb 17 22:50:14 UTC 2016


On 02/17/2016 03:14 PM, Nanley Chery wrote:
> On Wed, Feb 17, 2016 at 01:44:44PM -0700, Brian Paul wrote:
>> On 02/17/2016 12:37 PM, Nanley Chery wrote:
>>> From: Nanley Chery <nanley.g.chery at intel.com>
>>>
>>> Test that glReadPixels for an area which reaches out of bounds
>>> behaves like a glReadPixels into a sub-rectangle for the valid area.
>>> This behavior reduces the number of corner cases associated with
>>> this function.
>>>
>>> Bugzilla: https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.freedesktop.org_show-5Fbug.cgi-3Fid-3D92193&d=BQIDAw&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=T0t4QG7chq2ZwJo6wilkFznRSFy-8uDKartPGbomVj8&m=VqhvdRGnM3cYHJaEp2CUz9vIsz84HCFARlGvitYubNE&s=m9acf6_FdQWwNbQ3Ecy94zX5Q3l6StbHwYIYZ8icohk&e=
>>>
>>> Signed-off-by: Nanley Chery <nanley.g.chery at intel.com>
>>>
>>> v2 (Brian Paul)
>>> - Rename test and change path
>>> - Use larger window dimensions and compute big buffer dims at runtime
>>> - Use bool instead of GLboolean
>>> - Remove extra function arguments
>>> - Free allocated data
>>> - Exercise more clipping code
>>> ---
>>>   tests/all.py                        |   1 +
>>>   tests/spec/gl-1.0/CMakeLists.gl.txt |   1 +
>>>   tests/spec/gl-1.0/readpixels-oob.c  | 132 ++++++++++++++++++++++++++++++++++++
>>>   3 files changed, 134 insertions(+)
>>>   create mode 100644 tests/spec/gl-1.0/readpixels-oob.c
>>>
>>> diff --git a/tests/all.py b/tests/all.py
>>> index 89288a2..bb7c1dc 100644
>>> --- a/tests/all.py
>>> +++ b/tests/all.py
>>> @@ -997,6 +997,7 @@ with profile.group_manager(
>>>       g(['gl-1.0-edgeflag-const'])
>>>       g(['gl-1.0-edgeflag-quads'])
>>>       g(['gl-1.0-long-dlist'])
>>> +    g(['gl-1.0-readpixels-oob'])
>>>       g(['gl-1.0-rendermode-feedback'])
>>>       g(['gl-1.0-front-invalidate-back'], run_concurrent=False)
>>>       g(['gl-1.0-swapbuffers-behavior'], run_concurrent=False)
>>> diff --git a/tests/spec/gl-1.0/CMakeLists.gl.txt b/tests/spec/gl-1.0/CMakeLists.gl.txt
>>> index 6ef9059..e1c534c 100644
>>> --- a/tests/spec/gl-1.0/CMakeLists.gl.txt
>>> +++ b/tests/spec/gl-1.0/CMakeLists.gl.txt
>>> @@ -26,6 +26,7 @@ piglit_add_executable (gl-1.0-polygon-line-aa polygon-line-aa.c)
>>>   piglit_add_executable (gl-1.0-push-no-attribs push-no-attribs.c)
>>>   piglit_add_executable (gl-1.0-rastercolor rastercolor.c)
>>>   piglit_add_executable (gl-1.0-readpixsanity readpix.c)
>>> +piglit_add_executable (gl-1.0-readpixels-oob readpixels-oob.c)
>>>   piglit_add_executable (gl-1.0-rendermode-feedback rendermode-feedback.c)
>>>   piglit_add_executable (gl-1.0-swapbuffers-behavior swapbuffers-behavior.c)
>>>
>>> diff --git a/tests/spec/gl-1.0/readpixels-oob.c b/tests/spec/gl-1.0/readpixels-oob.c
>>> new file mode 100644
>>> index 0000000..44c0340
>>> --- /dev/null
>>> +++ b/tests/spec/gl-1.0/readpixels-oob.c
>>> @@ -0,0 +1,132 @@
>>> +/*
>>> + * Copyright © 2016 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-oob.c
>>> + *
>>> + * Test that requesting an area larger than the readbuffer (with
>>> + * glReadPixels) will only modify the valid area in the user's buffer.
>>> + * This is equivalent to a user requesting to read into a sub-rectangle
>>> + * of the larger rectangle contained in the provided buffer (via
>>> + * PACK_ROW_LENGTH, PACK_SKIP_ROWS, PACK_SKIP_PIXELS).
>>> + *
>>> + * This behaviour ensures that Mesa does as little work as possible in
>>> + * this imprecise usage of glReadpixels. It also reduces the bugs that
>>> + * may occur with this special case by converting it to an analogous
>>> + * common case.
>>> + *
>>> + */
>>> +
>>> +#include "piglit-util-gl.h"
>>> +
>>> +#define BIG_MULT 3
>>> +
>>> +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
>>> +
>>> +static bool
>>> +test_with_offsets(GLint x_offset, GLint y_offset)
>>> +{
>>> +	bool pass = true;
>>> +	int x, y, i;
>>> +
>>> +	/* Allocate into an oversized buffer. We'll check that the contents
>>> +	 * are still 0 after the glReadPixels.
>>> +	 */
>>> +	const size_t num_chan = 4;
>>> +	const size_t big_buf_h = piglit_height * BIG_MULT;
>>> +	const size_t big_buf_w = piglit_width * BIG_MULT;
>>> +	const size_t tot_elements = big_buf_h * big_buf_w * num_chan;
>>> +	const size_t fb_w = piglit_width - MAX2(0, x_offset);
>>> +	const size_t fb_h = piglit_height - MAX2(0, y_offset);
>>> +	const unsigned dst_x = abs(MIN2(0, x_offset));
>>> +	const unsigned dst_y = abs(MIN2(0, y_offset));
>>> +	GLubyte * black_img = (GLubyte*)calloc(tot_elements, sizeof(GLubyte));
>>> +	GLfloat * black_imgf = (GLfloat*)malloc(tot_elements *sizeof(GLfloat));
>>> +
>>> +	printf("Testing with offsets, x: %d\ty: %d\n", x_offset, y_offset);
>>
>> Is that left-over debug code?  If you want to keep it, that's fine by me.
>>
>>
>
> This wasn't left-over, but I suppose a better place to put this would be
> in the if (!pass) block and to change "Testing" to "Tested". Would that
> still be okay for pushing?

Sounds good.

-Brian




More information about the Piglit mailing list