Re: [Piglit] [PATCH] gl 1.0: Stress test GL_LINE_LOOP.

2018-01-11 Thread Brian Paul


Reviewed-by: Brian Paul 


On 01/11/2018 12:54 PM, Fabian Bieler wrote:

Draw circles with long line loops and check the result.

In particular, check that the last line segment is drawn and that no
additional segments in between arbitrary vertices are drawn.

V2: Increase vertex size and count to exhaust Mesa's vbo buffer and Gallium's
 auxiliary draw buffer.
 Add command line argument to set vertex count.
---


I fixed some long line loop issues in Mesa/gallium a few years ago.  I _think_ 
0x2000 (8192) is enough, but you might want to check more (Note that Mesa 
llvmpipe reports GL_MAX_ELEMENTS_VERTICES = 3000).



Also, I see you're stepping the vertex count by <<=2 in the test loop below.  
That _might_ skip some magic vertex counts which tickle bugs. Off-hand I don't have a 
good suggesting for how to be thorough about this without being really slow.

I toyed and dismissed the idea of additional tests with random vertex counts as 
I don't think the occasional failure of this test would be investigated. I did 
add a command line option to manually set the vertex count for easier 
debugging, though.


There's (at least) two places where the vertex buffer limits (the VBO module 
and draw/vbuf code) will trigger splitting/continuing long line loops.  This 
test should try to exceed both those buffer limits.  You might set some 
breakpoints in the Mesa/gallium code to see if they're getting hit.

Mesa's VBO buffer is 1MB. Gallium's auxiliary draw code buffer size varies from 
driver to driver, but it's never bigger than 1MB.
I increased the vertex size and count so that the vertex array is at least 3MB 
in size.

  tests/all.py|   1 +
  tests/spec/gl-1.0/CMakeLists.gl.txt |   1 +
  tests/spec/gl-1.0/long-line-loop.c  | 199 
  3 files changed, 201 insertions(+)
  create mode 100644 tests/spec/gl-1.0/long-line-loop.c

diff --git a/tests/all.py b/tests/all.py
index 32ab2b610..6abcb3aa8 100644
--- a/tests/all.py
+++ b/tests/all.py
@@ -727,6 +727,7 @@ with profile.test_list.group_manager(
  g(['gl-1.0-edgeflag-quads'])
  g(['gl-1.0-empty-begin-end-clause'])
  g(['gl-1.0-long-dlist'])
+g(['gl-1.0-long-line-loop'])
  g(['gl-1.0-readpixels-oob'])
  g(['gl-1.0-rendermode-feedback'])
  g(['gl-1.0-front-invalidate-back'], run_concurrent=False)
diff --git a/tests/spec/gl-1.0/CMakeLists.gl.txt 
b/tests/spec/gl-1.0/CMakeLists.gl.txt
index 355df7472..2df3a8cf4 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-fpexceptions fpexceptions.c)
  piglit_add_executable (gl-1.0-front-invalidate-back front-invalidate-back.c)
  piglit_add_executable (gl-1.0-logicop logicop.c)
  piglit_add_executable (gl-1.0-long-dlist long-dlist.c)
+piglit_add_executable (gl-1.0-long-line-loop long-line-loop.c)
  piglit_add_executable (gl-1.0-ortho-pos orthpos.c)
  piglit_add_executable (gl-1.0-no-op-paths no-op-paths.c)
  piglit_add_executable (gl-1.0-polygon-line-aa polygon-line-aa.c)
diff --git a/tests/spec/gl-1.0/long-line-loop.c 
b/tests/spec/gl-1.0/long-line-loop.c
new file mode 100644
index 0..02b756caa
--- /dev/null
+++ b/tests/spec/gl-1.0/long-line-loop.c
@@ -0,0 +1,199 @@
+/*
+ * Copyright © 2017 Fabian Bieler
+ *
+ * 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 long-line-loop.c
+ * Draw cricles with line loops and a line strips blended on top of each
+ * other and check that the renderings match.
+ */
+
+#include "piglit-util-gl.h"
+
+PIGLIT_GL_TEST_CONFIG_BEGIN
+
+   config.supports_gl_compat_version = 10;
+   config.window_width = 1024;
+   config.window_height = 1024;
+   config.window_visual = PIGLIT_GL_VISUAL_RGB;
+   config.khr_no_error_support = PIGLIT_NO_ERRORS;
+
+PIGLIT_GL_TEST_CONFIG_END
+
+static int max_vertices;
+static int num_vertice

[Piglit] [PATCH] gl 1.0: Stress test GL_LINE_LOOP.

2018-01-11 Thread Fabian Bieler
Draw circles with long line loops and check the result.

In particular, check that the last line segment is drawn and that no
additional segments in between arbitrary vertices are drawn.

V2: Increase vertex size and count to exhaust Mesa's vbo buffer and Gallium's
auxiliary draw buffer.
Add command line argument to set vertex count.
---

> I fixed some long line loop issues in Mesa/gallium a few years ago.  I 
> _think_ 0x2000 (8192) is enough, but you might want to check more (Note that 
> Mesa llvmpipe reports GL_MAX_ELEMENTS_VERTICES = 3000).

> Also, I see you're stepping the vertex count by <<=2 in the test loop below.  
> That _might_ skip some magic vertex counts which tickle bugs. Off-hand I 
> don't have a good suggesting for how to be thorough about this without being 
> really slow.
I toyed and dismissed the idea of additional tests with random vertex counts as 
I don't think the occasional failure of this test would be investigated. I did 
add a command line option to manually set the vertex count for easier 
debugging, though.

> There's (at least) two places where the vertex buffer limits (the VBO module 
> and draw/vbuf code) will trigger splitting/continuing long line loops.  This 
> test should try to exceed both those buffer limits.  You might set some 
> breakpoints in the Mesa/gallium code to see if they're getting hit.
Mesa's VBO buffer is 1MB. Gallium's auxiliary draw code buffer size varies from 
driver to driver, but it's never bigger than 1MB.
I increased the vertex size and count so that the vertex array is at least 3MB 
in size.

 tests/all.py|   1 +
 tests/spec/gl-1.0/CMakeLists.gl.txt |   1 +
 tests/spec/gl-1.0/long-line-loop.c  | 199 
 3 files changed, 201 insertions(+)
 create mode 100644 tests/spec/gl-1.0/long-line-loop.c

diff --git a/tests/all.py b/tests/all.py
index 32ab2b610..6abcb3aa8 100644
--- a/tests/all.py
+++ b/tests/all.py
@@ -727,6 +727,7 @@ with profile.test_list.group_manager(
 g(['gl-1.0-edgeflag-quads'])
 g(['gl-1.0-empty-begin-end-clause'])
 g(['gl-1.0-long-dlist'])
+g(['gl-1.0-long-line-loop'])
 g(['gl-1.0-readpixels-oob'])
 g(['gl-1.0-rendermode-feedback'])
 g(['gl-1.0-front-invalidate-back'], run_concurrent=False)
diff --git a/tests/spec/gl-1.0/CMakeLists.gl.txt 
b/tests/spec/gl-1.0/CMakeLists.gl.txt
index 355df7472..2df3a8cf4 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-fpexceptions fpexceptions.c)
 piglit_add_executable (gl-1.0-front-invalidate-back front-invalidate-back.c)
 piglit_add_executable (gl-1.0-logicop logicop.c)
 piglit_add_executable (gl-1.0-long-dlist long-dlist.c)
+piglit_add_executable (gl-1.0-long-line-loop long-line-loop.c)
 piglit_add_executable (gl-1.0-ortho-pos orthpos.c)
 piglit_add_executable (gl-1.0-no-op-paths no-op-paths.c)
 piglit_add_executable (gl-1.0-polygon-line-aa polygon-line-aa.c)
diff --git a/tests/spec/gl-1.0/long-line-loop.c 
b/tests/spec/gl-1.0/long-line-loop.c
new file mode 100644
index 0..02b756caa
--- /dev/null
+++ b/tests/spec/gl-1.0/long-line-loop.c
@@ -0,0 +1,199 @@
+/*
+ * Copyright © 2017 Fabian Bieler
+ *
+ * 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 long-line-loop.c
+ * Draw cricles with line loops and a line strips blended on top of each
+ * other and check that the renderings match.
+ */
+
+#include "piglit-util-gl.h"
+
+PIGLIT_GL_TEST_CONFIG_BEGIN
+
+   config.supports_gl_compat_version = 10;
+   config.window_width = 1024;
+   config.window_height = 1024;
+   config.window_visual = PIGLIT_GL_VISUAL_RGB;
+   config.khr_no_error_support = PIGLIT_NO_ERRORS;
+
+PIGLIT_GL_TEST_CONFIG_END
+
+static int max_vertices;
+static int num_vertices;
+static int probe_location[2];
+static const float radius = 0.9;
+
+static