[Piglit] [PATCH] arb_geometry_shader4: Test valid vertices out count.
Paul Berry
stereotype441 at gmail.com
Sun Jun 16 21:11:58 PDT 2013
On 16 June 2013 12:12, Fabian Bieler <fabianbieler at fastmail.fm> wrote:
> Test that glProgramParameter() triggers the required errors.
>
> The value of GEOMETRY_VERTICES_OUT is limited by the implementation
> dependend constants MAX_GEOMETRY_OUTPUT_VERTICES and
> MAX_VERTEX_VARYING_COMPONENTS.
>
> From the ARB_geometry_shader4 spec (section Errors):
> "The error INVALID_VALUE is generated by ProgramParameteriARB if <pname> is
> GEOMETRY_VERTICES_OUT_ARB and <value> is negative.
>
> The error INVALID_VALUE is generated by ProgramParameteriARB if <pname> is
> GEOMETRY_VERTICES_OUT_ARB and <value> exceeds
> MAX_GEOMETRY_OUTPUT_VERTICES_ARB.
>
> The error INVALID_VALUE is generated by ProgramParameteriARB if <pname> is
> set to GEOMETRY_VERTICES_OUT_ARB and the product of <value> and the sum of
> all components of all active varying variables exceeds
> MAX_GEOMETRY_TOTAL_OUTPUT_COMPONENTS_ARB."
>
There's an inconsistency in the spec here. Earlier in the spec, it says
that MAX_GEOMETRY_TOTAL_OUTPUT_COMPONENTS_ARB is checked at link time, not
at the time of the call to ProgramParameteriARB(). The OpenGL 3.2 and 4.3
specs also agree that this limit is checked at link time.
I think the spec writers just made a copy-paste error here--it's crazy to
check MAX_GEOMETRY_TOTAL_OUTPUT_COMPONENTS_ARB at the time of the call to
ProgramParameteriARB(), because at that time, it's possible that the
geometry shader hasn't even been attached yet.
On an unrelated note, in general comments like the ones above should go at
the top of the .c file rather than in the commit message. That way if
someone gets a test failure later, and goes to the .c file to investigate
why, they will see the relevant spec text rather than having to dig through
git history.
>
> v2: Don't count gl_Position against MAX_GEOMETRY_TOTAL_OUTPUT_COMPONENTS.
> ---
> tests/all.tests | 2 +
> .../execution/program-parameter/CMakeLists.gl.txt | 1 +
> .../execution/program-parameter/vertices-out.c | 209
> +++++++++++++++++++++
> 3 files changed, 212 insertions(+)
> create mode 100644
> tests/spec/arb_geometry_shader4/execution/program-parameter/vertices-out.c
>
> diff --git a/tests/all.tests b/tests/all.tests
> index 0d47323..0bb7f24 100644
> --- a/tests/all.tests
> +++ b/tests/all.tests
> @@ -2316,6 +2316,8 @@ add_plain_test(arb_map_buffer_alignment,
> 'arb_map_buffer_alignment-sanity_test')
> arb_geometry_shader4 = Group()
> add_concurrent_test(arb_geometry_shader4, 'program-parameter-input-type')
> add_concurrent_test(arb_geometry_shader4, 'program-parameter-output-type')
> +for mode in ['1', 'tf 1', '4', 'tf 4', '32', 'tf 32']:
> + add_concurrent_test(arb_geometry_shader4,
> 'program-parameter-vertices-out {0}'.format(mode))
> spec['ARB_geometry_shader4'] = arb_geometry_shader4
> add_shader_test_dir(spec['ARB_geometry_shader4'],
> os.path.join(testsDir, 'spec',
> 'arb_geometry_shader4'),
> diff --git
> a/tests/spec/arb_geometry_shader4/execution/program-parameter/CMakeLists.gl.txt
> b/tests/spec/arb_geometry_shader4/execution/program-parameter/CMakeLists.gl.txt
> index aee7a98..5768ead 100644
> ---
> a/tests/spec/arb_geometry_shader4/execution/program-parameter/CMakeLists.gl.txt
> +++
> b/tests/spec/arb_geometry_shader4/execution/program-parameter/CMakeLists.gl.txt
> @@ -12,3 +12,4 @@ link_libraries (
>
> piglit_add_executable (program-parameter-input-type input-type.c)
> piglit_add_executable (program-parameter-output-type output-type.c)
> +piglit_add_executable (program-parameter-vertices-out vertices-out.c)
> diff --git
> a/tests/spec/arb_geometry_shader4/execution/program-parameter/vertices-out.c
> b/tests/spec/arb_geometry_shader4/execution/program-parameter/vertices-out.c
> new file mode 100644
> index 0000000..3fdca1b
> --- /dev/null
> +++
> b/tests/spec/arb_geometry_shader4/execution/program-parameter/vertices-out.c
> @@ -0,0 +1,209 @@
> +/*
> + * Copyright © 2013 The Piglit project
> + *
> + * 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 vertices-out.c
> + *
> + * Test required errors for wrong GL_GEOMETRY_VERTICES_OUT parameters with
> + * variable number of output components.
>
+ */
> +
> +#include "common.h"
> +
> +
> +static const char gs_text_var[] =
> + "uniform int vertex_count;\n"
> + "varying out float var[n];\n"
> + "void main()\n"
> + "{\n"
> + " for (int i = 0; i < vertex_count; i++) {\n"
> + " gl_Position = vec4(0.0);\n"
> + " for (int j = 0; j < n; j++)\n"
> + " var[j] = 1.0 / exp2(float(j + 1));\n"
> + " EmitVertex();\n"
> + " }\n"
> + "}\n";
> +
> +static const char fs_text_var[] =
> + "varying float var[n];\n"
> + "void main()\n"
> + "{\n"
> + " gl_FragColor = vec4(0.0);\n"
> + " for (int j = 0; j < n; j++)\n"
> + " gl_FragColor += vec4(var[j]);\n"
> + "}\n";
> +
> +static bool transform_feedback = false;
> +static int components = -1;
> +
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> + config.supports_gl_compat_version = 20;
> + config.window_visual = PIGLIT_GL_VISUAL_DOUBLE |
> PIGLIT_GL_VISUAL_RGBA;
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +
> +static bool
> +test_geometry_vertices_out(const GLuint prog)
> +{
> + bool pass = true;
> + int max_geometry_output_vertices;
> + int max_geometry_total_output_components;
> + int max_vertices_out;
> +
> + glGetIntegerv(GL_MAX_GEOMETRY_OUTPUT_VERTICES,
> + &max_geometry_output_vertices);
> + glGetIntegerv(GL_MAX_GEOMETRY_TOTAL_OUTPUT_COMPONENTS,
> + &max_geometry_total_output_components);
> +
> + printf("Testing negative (-1) vertices out.\n");
> + glProgramParameteri(prog, GL_GEOMETRY_VERTICES_OUT_ARB, -1);
> + pass = piglit_check_gl_error(GL_INVALID_VALUE) && pass;
> +
> + /* Setting GEOMETRY_VERTICES_OUT to zero should generate no error
> but
> + * linking with a geometry shader should fail.
> + */
> + printf("Testing zero (0) vertices out.\n");
> + glProgramParameteri(prog, GL_GEOMETRY_VERTICES_OUT_ARB, 0);
> + glLinkProgram(prog);
> + pass = piglit_check_gl_error(GL_NO_ERROR) &&
> !piglit_link_check_status_quiet(prog) && pass;
> +
> + printf("Testing too many (%d) vertices out.\n",
> max_geometry_output_vertices + 1);
> + glProgramParameteri(prog, GL_GEOMETRY_VERTICES_OUT_ARB,
> + max_geometry_output_vertices + 1);
> + pass = piglit_check_gl_error(GL_INVALID_VALUE) && pass;
> +
> + /* If MAX_GEOMETRY_TOTAL_OUTPUT_COMPONENTS results in an even
> smaller limit for
> + * GEOMETRY_VERTICES_OUT than GL_MAX_GEOMETRY_OUTPUT_VERTICES test
> that, too.
> + */
> + if ((components != 0) &&
> + (max_geometry_total_output_components / components + 1 <
> max_geometry_output_vertices + 1)) {
>
The "+ 1"s seem unnecessary. How about just "if
(max_geometry_total_output_components / components <
max_geometry_output_vertices)"?
Also, the piglit coding conventions are to keep lines less than 80 columns,
except in all.tests, and in rare situations where it would make the code
difficult to read.
> + /* The spec requires ProgramParameter to throw an
> INVALID_VALUE if
> + * (GEOMETRY_VERTICES_OUT * (sum of components of all out
> varyings)) >
> + * MAX_GEOMETRY_TOTAL_OUTPUT_COMPONENTS. But the number of
> output
> + * components can only be infered from the geometry shader
> source
> + * (and a geometry shader might not even be attached to
> the program
> + * object when ProgramParameter is called). Also accept
> failed linking.
> + */
> + printf("Testing too many total output components (%d
> vertices => %d total components).\n",
> + max_geometry_total_output_components / components +
> 1,
> + (max_geometry_total_output_components / components
> + 1) * components);
> + glProgramParameteri(prog, GL_GEOMETRY_VERTICES_OUT_ARB,
> + max_geometry_total_output_components /
> components + 1);
> + if (!piglit_check_gl_error(GL_INVALID_VALUE) &&
> + (glLinkProgram(prog),
> piglit_link_check_status_quiet(prog))) {
>
Considering the discussion above, I think this code should just try to link
and check the link status. IMHO no implementation is going to generate
this error at the time of the call to glProgramParameteri().
> + pass = false;
> + }
> + }
> +
> + max_vertices_out = (components != 0) ?
> + MIN2(max_geometry_output_vertices,
> max_geometry_total_output_components / components) :
> + max_geometry_output_vertices;
> + printf("Testing maximal (%d) vertices out.\n", max_vertices_out);
> + glProgramParameteri(prog, GL_GEOMETRY_VERTICES_OUT_ARB,
> + max_vertices_out);
> + glLinkProgram(prog);
> + pass = piglit_check_gl_error(GL_NO_ERROR) &&
> piglit_link_check_status(prog) && pass;
> +
> + return pass;
> +}
> +
> +
> +/* Parse command line arguments.
> + *
> + * Recognized command line arguments are:
> + * * The optional argument "tf": Use transform feedback.
> + * * An integer indicating the number of per vertex varying components
> + * written by the geometry shader (including gl_Position if
> transform
> + * feedback is not used).
> + */
> +static void
> +parse_cmd_line(int argc, char **argv)
> +{
> + int i;
> +
> + for (i = 1; i < argc; i++) {
> + if (strcmp(argv[i], "tf") == 0) {
> + transform_feedback = true;
> + } else if(argv[i][0] != '-') {
> + char *e;
> + long int l = strtol(argv[i], &e, 10);
> + if (*e == '\0')
> + components = l;
> + }
> + }
> +
> + if ((components < 0) || components > 32) {
> + fprintf(stderr, "Please specify number of components "
> + "from 0 to 32 (inclusive) on the command line\n");
> + piglit_report_result(PIGLIT_FAIL);
> + }
>
It seems weird to place a maximum of 32 on the number of components. Why
not just have the test run once with 1 component and once with
MAX_GEOMETRY_VARYING_COMPONENTS_ARB?
> +}
> +
> +void
> +piglit_init(int argc, char **argv)
> +{
> + bool pass = true;
> + GLuint prog;
> + const char *gs_string, *fs_string;
> +
> + parse_cmd_line(argc, argv);
> +
> + piglit_require_extension("GL_ARB_geometry_shader4");
> + /* NV_geometry_shader4 relaxes some restrictions on valid program
> + * parameters.
> + */
> + piglit_require_not_extension("GL_NV_geometry_shader4");
> + if (transform_feedback)
> + piglit_require_extension("GL_EXT_transform_feedback");
> +
> + /* Prepare shader source strings. */
> + if (components == 0) {
> + gs_string = gs_text4;
> + fs_string = fs_text4;
> + } else {
> + asprintf((char **)&gs_string, "#extension
> GL_ARB_geometry_shader4: enable\n"
> + "const int n = %d;\n%s",
> components, gs_text_var);
> + if (transform_feedback)
> + fs_string = NULL;
> + else
> + asprintf((char **)&fs_string, "const int n =
> %d;\n%s", components, fs_text_var);
> + }
> +
> + printf("Running Test with %d component(s) per output vertex and
> transform feedback %s.\n",
> + components, transform_feedback ? "enabled" : "disabled");
> +
> + /* Create shader and run test. */
> + prog = create_shader(vs_text, gs_string, fs_string);
> + pass = test_geometry_vertices_out(prog) && pass;
> + glDeleteProgram(prog);
> +
> + piglit_report_result(pass ? PIGLIT_PASS : PIGLIT_FAIL);
> +}
> +
> +enum piglit_result
> +piglit_display(void)
> +{
> + /* Should never be reached */
> + return PIGLIT_FAIL;
> +}
> --
> 1.8.1.2
>
>
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20130617/b36d1d88/attachment-0001.html>
More information about the Piglit
mailing list