[Piglit] [PATCH v2 08/10] GL3.2 GL_ARB_sync: Test that a sync object is initialized with the correct properties

[Chad Versace]

On 08/20/2013 11:26 AM, Nicholas Mack wrote:
> v2: Fix comments, add extra checks for length variable being modified
> ---
>   tests/spec/arb_sync/CMakeLists.gl.txt |   1 +
>   tests/spec/arb_sync/sync-initialize.c | 125 ++++++++++++++++++++++++++++++++++
>   2 files changed, 126 insertions(+)
>   create mode 100644 tests/spec/arb_sync/sync-initialize.c


Again, the test should be added to all.tests.


>
> diff --git a/tests/spec/arb_sync/CMakeLists.gl.txt b/tests/spec/arb_sync/CMakeLists.gl.txt
> index 038e0e1..9855d69 100644
> --- a/tests/spec/arb_sync/CMakeLists.gl.txt
> +++ b/tests/spec/arb_sync/CMakeLists.gl.txt
> @@ -17,4 +17,5 @@ piglit_add_executable (arb_sync-fence-errors FenceSync-errors.c)
>   piglit_add_executable (arb_sync-get-sync-errors GetSynciv-errors.c)
>   piglit_add_executable (arb_sync-is-sync IsSync.c)
>   piglit_add_executable (arb_sync-repeat-wait repeat-wait.c)
> +piglit_add_executable (arb_sync-sync-initialize sync-initialize.c)
>   piglit_add_executable (arb_sync-timeout-zero timeout-zero.c)
> diff --git a/tests/spec/arb_sync/sync-initialize.c b/tests/spec/arb_sync/sync-initialize.c
> new file mode 100644
> index 0000000..eb1bb86
> --- /dev/null
> +++ b/tests/spec/arb_sync/sync-initialize.c
> @@ -0,0 +1,125 @@
> +/**
> + * 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.
> + */
> +
> +/**


Again, a \file command is needed here.


> + * Test that a sync is initialized correctly with FenceSync
> + *
> + * Section 5.2(Sync Objects and Fences) of OpenGL 3.2 Core says:
> + *  "Table 5.1: Initial properties of a sync object created with FenceSync."
> + *
> + *  Property Name	Property Value
> + * --------------------------------------
> + *  OBJECT_TYPE		SYNC_FENCE
> + *  SYNC_CONDITION	<condition>
> + *  SYNC_STATUS		UNSIGNALED
> + *  SYNC_FLAGS		<flags>
> + *


Indent the spec quote.


> + */
> +
> +#include "piglit-util-gl-common.h"
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +
> +config.supports_gl_compat_version = 10;
> +config.supports_gl_core_version = 31;
> +
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +enum piglit_result
> +piglit_display(void)
> +{
> +	/* UNREACHED */
> +	return PIGLIT_FAIL;
> +}
> +
> +static GLsync test;


This variable is used only inside piglit_init(), so move its declaration there
and remove 'static'.

Also, 'sync' or 'fence' would be a better variable name.


> +
> +void
> +piglit_init(int argc, char **argv)
> +{
> +	bool pass = true;
> +	GLsizei length = -5;
> +	GLint value;
> +
> +	if (piglit_get_gl_version() < 32) {
> +		piglit_require_extension("GL_ARB_sync");
> +	}
> +
> +	test = glFenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0);


In each testcase below, you check that `length == 1`. That's good, but you should
also reset length to an invalid value (-5) before each call glGetSynciv().


> +
> +	/* Test initialized as fence type */
> +	glGetSynciv(test, GL_OBJECT_TYPE, 1, &length, &value);
> +	if(length != 1) {
> +		printf("length should be 1 but incorrectly returned: %d\n",
> +			length);
> +		pass = false;
> +	}
> +	if(value != GL_SYNC_FENCE) {
> +		printf("Expected GL_SYNC_FENCE but returned: %s\n",
> +			piglit_get_gl_enum_name(value));
> +		pass = false;
> +	}
> +
> +	/* Test initialized to given condition */
> +	glGetSynciv(test, GL_SYNC_CONDITION, 1, &length, &value);
> +	if(length != 1) {
> +		printf("length should be 1 but incorrectly returned: %d\n",
> +			length);
> +		pass = false;
> +	}
> +	if(value != GL_SYNC_GPU_COMMANDS_COMPLETE) {
> +		printf("Expected GL_SYNC_GPU_COMMANDS_COMPLETE but returned: %s\n",
> +			piglit_get_gl_enum_name(value));
> +		pass = false;
> +	}
> +
> +	/* Test initialed to unsignaled */
                 ^^^ initialized

> +	glGetSynciv(test, GL_SYNC_STATUS, 1, &length, &value);
> +	if(length != 1) {
> +		printf("length should be 1 but incorrectly returned: %d\n",
> +			length);
> +		pass = false;
> +	}
> +	if(value != GL_UNSIGNALED) {
> +		printf("Expected GL_UNSIGNALED but returned: %s\n",
> +			piglit_get_gl_enum_name(value));
> +		pass = false;
> +	}


In this test, I believe the sync object's status here should be GL_SIGNALED because all commands
in the GL stream will have trivially completed by this point, because there are no GL render commands.

However, we still want to check that the initial value of a fence is GL_UNSIGNALED. To check that,
before calling glFenceSync() the test needs to do some non-trivial rendering. And we need to have
strong confidence that the rendering will not complete quickly.

glClear won't work, because in the Intel driver sometimes glClear is executed immediately.

glDraw* should work, at least for the Intel driver, because Intel usually doesn't execute the
draw until (a) the GL context has queued up several draw commands or (b) the GL context calls
glFlush().

An easy way to call glDraw* is piglit_draw_rect(). So, this test should call piglit_draw_rect()
immediately before calling glFenceSync(). And the test needs some comments explaining why we
do that.


> +
> +	/* Test initialized with given flag */
> +	glGetSynciv(test, GL_SYNC_FLAGS, 1, &length, &value);
> +	if(length != 1) {
> +		printf("length should be 1 but incorrectly returned: %d\n",
> +			length);
> +		pass = false;
> +	}
> +	if(value != 0) {
> +		printf("Expected GL_SYNC_FLAGS == 0 but returned: %d\n",
> +			value);
> +		pass = false;
> +	}
> +
> +	glDeleteSync(test);
> +
> +	piglit_report_result(pass ? PIGLIT_PASS : PIGLIT_FAIL);
> +}
>