primitive-restart: extend the test to catch regressions of bug-109451

Submitted by andrey simiklit on Feb. 1, 2019, 9:27 a.m.

Details

Message ID 20190201092718.17818-1-asimiklit.work@gmail.com
State New
Headers show
Series "primitive-restart: extend the test to catch regressions of bug-109451" ( rev: 1 ) in Piglit

Not browsing as part of any series.

Commit Message

andrey simiklit Feb. 1, 2019, 9:27 a.m.
From: Andrii Simiklit <andrii.simiklit@globallogic.com>

Looks like this test should be able to catch this kind of the regressions
os I add it here.
This test should draw green rectangle if everything is ok.
But in case of bug which makes us unable to disable
the primitive restart option it should draw just one triangle.

The possible solution was suggested.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109451
Signed-off-by: Andrii Simiklit <andrii.simiklit@globallogic.com>
---
 tests/general/primitive-restart.c | 72 +++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

Patch hide | download patch | download mbox

diff --git a/tests/general/primitive-restart.c b/tests/general/primitive-restart.c
index 4219a6c5b..aef9bed41 100644
--- a/tests/general/primitive-restart.c
+++ b/tests/general/primitive-restart.c
@@ -75,6 +75,11 @@  static bool Have_31;
 static bool TestGL31;
 
 
+static void
+enable_restart(GLuint restart_index);
+static void
+disable_restart(void);
+
 static bool
 check_rendering(void)
 {
@@ -184,6 +189,71 @@  test_begin_end(GLenum primMode)
    return pass;
 }
 
+static void
+write_vec2_value(GLfloat * verts, GLuint vidx, GLfloat x, GLfloat y)
+{
+   verts[(vidx * 2) + 0] = x;
+   verts[(vidx * 2) + 1] = y;
+}
+/* This test should draw green rectangle if everything is ok.
+ * But in case of bug which makes us unable to disable
+ * the primitive restart option it should draw just one triangle.
+ */
+static bool
+test_shared_ib_restart()
+{
+   bool pass = true;
+   GLfloat verts[256 * 2];
+   memset(&verts, 0, sizeof(verts));
+   //left-bottom
+   write_vec2_value(verts, 0, 0.0f, 0.0f);
+   //right-bottom
+   write_vec2_value(verts, 1, piglit_width, 0.0f);
+   //left-top
+   write_vec2_value(verts, 2, 0.0f, piglit_height);
+   //right-top
+   write_vec2_value(verts, 255, piglit_width, piglit_height);
+
+   const GLubyte indices[] = { 0, 1, 2, 255, 0 };
+   const GLfloat expected[3] = { 0.0f, 1.0f, 0.0f };
+   GLuint vbo1, vbo2;
+
+   piglit_ortho_projection(piglit_width, piglit_height, GL_FALSE);
+
+   glClear(GL_COLOR_BUFFER_BIT);
+
+   glColor4fv(green);
+
+   glGenBuffers(1, &vbo1);
+   glBindBuffer(GL_ARRAY_BUFFER, vbo1);
+   glBufferData(GL_ARRAY_BUFFER, sizeof(verts), verts, GL_DYNAMIC_DRAW);
+   glVertexPointer(2, GL_FLOAT, 0, (void *)0);
+
+   glGenBuffers(1, &vbo2);
+   glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, vbo2);
+   glBufferData(GL_ELEMENT_ARRAY_BUFFER, sizeof(indices), indices, GL_STATIC_DRAW);
+   glEnableClientState(GL_VERTEX_ARRAY);
+
+   //We should draw something with an enabled restart option to check that
+   //it could be correctly disabled
+   enable_restart(0xff);
+   glDrawElements(GL_LINE_STRIP, ARRAY_SIZE(indices), GL_UNSIGNED_BYTE, (void*)0);
+   disable_restart();
+
+   //Draw full screen rectangle
+   glDrawElements(GL_TRIANGLE_STRIP, ARRAY_SIZE(indices) - 1, GL_UNSIGNED_BYTE, (void*)0);
+
+   glDisableClientState(GL_VERTEX_ARRAY);
+   glBindBuffer(GL_ARRAY_BUFFER, 0);
+   glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0);
+   glDeleteBuffers(1, &vbo1);
+   glDeleteBuffers(1, &vbo2);
+   glFinish();
+   pass = (piglit_probe_rect_rgb(0, 0, piglit_width, piglit_height, expected) != 0) && pass;
+   piglit_present_results();
+   return pass;
+}
+
 
 static void
 enable_restart(GLuint restart_index)
@@ -503,6 +573,7 @@  primitive_restart_test(VBO_CFG vbo_cfg)
 
    if (Have_NV) {
       TestGL31 = false;
+      pass = test_shared_ib_restart() && pass;
       pass = test_begin_end(GL_TRIANGLE_STRIP) && pass;
       pass = test_begin_end(GL_LINE_STRIP) && pass;
       pass = test_draw_elements(vbo_cfg, GL_TRIANGLE_STRIP, GL_UNSIGNED_BYTE) && pass;
@@ -521,6 +592,7 @@  primitive_restart_test(VBO_CFG vbo_cfg)
 
    if (Have_31) {
       TestGL31 = true;
+      pass = test_shared_ib_restart() && pass;
       pass = test_draw_elements(vbo_cfg, GL_TRIANGLE_STRIP, GL_UNSIGNED_BYTE) && pass;
       pass = test_draw_elements(vbo_cfg, GL_TRIANGLE_STRIP, GL_UNSIGNED_SHORT) && pass;
       pass = test_draw_elements(vbo_cfg, GL_TRIANGLE_STRIP, GL_UNSIGNED_INT) && pass;

Comments

ping

On Fri, Feb 1, 2019 at 11:27 AM <asimiklit.work@gmail.com> wrote:

> From: Andrii Simiklit <andrii.simiklit@globallogic.com>
>
> Looks like this test should be able to catch this kind of the regressions
> os I add it here.
> This test should draw green rectangle if everything is ok.
> But in case of bug which makes us unable to disable
> the primitive restart option it should draw just one triangle.
>
> The possible solution was suggested.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109451
> Signed-off-by: Andrii Simiklit <andrii.simiklit@globallogic.com>
> ---
>  tests/general/primitive-restart.c | 72 +++++++++++++++++++++++++++++++
>  1 file changed, 72 insertions(+)
>
> diff --git a/tests/general/primitive-restart.c
> b/tests/general/primitive-restart.c
> index 4219a6c5b..aef9bed41 100644
> --- a/tests/general/primitive-restart.c
> +++ b/tests/general/primitive-restart.c
> @@ -75,6 +75,11 @@ static bool Have_31;
>  static bool TestGL31;
>
>
> +static void
> +enable_restart(GLuint restart_index);
> +static void
> +disable_restart(void);
> +
>  static bool
>  check_rendering(void)
>  {
> @@ -184,6 +189,71 @@ test_begin_end(GLenum primMode)
>     return pass;
>  }
>
> +static void
> +write_vec2_value(GLfloat * verts, GLuint vidx, GLfloat x, GLfloat y)
> +{
> +   verts[(vidx * 2) + 0] = x;
> +   verts[(vidx * 2) + 1] = y;
> +}
> +/* This test should draw green rectangle if everything is ok.
> + * But in case of bug which makes us unable to disable
> + * the primitive restart option it should draw just one triangle.
> + */
> +static bool
> +test_shared_ib_restart()
> +{
> +   bool pass = true;
> +   GLfloat verts[256 * 2];
> +   memset(&verts, 0, sizeof(verts));
> +   //left-bottom
> +   write_vec2_value(verts, 0, 0.0f, 0.0f);
> +   //right-bottom
> +   write_vec2_value(verts, 1, piglit_width, 0.0f);
> +   //left-top
> +   write_vec2_value(verts, 2, 0.0f, piglit_height);
> +   //right-top
> +   write_vec2_value(verts, 255, piglit_width, piglit_height);
> +
> +   const GLubyte indices[] = { 0, 1, 2, 255, 0 };
> +   const GLfloat expected[3] = { 0.0f, 1.0f, 0.0f };
> +   GLuint vbo1, vbo2;
> +
> +   piglit_ortho_projection(piglit_width, piglit_height, GL_FALSE);
> +
> +   glClear(GL_COLOR_BUFFER_BIT);
> +
> +   glColor4fv(green);
> +
> +   glGenBuffers(1, &vbo1);
> +   glBindBuffer(GL_ARRAY_BUFFER, vbo1);
> +   glBufferData(GL_ARRAY_BUFFER, sizeof(verts), verts, GL_DYNAMIC_DRAW);
> +   glVertexPointer(2, GL_FLOAT, 0, (void *)0);
> +
> +   glGenBuffers(1, &vbo2);
> +   glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, vbo2);
> +   glBufferData(GL_ELEMENT_ARRAY_BUFFER, sizeof(indices), indices,
> GL_STATIC_DRAW);
> +   glEnableClientState(GL_VERTEX_ARRAY);
> +
> +   //We should draw something with an enabled restart option to check that
> +   //it could be correctly disabled
> +   enable_restart(0xff);
> +   glDrawElements(GL_LINE_STRIP, ARRAY_SIZE(indices), GL_UNSIGNED_BYTE,
> (void*)0);
> +   disable_restart();
> +
> +   //Draw full screen rectangle
> +   glDrawElements(GL_TRIANGLE_STRIP, ARRAY_SIZE(indices) - 1,
> GL_UNSIGNED_BYTE, (void*)0);
> +
> +   glDisableClientState(GL_VERTEX_ARRAY);
> +   glBindBuffer(GL_ARRAY_BUFFER, 0);
> +   glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0);
> +   glDeleteBuffers(1, &vbo1);
> +   glDeleteBuffers(1, &vbo2);
> +   glFinish();
> +   pass = (piglit_probe_rect_rgb(0, 0, piglit_width, piglit_height,
> expected) != 0) && pass;
> +   piglit_present_results();
> +   return pass;
> +}
> +
>
>  static void
>  enable_restart(GLuint restart_index)
> @@ -503,6 +573,7 @@ primitive_restart_test(VBO_CFG vbo_cfg)
>
>     if (Have_NV) {
>        TestGL31 = false;
> +      pass = test_shared_ib_restart() && pass;
>        pass = test_begin_end(GL_TRIANGLE_STRIP) && pass;
>        pass = test_begin_end(GL_LINE_STRIP) && pass;
>        pass = test_draw_elements(vbo_cfg, GL_TRIANGLE_STRIP,
> GL_UNSIGNED_BYTE) && pass;
> @@ -521,6 +592,7 @@ primitive_restart_test(VBO_CFG vbo_cfg)
>
>     if (Have_31) {
>        TestGL31 = true;
> +      pass = test_shared_ib_restart() && pass;
>        pass = test_draw_elements(vbo_cfg, GL_TRIANGLE_STRIP,
> GL_UNSIGNED_BYTE) && pass;
>        pass = test_draw_elements(vbo_cfg, GL_TRIANGLE_STRIP,
> GL_UNSIGNED_SHORT) && pass;
>        pass = test_draw_elements(vbo_cfg, GL_TRIANGLE_STRIP,
> GL_UNSIGNED_INT) && pass;
> --
> 2.17.1
>
>
On 01/02/2019 09:27, asimiklit.work@gmail.com wrote:
> From: Andrii Simiklit <andrii.simiklit@globallogic.com>
>
> Looks like this test should be able to catch this kind of the regressions
> os I add it here.
> This test should draw green rectangle if everything is ok.
> But in case of bug which makes us unable to disable
> the primitive restart option it should draw just one triangle.
>
> The possible solution was suggested.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109451
> Signed-off-by: Andrii Simiklit <andrii.simiklit@globallogic.com>


Looks correct to me and catches our i965 issue. Maybe you could add the 
link to the bug in the comment of the new function.


Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>


> ---
>   tests/general/primitive-restart.c | 72 +++++++++++++++++++++++++++++++
>   1 file changed, 72 insertions(+)
>
> diff --git a/tests/general/primitive-restart.c b/tests/general/primitive-restart.c
> index 4219a6c5b..aef9bed41 100644
> --- a/tests/general/primitive-restart.c
> +++ b/tests/general/primitive-restart.c
> @@ -75,6 +75,11 @@ static bool Have_31;
>   static bool TestGL31;
>   
>   
> +static void
> +enable_restart(GLuint restart_index);
> +static void
> +disable_restart(void);
> +
>   static bool
>   check_rendering(void)
>   {
> @@ -184,6 +189,71 @@ test_begin_end(GLenum primMode)
>      return pass;
>   }
>   
> +static void
> +write_vec2_value(GLfloat * verts, GLuint vidx, GLfloat x, GLfloat y)
> +{
> +   verts[(vidx * 2) + 0] = x;
> +   verts[(vidx * 2) + 1] = y;
> +}
> +/* This test should draw green rectangle if everything is ok.
> + * But in case of bug which makes us unable to disable
> + * the primitive restart option it should draw just one triangle.
> + */
> +static bool
> +test_shared_ib_restart()
> +{
> +   bool pass = true;
> +   GLfloat verts[256 * 2];
> +   memset(&verts, 0, sizeof(verts));
> +   //left-bottom
> +   write_vec2_value(verts, 0, 0.0f, 0.0f);
> +   //right-bottom
> +   write_vec2_value(verts, 1, piglit_width, 0.0f);
> +   //left-top
> +   write_vec2_value(verts, 2, 0.0f, piglit_height);
> +   //right-top
> +   write_vec2_value(verts, 255, piglit_width, piglit_height);
> +
> +   const GLubyte indices[] = { 0, 1, 2, 255, 0 };
> +   const GLfloat expected[3] = { 0.0f, 1.0f, 0.0f };
> +   GLuint vbo1, vbo2;
> +
> +   piglit_ortho_projection(piglit_width, piglit_height, GL_FALSE);
> +
> +   glClear(GL_COLOR_BUFFER_BIT);
> +
> +   glColor4fv(green);
> +
> +   glGenBuffers(1, &vbo1);
> +   glBindBuffer(GL_ARRAY_BUFFER, vbo1);
> +   glBufferData(GL_ARRAY_BUFFER, sizeof(verts), verts, GL_DYNAMIC_DRAW);
> +   glVertexPointer(2, GL_FLOAT, 0, (void *)0);
> +
> +   glGenBuffers(1, &vbo2);
> +   glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, vbo2);
> +   glBufferData(GL_ELEMENT_ARRAY_BUFFER, sizeof(indices), indices, GL_STATIC_DRAW);
> +   glEnableClientState(GL_VERTEX_ARRAY);
> +
> +   //We should draw something with an enabled restart option to check that
> +   //it could be correctly disabled
> +   enable_restart(0xff);
> +   glDrawElements(GL_LINE_STRIP, ARRAY_SIZE(indices), GL_UNSIGNED_BYTE, (void*)0);
> +   disable_restart();
> +
> +   //Draw full screen rectangle
> +   glDrawElements(GL_TRIANGLE_STRIP, ARRAY_SIZE(indices) - 1, GL_UNSIGNED_BYTE, (void*)0);
> +
> +   glDisableClientState(GL_VERTEX_ARRAY);
> +   glBindBuffer(GL_ARRAY_BUFFER, 0);
> +   glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0);
> +   glDeleteBuffers(1, &vbo1);
> +   glDeleteBuffers(1, &vbo2);
> +   glFinish();
> +   pass = (piglit_probe_rect_rgb(0, 0, piglit_width, piglit_height, expected) != 0) && pass;
> +   piglit_present_results();
> +   return pass;
> +}
> +
>   
>   static void
>   enable_restart(GLuint restart_index)
> @@ -503,6 +573,7 @@ primitive_restart_test(VBO_CFG vbo_cfg)
>   
>      if (Have_NV) {
>         TestGL31 = false;
> +      pass = test_shared_ib_restart() && pass;
>         pass = test_begin_end(GL_TRIANGLE_STRIP) && pass;
>         pass = test_begin_end(GL_LINE_STRIP) && pass;
>         pass = test_draw_elements(vbo_cfg, GL_TRIANGLE_STRIP, GL_UNSIGNED_BYTE) && pass;
> @@ -521,6 +592,7 @@ primitive_restart_test(VBO_CFG vbo_cfg)
>   
>      if (Have_31) {
>         TestGL31 = true;
> +      pass = test_shared_ib_restart() && pass;
>         pass = test_draw_elements(vbo_cfg, GL_TRIANGLE_STRIP, GL_UNSIGNED_BYTE) && pass;
>         pass = test_draw_elements(vbo_cfg, GL_TRIANGLE_STRIP, GL_UNSIGNED_SHORT) && pass;
>         pass = test_draw_elements(vbo_cfg, GL_TRIANGLE_STRIP, GL_UNSIGNED_INT) && pass;