[Piglit] [PATCH v2 02/10] GL3.2 GL_ARB_sync: Test for valid return values from ClientWaitSync

[Chad Versace]

On 08/20/2013 11:26 AM, Nicholas Mack wrote:
> v2: Fix comments, initialize variables.  Still need to figure out if GPU commands
>      needed before testing these.
> ---
>   tests/spec/arb_sync/CMakeLists.gl.txt        |   3 +-
>   tests/spec/arb_sync/ClientWaitSync-returns.c | 109 +++++++++++++++++++++++++++
>   2 files changed, 111 insertions(+), 1 deletion(-)
>   create mode 100644 tests/spec/arb_sync/ClientWaitSync-returns.c
>
> diff --git a/tests/spec/arb_sync/CMakeLists.gl.txt b/tests/spec/arb_sync/CMakeLists.gl.txt
> index 05f0972..ff8ca85 100644
> --- a/tests/spec/arb_sync/CMakeLists.gl.txt
> +++ b/tests/spec/arb_sync/CMakeLists.gl.txt
> @@ -10,6 +10,7 @@ link_libraries (
>   	${OPENGL_glu_LIBRARY}
>   )
>
> +piglit_add_executable (arb_sync-client-wait-errors ClientWaitSync-errors.c)
> +piglit_add_executable (arb_sync-client-wait-returns ClientWaitSync-returns.c)
>   piglit_add_executable (arb_sync-repeat-wait repeat-wait.c)
>   piglit_add_executable (arb_sync-timeout-zero timeout-zero.c)
> -piglit_add_executable (arb_sync-ClientWaitSync-errors ClientWaitSync-errors.c)
> diff --git a/tests/spec/arb_sync/ClientWaitSync-returns.c b/tests/spec/arb_sync/ClientWaitSync-returns.c
> new file mode 100644
> index 0000000..1002679
> --- /dev/null
> +++ b/tests/spec/arb_sync/ClientWaitSync-returns.c
> @@ -0,0 +1,109 @@
> +/**
> + * 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 ClientWaitSync() returns correct values
> + *
> + *
> + * Section 5.2.1(Waiting for Sync Objects) of OpenGL 3.2 Core says:
> + * "ClientWaitSync returns one of four status values. A return value of
> + *  ALREADY_SIGNALED indicates that sync was signaled at the time
> + *  ClientWaitSync was called. ALREADY_SIGNALED will always be
> + *  returned if sync was signaled, even if the value of timeout is
> + *  zero. A return value of TIMEOUT_EXPIRED indicates that the
> + *  specified timeout period expired before sync was signaled. A re-
> + *  turn value of CONDITION_SATISFIED indicates that sync was signaled
> + *  before the timeout expired. Finally, if an error occurs, in
> + *  addition to generating a GL error as specified below,
> + *  ClientWaitSync immediately returns WAIT_FAILED withoutblocking."


The spec quote should be indented. That's the style used inside Mesa.


> + */
> +
> +#include "piglit-util-gl-common.h"
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +
> +config.supports_gl_compat_version = 10;


Lines inside the CONFIG block should be indented.

Also, this test `supports_gl_core_version = 31`.


> +
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +enum piglit_result
> +piglit_display(void)
> +{
> +	/* UNREACHED */
> +	return PIGLIT_FAIL;
> +}
> +
> +/* One second in nanoseconds */
> +#define ONE_SECOND 1000000


Should be 1000000000 (9 zeros).


> +
> +void
> +piglit_init(int argc, char **argv)
> +{
> +	bool pass = true;
> +	GLsync a, b;
> +	GLenum status1;


The top of piglit_init() should require GL 3.2 or GL_ARB_sync.


> +
> +	/* create syncs */
> +	a = glFenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0);
> +
> +	/* sync <a> has not been signaled yet */
> +	status1 = glClientWaitSync(a, GL_SYNC_FLUSH_COMMANDS_BIT, ONE_SECOND);
> +	if(status1 != GL_CONDITION_SATISFIED) {
> +		printf("Expected GL_CONDITION_SATISFIED but returned: %s\n",
> +			piglit_get_gl_enum_name(status1));
> +		pass = false;
> +	}
> +
> +	/* sync <a> has already been signaled */
> +	status1 = glClientWaitSync(a, GL_SYNC_FLUSH_COMMANDS_BIT, ONE_SECOND);
> +	if(status1 != GL_ALREADY_SIGNALED) {
> +		printf("Expected GL_ALREADY_SIGNALED but returned: %s\n",
> +			piglit_get_gl_enum_name(status1));
> +		pass = false;
> +	}
> +
> +	/* sync <a> has already been signaled even though timeout is 0 */
> +	status1 = glClientWaitSync(a, GL_SYNC_FLUSH_COMMANDS_BIT, 0);
> +	if(status1 != GL_ALREADY_SIGNALED) {
> +		printf("Expected GL_ALREADY_SIGNALED with timeout but returned: %s\n",
> +			piglit_get_gl_enum_name(status1));
> +		pass = false;
> +	}
> +
> +	/* create new sync and call ClientWaitSync with a quick timeout */
> +	b = glFenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0);
> +
> +	/* sync <b> will be checked after timeout expires */
> +	status1 = glClientWaitSync(b, GL_SYNC_FLUSH_COMMANDS_BIT, 0);
> +	if(status1 != GL_TIMEOUT_EXPIRED) {
> +		printf("Expected GL_TIMEOUT_EXPIRED but returned: %s\n",
> +			piglit_get_gl_enum_name(status1));
> +		pass = false;
> +	}
> +
> +	glDeleteSync(a);
> +	glDeleteSync(b);
> +
> +	piglit_report_result(pass ? PIGLIT_PASS : PIGLIT_FAIL);
> +}


This test is attempting to verify some difficult things: correct signaling of
fences in the command stream and timeouts. The test needs some changes to do
that correctly.

Let's discuss each of the testcases attempted by this test:

1. Goal: Insert a fence into the command stream. Wait on the fence with a 1 second
    timeout. Verify that it signals after 1 second with GL_TIMEOUT_EXPIRED.

    To test this, we need to queue some rendering commands that will take longer
    than 1 second to complete. Otherwise, the fence will signal before the timeout,
    and that prevents us from testing the timeout functionality.

    If we call piglit_draw_rect(), that will queue a glDraw* command. As long as we
    queue no other commands, and don't call glFlush(), then that command should never
    complete (at least on the Intel driver).

    So, this testcase should look like this:

       piglit_draw_rect();
       fence1 = glFenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0);

       // TODO: Use clock_gettime(CLOCK_MONOTONIC) to get a timestamp.

       status1 = glClientWaitSync(a, GL_SYNC_FLUSH_COMMANDS_BIT, ONE_SECOND);

       /* The draw call should never complete, because we didn't call glFlush(). */
       if(status1 != GL_TIMEOUT_EXPIRED) {
             // TODO: Fail the test
       }

       // TODO: Use clock_gettime(CLOCK_MONOTONIC) to get a timestamp.
       // TODO: If less than one second expired, or significantly more than
       // TODO: one second, then fail the test.

       /* Cleanup this testcase in preparation for the next one. Force the
        * draw call to complete and the fence to signal.
        */
       glFlush();


2. Goal: Insert a fence into the command stream. Wait on it with no timeout.
    Verify that glClientWaitSync() returns immediately with GL_TIMEOUT_EXPIRED.

    This testcase should look very similar to #1. The only difference is it
    should verify with timestamps that the duration of glClientWaitSync()
    is very small.

3. Goal: Insert a fence into the command stream. Flush the command stream
    and block on its completion. Afterwards, verify that the fence was signaled.

       piglit_draw_rect();
       fence3 = glFenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0);
       glFinish();
       status3 = glClientWaitSync(a, GL_SYNC_FLUSH_COMMANDS_BIT, 0);
       if (status3 != GL_ALREADY_SIGNALED) {
           // TODO: Fail the test.
       }

4. Goal: Insert a fence into the command stream. Call glClientWaitSync() *before*
    the commands complete. Verify that glClientWaitSync returns GL_CONDITION_SATISFIED.

    Testing this will be difficult. It's necessary to make a draw call that takes a long
    time; or, write a test that uses multiple threads. I believe the correct way to
    write such a test is to use multiple threads, because using a long draw call may lead to
    indeterminate test results.

    Writing a multithreaded Piglit test for #4 looks difficult to me. Let's save that testcase
    for another day.


Since this test will use the Posix clock API, we need to build this test *only*
if the system supports Posix clocks. That requires CMake magic. If you finish
writing and fixing the tests, I'll take care of the CMake magic.