primitive-restart: change test for new OpenGL 4.5 behavior

Submitted by Nicolai Hähnle on Aug. 11, 2016, 11:11 a.m.

Details

Message ID 1470913898-15542-1-git-send-email-nhaehnle@gmail.com
State New
Headers show
Series "primitive-restart: change test for new OpenGL 4.5 behavior" ( rev: 1 ) in Piglit

Not browsing as part of any series.

Commit Message

Nicolai Hähnle Aug. 11, 2016, 11:11 a.m.
From: "Yang, Kefeng" <Kefeng.Yang@amd.com>

With OpenGL 4.5, the spec was changed to remove primitive restart from
glDrawArrays and friends.
---
 tests/general/primitive-restart.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

Patch hide | download patch | download mbox

diff --git a/tests/general/primitive-restart.c b/tests/general/primitive-restart.c
index 5cd8be3..48c2a11 100644
--- a/tests/general/primitive-restart.c
+++ b/tests/general/primitive-restart.c
@@ -63,21 +63,21 @@  static char* vbo_cfg_names[] = {
    "all",
 };
 
 static VBO_CFG vbo_init_cfg = DISABLE_VBO;
 
 static const GLfloat red[4] = {1.0, 0.0, 0.0, 1.0};
 static const GLfloat green[4] = {0.0, 1.0, 0.0, 0.0};
 static const GLfloat black[4] = {0.0, 0.0, 0.0, 0.0};
 
 static bool Have_NV;
-static bool Have_31;
+static bool Have_31_to_44;
 static bool TestGL31;
 
 
 static bool
 check_rendering(void)
 {
    const GLfloat x0 = 0.0, x1 = piglit_width - 10.0, dx = 20.0;
    const GLint iy = piglit_height / 2;
    bool draw = true;
    GLfloat x;
@@ -638,21 +638,21 @@  primitive_restart_test(VBO_CFG vbo_cfg)
       pass = pass && test_draw_elements(vbo_cfg, GL_LINE_STRIP, GL_UNSIGNED_INT);
       pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP, GL_UNSIGNED_BYTE);
       pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP, GL_UNSIGNED_SHORT);
       pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP, GL_UNSIGNED_INT);
       pass = pass && test_array_element(vbo_cfg, GL_LINE_STRIP, GL_UNSIGNED_BYTE);
       pass = pass && test_array_element(vbo_cfg, GL_LINE_STRIP, GL_UNSIGNED_SHORT);
       pass = pass && test_array_element(vbo_cfg, GL_LINE_STRIP, GL_UNSIGNED_INT);
       pass = pass && test_draw_arrays(vbo_cfg);
    }
 
-   if (Have_31) {
+   if (Have_31_to_44) {
       TestGL31 = true;
       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 = pass && test_draw_elements(vbo_cfg, GL_LINE_STRIP, GL_UNSIGNED_BYTE);
       pass = pass && test_draw_elements(vbo_cfg, GL_LINE_STRIP, GL_UNSIGNED_SHORT);
       pass = pass && test_draw_elements(vbo_cfg, GL_LINE_STRIP, GL_UNSIGNED_INT);
       pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP, GL_UNSIGNED_BYTE);
       pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP, GL_UNSIGNED_SHORT);
       pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP, GL_UNSIGNED_INT);
@@ -680,35 +680,37 @@  piglit_display(void)
    } else {
       return primitive_restart_test(vbo_init_cfg) ? PIGLIT_PASS : PIGLIT_FAIL;
    }
 }
 
 
 void
 piglit_init(int argc, char **argv)
 {
    Have_NV = piglit_is_extension_supported("GL_NV_primitive_restart");
-   Have_31 = piglit_get_gl_version() >= 31;
+
+   //In OGL 4.5, the PRIMITIVE_RESTART feature for DA-style draws is deprecated
+   Have_31_to_44 = piglit_get_gl_version() >= 31 && piglit_get_gl_version() <= 44;
 
    if (argc >= 2) {
       VBO_CFG vbo_cfg;
       for (vbo_cfg = 0; vbo_cfg < ARRAY_SIZE(vbo_cfg_names); vbo_cfg++) {
          if (strcmp(argv[1], vbo_cfg_names[vbo_cfg]) == 0) {
             vbo_init_cfg = vbo_cfg;
             break;
          }
       }
    }
 
    /* Debug */
    if (0) {
       printf("Have NV: %d\n", Have_NV);
-      printf("Have 31: %d\n", Have_31);
+      printf("Have 31 to 44: %d\n", Have_31_to_44);
    }
 
-   if (!Have_NV && !Have_31) {
+   if (!Have_NV && !Have_31_to_44) {
       piglit_report_result(PIGLIT_SKIP);
       exit(1);
    }
 
    glClearColor(0, 0, 0, 0);
 }

Comments

On 08/11/2016 05:11 AM, Nicolai Hähnle wrote:
> From: "Yang, Kefeng" <Kefeng.Yang@amd.com>
>
> With OpenGL 4.5, the spec was changed to remove primitive restart from
> glDrawArrays and friends.
> ---
>   tests/general/primitive-restart.c | 12 +++++++-----
>   1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/tests/general/primitive-restart.c b/tests/general/primitive-restart.c
> index 5cd8be3..48c2a11 100644
> --- a/tests/general/primitive-restart.c
> +++ b/tests/general/primitive-restart.c
> @@ -63,21 +63,21 @@ static char* vbo_cfg_names[] = {
>      "all",
>   };
>
>   static VBO_CFG vbo_init_cfg = DISABLE_VBO;
>
>   static const GLfloat red[4] = {1.0, 0.0, 0.0, 1.0};
>   static const GLfloat green[4] = {0.0, 1.0, 0.0, 0.0};
>   static const GLfloat black[4] = {0.0, 0.0, 0.0, 0.0};
>
>   static bool Have_NV;
> -static bool Have_31;
> +static bool Have_31_to_44;
>   static bool TestGL31;
>
>
>   static bool
>   check_rendering(void)
>   {
>      const GLfloat x0 = 0.0, x1 = piglit_width - 10.0, dx = 20.0;
>      const GLint iy = piglit_height / 2;
>      bool draw = true;
>      GLfloat x;
> @@ -638,21 +638,21 @@ primitive_restart_test(VBO_CFG vbo_cfg)
>         pass = pass && test_draw_elements(vbo_cfg, GL_LINE_STRIP, GL_UNSIGNED_INT);
>         pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP, GL_UNSIGNED_BYTE);
>         pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP, GL_UNSIGNED_SHORT);
>         pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP, GL_UNSIGNED_INT);
>         pass = pass && test_array_element(vbo_cfg, GL_LINE_STRIP, GL_UNSIGNED_BYTE);
>         pass = pass && test_array_element(vbo_cfg, GL_LINE_STRIP, GL_UNSIGNED_SHORT);
>         pass = pass && test_array_element(vbo_cfg, GL_LINE_STRIP, GL_UNSIGNED_INT);
>         pass = pass && test_draw_arrays(vbo_cfg);
>      }
>
> -   if (Have_31) {
> +   if (Have_31_to_44) {
>         TestGL31 = true;
>         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 = pass && test_draw_elements(vbo_cfg, GL_LINE_STRIP, GL_UNSIGNED_BYTE);
>         pass = pass && test_draw_elements(vbo_cfg, GL_LINE_STRIP, GL_UNSIGNED_SHORT);
>         pass = pass && test_draw_elements(vbo_cfg, GL_LINE_STRIP, GL_UNSIGNED_INT);
>         pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP, GL_UNSIGNED_BYTE);
>         pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP, GL_UNSIGNED_SHORT);
>         pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP, GL_UNSIGNED_INT);
> @@ -680,35 +680,37 @@ piglit_display(void)
>      } else {
>         return primitive_restart_test(vbo_init_cfg) ? PIGLIT_PASS : PIGLIT_FAIL;
>      }
>   }
>
>
>   void
>   piglit_init(int argc, char **argv)
>   {
>      Have_NV = piglit_is_extension_supported("GL_NV_primitive_restart");
> -   Have_31 = piglit_get_gl_version() >= 31;
> +
> +   //In OGL 4.5, the PRIMITIVE_RESTART feature for DA-style draws is deprecated
> +   Have_31_to_44 = piglit_get_gl_version() >= 31 && piglit_get_gl_version() <= 44;
>
>      if (argc >= 2) {
>         VBO_CFG vbo_cfg;
>         for (vbo_cfg = 0; vbo_cfg < ARRAY_SIZE(vbo_cfg_names); vbo_cfg++) {
>            if (strcmp(argv[1], vbo_cfg_names[vbo_cfg]) == 0) {
>               vbo_init_cfg = vbo_cfg;
>               break;
>            }
>         }
>      }
>
>      /* Debug */
>      if (0) {
>         printf("Have NV: %d\n", Have_NV);
> -      printf("Have 31: %d\n", Have_31);
> +      printf("Have 31 to 44: %d\n", Have_31_to_44);
>      }
>
> -   if (!Have_NV && !Have_31) {
> +   if (!Have_NV && !Have_31_to_44) {
>         piglit_report_result(PIGLIT_SKIP);
>         exit(1);
>      }
>
>      glClearColor(0, 0, 0, 0);
>   }
>

Reviewed-by: Brian Paul <brianp@vmware.com>
Hi,

On Thursday, 11 August 2016 13:11:38 CEST Nicolai Hähnle wrote:
> From: "Yang, Kefeng" <Kefeng.Yang@amd.com>
> 
> With OpenGL 4.5, the spec was changed to remove primitive restart from
> glDrawArrays and friends.

May be I am missing someting, but at least I do still read 10.3.6 from the 
core opengl 4.5 (glspec45.core.pdf) as well as the compat 
(glspec45.compatibility.pdf) specification that primitive restart should be 
there. There are some restrictions to which api calls the implicitly defined 
PRIMITIVE_RESTART_FIXED_INDEX is available, but this reads to me like the 
barely needed not to be ambigous with the implicitly defined value.
So, where can I find somthing about primitive index going away?

Thanks!

Mathias


> ---
>  tests/general/primitive-restart.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/general/primitive-restart.c b/tests/general/primitive-
restart.c
> index 5cd8be3..48c2a11 100644
> --- a/tests/general/primitive-restart.c
> +++ b/tests/general/primitive-restart.c
> @@ -63,21 +63,21 @@ static char* vbo_cfg_names[] = {
>     "all",
>  };
>  
>  static VBO_CFG vbo_init_cfg = DISABLE_VBO;
>  
>  static const GLfloat red[4] = {1.0, 0.0, 0.0, 1.0};
>  static const GLfloat green[4] = {0.0, 1.0, 0.0, 0.0};
>  static const GLfloat black[4] = {0.0, 0.0, 0.0, 0.0};
>  
>  static bool Have_NV;
> -static bool Have_31;
> +static bool Have_31_to_44;
>  static bool TestGL31;
>  
>  
>  static bool
>  check_rendering(void)
>  {
>     const GLfloat x0 = 0.0, x1 = piglit_width - 10.0, dx = 20.0;
>     const GLint iy = piglit_height / 2;
>     bool draw = true;
>     GLfloat x;
> @@ -638,21 +638,21 @@ primitive_restart_test(VBO_CFG vbo_cfg)
>        pass = pass && test_draw_elements(vbo_cfg, GL_LINE_STRIP, 
GL_UNSIGNED_INT);
>        pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP, 
GL_UNSIGNED_BYTE);
>        pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP, 
GL_UNSIGNED_SHORT);
>        pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP, 
GL_UNSIGNED_INT);
>        pass = pass && test_array_element(vbo_cfg, GL_LINE_STRIP, 
GL_UNSIGNED_BYTE);
>        pass = pass && test_array_element(vbo_cfg, GL_LINE_STRIP, 
GL_UNSIGNED_SHORT);
>        pass = pass && test_array_element(vbo_cfg, GL_LINE_STRIP, 
GL_UNSIGNED_INT);
>        pass = pass && test_draw_arrays(vbo_cfg);
>     }
>  
> -   if (Have_31) {
> +   if (Have_31_to_44) {
>        TestGL31 = true;
>        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 = pass && test_draw_elements(vbo_cfg, GL_LINE_STRIP, 
GL_UNSIGNED_BYTE);
>        pass = pass && test_draw_elements(vbo_cfg, GL_LINE_STRIP, 
GL_UNSIGNED_SHORT);
>        pass = pass && test_draw_elements(vbo_cfg, GL_LINE_STRIP, 
GL_UNSIGNED_INT);
>        pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP, 
GL_UNSIGNED_BYTE);
>        pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP, 
GL_UNSIGNED_SHORT);
>        pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP, 
GL_UNSIGNED_INT);
> @@ -680,35 +680,37 @@ piglit_display(void)
>     } else {
>        return primitive_restart_test(vbo_init_cfg) ? PIGLIT_PASS : 
PIGLIT_FAIL;
>     }
>  }
>  
>  
>  void
>  piglit_init(int argc, char **argv)
>  {
>     Have_NV = piglit_is_extension_supported("GL_NV_primitive_restart");
> -   Have_31 = piglit_get_gl_version() >= 31;
> +
> +   //In OGL 4.5, the PRIMITIVE_RESTART feature for DA-style draws is 
deprecated
> +   Have_31_to_44 = piglit_get_gl_version() >= 31 && piglit_get_gl_version() 
<= 44;
>  
>     if (argc >= 2) {
>        VBO_CFG vbo_cfg;
>        for (vbo_cfg = 0; vbo_cfg < ARRAY_SIZE(vbo_cfg_names); vbo_cfg++) {
>           if (strcmp(argv[1], vbo_cfg_names[vbo_cfg]) == 0) {
>              vbo_init_cfg = vbo_cfg;
>              break;
>           }
>        }
>     }
>  
>     /* Debug */
>     if (0) {
>        printf("Have NV: %d\n", Have_NV);
> -      printf("Have 31: %d\n", Have_31);
> +      printf("Have 31 to 44: %d\n", Have_31_to_44);
>     }
>  
> -   if (!Have_NV && !Have_31) {
> +   if (!Have_NV && !Have_31_to_44) {
>        piglit_report_result(PIGLIT_SKIP);
>        exit(1);
>     }
>  
>     glClearColor(0, 0, 0, 0);
>  }
>
On Thursday, 11 August 2016 13:11:38 CEST Nicolai Hähnle wrote:
> From: "Yang, Kefeng" <Kefeng.Yang@amd.com>
> 
> With OpenGL 4.5, the spec was changed to remove primitive restart from
> glDrawArrays and friends.

Ok, I have now spotted a difference between the 4.4 and 4.5 specs.
But it appears to me that primitive restart is basically staying, but is 
reduced to these cases that actually make sense somehow.

That is, primitive restart *stays* for all DrawElements calls and vanishes for 
almost all other cases.

So, the test should still test the DrawElement calls as before - also for gl 
>= 4.5.

best
Mathias

> ---
>  tests/general/primitive-restart.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/general/primitive-restart.c b/tests/general/primitive-
restart.c
> index 5cd8be3..48c2a11 100644
> --- a/tests/general/primitive-restart.c
> +++ b/tests/general/primitive-restart.c
> @@ -63,21 +63,21 @@ static char* vbo_cfg_names[] = {
>     "all",
>  };
>  
>  static VBO_CFG vbo_init_cfg = DISABLE_VBO;
>  
>  static const GLfloat red[4] = {1.0, 0.0, 0.0, 1.0};
>  static const GLfloat green[4] = {0.0, 1.0, 0.0, 0.0};
>  static const GLfloat black[4] = {0.0, 0.0, 0.0, 0.0};
>  
>  static bool Have_NV;
> -static bool Have_31;
> +static bool Have_31_to_44;
>  static bool TestGL31;
>  
>  
>  static bool
>  check_rendering(void)
>  {
>     const GLfloat x0 = 0.0, x1 = piglit_width - 10.0, dx = 20.0;
>     const GLint iy = piglit_height / 2;
>     bool draw = true;
>     GLfloat x;
> @@ -638,21 +638,21 @@ primitive_restart_test(VBO_CFG vbo_cfg)
>        pass = pass && test_draw_elements(vbo_cfg, GL_LINE_STRIP, 
GL_UNSIGNED_INT);
>        pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP, 
GL_UNSIGNED_BYTE);
>        pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP, 
GL_UNSIGNED_SHORT);
>        pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP, 
GL_UNSIGNED_INT);
>        pass = pass && test_array_element(vbo_cfg, GL_LINE_STRIP, 
GL_UNSIGNED_BYTE);
>        pass = pass && test_array_element(vbo_cfg, GL_LINE_STRIP, 
GL_UNSIGNED_SHORT);
>        pass = pass && test_array_element(vbo_cfg, GL_LINE_STRIP, 
GL_UNSIGNED_INT);
>        pass = pass && test_draw_arrays(vbo_cfg);
>     }
>  
> -   if (Have_31) {
> +   if (Have_31_to_44) {
>        TestGL31 = true;
>        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 = pass && test_draw_elements(vbo_cfg, GL_LINE_STRIP, 
GL_UNSIGNED_BYTE);
>        pass = pass && test_draw_elements(vbo_cfg, GL_LINE_STRIP, 
GL_UNSIGNED_SHORT);
>        pass = pass && test_draw_elements(vbo_cfg, GL_LINE_STRIP, 
GL_UNSIGNED_INT);
>        pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP, 
GL_UNSIGNED_BYTE);
>        pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP, 
GL_UNSIGNED_SHORT);
>        pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP, 
GL_UNSIGNED_INT);
> @@ -680,35 +680,37 @@ piglit_display(void)
>     } else {
>        return primitive_restart_test(vbo_init_cfg) ? PIGLIT_PASS : 
PIGLIT_FAIL;
>     }
>  }
>  
>  
>  void
>  piglit_init(int argc, char **argv)
>  {
>     Have_NV = piglit_is_extension_supported("GL_NV_primitive_restart");
> -   Have_31 = piglit_get_gl_version() >= 31;
> +
> +   //In OGL 4.5, the PRIMITIVE_RESTART feature for DA-style draws is 
deprecated
> +   Have_31_to_44 = piglit_get_gl_version() >= 31 && piglit_get_gl_version() 
<= 44;
>  
>     if (argc >= 2) {
>        VBO_CFG vbo_cfg;
>        for (vbo_cfg = 0; vbo_cfg < ARRAY_SIZE(vbo_cfg_names); vbo_cfg++) {
>           if (strcmp(argv[1], vbo_cfg_names[vbo_cfg]) == 0) {
>              vbo_init_cfg = vbo_cfg;
>              break;
>           }
>        }
>     }
>  
>     /* Debug */
>     if (0) {
>        printf("Have NV: %d\n", Have_NV);
> -      printf("Have 31: %d\n", Have_31);
> +      printf("Have 31 to 44: %d\n", Have_31_to_44);
>     }
>  
> -   if (!Have_NV && !Have_31) {
> +   if (!Have_NV && !Have_31_to_44) {
>        piglit_report_result(PIGLIT_SKIP);
>        exit(1);
>     }
>  
>     glClearColor(0, 0, 0, 0);
>  }
>
Hi,

On Monday, 15 August 2016 09:47:55 CEST Yang, Kefeng wrote:
> Patch is updated to remove the DrawArray test for GL_PRIMITIVE_RESTART, 
retroactively for all GL versions.
> DrawElement test still need to be kept.

I am fine with just removing these tests also for older than 4.5 GL versions.

But for the type of tests I really meant DrawElement* calls and nothing else.
Probably the best description in the 4.5 spec is:


10.3.6 Primitive Restart
[...]
Note that primitive restart is not performed for array elements transferred by
any drawing command not taking a type parameter, including ArrayElement and
all of the *Draw* commands other than *DrawElements*.


For this test this means that only the lines calling test_draw_elements stay.
The lines from the Have_31 scope calling test_draw_arrays, as you already did, 
as well as the lines calling test_array_element should go away to be 
compatible with 4.5.

best
Mathias

> 
> From 02c464d0d1b2f6f600e4e080835087ebe94d2f84 Mon Sep 17 00:00:00 2001
> From: Kefeng Yang <kefeng.yang@amd.com>
> Date: Mon, 15 Aug 2016 17:24:04 +0800
> Subject: [PATCH] Remove glDrawArray test for GL_PRIMITIVE_RESTART
>  retroactively for all GL versions
> 
> ---
>  tests/general/primitive-restart.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/tests/general/primitive-restart.c b/tests/general/primitive-
restart.c
> index 5cd8be3..f946316 100644
> --- a/tests/general/primitive-restart.c
> +++ b/tests/general/primitive-restart.c
> @@ -659,7 +659,6 @@ primitive_restart_test(VBO_CFG vbo_cfg)
>        pass = pass && test_array_element(vbo_cfg, GL_LINE_STRIP, 
GL_UNSIGNED_BYTE);
>        pass = pass && test_array_element(vbo_cfg, GL_LINE_STRIP, 
GL_UNSIGNED_SHORT);
>        pass = pass && test_array_element(vbo_cfg, GL_LINE_STRIP, 
GL_UNSIGNED_INT);
> -      pass = pass && test_draw_arrays(vbo_cfg);
>     }
> 
>     return pass;
> --
> 1.9.1
> 
> 
> Cheers,
> Kefeng
> 
> -----Original Message-----
> From: Mathias Fröhlich [mailto:Mathias.Froehlich@gmx.net] 
> Sent: Saturday, August 13, 2016 4:36 PM
> To: piglit@lists.freedesktop.org
> Cc: Nicolai Hähnle; Yang, Kefeng
> Subject: Re: [Piglit] [PATCH] primitive-restart: change test for new OpenGL 
4.5 behavior
> 
> On Thursday, 11 August 2016 13:11:38 CEST Nicolai Hähnle wrote:
> > From: "Yang, Kefeng" <Kefeng.Yang@amd.com>
> > 
> > With OpenGL 4.5, the spec was changed to remove primitive restart from 
> > glDrawArrays and friends.
> 
> Ok, I have now spotted a difference between the 4.4 and 4.5 specs.
> But it appears to me that primitive restart is basically staying, but is 
reduced to these cases that actually make sense somehow.
> 
> That is, primitive restart *stays* for all DrawElements calls and vanishes 
for almost all other cases.
> 
> So, the test should still test the DrawElement calls as before - also for gl 
> >= 4.5.
> 
> best
> Mathias
> 
> > ---
> >  tests/general/primitive-restart.c | 12 +++++++-----
> >  1 file changed, 7 insertions(+), 5 deletions(-)
> > 
> > diff --git a/tests/general/primitive-restart.c 
> > b/tests/general/primitive-
> restart.c
> > index 5cd8be3..48c2a11 100644
> > --- a/tests/general/primitive-restart.c
> > +++ b/tests/general/primitive-restart.c
> > @@ -63,21 +63,21 @@ static char* vbo_cfg_names[] = {
> >     "all",
> >  };
> >  
> >  static VBO_CFG vbo_init_cfg = DISABLE_VBO;
> >  
> >  static const GLfloat red[4] = {1.0, 0.0, 0.0, 1.0};  static const 
> > GLfloat green[4] = {0.0, 1.0, 0.0, 0.0};  static const GLfloat 
> > black[4] = {0.0, 0.0, 0.0, 0.0};
> >  
> >  static bool Have_NV;
> > -static bool Have_31;
> > +static bool Have_31_to_44;
> >  static bool TestGL31;
> >  
> >  
> >  static bool
> >  check_rendering(void)
> >  {
> >     const GLfloat x0 = 0.0, x1 = piglit_width - 10.0, dx = 20.0;
> >     const GLint iy = piglit_height / 2;
> >     bool draw = true;
> >     GLfloat x;
> > @@ -638,21 +638,21 @@ primitive_restart_test(VBO_CFG vbo_cfg)
> >        pass = pass && test_draw_elements(vbo_cfg, GL_LINE_STRIP,
> GL_UNSIGNED_INT);
> >        pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP,
> GL_UNSIGNED_BYTE);
> >        pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP,
> GL_UNSIGNED_SHORT);
> >        pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP,
> GL_UNSIGNED_INT);
> >        pass = pass && test_array_element(vbo_cfg, GL_LINE_STRIP,
> GL_UNSIGNED_BYTE);
> >        pass = pass && test_array_element(vbo_cfg, GL_LINE_STRIP,
> GL_UNSIGNED_SHORT);
> >        pass = pass && test_array_element(vbo_cfg, GL_LINE_STRIP,
> GL_UNSIGNED_INT);
> >        pass = pass && test_draw_arrays(vbo_cfg);
> >     }
> >  
> > -   if (Have_31) {
> > +   if (Have_31_to_44) {
> >        TestGL31 = true;
> >        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 = pass && test_draw_elements(vbo_cfg, GL_LINE_STRIP,
> GL_UNSIGNED_BYTE);
> >        pass = pass && test_draw_elements(vbo_cfg, GL_LINE_STRIP,
> GL_UNSIGNED_SHORT);
> >        pass = pass && test_draw_elements(vbo_cfg, GL_LINE_STRIP,
> GL_UNSIGNED_INT);
> >        pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP,
> GL_UNSIGNED_BYTE);
> >        pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP,
> GL_UNSIGNED_SHORT);
> >        pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP,
> GL_UNSIGNED_INT);
> > @@ -680,35 +680,37 @@ piglit_display(void)
> >     } else {
> >        return primitive_restart_test(vbo_init_cfg) ? PIGLIT_PASS : 
> PIGLIT_FAIL;
> >     }
> >  }
> >  
> >  
> >  void
> >  piglit_init(int argc, char **argv)
> >  {
> >     Have_NV = piglit_is_extension_supported("GL_NV_primitive_restart");
> > -   Have_31 = piglit_get_gl_version() >= 31;
> > +
> > +   //In OGL 4.5, the PRIMITIVE_RESTART feature for DA-style draws is
> deprecated
> > +   Have_31_to_44 = piglit_get_gl_version() >= 31 && 
> > + piglit_get_gl_version()
> <= 44;
> >  
> >     if (argc >= 2) {
> >        VBO_CFG vbo_cfg;
> >        for (vbo_cfg = 0; vbo_cfg < ARRAY_SIZE(vbo_cfg_names); vbo_cfg++) {
> >           if (strcmp(argv[1], vbo_cfg_names[vbo_cfg]) == 0) {
> >              vbo_init_cfg = vbo_cfg;
> >              break;
> >           }
> >        }
> >     }
> >  
> >     /* Debug */
> >     if (0) {
> >        printf("Have NV: %d\n", Have_NV);
> > -      printf("Have 31: %d\n", Have_31);
> > +      printf("Have 31 to 44: %d\n", Have_31_to_44);
> >     }
> >  
> > -   if (!Have_NV && !Have_31) {
> > +   if (!Have_NV && !Have_31_to_44) {
> >        piglit_report_result(PIGLIT_SKIP);
> >        exit(1);
> >     }
> >  
> >     glClearColor(0, 0, 0, 0);
> >  }
> > 
> 
> 
>
On Wednesday, 17 August 2016 07:51:06 CEST Yang, Kefeng wrote:
> Agreed. The ArrayElement tests shall also be removed for 
GL_PRIMITIVE_RESTART.

Thanks!
Reviewed-by: Mathias Fröhlich <Mathias.Froehlich@web.de>

Mathias

> 
> Patch is updated:
> 
> From 447ec5a1833fded09d9500d8ee3dcaccbdcd82e2 Mon Sep 17 00:00:00 2001
> From: Kefeng Yang <kefeng.yang@amd.com>
> Date: Wed, 17 Aug 2016 15:32:19 +0800
> Subject: [PATCH] Remove glDrawArray and glArrayElement test for
>  GL_PRIMITIVE_RESTART retroactively for all GL versions
> 
> ---
>  tests/general/primitive-restart.c | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/tests/general/primitive-restart.c b/tests/general/primitive-
restart.c
> index 5cd8be3..83f494d 100644
> --- a/tests/general/primitive-restart.c
> +++ b/tests/general/primitive-restart.c
> @@ -653,13 +653,6 @@ primitive_restart_test(VBO_CFG vbo_cfg)
>        pass = pass && test_draw_elements(vbo_cfg, GL_LINE_STRIP, 
GL_UNSIGNED_BYTE);
>        pass = pass && test_draw_elements(vbo_cfg, GL_LINE_STRIP, 
GL_UNSIGNED_SHORT);
>        pass = pass && test_draw_elements(vbo_cfg, GL_LINE_STRIP, 
GL_UNSIGNED_INT);
> -      pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP, 
GL_UNSIGNED_BYTE);
> -      pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP, 
GL_UNSIGNED_SHORT);
> -      pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP, 
GL_UNSIGNED_INT);
> -      pass = pass && test_array_element(vbo_cfg, GL_LINE_STRIP, 
GL_UNSIGNED_BYTE);
> -      pass = pass && test_array_element(vbo_cfg, GL_LINE_STRIP, 
GL_UNSIGNED_SHORT);
> -      pass = pass && test_array_element(vbo_cfg, GL_LINE_STRIP, 
GL_UNSIGNED_INT);
> -      pass = pass && test_draw_arrays(vbo_cfg);
>     }
> 
>     return pass;
> --
> 1.9.1
> 
> 
> Thanks, 
> Kefeng
> 
> -----Original Message-----
> From: Mathias Fröhlich [mailto:Mathias.Froehlich@gmx.net] 
> Sent: Wednesday, August 17, 2016 1:53 PM
> To: Yang, Kefeng
> Cc: piglit@lists.freedesktop.org; Nicolai Hähnle; Krowicki, Eric
> Subject: Re: [Piglit] [PATCH] primitive-restart: change test for new OpenGL 
4.5 behavior
> 
> Hi,
> 
> On Monday, 15 August 2016 09:47:55 CEST Yang, Kefeng wrote:
> > Patch is updated to remove the DrawArray test for 
> > GL_PRIMITIVE_RESTART,
> retroactively for all GL versions.
> > DrawElement test still need to be kept.
> 
> I am fine with just removing these tests also for older than 4.5 GL 
versions.
> 
> But for the type of tests I really meant DrawElement* calls and nothing 
else.
> Probably the best description in the 4.5 spec is:
> 
> 
> 10.3.6 Primitive Restart
> [...]
> Note that primitive restart is not performed for array elements transferred 
by any drawing command not taking a type parameter, including ArrayElement and 
all of the *Draw* commands other than *DrawElements*.
> 
> 
> For this test this means that only the lines calling test_draw_elements 
stay.
> The lines from the Have_31 scope calling test_draw_arrays, as you already 
did, as well as the lines calling test_array_element should go away to be 
compatible with 4.5.
> 
> best
> Mathias
> 
> > 
> > From 02c464d0d1b2f6f600e4e080835087ebe94d2f84 Mon Sep 17 00:00:00 2001
> > From: Kefeng Yang <kefeng.yang@amd.com>
> > Date: Mon, 15 Aug 2016 17:24:04 +0800
> > Subject: [PATCH] Remove glDrawArray test for GL_PRIMITIVE_RESTART  
> > retroactively for all GL versions
> > 
> > ---
> >  tests/general/primitive-restart.c | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/tests/general/primitive-restart.c 
> > b/tests/general/primitive-
> restart.c
> > index 5cd8be3..f946316 100644
> > --- a/tests/general/primitive-restart.c
> > +++ b/tests/general/primitive-restart.c
> > @@ -659,7 +659,6 @@ primitive_restart_test(VBO_CFG vbo_cfg)
> >        pass = pass && test_array_element(vbo_cfg, GL_LINE_STRIP,
> GL_UNSIGNED_BYTE);
> >        pass = pass && test_array_element(vbo_cfg, GL_LINE_STRIP,
> GL_UNSIGNED_SHORT);
> >        pass = pass && test_array_element(vbo_cfg, GL_LINE_STRIP,
> GL_UNSIGNED_INT);
> > -      pass = pass && test_draw_arrays(vbo_cfg);
> >     }
> > 
> >     return pass;
> > --
> > 1.9.1
> > 
> > 
> > Cheers,
> > Kefeng
> > 
> > -----Original Message-----
> > From: Mathias Fröhlich [mailto:Mathias.Froehlich@gmx.net]
> > Sent: Saturday, August 13, 2016 4:36 PM
> > To: piglit@lists.freedesktop.org
> > Cc: Nicolai Hähnle; Yang, Kefeng
> > Subject: Re: [Piglit] [PATCH] primitive-restart: change test for new 
> > OpenGL
> 4.5 behavior
> > 
> > On Thursday, 11 August 2016 13:11:38 CEST Nicolai Hähnle wrote:
> > > From: "Yang, Kefeng" <Kefeng.Yang@amd.com>
> > > 
> > > With OpenGL 4.5, the spec was changed to remove primitive restart 
> > > from glDrawArrays and friends.
> > 
> > Ok, I have now spotted a difference between the 4.4 and 4.5 specs.
> > But it appears to me that primitive restart is basically staying, but 
> > is
> reduced to these cases that actually make sense somehow.
> > 
> > That is, primitive restart *stays* for all DrawElements calls and 
> > vanishes
> for almost all other cases.
> > 
> > So, the test should still test the DrawElement calls as before - also 
> > for gl
> > >= 4.5.
> > 
> > best
> > Mathias
> > 
> > > ---
> > >  tests/general/primitive-restart.c | 12 +++++++-----
> > >  1 file changed, 7 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/tests/general/primitive-restart.c
> > > b/tests/general/primitive-
> > restart.c
> > > index 5cd8be3..48c2a11 100644
> > > --- a/tests/general/primitive-restart.c
> > > +++ b/tests/general/primitive-restart.c
> > > @@ -63,21 +63,21 @@ static char* vbo_cfg_names[] = {
> > >     "all",
> > >  };
> > >  
> > >  static VBO_CFG vbo_init_cfg = DISABLE_VBO;
> > >  
> > >  static const GLfloat red[4] = {1.0, 0.0, 0.0, 1.0};  static const 
> > > GLfloat green[4] = {0.0, 1.0, 0.0, 0.0};  static const GLfloat 
> > > black[4] = {0.0, 0.0, 0.0, 0.0};
> > >  
> > >  static bool Have_NV;
> > > -static bool Have_31;
> > > +static bool Have_31_to_44;
> > >  static bool TestGL31;
> > >  
> > >  
> > >  static bool
> > >  check_rendering(void)
> > >  {
> > >     const GLfloat x0 = 0.0, x1 = piglit_width - 10.0, dx = 20.0;
> > >     const GLint iy = piglit_height / 2;
> > >     bool draw = true;
> > >     GLfloat x;
> > > @@ -638,21 +638,21 @@ primitive_restart_test(VBO_CFG vbo_cfg)
> > >        pass = pass && test_draw_elements(vbo_cfg, GL_LINE_STRIP,
> > GL_UNSIGNED_INT);
> > >        pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP,
> > GL_UNSIGNED_BYTE);
> > >        pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP,
> > GL_UNSIGNED_SHORT);
> > >        pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP,
> > GL_UNSIGNED_INT);
> > >        pass = pass && test_array_element(vbo_cfg, GL_LINE_STRIP,
> > GL_UNSIGNED_BYTE);
> > >        pass = pass && test_array_element(vbo_cfg, GL_LINE_STRIP,
> > GL_UNSIGNED_SHORT);
> > >        pass = pass && test_array_element(vbo_cfg, GL_LINE_STRIP,
> > GL_UNSIGNED_INT);
> > >        pass = pass && test_draw_arrays(vbo_cfg);
> > >     }
> > >  
> > > -   if (Have_31) {
> > > +   if (Have_31_to_44) {
> > >        TestGL31 = true;
> > >        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 = pass && test_draw_elements(vbo_cfg, GL_LINE_STRIP,
> > GL_UNSIGNED_BYTE);
> > >        pass = pass && test_draw_elements(vbo_cfg, GL_LINE_STRIP,
> > GL_UNSIGNED_SHORT);
> > >        pass = pass && test_draw_elements(vbo_cfg, GL_LINE_STRIP,
> > GL_UNSIGNED_INT);
> > >        pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP,
> > GL_UNSIGNED_BYTE);
> > >        pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP,
> > GL_UNSIGNED_SHORT);
> > >        pass = pass && test_array_element(vbo_cfg, GL_TRIANGLE_STRIP,
> > GL_UNSIGNED_INT);
> > > @@ -680,35 +680,37 @@ piglit_display(void)
> > >     } else {
> > >        return primitive_restart_test(vbo_init_cfg) ? PIGLIT_PASS : 
> > PIGLIT_FAIL;
> > >     }
> > >  }
> > >  
> > >  
> > >  void
> > >  piglit_init(int argc, char **argv)
> > >  {
> > >     Have_NV = piglit_is_extension_supported("GL_NV_primitive_restart");
> > > -   Have_31 = piglit_get_gl_version() >= 31;
> > > +
> > > +   //In OGL 4.5, the PRIMITIVE_RESTART feature for DA-style draws 
> > > + is
> > deprecated
> > > +   Have_31_to_44 = piglit_get_gl_version() >= 31 &&
> > > + piglit_get_gl_version()
> > <= 44;
> > >  
> > >     if (argc >= 2) {
> > >        VBO_CFG vbo_cfg;
> > >        for (vbo_cfg = 0; vbo_cfg < ARRAY_SIZE(vbo_cfg_names); vbo_cfg++) 
{
> > >           if (strcmp(argv[1], vbo_cfg_names[vbo_cfg]) == 0) {
> > >              vbo_init_cfg = vbo_cfg;
> > >              break;
> > >           }
> > >        }
> > >     }
> > >  
> > >     /* Debug */
> > >     if (0) {
> > >        printf("Have NV: %d\n", Have_NV);
> > > -      printf("Have 31: %d\n", Have_31);
> > > +      printf("Have 31 to 44: %d\n", Have_31_to_44);
> > >     }
> > >  
> > > -   if (!Have_NV && !Have_31) {
> > > +   if (!Have_NV && !Have_31_to_44) {
> > >        piglit_report_result(PIGLIT_SKIP);
> > >        exit(1);
> > >     }
> > >  
> > >     glClearColor(0, 0, 0, 0);
> > >  }
> > > 
> > 
> > 
> > 
> 
> 
>