Without commenting on this change in particular, instancing is
significantly undertested in piglit. Right now a3xx with freedreno
passes all instancing tests but it (a) doesn't support more than 256
instances, and even if it did, then (b) it'd have to have a very
annoying implementation if the lcm of all the divisors were > 256. You
don't have to write such tests, but if you were feeling inspired... :)

On Tue, Jul 14, 2015 at 9:35 AM, Neil Roberts <n...@linux.intel.com> wrote:
> Adds an option to use an instanced attribute so that it can be tested
> in combination with glPolygonMode. This demonstrates a bug on the Mesa
> i965 driver on BDW which was reported as bug #91292. The ‘divisor’
> argument can be added in addition to any of the other arguments so
> that all of the combinations can be test.
> ---
>  tests/all.py                    |  4 +++
>  tests/general/point-vertex-id.c | 75 
> ++++++++++++++++++++++++++++++++++-------
>  2 files changed, 67 insertions(+), 12 deletions(-)
>
> diff --git a/tests/all.py b/tests/all.py
> index 3bd7baa..114959d 100644
> --- a/tests/all.py
> +++ b/tests/all.py
> @@ -580,6 +580,10 @@ with profile.group_manager(PiglitGLTest, 'shaders') as g:
>      g(['point-vertex-id', 'gl_VertexID'])
>      g(['point-vertex-id', 'gl_InstanceID'])
>      g(['point-vertex-id', 'gl_VertexID', 'gl_InstanceID'])
> +    g(['point-vertex-id', 'divisor'])
> +    g(['point-vertex-id', 'gl_VertexID', 'divisor'])
> +    g(['point-vertex-id', 'gl_InstanceID', 'divisor'])
> +    g(['point-vertex-id', 'gl_VertexID', 'gl_InstanceID', 'divisor'])
>      g(['glsl-vs-int-attrib'])
>      g(['glsl-link-test',
>         os.path.join('shaders', 'glsl-link-initializer-01a.vert'),
> diff --git a/tests/general/point-vertex-id.c b/tests/general/point-vertex-id.c
> index 7a400fb..5823ff1 100644
> --- a/tests/general/point-vertex-id.c
> +++ b/tests/general/point-vertex-id.c
> @@ -31,9 +31,10 @@
>   *
>   * Specify gl_VertexID or gl_InstanceID as an argument to specify
>   * which to test. Alternatively you can specify both in order to test
> - * a combination of both.
> + * a combination of both. Additionally ‘divisor’ can be specified to
> + * make it use an instanced array.
>   *
> - * See bug #84677
> + * See bug #84677 and #91292
>   */
>
>  PIGLIT_GL_TEST_CONFIG_BEGIN
> @@ -54,6 +55,10 @@ vertex_shader[] =
>         "in vec2 pos;\n"
>         "#endif"
>         "\n"
> +       "#ifdef USE_DIVISOR\n"
> +       "in vec2 triangle_offset;\n"
> +       "#endif\n"
> +       "\n"
>         "void\n"
>         "main()\n"
>         "{\n"
> @@ -62,7 +67,14 @@ vertex_shader[] =
>         "#endif\n"
>         "        gl_Position = vec4(pos, 0.0, 1.0);\n"
>         "#ifdef USE_INSTANCE_ID\n"
> -       "        gl_Position.t += float(gl_InstanceID) * 20.0;\n"
> +       "        float instance_offset = 20.0;\n"
> +       "#ifdef USE_DIVISOR\n"
> +       "        instance_offset = 10.0;\n"
> +       "#endif\n"
> +       "        gl_Position.t += float(gl_InstanceID) * instance_offset;\n"
> +       "#endif\n"
> +       "#ifdef USE_DIVISOR\n"
> +       "        gl_Position.st += triangle_offset;\n"
>         "#endif\n"
>         "        gl_Position.st = ((gl_Position.st + 0.5) * 2.0 /\n"
>         "                          viewport_size - 1.0);\n"
> @@ -77,6 +89,7 @@ struct vertex {
>  enum test_mode_flags {
>         TEST_MODE_VERTEX_ID = (1 << 0),
>         TEST_MODE_INSTANCE_ID = (1 << 1),
> +       TEST_MODE_DIVISOR = (1 << 2),
>  };
>
>  static enum test_mode_flags test_modes;
> @@ -93,7 +106,7 @@ vertices[] = {
>         { 30, 20, GL_FALSE },
>         /* Copy of the above two triangles but shifted up by 20. If
>          * instanced rendering is used these will be generated based
> -        * on the gl_InstanceID instead.
> +        * on the gl_InstanceID or an instanced array instead.
>          */
>         { 10, 30, GL_TRUE },
>         { 20, 30, GL_TRUE },
> @@ -103,6 +116,25 @@ vertices[] = {
>         { 30, 40, GL_FALSE },
>  };
>
> +/* If a divisor is set then each pair of triangles will be drawn as an
> + * instance and these offsets will be added to each instance.
> + */
> +static const struct vertex
> +triangle_offsets[] = {
> +       { 0, 0 },
> +       { 0, 20 },
> +};
> +
> +/* If both the divisor and the instance id is used then these offsets
> + * will be used instead. The remainder of the offset will be added by
> + * the instance ID.
> + */
> +static const struct vertex
> +triangle_offsets_with_instance_id[] = {
> +       { 0, 0 },
> +       { 0, 10 },
> +};
> +
>  enum piglit_result
>  piglit_display(void)
>  {
> @@ -119,6 +151,8 @@ piglit_display(void)
>                        "#extension GL_ARB_draw_instanced : require\n"
>                        "#define USE_INSTANCE_ID\n");
>         }
> +       if (test_modes & TEST_MODE_DIVISOR)
> +               strcat(shader_buf, "#define USE_DIVISOR\n");
>         if (test_modes & TEST_MODE_VERTEX_ID)
>                 strcat(shader_buf, "#define USE_VERTEX_ID\n");
>         strcat(shader_buf, vertex_shader);
> @@ -161,9 +195,25 @@ piglit_display(void)
>                                       &vertices[0].x);
>         }
>
> +       if (test_modes & TEST_MODE_DIVISOR) {
> +               pos_location = glGetAttribLocation(program, 
> "triangle_offset");
> +               if (pos_location == -1)
> +                       piglit_report_result(PIGLIT_FAIL);
> +               glEnableVertexAttribArray(pos_location);
> +               glVertexAttribDivisor(pos_location, 1);
> +               glVertexAttribPointer(pos_location,
> +                                     2, /* size */
> +                                     GL_INT,
> +                                     GL_FALSE, /* normalized */
> +                                     sizeof (struct vertex),
> +                                     (test_modes & TEST_MODE_INSTANCE_ID) ?
> +                                     &triangle_offsets_with_instance_id[0].x 
> :
> +                                     &triangle_offsets[0].x);
> +       }
> +
>         glPolygonMode(GL_FRONT_AND_BACK, GL_POINT);
>
> -       if ((test_modes & TEST_MODE_INSTANCE_ID)) {
> +       if (test_modes & (TEST_MODE_INSTANCE_ID | TEST_MODE_DIVISOR)) {
>                 glDrawArraysInstanced(GL_TRIANGLES,
>                                       0, /* first */
>                                       ARRAY_SIZE(vertices) / 2,
> @@ -174,9 +224,6 @@ piglit_display(void)
>                              ARRAY_SIZE(vertices));
>         }
>
> -       if (!(test_modes & TEST_MODE_VERTEX_ID))
> -               glDisableVertexAttribArray(pos_location);
> -
>         ref_image = malloc(piglit_width * piglit_height * 3 *
>                            sizeof (float));
>         memset(ref_image, 0, piglit_width * piglit_height * 3 * sizeof 
> (float));
> @@ -211,6 +258,8 @@ piglit_init(int argc, char **argv)
>                         test_modes |= TEST_MODE_VERTEX_ID;
>                 } else if (!strcmp(argv[i], "gl_InstanceID")) {
>                         test_modes |= TEST_MODE_INSTANCE_ID;
> +               } else if (!strcmp(argv[i], "divisor")) {
> +                       test_modes |= TEST_MODE_DIVISOR;
>                 } else {
>                         fprintf(stderr, "Unknown argument: %s\n", argv[i]);
>                         piglit_report_result(PIGLIT_FAIL);
> @@ -219,14 +268,16 @@ piglit_init(int argc, char **argv)
>
>         if (test_modes == 0) {
>                 fprintf(stderr,
> -                       "usage: point-vertex-id [gl_VertexID] 
> [gl_InstanceID]\n"
> -                       "Either one or both of the arguments must be "
> -                       "specified\n");
> +                       "usage: point-vertex-id [gl_VertexID] [gl_InstanceID] 
> "
> +                       "[divisor]\n"
> +                       "At least one of the arguments must be specified\n");
>                 piglit_report_result(PIGLIT_FAIL);
>         }
>
> -       if ((test_modes & TEST_MODE_INSTANCE_ID))
> +       if (test_modes & (TEST_MODE_INSTANCE_ID | TEST_MODE_DIVISOR))
>                 piglit_require_extension("GL_ARB_draw_instanced");
> +       if (test_modes & TEST_MODE_DIVISOR)
> +               piglit_require_extension("GL_ARB_instanced_arrays");
>
>         piglit_require_GLSL_version(130);
>  }
> --
> 1.9.3
>
> _______________________________________________
> Piglit mailing list
> Piglit@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
_______________________________________________
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit

Reply via email to