[Piglit] [PATCH] V4. Add a case to valid gl_VertexID for bug 87611
Wang, Shuo
shuo.wang at intel.com
Tue Apr 7 18:27:44 PDT 2015
Thank you for the clearly information, I will work on it and send out a new version later.
Thanks,
Shuo
-----Original Message-----
From: Ian Romanick [mailto:idr at freedesktop.org]
Sent: Wednesday, April 8, 2015 9:22 AM
To: Wang, Shuo; piglit at lists.freedesktop.org
Cc: Jin, Gordon
Subject: Re: [Piglit] [PATCH] V4. Add a case to valid gl_VertexID for bug 87611
This test uses tabs for indentation. The coding style for piglit is to never used tabs. Instead, use 4-spaces for each indentation level.
Other comments are below.
On 04/01/2015 02:47 AM, Wang Shuo wrote:
> This case is passed by master since Ken's commit is already pushed
> (c633528cbac007a73a066f269b3c9a25daf1e21a)
> Below is the test point of this case:
> 1. Use user-allocated vertex arrays (NOT vertex buffer objects).
> 2. Use a glDrawArrays with a non-zero 'first'.
> 3. Use primitive restart to trigger multiple _mesa_prim without an
> intervening state change.
> For primitive restart test:
> In OpenGL, primitive restarting to work with all rendering
> commands, including the non-indexed commands. However,
> implementations did not implement this, and the rule itself
> makes no sense.(https://www.opengl.org/wiki/Vertex_Rendering)
> Also apparently GL 4.5 changed this de-facto behaviour into
> specified behaviour -- primitive restart is only supposed to
> work on Draw*Elements*.
> 4. Validate the value of gl_VertexID in the vertex shader.
> Both glDrawArrays testing and glDrawElements testing test the
> value of gl_VertexID
> 5. Using GL_TRIANGLE_FAN because GL_TRIANGLE_STRIP are supported
> by the HW path, and GL_TRIANGLE_FAN specifically to hit the
> SW primitive restart path which force the draw to be decomposed
> into multiple draws
>
> V2:
> 1.Fix some Piglit style issue
>
> V3:
> 1.Make sure GL_TRIANGLE_FAN should be used.
>
> V4:
> 1.Fix some GL version issue and coding style issue.
>
> Signed-off-by: Wang Shuo <shuo.wang at intel.com>
> ---
> tests/all.py | 1 +
> tests/spec/gl-3.1/CMakeLists.gl.txt | 1 +
> tests/spec/gl-3.1/primitive-restart-vertex-id.c | 137
> ++++++++++++++++++++++++
> 3 files changed, 139 insertions(+)
> create mode 100644 tests/spec/gl-3.1/primitive-restart-vertex-id.c
>
> diff --git a/tests/all.py b/tests/all.py index b2266db..465c2b5 100644
> --- a/tests/all.py
> +++ b/tests/all.py
> @@ -1152,6 +1152,7 @@ with profile.group_manager(
> g(['gl-3.1-genned-names'], 'genned-names')
> g(['gl-3.1-minmax'], 'minmax')
> g(['gl-3.1-vao-broken-attrib'], 'vao-broken-attrib')
> + g(['gl-3.1-primitive-restart-vertex-id'],
> + 'primitive-restart-vertex-id')
> g(['gl-3.0-required-renderbuffer-attachment-formats', '31'],
> 'required-renderbuffer-attachment-formats')
> g(['gl-3.0-required-sized-texture-formats', '31'], diff --git
> a/tests/spec/gl-3.1/CMakeLists.gl.txt
> b/tests/spec/gl-3.1/CMakeLists.gl.txt
> index a5f28c1..4ecb0b2 100644
> --- a/tests/spec/gl-3.1/CMakeLists.gl.txt
> +++ b/tests/spec/gl-3.1/CMakeLists.gl.txt
> @@ -15,5 +15,6 @@ piglit_add_executable (gl-3.1-genned-names
> genned-names.c) piglit_add_executable (gl-3.1-minmax minmax.c)
> piglit_add_executable (gl-3.1-primitive-restart-xfb
> primitive-restart-xfb.c) piglit_add_executable
> (gl-3.1-vao-broken-attrib vao-broken-attrib.c)
> +piglit_add_executable (gl-3.1-primitive-restart-vertex-id
> +primitive-restart-vertex-id.c)
>
> # vim: ft=cmake:
> diff --git a/tests/spec/gl-3.1/primitive-restart-vertex-id.c
> b/tests/spec/gl-3.1/primitive-restart-vertex-id.c
> new file mode 100644
> index 0000000..0f92d1f
> --- /dev/null
> +++ b/tests/spec/gl-3.1/primitive-restart-vertex-id.c
> @@ -0,0 +1,137 @@
> +/*
> + * Copyright © 2015 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 primitive-restart-vertex-id.c
> + *
> + * Check gl_VertexID
> + * This case is added for *Bug 87611 - Exercise gl_VertexID corner
> + * cases in i965 driver*, and used to watch the status of *Bug 85529
> + * - Surfaces not drawn in Unvanquished* This case is
> + * passed by master now.
> + *
> + *
> + * \author Wang Shuo <shuo.wang at intel.com> */
> +
> +
> +#include "piglit-util-gl.h"
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +
> + config.supports_gl_compat_version = 10;
With only this in the configuration, the test will never get an OpenGL
3.1 context on Mesa. You should add:
config.supports_gl_core_version = 31;
This test also needs GLSL 1.30, and that is (currently) only available with OpenGL 3.0 or later. You should change the "supports_gl_compat_version" line to:
config.supports_gl_compat_version = 30;
Then you can delete the GLSL version check in piglit_init.
> +
> + config.window_visual = PIGLIT_GL_VISUAL_RGB |
> +PIGLIT_GL_VISUAL_DOUBLE;
> +
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +/* Validate the value of gl_VertexID in the vertex shader. */ static
> +const GLchar *vertShaderText =
> + "#version 130\n"
> + "in vec4 piglit_vertex;\n"
> + "void main()\n"
> + "{\n"
> + " gl_Position = piglit_vertex;\n"
> + " if (gl_VertexID >= 2 && gl_VertexID <= 5 ) {\n"
> + " gl_FrontColor = vec4(0.0, 1.0, 0.0, 1.0);\n"
> + " } else {\n"
> + " gl_FrontColor = vec4(1.0, 0.0, 0.0, 1.0);\n"
> + " }\n"
> + "}\n";
In core profile (or in OpenGL 3.1 on Mesa), you must have a fragment shader. Since the test requires GLSL 1.30 anyway, I think these shaders would work best:
static const GLchar *vertShaderText =
"#version 130\n"
"in vec4 piglit_vertex;\n"
"flat out int pass;\n"
"\n"
"void main()\n"
"{\n"
" gl_Position = piglit_vertex;\n"
" pass = int(gl_VertexID >= 2 && gl_VertexID <= 5);\n"
"}\n";
static const GLchar *fragShaderText =
"#version 130\n"
"flat in int pass;\n"
"\n"
"void main()\n"
"{\n"
" gl_FragColor = bool(pass)\n"
" ? vec4(0.0, 1.0, 0.0, 1.0)\n"
" : vec4(1.0, 0.0, 0.0, 1.0);\n"
"}\n";
> +
> +static GLint prog;
> +
> +static const GLfloat Vcoords[6][2] = { {-1, -1}, {-1, -1}, {-1, -1},
> +{1, -1}, {1, 1}, {-1, 1}};
> +
> +static const unsigned short indices[] = {
> + 2, 0xFFFF, 2, 3, 0xFFFF, 0xFFFF, 2, 3, 4, 5, 0xFFFF, 0xFFFF,
> +0xFFFF,5 };
> +
> +
> +enum piglit_result
> +piglit_display(void)
> +{
> + static const GLfloat expColor[4] = {0, 1, 0, 1};
> + GLboolean pass = GL_TRUE;
bool pass = true;
> + enum piglit_result result;
I think "result" is not used. It should be removed.
> +
> + glUseProgram(prog);
> +
> + glClear(GL_COLOR_BUFFER_BIT);
I think having the clear between the two draws prevents the test from exercising the case we wanted to test. Did you try this with Mesa from just before Ken's fix?
[Shuo] I had test it by V3, and I will double check it during the new version.
> +
> + /* Use user-allocated vertex arrays (NOT vertex buffer objects). */
> + glVertexAttribPointer(PIGLIT_ATTRIB_POS, 2, GL_FLOAT, GL_FALSE, 0, Vcoords);
> + glEnableVertexAttribArray(PIGLIT_ATTRIB_POS);
> +
> + /* Use a glDrawArrays with a non-zero 'first'. */
> + glDrawArrays(GL_TRIANGLE_FAN, 2, 4);
> +
> + pass = piglit_probe_pixel_rgba(20, 20, expColor);
> +
> + glDisableVertexAttribArray(PIGLIT_ATTRIB_POS);
> +
> + /* Use primitive restart to trigger multiple _mesa_prim without an
> + * intervening state change.
> + * Because for OpenGL, PRIMITIVE_RESTART work with all
> + * rendering commands, including the non-indexed commands. However,
> + * implementations did not implement this, and the rule itself makes
> + * no sense (https://www.opengl.org/wiki/Vertex_Rendering)
> + * So for testing PRIMITIVE_RESTART I used glDrawElements
> + */
> + glUseProgram(prog);
> +
> + glClear(GL_COLOR_BUFFER_BIT);
> +
> + glEnable(GL_PRIMITIVE_RESTART_FIXED_INDEX);
GL_PRIMITIVE_RESTART_FIXED_INDEX only exists when the extension GL_ARB_ES3_compatibility is available. You can get the same result in OpenGL 3.1 by using GL_PRIMITIVE_RESTART and setting the restart index to 0xffff.
The function glPrimitiveRestartIndex only exists in OpenGL 3.1. In earlier versions with GL_NV_primitive_restart you use GL_PRIMITIVE_RESTART_NV (as client state) and glPrimitiveRestartNV.
if (piglit_get_gl_version() >= 31) {
glEnable(GL_PRIMITIVE_RESTART);
glPrimitiveRestartIndex(0x0ffff);
} else {
glEnableClientState(GL_PRIMITIVE_RESTART_NV);
glPrimitiveRestartIndexNV(0x0ffff);
}
> +
> + glVertexAttribPointer(PIGLIT_ATTRIB_POS, 2, GL_FLOAT, GL_FALSE, 0, Vcoords);
> + glEnableVertexAttribArray(PIGLIT_ATTRIB_POS);
> +
> + glDrawElements(GL_TRIANGLE_FAN, ARRAY_SIZE(indices),
> +GL_UNSIGNED_SHORT, indices);
> +
> + pass = piglit_probe_pixel_rgba(20, 20, expColor) && pass;
> +
> + glDisableVertexAttribArray(PIGLIT_ATTRIB_POS);
> +
> + glDisable(GL_PRIMITIVE_RESTART_FIXED_INDEX);
Same here.
if (piglit_get_gl_version() >= 31)
glDisable(GL_PRIMITIVE_RESTART);
else
glDisableClientState(GL_PRIMITIVE_RESTART_NV);
> +
> + piglit_present_results();
> +
> + return pass ? PIGLIT_PASS : PIGLIT_FAIL; }
> +
> +
> +void
> +piglit_init(int argc, char **argv)
> +{
> +
> + piglit_require_GLSL_version(130);
> +
> + if (!piglit_is_extension_supported("GL_NV_primitive_restart") &&
> + piglit_get_gl_version() < 31) {
> + printf("GL_NV_primitive_restart or GL 3.1 required\n");
> + piglit_report_result(PIGLIT_SKIP);
> + }
> +
> + prog = piglit_build_simple_program(vertShaderText, NULL); }
>
More information about the Piglit
mailing list