Re: [Piglit] [PATCH 2/8] GL3.2 GL_ARB_sync: Test for valid return values from ClientWaitSync

2013-10-14 Thread Chad Versace

On 10/07/2013 08:46 AM, Nicholas Mack wrote:

v2: Fix comments, initialize variables.  Still need to figure out if GPU 
commands
 needed before testing these.

v3: Rewrite test cases, fix comment and config block and add to all.tests
---
  tests/all.tests  |   1 +
  tests/spec/arb_sync/CMakeLists.gl.txt|   3 +-
  tests/spec/arb_sync/ClientWaitSync-returns.c | 195 +++
  3 files changed, 198 insertions(+), 1 deletion(-)
  create mode 100644 tests/spec/arb_sync/ClientWaitSync-returns.c

diff --git a/tests/all.tests b/tests/all.tests
index 4aa77f4..0118cc9 100644
--- a/tests/all.tests
+++ b/tests/all.tests
@@ -1100,6 +1100,7 @@ 
import_glsl_parser_tests(spec['ARB_shader_stencil_export'],
  arb_sync = Group()
  spec['ARB_sync'] = arb_sync
  arb_sync['ClientWaitSync-errors'] = 
concurrent_test('arb_sync-ClientWaitSync-errors')
+arb_sync['ClientWaitSync-returns'] = 
concurrent_test('arb_sync-ClientWaitSync-returns')


The actual executable name in the CMake is arb_sync-client-wait-returns.

Before submitting a patch series, it's always a good idea to do something like 
this
to check that the Python and CMake agree, and that all the expected tests show
up in the results file.

  make clean  (make help | grep arb_sync | cut -d: -f1 | xargs make)  
./piglit-run.py -t arb_sync



I submitted a patch, with you CC'd, that introduces the needed CMake magic to
detect if the system supports Posix clocks. Please wait until that patch lands
before committing this one. The CMake snippet in this patch should look like
below to ensure that this test gets built only on systems with Posix clocks.

if(PIGLIT_HAS_POSIX_CLOCKS)
piglit_add_executable(my-test-name ...)
endif()

After that, you can remove all the requires Posix comments in the test.


+/* One second in nanoseconds */
+#define ONE_SECOND 10
+
+void
+piglit_init(int argc, char **argv)
+{
+   bool pass = true;
+   GLsync fence1,  fence2,  fence3;
+   GLenum status1, status2, status3;
+   /* requires Posix clock API
+   timespec startTime, endTime;
+   int timeStatus = -1, seconds_elapsed = -1;
+   */
+
+   if (piglit_get_gl_version()  32) {
+   piglit_require_extension(GL_ARB_sync);
+   }
+
+   /* Test Case 1: Verify that fence times out correctly after set time */
+
+   /* queue a draw command */
+   piglit_draw_rect(-1, -1, 2, 2);
+
+   /* create fence sync */
+   fence1 = glFenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0);
+
+   /* check fence status */
+   status1 = glClientWaitSync(fence1, GL_SYNC_FLUSH_COMMANDS_BIT,
+   ONE_SECOND);
+
+   /* requires Posix clock API
+   timeStatus = clock_gettime(CLOCK_MONOTONIC, startTime);
+   if (timeStatus != 0) {
+   printf(Intial clock time not set correctly\n);
+   piglit_report_result(PIGLIT_FAIL);
+   }
+   */


The test must get the start timestamp immediately before calling 
glClientWaitSync!
The way this patch is currently written, startTime and endTime will always be
*very* close, because there is no blocking before the first call to 
clock_gettime()
and the second call.


+
+   /* draw call should not be complete since glFlush() wasn't called */
+   if (status1 != GL_TIMEOUT_EXPIRED) {
+   printf(Expected: status1 == GL_TIMEOUT_EXPIRED\n
+   Actual: status1 == %s\n,
+   piglit_get_gl_enum_name(status1));
+   pass = false;
+   }
+
+   /* requires Posix clock API
+   timeStatus = clock_gettime(CLOCK_MONOTONIC, endTime);
+   if (timeStatus != 0) {
+   printf(End clock time not set correctly\n);
+   piglit_report_result(PIGLIT_FAIL);
+   }
+   seconds_elapsed = endTime.tv_sec - startTime.tv_sec;
+   if (seconds_elapsed  1) { //TODO: how to check for significantly 
greater?
+   printf(Expected: wait = 1 second\n
+   Actual: wait = %d seconds, seconds_elapsed);
+   }
+   */
+
+   /* clean up */
+   glFlush();
+   glDeleteSync(fence1);
+
+   /* Test Case 2: Verify that fence times out correctly with no timeout */
+
+   /* queue a draw command */
+   piglit_draw_rect(-1, -1, 2, 2);
+
+   /* create fence sync */
+   fence2 = glFenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0);
+
+   /* check fence status */
+   status2 = glClientWaitSync(fence2, GL_SYNC_FLUSH_COMMANDS_BIT,
+   0);
+
+   /* requires Posix clock API
+   timeStatus = clock_gettime(CLOCK_MONOTONIC, startTime);
+   if (timeStatus != 0) {
+   printf(Intial clock time not set correctly\n);
+   piglit_report_result(PIGLIT_FAIL);
+   }
+   */


Same problem for testcase 2. Get startTime immediately before calling 
glClientWaitSync.


+
+   

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

2013-10-07 Thread Nicholas Mack
v2: Fix comments, initialize variables.  Still need to figure out if GPU 
commands
needed before testing these.

v3: Rewrite test cases, fix comment and config block and add to all.tests
---
 tests/all.tests  |   1 +
 tests/spec/arb_sync/CMakeLists.gl.txt|   3 +-
 tests/spec/arb_sync/ClientWaitSync-returns.c | 195 +++
 3 files changed, 198 insertions(+), 1 deletion(-)
 create mode 100644 tests/spec/arb_sync/ClientWaitSync-returns.c

diff --git a/tests/all.tests b/tests/all.tests
index 4aa77f4..0118cc9 100644
--- a/tests/all.tests
+++ b/tests/all.tests
@@ -1100,6 +1100,7 @@ 
import_glsl_parser_tests(spec['ARB_shader_stencil_export'],
 arb_sync = Group()
 spec['ARB_sync'] = arb_sync
 arb_sync['ClientWaitSync-errors'] = 
concurrent_test('arb_sync-ClientWaitSync-errors')
+arb_sync['ClientWaitSync-returns'] = 
concurrent_test('arb_sync-ClientWaitSync-returns')
 arb_sync['repeat-wait'] = concurrent_test('arb_sync-repeat-wait')
 arb_sync['timeout-zero'] = concurrent_test('arb_sync-timeout-zero')
 add_plain_test(arb_sync, 'sync_api')
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 000..e801e04
--- /dev/null
+++ b/tests/spec/arb_sync/ClientWaitSync-returns.c
@@ -0,0 +1,195 @@
+/**
+ * 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
+ * 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.
+ *
+ */
+
+#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;
+}
+
+/* One second in nanoseconds */
+#define ONE_SECOND 10
+
+void
+piglit_init(int argc, char **argv)
+{
+   bool pass = true;
+   GLsync fence1,  fence2,  fence3;
+   GLenum status1, status2, status3;
+   /* requires Posix clock API
+   timespec startTime, endTime;
+   int timeStatus = -1, seconds_elapsed = -1;
+   */
+
+   if (piglit_get_gl_version()  32) {
+   piglit_require_extension(GL_ARB_sync);
+   }
+
+   /* Test Case 1: Verify that fence times out correctly after set time */
+
+   /* queue a draw command */
+   piglit_draw_rect(-1, -1, 2, 2);
+
+   /* create fence sync */
+   fence1 =