<div dir="ltr">On 4 September 2013 12:57, Ian Romanick <span dir="ltr"><<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">

<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">From: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com" target="_blank">ian.d.romanick@intel.com</a>><br>


<br>
This is a nearly exhaustive test of all the glProgramUniform functions<br>
added by GL_ARB_separate_shader_objects.<br>
<br>
For every entrypoint, a shader using a uniform of the correct type is<br>
created using glCreateShaderProgramv.  The uniform is then set and<br>
queried (using glGetUniform).  The test passes if the correct data is<br>
returned and no GL errors are generated.<br>
<br>
The following aspects of these interfaces are not tested by this test:<br>
<br>
    - Uniform arrays set by glProgramUniform*v or by<br>
      glProgramUniformMatrix*v with count > 1.<br>
<br>
    - Transpose matrices set by glProgramUniformMatrix*v with transpose<br>
      set to GL_TRUE.<br>
<br>
Signed-off-by: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com" target="_blank">ian.d.romanick@intel.com</a>><br>
---<br>
 tests/all.tests                                    |    1 +<br>
 .../arb_separate_shader_objects/CMakeLists.gl.txt  |    1 +<br>
 .../ProgramUniform-coverage.c                      | 1211 ++++++++++++++++++++<br>
 3 files changed, 1213 insertions(+)<br>
 create mode 100644 tests/spec/arb_separate_shader_objects/ProgramUniform-coverage.c<br>
<br>
diff --git a/tests/all.tests b/tests/all.tests<br>
index a8e389c..9baaa89 100644<br>
--- a/tests/all.tests<br>
+++ b/tests/all.tests<br>
@@ -1241,6 +1241,7 @@ arb_separate_shader_objects['GetProgramPipelineiv'] = concurrent_test('arb_separ<br>
 arb_separate_shader_objects['IsProgramPipeline'] = concurrent_test('arb_separate_shader_object-IsProgramPipeline')<br>
 arb_separate_shader_objects['UseProgramStages - non-separable program'] = concurrent_test('arb_separate_shader_object-UseProgramStages-non-separable')<br>
 arb_separate_shader_objects['Mix BindProgramPipeline and UseProgram'] = concurrent_test('arb_separate_shader_object-mix_pipeline_useprogram')<br>
+arb_separate_shader_objects['ProgramUniform coverage'] = concurrent_test('arb_separate_shader_object-ProgramUniform-coverage')<br>
 arb_separate_shader_objects['Rendezvous by location'] = plain_test('arb_separate_shader_object-rendezvous_by_location -fbo')<br>
 arb_separate_shader_objects['ValidateProgramPipeline'] = concurrent_test('arb_separate_shader_object-ValidateProgramPipeline')<br>
<br>
diff --git a/tests/spec/arb_separate_shader_objects/CMakeLists.gl.txt b/tests/spec/arb_separate_shader_objects/CMakeLists.gl.txt<br>
index 66176f1..32a28ba 100644<br>
--- a/tests/spec/arb_separate_shader_objects/CMakeLists.gl.txt<br>
+++ b/tests/spec/arb_separate_shader_objects/CMakeLists.gl.txt<br>
@@ -12,6 +12,7 @@ link_libraries (<br>
 piglit_add_executable (arb_separate_shader_object-GetProgramPipelineiv GetProgramPipelineiv.c)<br>
 piglit_add_executable (arb_separate_shader_object-IsProgramPipeline IsProgramPipeline.c)<br>
 piglit_add_executable (arb_separate_shader_object-mix_pipeline_useprogram mix_pipeline_useprogram.c)<br>
+piglit_add_executable (arb_separate_shader_object-ProgramUniform-coverage ProgramUniform-coverage.c)<br>
 piglit_add_executable (arb_separate_shader_object-rendezvous_by_location rendezvous_by_location.c)<br>
 piglit_add_executable (arb_separate_shader_object-UseProgramStages-non-separable UseProgramStages-non-separable.c)<br>
 piglit_add_executable (arb_separate_shader_object-ValidateProgramPipeline ValidateProgramPipeline.c)<br>
diff --git a/tests/spec/arb_separate_shader_objects/ProgramUniform-coverage.c b/tests/spec/arb_separate_shader_objects/ProgramUniform-coverage.c<br>
new file mode 100644<br>
index 0000000..c1faf60<br>
--- /dev/null<br>
+++ b/tests/spec/arb_separate_shader_objects/ProgramUniform-coverage.c<br>
@@ -0,0 +1,1211 @@<br>
+/*<br>
+ * Copyright © 2013 Intel Corporation<br>
+ *<br>
+ * Permission is hereby granted, free of charge, to any person obtaining a<br>
+ * copy of this software and associated documentation files (the "Software"),<br>
+ * to deal in the Software without restriction, including without limitation<br>
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,<br>
+ * and/or sell copies of the Software, and to permit persons to whom the<br>
+ * Software is furnished to do so, subject to the following conditions:<br>
+ *<br>
+ * The above copyright notice and this permission notice (including the next<br>
+ * paragraph) shall be included in all copies or substantial portions of the<br>
+ * Software.<br>
+ *<br>
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR<br>
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,<br>
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL<br>
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER<br>
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING<br>
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS<br>
+ * IN THE SOFTWARE.<br>
+ */<br>
+<br>
+/**<br>
+ * \name ProgramUniform-coverage.c<br>
+ * Nearly exhaustive test of all the glProgramUniform functions added by<br>
+ * GL_ARB_separate_shader_objects.<br>
+ *<br>
+ * For every entrypoint, a shader using a uniform of the correct type is<br>
+ * created using \c glCreateShaderProgramv.  The uniform is then set and<br>
+ * queried (using \c glGetUniform).  The test passes if the correct data is<br>
+ * returned and no GL errors are generated.<br>
+ *<br>
+ * \note<br>
+ * The following aspects of these interfaces are not tested by this test:<br>
+ *<br>
+ *     - Uniform arrays set by \c glProgramUniform*v or by<br>
+ *       \c glProgramUniformMatrix*v with \c count > 1.<br>
+ *<br>
+ *     - Transpose matrices set by \c glProgramUniformMatrix*v with \c transpose<br>
+ *       set to \c GL_TRUE.<br>
+ */<br>
+#include "piglit-util-gl-common.h"<br>
+<br>
+PIGLIT_GL_TEST_CONFIG_BEGIN<br>
+<br>
+       config.supports_gl_compat_version = 10;<br>
+       config.window_visual = PIGLIT_GL_VISUAL_RGB | PIGLIT_GL_VISUAL_DOUBLE;<br>
+<br>
+PIGLIT_GL_TEST_CONFIG_END<br>
+<br>
+static const char common_body[] =<br>
+       "\n"<br>
+       "void main()\n"<br>
+       "{\n"<br>
+       "    gl_Position = vec4(v4) + vec4(v3, v1) + vec4(v2, 0, 0);\n"<br>
+       "}\n"<br>
+       ;<br>
+<br>
+static const char float_code[] =<br>
+       "uniform float v1;\n"<br>
+       "uniform vec2 v2;\n"<br>
+       "uniform vec3 v3;\n"<br>
+       "uniform vec4 v4;\n"<br>
+       ;<br>
+<br>
+static const char int_code[] =<br>
+       "uniform int v1;\n"<br>
+       "uniform ivec2 v2;\n"<br>
+       "uniform ivec3 v3;\n"<br>
+       "uniform ivec4 v4;\n"<br>
+       ;<br>
+<br>
+static const char uint_code[] =<br>
+       "uniform uint v1;\n"<br>
+       "uniform uvec2 v2;\n"<br>
+       "uniform uvec3 v3;\n"<br>
+       "uniform uvec4 v4;\n"<br>
+       ;<br>
+<br>
+static const char double_code[] =<br>
+       "uniform uint v1;\n"<br>
+       "uniform uvec2 v2;\n"<br>
+       "uniform uvec3 v3;\n"<br>
+       "uniform uvec4 v4;\n"<br>
+       ;<br></blockquote><div><br></div><div>I think you mean:<br><br></div><div>uniform double v1;<br></div><div>uniform dvec2 v2;<br>uniform dvec3 v3;<br>uniform dvec4 v4;<br></div><div><br>Copy and paste error?<br><br>

</div><div>I didn't have the energy to read through the whole test, so with the above fixed, consider this patch:<br><br>Acked-by: Paul Berry <<a href="mailto:stereotype441@gmail.com" target="_blank">stereotype441@gmail.com</a>><br>

<br></div><div>I've sent comments on everything but patch 4.  Patch 4 is:<br><br>Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com" target="_blank">stereotype441@gmail.com</a>><br><br><br><br></div>
<div>You also asked yesterday if I could send out a list of additional ARB_sso tests that I think we need.  Here's what I was able to come up with:<br>
<br>- For i965, it would be nice to have some validation that rendezvous by location works properly for VS-FS when there are more than 16 slots used by the FS, and the set of locations output by the VS does not match the set of locations input by the FS.  (This is the one case I wasn't able to verify when testing my "i965/gen6+: Support 128 varying components" series, because ARB_sso is really needed in order to test it effectively).<br>
<br></div><div>- Test that varying structs and arrays are *not* packed when there is an explicit location.  For example, this:<br><br>struct Foo {<br></div><div>   vec2 a;<br></div><div>   vec2 b;<br></div><div>   vec2 c[2];<br>
};<br>layout(location = 5) out Foo f;<br><br></div><div>should cause f.a to be assigned location 5, f.b to be assigned location 6, f.c[0] to be assigned to location 7, and f.c[1] to be assigned to location 8.  (Currently Mesa packs varying structs unconditionally, so this test would fail unless we do additional work).<br>
<br></div><div>- Test that varying arrays that are assigned an explicit location still work properly with transform feedback, both when capturing a single element of the array and when capturing the entire array all at once.  (Currently Mesa's transform feedback code assumes that varying arrays are always packed).<br>
</div><div><br></div><div>- Verify that if you create a separable program with GS but no VS, and then you try to use it using UseProgram(): (a) ValidateProgram() says it's invalid, and (b) trying to draw with it results in INVALID_OPERATION.  (This is the "loophole" that I erroneously thought I discovered in the spec yesterday.  It would be good to make sure the implementation doesn't have such a loophole).<br>
<br></div><div>- I didn't see any tests to verify that glActiveShaderProgram() has the proper effect (only to verify that it generates the proper error messages).  I could be mistaken about that though.<br><br></div><div>
- Verify that the state set by glActiveShaderProgram() is per-pipeline state, not global state.<br><br></div><div>- Test that INVALID_OPERATION is generated by ResumeTransformFeedback if the program pipeline object being used by the current transform feedback object is not bound, any of its shader stage bindings has changed, or a single program object is active and overriding it (see the text "Add to list of INVALID_OPERATION errors on page 172" from the ARB_sso spec).<br>
<br></div><div>- Patch 8/9 was called "sso: Verify that rendezvous by location also works", but I don't see any tests of rendezvous by name yet.  Am I missing something?<br><br></div><div>- Some tests that mix rendezvous-by-name with rendezvous-by-location are probably in order, especially given that the rendezvous-by-name variables have to get allocated to unassigned locations.<br>

</div></div></div></div>