Re: [Piglit] [PATCH 1/2] status.py: convert status object attributes from instance to class
On Tue, Oct 15, 2013 at 05:28:48AM -0700, Dylan Baker wrote: > This converts Status objects attributes to class attributes. This should > reduce lookup time and memory consumption of these classes. > > Signed-off-by: Dylan Baker I don't pretend to understand why this is needed other than Dylan said it will help get to the point of subtest failure causing an overall failure (which I want). To help speed things up, this is: Tested-by: Ben Widawsky -- Ben Widawsky, Intel Open Source Technology Center ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH v2] Interface Blocks: Test how interface block members are accessed from API
On 21 October 2013 16:04, Nicholas Mack wrote:
> v2: Add checks to also test invalid names
> ---
> tests/all.tests| 1 +
> tests/spec/glsl-1.50/execution/CMakeLists.gl.txt | 1 +
> .../interface-blocks-api-access-members.c | 173
> +
> 3 files changed, 175 insertions(+)
> create mode 100644
> tests/spec/glsl-1.50/execution/interface-blocks-api-access-members.c
>
> diff --git a/tests/all.tests b/tests/all.tests
> index c919f19..d2e685c 100644
> --- a/tests/all.tests
> +++ b/tests/all.tests
> @@ -966,6 +966,7 @@ import_glsl_parser_tests(spec['glsl-1.50'],
> add_shader_test_dir(spec['glsl-1.50'],
> os.path.join(testsDir, 'spec', 'glsl-1.50'),
> recursive=True)
> +spec['glsl-1.50']['execution']['interface-blocks-api-access-members'] =
> concurrent_test('glsl-1.50-interface-blocks-api-access-members')
> spec['glsl-1.50']['execution']['get-active-attrib-array'] =
> concurrent_test('glsl-1.50-get-active-attrib-array')
> spec['glsl-1.50']['execution']['vs-input-arrays'] =
> concurrent_test('glsl-1.50-vs-input-arrays')
> spec['glsl-1.50']['execution']['vs-named-block-no-modify'] =
> concurrent_test('glsl-1.50-vs-named-block-no-modify')
> diff --git a/tests/spec/glsl-1.50/execution/CMakeLists.gl.txt
> b/tests/spec/glsl-1.50/execution/CMakeLists.gl.txt
> index 693f69a..114867b 100644
> --- a/tests/spec/glsl-1.50/execution/CMakeLists.gl.txt
> +++ b/tests/spec/glsl-1.50/execution/CMakeLists.gl.txt
> @@ -13,3 +13,4 @@ ${OPENGL_glu_LIBRARY}
> piglit_add_executable (glsl-1.50-vs-input-arrays vs-input-arrays.c)
> piglit_add_executable (glsl-1.50-get-active-attrib-array
> get-active-attrib-array.c)
> piglit_add_executable (glsl-1.50-vs-named-block-no-modify
> vs-named-block-no-modify.c)
> +piglit_add_executable (glsl-1.50-interface-blocks-api-access-members
> interface-blocks-api-access-members.c)
> diff --git
> a/tests/spec/glsl-1.50/execution/interface-blocks-api-access-members.c
> b/tests/spec/glsl-1.50/execution/interface-blocks-api-access-members.c
> new file mode 100644
> index 000..77089f8
> --- /dev/null
> +++ b/tests/spec/glsl-1.50/execution/interface-blocks-api-access-members.c
> @@ -0,0 +1,173 @@
> +/**
> + * Copyright © 2013 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.
> + */
> +
> +/**
> + * Test the syntax for accessing interface block members through the API
> + *
> + * From the GLSL 1.50 core spec, section 4.3.7 (Interface Blocks):
> + * "Outside the shading language (i.e., in the API), members are similarly
> + * identified except the block name is always used in place of the
> instance
> + * name (API accesses are to interfaces, not to shaders). If there is no
> + * instance name, then the API does not use the block name to access a
> member,
> + * just the member name."
> + *
> + * "For blocks declared as arrays, the array index must also be included
> when
> + * accessing members"
> + */
> +
> +#include "piglit-util-gl-common.h"
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +
> + config.supports_gl_compat_version = 32;
> + config.supports_gl_core_version = 32;
> +
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +static const char *vstext =
> + "#version 150\n"
> + "in vec4 vertex;\n"
> + "void main()\n"
> + "{\n"
> + " gl_Position = vertex;\n"
> + "}\n";
> +
> +static const char *gstext =
> + "#version 150\n"
> + "layout(points) in;\n"
> + "layout(points, max_vertices = 3) out;\n"
> + "out NoInst {\n"
> + " float a;\n"
> + " vec3 b;\n"
> + "};\n"
> + "out WithInst {\n"
> + " float c;\n"
> + " vec3 d;\n"
> + "} inst;\n"
> + "out WithInstArray {\n"
> + " float e;\n"
> + " vec3 f;\n"
> +
Re: [Piglit] [PATCH v2] Interface Blocks: Test how interface block members are accessed from API
On 21 October 2013 16:04, Nicholas Mack wrote:
> v2: Add checks to also test invalid names
> ---
> tests/all.tests| 1 +
> tests/spec/glsl-1.50/execution/CMakeLists.gl.txt | 1 +
> .../interface-blocks-api-access-members.c | 173
> +
> 3 files changed, 175 insertions(+)
> create mode 100644
> tests/spec/glsl-1.50/execution/interface-blocks-api-access-members.c
>
Reviewed-by: Paul Berry
>
> diff --git a/tests/all.tests b/tests/all.tests
> index c919f19..d2e685c 100644
> --- a/tests/all.tests
> +++ b/tests/all.tests
> @@ -966,6 +966,7 @@ import_glsl_parser_tests(spec['glsl-1.50'],
> add_shader_test_dir(spec['glsl-1.50'],
> os.path.join(testsDir, 'spec', 'glsl-1.50'),
> recursive=True)
> +spec['glsl-1.50']['execution']['interface-blocks-api-access-members'] =
> concurrent_test('glsl-1.50-interface-blocks-api-access-members')
> spec['glsl-1.50']['execution']['get-active-attrib-array'] =
> concurrent_test('glsl-1.50-get-active-attrib-array')
> spec['glsl-1.50']['execution']['vs-input-arrays'] =
> concurrent_test('glsl-1.50-vs-input-arrays')
> spec['glsl-1.50']['execution']['vs-named-block-no-modify'] =
> concurrent_test('glsl-1.50-vs-named-block-no-modify')
> diff --git a/tests/spec/glsl-1.50/execution/CMakeLists.gl.txt
> b/tests/spec/glsl-1.50/execution/CMakeLists.gl.txt
> index 693f69a..114867b 100644
> --- a/tests/spec/glsl-1.50/execution/CMakeLists.gl.txt
> +++ b/tests/spec/glsl-1.50/execution/CMakeLists.gl.txt
> @@ -13,3 +13,4 @@ ${OPENGL_glu_LIBRARY}
> piglit_add_executable (glsl-1.50-vs-input-arrays vs-input-arrays.c)
> piglit_add_executable (glsl-1.50-get-active-attrib-array
> get-active-attrib-array.c)
> piglit_add_executable (glsl-1.50-vs-named-block-no-modify
> vs-named-block-no-modify.c)
> +piglit_add_executable (glsl-1.50-interface-blocks-api-access-members
> interface-blocks-api-access-members.c)
> diff --git
> a/tests/spec/glsl-1.50/execution/interface-blocks-api-access-members.c
> b/tests/spec/glsl-1.50/execution/interface-blocks-api-access-members.c
> new file mode 100644
> index 000..77089f8
> --- /dev/null
> +++ b/tests/spec/glsl-1.50/execution/interface-blocks-api-access-members.c
> @@ -0,0 +1,173 @@
> +/**
> + * Copyright © 2013 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.
> + */
> +
> +/**
> + * Test the syntax for accessing interface block members through the API
> + *
> + * From the GLSL 1.50 core spec, section 4.3.7 (Interface Blocks):
> + * "Outside the shading language (i.e., in the API), members are similarly
> + * identified except the block name is always used in place of the
> instance
> + * name (API accesses are to interfaces, not to shaders). If there is no
> + * instance name, then the API does not use the block name to access a
> member,
> + * just the member name."
> + *
> + * "For blocks declared as arrays, the array index must also be included
> when
> + * accessing members"
> + */
> +
> +#include "piglit-util-gl-common.h"
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +
> + config.supports_gl_compat_version = 32;
> + config.supports_gl_core_version = 32;
> +
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +static const char *vstext =
> + "#version 150\n"
> + "in vec4 vertex;\n"
> + "void main()\n"
> + "{\n"
> + " gl_Position = vertex;\n"
> + "}\n";
> +
> +static const char *gstext =
> + "#version 150\n"
> + "layout(points) in;\n"
> + "layout(points, max_vertices = 3) out;\n"
> + "out NoInst {\n"
> + " float a;\n"
> + " vec3 b;\n"
> + "};\n"
> + "out WithInst {\n"
> + " float c;\n"
> + " vec3 d;\n"
> + "} inst;\n"
> + "out WithInstArray {\n"
> + " float e;\n"
> +
Re: [Piglit] [PATCH] summary.py: Fix empty generation of problems, changes, etc
On 21 October 2013 17:36, Dylan Baker wrote:
> The list comprehension can be further refined to be fixed, but the
> resulting code is much more difficult to read than the try/except
> statement.
>
> This reverts commit aaae592490a6459aba52bc306b819a7fcbaa3008.
> This reverts commit 450935b06f8508d3f4177fe7336e3bdee3973a9e.
>
> CC: Paul Berry
> Signed-off-by: Dylan Baker
>
Thanks, Dylan!
Tested-by: Paul Berry
> ---
> framework/summary.py | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/framework/summary.py b/framework/summary.py
> index 3e5c24e..6084621 100644
> --- a/framework/summary.py
> +++ b/framework/summary.py
> @@ -361,8 +361,12 @@ class Summary:
> # Create the lists of statuses like problems, regressions, fixes,
> # changes and skips
> for test in self.tests['all']:
> -status = [i.tests.get('test', {}).get('result', so.NotRun())
> - for i in self.results]
> +status = []
> +for each in self.results:
> +try:
> +status.append(each.tests[test]['result'])
> +except KeyError:
> +status.append(so.NotRun())
>
> # Problems include: warn, dmesg-warn, fail, dmesg-fail, and
> crash.
> # Skip does not go on this page, it has the 'skipped' page
> --
> 1.8.1.5
>
>
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
[Piglit] [PATCH] summary.py: Fix empty generation of problems, changes, etc
The list comprehension can be further refined to be fixed, but the
resulting code is much more difficult to read than the try/except
statement.
This reverts commit aaae592490a6459aba52bc306b819a7fcbaa3008.
This reverts commit 450935b06f8508d3f4177fe7336e3bdee3973a9e.
CC: Paul Berry
Signed-off-by: Dylan Baker
---
framework/summary.py | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/framework/summary.py b/framework/summary.py
index 3e5c24e..6084621 100644
--- a/framework/summary.py
+++ b/framework/summary.py
@@ -361,8 +361,12 @@ class Summary:
# Create the lists of statuses like problems, regressions, fixes,
# changes and skips
for test in self.tests['all']:
-status = [i.tests.get('test', {}).get('result', so.NotRun())
- for i in self.results]
+status = []
+for each in self.results:
+try:
+status.append(each.tests[test]['result'])
+except KeyError:
+status.append(so.NotRun())
# Problems include: warn, dmesg-warn, fail, dmesg-fail, and crash.
# Skip does not go on this page, it has the 'skipped' page
--
1.8.1.5
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH 9/9] summary.py: Use with open(...) as syntax
On 15 October 2013 03:13, Dylan Baker wrote:
> This is the commonly excepted "right way" in python.
>
> Signed-off-by: Dylan Baker
>
Something in this series regressed the "changes", "problems", and
"regressions" links in the piglit HTML output. As of commit aaae592
(summary.py: Fix regression in Summary class) those pages show up empty,
even if there really are changes, problems, or regressions. In commit
ad97d8b (summary.py: Replace ``import from'' syntax with import), the links
work fine. I can't bisect further because of the regression fixed in
commit aaae592.
I don't know whether the "skipped" and "fixes" links work properly or not.
> ---
> framework/summary.py | 77
>
> 1 file changed, 36 insertions(+), 41 deletions(-)
>
> diff --git a/framework/summary.py b/framework/summary.py
> index 3ab71f3..359d078 100644
> --- a/framework/summary.py
> +++ b/framework/summary.py
> @@ -428,13 +428,12 @@ class Summary:
> for each in self.results:
> os.mkdir(path.join(destination, each.name))
>
> -file = open(path.join(destination, each.name, "index.html"),
> 'w')
> -file.write(testindex.render(name=each.name,
> -time=each.time_elapsed,
> -options=each.options,
> -glxinfo=each.glxinfo,
> -lspci=each.lspci))
> -file.close()
> +with open(path.join(destination, each.name, "index.html"),
> 'w') as out:
> +out.write(testindex.render(name=each.name,
> + time=each.time_elapsed,
> + options=each.options,
> + glxinfo=each.glxinfo,
> + lspci=each.lspci))
>
> # Then build the individual test results
> for key, value in each.tests.iteritems():
> @@ -447,21 +446,19 @@ class Summary:
> if not path.exists(temp_path):
> os.makedirs(temp_path)
>
> -file = open(path.join(destination,
> - each.name,
> - key + ".html"), 'w')
> -file.write(testfile.render(
> -testname=key,
> -status=value.get('result', 'None'),
> -returncode=value.get('returncode', 'None'),
> -time=value.get('time', 'None'),
> -info=value.get('info', 'None'),
> -traceback=value.get('traceback', 'None'),
> -command=value.get('command', 'None'),
> -dmesg=value.get('dmesg', 'None'),
> -css=path.relpath(result_css, temp_path),
> -index=path.relpath(index, temp_path)))
> -file.close()
> +with open(path.join(destination, each.name, key +
> ".html"),
> + 'w') as out:
> +out.write(testfile.render(
> +testname=key,
> +status=value.get('result', 'None'),
> +returncode=value.get('returncode', 'None'),
> +time=value.get('time', 'None'),
> +info=value.get('info', 'None'),
> +traceback=value.get('traceback', 'None'),
> +command=value.get('command', 'None'),
> +dmesg=value.get('dmesg', 'None'),
> +css=path.relpath(result_css, temp_path),
> +index=path.relpath(index, temp_path)))
>
> # Finally build the root html files: index, regressions, etc
> index = Template(filename="templates/index.mako",
> @@ -477,30 +474,28 @@ class Summary:
> # Index.html is a bit of a special case since there is index,
> all, and
> # alltests, where the other pages all use the same name. ie,
> # changes.html, self.changes, and page=changes.
> -file = open(path.join(destination, "index.html"), 'w')
> -file.write(index.render(results=HTMLIndex(self,
> self.tests['all']),
> -page='all',
> -pages=pages,
> -colnum=len(self.results),
> -exclude=exclude))
> -file.close()
> +with open(path.join(destination, "index.html"), 'w') as out:
> +out.write(index.render(results=HTMLIndex(self,
> self.tests['all']),
> + page='all',
> + pages=pages,
> + c
Re: [Piglit] [PATCH v3 2/5] util: Add common framework for command-line invoked subtests
On 21 October 2013 16:55, Chad Versace wrote:
> On 10/21/2013 11:29 AM, Paul Berry wrote:
>
>> On 15 October 2013 17:32, Ian Romanick wrote:
>>
>> From: Ian Romanick
>>>
>>
>
>
> @@ -43,6 +43,20 @@ enum piglit_gl_visual {
>>> };
>>>
>>> /**
>>> + * An idividual subtest that makes up part of a test group.
>>> + */
>>> +struct piglit_gl_subtest {
>>> + /** Name of the subtest as it will appear in the log. */
>>> + const char *name;
>>> +
>>> + /** Command line name used to select this test. */
>>> + const char *option;
>>> +
>>> + /** Function that implements the test. */
>>> + enum piglit_result (*subtest_func)(void);
>>>
>>>
>> Would you have any objection to adding a void * to this structure, which
>> would then get passed to subtest_func? That would give the test
>> implementor extra flexibility if they want to share a single subtest_func
>> between multiple similar subtests. It would also allow for the
>> possibility
>> that the set of available subtests is determined at run time rather than
>> compile time.
>>
>
> I second Paul's request to add a void* parameter.
>
> However, I don't see how the presence of a void* parameter enables a test
> to determine the list of available subtests at runtime. A test can already
> do that at runtime by generating piglit_gl_test_config::**subtests in
> main().
> Nothing requires that the test declare that structure statically.
>
>
Technically you're right, but without a void * parameter to subtest_func(),
it can only generate subtests that are drawn from a fixed set of
possibilities that's determined at compile-time, since the only way for the
tests to be different from each other is by using different function
pointers, and the set of functions available is fixed at compile time.
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH v3 5/5] arb_transform_feedback2: Misc. API error checks
On 10/15/2013 05:32 PM, Ian Romanick wrote:
From: Ian Romanick
This covers most of the errors mentioned in the spec that aren't
already covered in cannot-bind-when-active.c and gen-names-only.c.
Most of the other errors should be covered by existing
EXT_transform_feedback tests.
Mesa currently passes all of these tests.
NVIDIA (304.64 on GTX 260) fails several subtests.
Pause active feedback only: Incorrectly generates error in
glEndTransformFeedback.
TransformFeedbackVaryings only when inactive: Doesn't generate any
errors when it should.
Draw only from "ended" object: Generates GL_INVALID_VALUE instead
of GL_INVALID_OPERATION.
AMD has not been tested.
v2: Convert to use piglit subtest reporting. Fix fs_only_prog to
actually only have a fragment shader attached.
v3: Quite significant re-write. Per Paul's suggestion, each of the
tests is no a subtest that can be individually selected from the command
line. This uses infrastructure in piglit-framework-gl to handle the
subtests. This also eliminates the work-around for NVIDIA's
glEndTransformFeedback-while-paused bug.
The code currently in all.tests for getting the list of subtests is
just a place holder. We'll want to refactor this out somewhere else
before pushing.
Agreed. Let's create a utility function before pushing.
+static enum piglit_result draw_from_ended_object_only(void);
+
+static const struct piglit_gl_subtest subtests[] = {
+ {
+ "Paired begin and end",
+ "paired-begin-end",
+ paired_begin_end
+ },
+ {
+ "Pause active feedback only",
+ "pause-active-only",
+ pause_active_only
+ },
+ {
+ "Resume paused feedback only",
+ "resume-paused-only",
+ resume_paused_only
+ },
[...]
+ {
+ NULL,
+ NULL,
+ NULL
+ }
+};
+
+PIGLIT_GL_TEST_CONFIG_BEGIN
+
+ config.supports_gl_compat_version = 10;
+ config.window_visual = PIGLIT_GL_VISUAL_RGB;
+ config.subtests = subtests;
+
+PIGLIT_GL_TEST_CONFIG_END
I really like how your subtest work looks. This will bring us closer
to uniformity among piglit tests, and simplify all.tests for many future tests.
I'm not familiar with the test body itself, so I refrain from reviewing this
one.
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH 4/5] util: Add function to get the set of subtests selected from the command-line
On 10/15/2013 05:32 PM, Ian Romanick wrote:
From: Ian Romanick
Signed-off-by: Ian Romanick
Cc: Chad Versace
---
tests/util/piglit-framework-gl.c | 7 +++
tests/util/piglit-framework-gl.h | 3 +++
2 files changed, 10 insertions(+)
diff --git a/tests/util/piglit-framework-gl.c b/tests/util/piglit-framework-gl.c
index c297a6a..d1883c2 100644
--- a/tests/util/piglit-framework-gl.c
+++ b/tests/util/piglit-framework-gl.c
@@ -326,3 +326,10 @@ piglit_run_selected_subtests(const struct
piglit_gl_subtest *all_subtests,
return result;
}
+
+size_t
+piglit_get_selected_tests(const char ***selected_subtests)
+{
+ *selected_subtests = gl_fw->test_config->selected_subtests;
+ return gl_fw->test_config->num_selected_subtests;
+}
diff --git a/tests/util/piglit-framework-gl.h b/tests/util/piglit-framework-gl.h
index 0eb839e..120f2e8 100644
--- a/tests/util/piglit-framework-gl.h
+++ b/tests/util/piglit-framework-gl.h
@@ -212,6 +212,9 @@ void
piglit_gl_process_args(int *argc, char *argv[],
struct piglit_gl_test_config *config);
+size_t
+piglit_get_selected_tests(const char ***selected_subtests);
+
/**
* Run the OpenGL test described by @a config. Does not return.
*/
Document what this function does, and this patch is
Reviewed-by: Chad Versace
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH v3 2/5] util: Add common framework for command-line invoked subtests
On 10/21/2013 11:29 AM, Paul Berry wrote:
On 15 October 2013 17:32, Ian Romanick wrote:
From: Ian Romanick
@@ -43,6 +43,20 @@ enum piglit_gl_visual {
};
/**
+ * An idividual subtest that makes up part of a test group.
+ */
+struct piglit_gl_subtest {
+ /** Name of the subtest as it will appear in the log. */
+ const char *name;
+
+ /** Command line name used to select this test. */
+ const char *option;
+
+ /** Function that implements the test. */
+ enum piglit_result (*subtest_func)(void);
Would you have any objection to adding a void * to this structure, which
would then get passed to subtest_func? That would give the test
implementor extra flexibility if they want to share a single subtest_func
between multiple similar subtests. It would also allow for the possibility
that the set of available subtests is determined at run time rather than
compile time.
I second Paul's request to add a void* parameter.
However, I don't see how the presence of a void* parameter enables a test
to determine the list of available subtests at runtime. A test can already
do that at runtime by generating piglit_gl_test_config::subtests in main().
Nothing requires that the test declare that structure statically.
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH v2 3/5] util: Add a -list-subtests option that will list all the subtests
On 10/15/2013 05:32 PM, Ian Romanick wrote: From: Ian Romanick The utility is that all.tests can do 'sometest -list-subtests' to automatically get the list of subtests to run. Since the syntax for selecting subtests is regularized, this works quite well. Signed-off-by: Ian Romanick Cc: Chad Versace --- tests/util/piglit-framework-gl.c | 15 +++ tests/util/piglit-framework-gl.h | 8 2 files changed, 23 insertions(+) Patch 3 is Reviewed-by: Chad Versace ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH 1/5] util: Move command-line parsing out of piglit_gl_test_config_init
On 10/15/2013 05:32 PM, Ian Romanick wrote: From: Ian Romanick Create a new function, piglit_gl_process_args, that does the command-line parsing. This happens after the test code between PIGLIT_GL_TEST_CONFIG_BEGIN and PIGLIT_GL_TEST_CONFIG_END. By having an explicit function that does this, tests can call it inside the BEGIN/END block. This may be useful for tests that expect certain arguments to be in specific positions. Signed-off-by: Ian Romanick Cc: Chad Versace --- tests/texturing/shaders/texelFetch.c | 10 ++ tests/texturing/shaders/textureSize.c | 10 ++ tests/util/piglit-framework-gl.c | 24 ++-- tests/util/piglit-framework-gl.h | 10 +++--- 4 files changed, 33 insertions(+), 21 deletions(-) As Paul requested, explain in a comment that it's safe to call piglit_gl_process_args() twice b/c it's idempotent, and patch 1 is Reviewed-by: Chad Versace ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit
[Piglit] [PATCH v2] Interface Blocks: Test how interface block members are accessed from API
v2: Add checks to also test invalid names
---
tests/all.tests| 1 +
tests/spec/glsl-1.50/execution/CMakeLists.gl.txt | 1 +
.../interface-blocks-api-access-members.c | 173 +
3 files changed, 175 insertions(+)
create mode 100644
tests/spec/glsl-1.50/execution/interface-blocks-api-access-members.c
diff --git a/tests/all.tests b/tests/all.tests
index c919f19..d2e685c 100644
--- a/tests/all.tests
+++ b/tests/all.tests
@@ -966,6 +966,7 @@ import_glsl_parser_tests(spec['glsl-1.50'],
add_shader_test_dir(spec['glsl-1.50'],
os.path.join(testsDir, 'spec', 'glsl-1.50'),
recursive=True)
+spec['glsl-1.50']['execution']['interface-blocks-api-access-members'] =
concurrent_test('glsl-1.50-interface-blocks-api-access-members')
spec['glsl-1.50']['execution']['get-active-attrib-array'] =
concurrent_test('glsl-1.50-get-active-attrib-array')
spec['glsl-1.50']['execution']['vs-input-arrays'] =
concurrent_test('glsl-1.50-vs-input-arrays')
spec['glsl-1.50']['execution']['vs-named-block-no-modify'] =
concurrent_test('glsl-1.50-vs-named-block-no-modify')
diff --git a/tests/spec/glsl-1.50/execution/CMakeLists.gl.txt
b/tests/spec/glsl-1.50/execution/CMakeLists.gl.txt
index 693f69a..114867b 100644
--- a/tests/spec/glsl-1.50/execution/CMakeLists.gl.txt
+++ b/tests/spec/glsl-1.50/execution/CMakeLists.gl.txt
@@ -13,3 +13,4 @@ ${OPENGL_glu_LIBRARY}
piglit_add_executable (glsl-1.50-vs-input-arrays vs-input-arrays.c)
piglit_add_executable (glsl-1.50-get-active-attrib-array
get-active-attrib-array.c)
piglit_add_executable (glsl-1.50-vs-named-block-no-modify
vs-named-block-no-modify.c)
+piglit_add_executable (glsl-1.50-interface-blocks-api-access-members
interface-blocks-api-access-members.c)
diff --git
a/tests/spec/glsl-1.50/execution/interface-blocks-api-access-members.c
b/tests/spec/glsl-1.50/execution/interface-blocks-api-access-members.c
new file mode 100644
index 000..77089f8
--- /dev/null
+++ b/tests/spec/glsl-1.50/execution/interface-blocks-api-access-members.c
@@ -0,0 +1,173 @@
+/**
+ * Copyright © 2013 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.
+ */
+
+/**
+ * Test the syntax for accessing interface block members through the API
+ *
+ * From the GLSL 1.50 core spec, section 4.3.7 (Interface Blocks):
+ * "Outside the shading language (i.e., in the API), members are similarly
+ * identified except the block name is always used in place of the instance
+ * name (API accesses are to interfaces, not to shaders). If there is no
+ * instance name, then the API does not use the block name to access a member,
+ * just the member name."
+ *
+ * "For blocks declared as arrays, the array index must also be included when
+ * accessing members"
+ */
+
+#include "piglit-util-gl-common.h"
+
+PIGLIT_GL_TEST_CONFIG_BEGIN
+
+ config.supports_gl_compat_version = 32;
+ config.supports_gl_core_version = 32;
+
+PIGLIT_GL_TEST_CONFIG_END
+
+static const char *vstext =
+ "#version 150\n"
+ "in vec4 vertex;\n"
+ "void main()\n"
+ "{\n"
+ " gl_Position = vertex;\n"
+ "}\n";
+
+static const char *gstext =
+ "#version 150\n"
+ "layout(points) in;\n"
+ "layout(points, max_vertices = 3) out;\n"
+ "out NoInst {\n"
+ " float a;\n"
+ " vec3 b;\n"
+ "};\n"
+ "out WithInst {\n"
+ " float c;\n"
+ " vec3 d;\n"
+ "} inst;\n"
+ "out WithInstArray {\n"
+ " float e;\n"
+ " vec3 f;\n"
+ "} instArray[3];\n"
+ "void main()\n"
+ "{\n"
+ " a = 1.0;\n"
+ " b = vec3(2.0);\n"
+ " inst.c = 3.0;\n"
+ " inst.d = vec3(4.0);\n"
+ " for(int i = 0; i < 3; i++) {\n"
+ " instArray[i].e = 5.0 + 2 * i;\n
[Piglit] [PATCH] summary-html: Add Environment to HTML pages
Specifically for glean tests setting certain environment variables are
needed to run a certain subtest in glean. This patch prints that in the
HTML results.
CC: matts...@gmail.com
Signed-off-by: Dylan Baker
---
framework/summary.py | 3 +++
templates/test_result.mako | 8
2 files changed, 11 insertions(+)
diff --git a/framework/summary.py b/framework/summary.py
index 3e5c24e..14fe6e6 100644
--- a/framework/summary.py
+++ b/framework/summary.py
@@ -452,6 +452,9 @@ class Summary:
out.write(testfile.render(
testname=key,
status=value.get('result', 'None'),
+# Return a NoneType if envornment doesn't exist,
+# which stops it from being printed empty.
+env=value.get('environment', None),
returncode=value.get('returncode', 'None'),
time=value.get('time', 'None'),
info=value.get('info', 'None'),
diff --git a/templates/test_result.mako b/templates/test_result.mako
index b23fb8e..b06fd32 100644
--- a/templates/test_result.mako
+++ b/templates/test_result.mako
@@ -34,6 +34,14 @@
${info}
+ % if env:
+
+Environment
+
+ ${env}
+
+
+ % endif
Command
--
1.8.1.5
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
[Piglit] [PATCH v4] GL3.2 GL_ARB_sync: Test for valid return values from ClientWaitSync
v2: Fix comments, initialize variables. Still need to figure out if GPU
commands
needed before testing these.
v3: Rewrite test cases, fix comment and config block and add to all.tests
v4: Make test work with POSIX_CLOCKS and some other minor changes
---
tests/all.tests | 1 +
tests/spec/arb_sync/CMakeLists.gl.txt| 1 +
tests/spec/arb_sync/ClientWaitSync-returns.c | 183 +++
3 files changed, 185 insertions(+)
create mode 100644 tests/spec/arb_sync/ClientWaitSync-returns.c
diff --git a/tests/all.tests b/tests/all.tests
index c919f19..bdebbcd 100644
--- a/tests/all.tests
+++ b/tests/all.tests
@@ -1108,6 +1108,7 @@
import_glsl_parser_tests(spec['ARB_shader_stencil_export'],
arb_sync = Group()
spec['ARB_sync'] = arb_sync
arb_sync['ClientWaitSync-errors'] =
concurrent_test('arb_sync-client-wait-errors')
+arb_sync['ClientWaitSync-returns'] =
concurrent_test('arb_sync-client-wait-returns')
arb_sync['DeleteSync'] = concurrent_test('arb_sync-delete')
arb_sync['FenceSync-errors'] = concurrent_test('arb_sync-fence-sync-errors')
arb_sync['GetSynciv-errors'] = concurrent_test('arb_sync-get-sync-errors')
diff --git a/tests/spec/arb_sync/CMakeLists.gl.txt
b/tests/spec/arb_sync/CMakeLists.gl.txt
index e5fcb90..901e20f 100644
--- a/tests/spec/arb_sync/CMakeLists.gl.txt
+++ b/tests/spec/arb_sync/CMakeLists.gl.txt
@@ -11,6 +11,7 @@ link_libraries (
)
piglit_add_executable (arb_sync-client-wait-errors ClientWaitSync-errors.c)
+piglit_add_executable (arb_sync-client-wait-returns ClientWaitSync-returns.c)
piglit_add_executable (arb_sync-delete DeleteSync.c)
piglit_add_executable (arb_sync-fence-sync-errors FenceSync-errors.c)
piglit_add_executable (arb_sync-get-sync-errors GetSynciv-errors.c)
diff --git a/tests/spec/arb_sync/ClientWaitSync-returns.c
b/tests/spec/arb_sync/ClientWaitSync-returns.c
new file mode 100644
index 000..c78fb57
--- /dev/null
+++ b/tests/spec/arb_sync/ClientWaitSync-returns.c
@@ -0,0 +1,183 @@
+/**
+ * Copyright © 2013 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
+ * Test ClientWaitSync() returns correct values
+ *
+ *
+ * Section 5.2.1(Waiting for Sync Objects) of OpenGL 3.2 Core says:
+ * "ClientWaitSync returns one of four status values. A return value of
+ * ALREADY_SIGNALED indicates that sync was signaled at the time
+ * ClientWaitSync was called. ALREADY_SIGNALED will always be
+ * returned if sync was signaled, even if the value of timeout is
+ * zero. A return value of TIMEOUT_EXPIRED indicates that the
+ * specified timeout period expired before sync was signaled. A re-
+ * turn value of CONDITION_SATISFIED indicates that sync was signaled
+ * before the timeout expired. Finally, if an error occurs, in
+ * addition to generating a GL error as specified below,
+ * ClientWaitSync immediately returns WAIT_FAILED withoutblocking."
+ *
+ */
+
+#include "piglit-util-gl-common.h"
+
+PIGLIT_GL_TEST_CONFIG_BEGIN
+
+ config.supports_gl_compat_version = 10;
+ config.supports_gl_core_version = 31;
+
+PIGLIT_GL_TEST_CONFIG_END
+
+enum piglit_result
+piglit_display(void)
+{
+ /* UNREACHED */
+ return PIGLIT_FAIL;
+}
+
+/* One second in nanoseconds */
+#define ONE_SECOND 10
+
+void
+piglit_init(int argc, char **argv)
+{
+ bool pass = true;
+ GLsync fence1, fence2, fence3;
+ GLenum status1, status2, status3;
+
+ double seconds_elapsed = -1;
+ int64_t start = 0, end = 0;
+
+
+ if (piglit_get_gl_version() < 32) {
+ piglit_require_extension("GL_ARB_sync");
+ }
+
+ /* Test Case 1: Verify that fence times out correctly after set time */
+
+ /* queue a draw command */
+ piglit_draw_rect(-1, -1, 2, 2);
+
+ /* create fence sync */
+ fence1 = glFenceSync(GL
Re: [Piglit] [PATCH] util/x11: Propagate window resize events to piglit_width/height
Chad Versace writes: > On 10/18/2013 05:19 PM, Eric Anholt wrote: >> Chad Versace writes: >> >>> When I switched Piglit from GLUT to Waffle, I broke detection of X11 >>> window resizes. When the user resized the window, Piglit called >>> piglit_display() but neglected to update piglit_width/height. >>> >>> This patch ensures that Piglit updates piglit_width/height correctly. >>> >>> (It's amazing that it took this long for anyone to fix it. People must >>> rarely resize Piglit windows). >> >> ConfigureNotify tells you the new size. And I'm not clear why you'd >> need to update the size at Expose time. > > I admit that I don't understand X protocol. I knew that ConfigureNotify > tells us the size, and I was unsure if Expose did also. So I played it > safe and updated the size on both events. > > So, the size update on Event needs to get removed. > > Is there a cleaner way to get the window size from the ConfigureNotify > event than calling XGetGeometry? ConfigureNotify tells you the correct new size of the window (event.xconfigure.width/height, visible in the hunk of the patch you proposed), and we just need to update piglit_width/height with it. You don't need to update other times, because at other times you would have had a configurenotify anyway. pgp0rVcdmWAGF.pgp Description: PGP signature ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH v2 3/5] util: Add a -list-subtests option that will list all the subtests
On 15 October 2013 17:32, Ian Romanick wrote:
> From: Ian Romanick
>
> The utility is that all.tests can do 'sometest -list-subtests' to
> automatically get the list of subtests to run. Since the syntax for
> selecting subtests is regularized, this works quite well.
>
> Signed-off-by: Ian Romanick
> Cc: Chad Versace
> ---
> tests/util/piglit-framework-gl.c | 15 +++
> tests/util/piglit-framework-gl.h | 8
> 2 files changed, 23 insertions(+)
>
> diff --git a/tests/util/piglit-framework-gl.c
> b/tests/util/piglit-framework-gl.c
> index 635b52c..c297a6a 100644
> --- a/tests/util/piglit-framework-gl.c
> +++ b/tests/util/piglit-framework-gl.c
> @@ -140,6 +140,21 @@ process_args(int *argc, char *argv[], unsigned
> *force_samples,
> }
> *argc -= 2;
> j -= 2;
> + } else if (!strcmp(argv[j], "-list-subtests")) {
> + unsigned i;
> +
> + if (config->subtests == NULL) {
> + fprintf(stderr, "Test defines no
> subtests!\n");
> + exit(1);
> + }
> +
> + for (i = 0; config->subtests[i].name != NULL; i++)
> {
> + printf("%s: %s\n",
> + config->subtests[i].option,
> + config->subtests[i].name);
> + }
> +
> + exit(0);
>
Can we choose a standard way of marking the end of the subtest list, and
document it? This patch uses subtests[i].name == NULL to mark the end of
the list, and the previous patch uses subtests[i].subtest_func == NULL.
With that fixed, this patch is:
Reviewed-by: Paul Berry
> }
> }
> }
> diff --git a/tests/util/piglit-framework-gl.h
> b/tests/util/piglit-framework-gl.h
> index 0aca75c..0eb839e 100644
> --- a/tests/util/piglit-framework-gl.h
> +++ b/tests/util/piglit-framework-gl.h
> @@ -184,6 +184,14 @@ struct piglit_gl_test_config {
> (*display)(void);
>
> /**
> +* List of subtests supported by this test case
> +*
> +* This is only used during command line argument parsing to
> implement
> +* the -list-subtests option.
> +*/
> + const struct piglit_gl_subtest *subtests;
> +
> + /**
> * Names of subtests supplied on the command line.
> *
> * The paramaters passed to each -subtest command line option is
> --
> 1.8.1.4
>
> ___
> 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
Re: [Piglit] [PATCH v3 2/5] util: Add common framework for command-line invoked subtests
On 15 October 2013 17:32, Ian Romanick wrote:
> From: Ian Romanick
>
> There are two parts to this.
>
> First, command line options of the form "-subtest foo" have their
> argument (the "foo" part) stored in a list in the
> piglit_gl_test_config. Tests can use this functionality without having
> to use any of the other parts. This allows every piglit test to have
> the same syntax for specifying which subtests to run.
>
> Second, tests can create a list of piglit_gl_subtest structures. Each
> structure describes a single subtest: the name (as it will apear in the
> log), the command-line name (as supplied to -subtest), and the function
> that implements the test. Helper function use this table and the list
> of tests specified by -subtest options to run a group of tests.
>
> A later patch shows an example of using this functionality.
>
> v2: Use piglit_merge_result instead of
> piglit_update_result_from_subtest_result. Suggested by Eric, seconded
> by Chad.
>
> v3: Rename piglit_gl_subtest::subtest to
> piglit_gl_subtest::subtest_func. Fix some Doxygen comments. Both
> suggested by Chad.
>
> Signed-off-by: Ian Romanick
> Cc: Chad Versace
> ---
> tests/util/piglit-framework-gl.c | 94
> ++--
> tests/util/piglit-framework-gl.h | 32 ++
> 2 files changed, 123 insertions(+), 3 deletions(-)
>
> diff --git a/tests/util/piglit-framework-gl.c
> b/tests/util/piglit-framework-gl.c
> index dd2e6a5..635b52c 100644
> --- a/tests/util/piglit-framework-gl.c
> +++ b/tests/util/piglit-framework-gl.c
> @@ -40,7 +40,8 @@ int piglit_width;
> int piglit_height;
>
> static void
> -process_args(int *argc, char *argv[], unsigned *force_samples);
> +process_args(int *argc, char *argv[], unsigned *force_samples,
> +struct piglit_gl_test_config *config);
>
> void
> piglit_gl_test_config_init(struct piglit_gl_test_config *config)
> @@ -63,7 +64,8 @@ delete_arg(char *argv[], int argc, int arg)
> * length is returned in @a argc.
> */
> static void
> -process_args(int *argc, char *argv[], unsigned *force_samples)
> +process_args(int *argc, char *argv[], unsigned *force_samples,
> +struct piglit_gl_test_config *config)
> {
> int j;
>
> @@ -111,6 +113,33 @@ process_args(int *argc, char *argv[], unsigned
> *force_samples)
> *force_samples = atoi(argv[j]+9);
> delete_arg(argv, *argc, j--);
> *argc -= 1;
> + } else if (!strcmp(argv[j], "-subtest")) {
> + int i;
> +
> + j++;
> + if (j >= *argc) {
> + fprintf(stderr,
> + "-subtest requires an argument\n");
> + piglit_report_result(PIGLIT_FAIL);
> + }
> +
> + config->selected_subtests =
> + realloc(config->selected_subtests,
> + sizeof(char *)
> + * (config->num_selected_subtests +
> 1));
> +
> config->selected_subtests[config->num_selected_subtests] =
> + argv[j];
> +
> + config->num_selected_subtests++;
> +
> + /* Remove 2 arguments (hence the 'i - 2') from the
> +* command line.
> +*/
> + for (i = j + 1; i < *argc; i++) {
> + argv[i - 2] = argv[i];
> + }
> + *argc -= 2;
> + j -= 2;
> }
> }
> }
> @@ -121,7 +150,7 @@ piglit_gl_process_args(int *argc, char *argv[],
> {
> unsigned force_samples = 0;
>
> - process_args(argc, argv, &force_samples);
> + process_args(argc, argv, &force_samples, config);
>
> if (force_samples > 1)
> config->window_samples = force_samples;
> @@ -223,3 +252,62 @@ piglit_destroy_dma_buf(struct piglit_dma_buf *buf)
> if (gl_fw->destroy_dma_buf)
> gl_fw->destroy_dma_buf(buf);
> }
> +
> +const struct piglit_gl_subtest *
> +piglit_find_subtest(const struct piglit_gl_subtest *subtests, const char
> *name)
> +{
> + unsigned i;
> +
> + for (i = 0; subtests[i].subtest_func != NULL; i++) {
> + if (strcmp(subtests[i].option, name) == 0)
> + return &subtests[i];
> + }
> +
> + return NULL;
> +}
> +
> +enum piglit_result
> +piglit_run_selected_subtests(const struct piglit_gl_subtest *all_subtests,
> +const char **selected_subtests,
> +size_t num_selected_subtests,
> +enum piglit_result previous_result)
> +{
> + enum piglit_result result = previous_result;
> +
> + if (num_selected_subtests) {
> +
Re: [Piglit] [PATCH 1/5] util: Move command-line parsing out of piglit_gl_test_config_init
On 15 October 2013 17:32, Ian Romanick wrote:
> From: Ian Romanick
>
> Create a new function, piglit_gl_process_args, that does the
> command-line parsing. This happens after the test code between
> PIGLIT_GL_TEST_CONFIG_BEGIN and PIGLIT_GL_TEST_CONFIG_END. By having an
> explicit function that does this, tests can call it inside the BEGIN/END
> block. This may be useful for tests that expect certain arguments to be
> in specific positions.
>
> Signed-off-by: Ian Romanick
> Cc: Chad Versace
>
I spent a while worrying about the fact that this change causes some tests
to call piglit_gl_process_args() twice (once explicitly, and once
implicitly from PIGLIT_GL_TEST_CONFIG_END). Would you mind adding a note
to the commit message explaining that piglit_gl_process_args() is
idempotent (since it removes the args it processes), so that it's safe to
call more than once from a single test?
With that change, this patch is:
Reviewed-by: Paul Berry
> ---
> tests/texturing/shaders/texelFetch.c | 10 ++
> tests/texturing/shaders/textureSize.c | 10 ++
> tests/util/piglit-framework-gl.c | 24 ++--
> tests/util/piglit-framework-gl.h | 10 +++---
> 4 files changed, 33 insertions(+), 21 deletions(-)
>
> diff --git a/tests/texturing/shaders/texelFetch.c
> b/tests/texturing/shaders/texelFetch.c
> index ebd48cd..bc6cc8f 100644
> --- a/tests/texturing/shaders/texelFetch.c
> +++ b/tests/texturing/shaders/texelFetch.c
> @@ -84,6 +84,12 @@ static enum shader_target test_stage = UNKNOWN;
>
> PIGLIT_GL_TEST_CONFIG_BEGIN
>
> + config.window_width = 900;
> + config.window_height = 600;
> + config.window_visual = PIGLIT_GL_VISUAL_RGBA |
> PIGLIT_GL_VISUAL_DOUBLE;
> +
> + piglit_gl_process_args(&argc, argv, &config);
> +
> parse_args(argc, argv);
> if (test_stage == GS) {
> config.supports_gl_compat_version = 32;
> @@ -93,10 +99,6 @@ PIGLIT_GL_TEST_CONFIG_BEGIN
> config.supports_gl_core_version = 31;
> }
>
> - config.window_width = 900;
> - config.window_height = 600;
> - config.window_visual = PIGLIT_GL_VISUAL_RGBA |
> PIGLIT_GL_VISUAL_DOUBLE;
> -
> PIGLIT_GL_TEST_CONFIG_END
>
> #define MAX_LOD_OR_SAMPLES 10.0
> diff --git a/tests/texturing/shaders/textureSize.c
> b/tests/texturing/shaders/textureSize.c
> index f010d9c..ee64b07 100644
> --- a/tests/texturing/shaders/textureSize.c
> +++ b/tests/texturing/shaders/textureSize.c
> @@ -52,6 +52,12 @@ static enum shader_target test_stage = UNKNOWN;
>
> PIGLIT_GL_TEST_CONFIG_BEGIN
>
> + config.window_width = 150;
> + config.window_height = 30;
> + config.window_visual = PIGLIT_GL_VISUAL_RGB |
> PIGLIT_GL_VISUAL_DOUBLE;
> +
> + piglit_gl_process_args(&argc, argv, &config);
> +
> parse_args(argc, argv);
> if (test_stage == GS) {
> config.supports_gl_compat_version = 32;
> @@ -61,10 +67,6 @@ PIGLIT_GL_TEST_CONFIG_BEGIN
> config.supports_gl_core_version = 31;
> }
>
> - config.window_width = 150;
> - config.window_height = 30;
> - config.window_visual = PIGLIT_GL_VISUAL_RGB |
> PIGLIT_GL_VISUAL_DOUBLE;
> -
> PIGLIT_GL_TEST_CONFIG_END
>
> static int lod_location;
> diff --git a/tests/util/piglit-framework-gl.c
> b/tests/util/piglit-framework-gl.c
> index 2a315be..dd2e6a5 100644
> --- a/tests/util/piglit-framework-gl.c
> +++ b/tests/util/piglit-framework-gl.c
> @@ -43,18 +43,9 @@ static void
> process_args(int *argc, char *argv[], unsigned *force_samples);
>
> void
> -piglit_gl_test_config_init(int *argc, char *argv[],
> - struct piglit_gl_test_config *config)
> +piglit_gl_test_config_init(struct piglit_gl_test_config *config)
> {
> - unsigned force_samples = 0;
> -
> memset(config, 0, sizeof(*config));
> -
> - process_args(argc, argv, &force_samples);
> -
> - if (force_samples > 1)
> - config->window_samples = force_samples;
> -
> }
>
> static void
> @@ -125,6 +116,19 @@ process_args(int *argc, char *argv[], unsigned
> *force_samples)
> }
>
> void
> +piglit_gl_process_args(int *argc, char *argv[],
> + struct piglit_gl_test_config *config)
> +{
> + unsigned force_samples = 0;
> +
> + process_args(argc, argv, &force_samples);
> +
> + if (force_samples > 1)
> + config->window_samples = force_samples;
> +
> +}
> +
> +void
> piglit_gl_test_run(int argc, char *argv[],
>const struct piglit_gl_test_config *config)
> {
> diff --git a/tests/util/piglit-framework-gl.h
> b/tests/util/piglit-framework-gl.h
> index 7520f38..fcc1594 100644
> --- a/tests/util/piglit-framework-gl.h
> +++ b/tests/util/piglit-framework-gl.h
> @@ -175,8 +175,11 @@ struct piglit_gl_test_config {
> * from command line arguments.
> */
> void
> -piglit_gl_test_config_init(int *argc, char *argv[],
> - s
[Piglit] [PATCH 1/4 v3] GS: Test that geometry shader input/output layout qualifiers only compile if valid
v2: Tests check against list of valid layouts instead of invalid layouts
v3: Remove vertex shader info, remove linking, check compile status, rewrite
error messages
---
tests/all.tests| 12 ++
.../glsl-1.50/execution/geometry/CMakeLists.gl.txt | 2 +
.../geometry/gs-input-layout-qualifiers.c | 133 +
.../geometry/gs-output-layout-qualifiers.c | 130
4 files changed, 277 insertions(+)
create mode 100644
tests/spec/glsl-1.50/execution/geometry/gs-input-layout-qualifiers.c
create mode 100644
tests/spec/glsl-1.50/execution/geometry/gs-output-layout-qualifiers.c
diff --git a/tests/all.tests b/tests/all.tests
index c919f19..2485b39 100644
--- a/tests/all.tests
+++ b/tests/all.tests
@@ -1017,6 +1017,18 @@ for prim_type in ['GL_TRIANGLE_STRIP',
'GL_TRIANGLE_STRIP_ADJACENCY']:
'glsl-1.50-geometry-tri-strip-ordering-with-prim-restart {0} {1}'.format(
prim_type, restart_index))
+for input_layout in ['points', 'lines', 'lines_adjacency', 'triangles',
+ 'triangles_adjacency', 'line_strip', 'triangle_strip']:
+add_concurrent_test(spec['glsl-1.50'],
+'glsl-1.50-gs-input-layout-qualifiers {0}'.format(
+input_layout))
+
+for output_layout in ['points', 'lines', 'lines_adjacency', 'triangles',
+ 'triangles_adjacency', 'line_strip', 'triangle_strip']:
+add_concurrent_test(spec['glsl-1.50'],
+'glsl-1.50-gs-output-layout-qualifiers {0}'.format(
+output_layout))
+
spec['glsl-3.30'] = Group()
import_glsl_parser_tests(spec['glsl-3.30'],
os.path.join(testsDir, 'spec', 'glsl-3.30'),
diff --git a/tests/spec/glsl-1.50/execution/geometry/CMakeLists.gl.txt
b/tests/spec/glsl-1.50/execution/geometry/CMakeLists.gl.txt
index 3e6bc4b..d759c6b 100644
--- a/tests/spec/glsl-1.50/execution/geometry/CMakeLists.gl.txt
+++ b/tests/spec/glsl-1.50/execution/geometry/CMakeLists.gl.txt
@@ -19,3 +19,5 @@ piglit_add_executable (glsl-1.50-gs-emits-too-few-verts
gs-emits-too-few-verts.c
piglit_add_executable (glsl-1.50-getshaderiv-may-return-GS
getshaderiv-may-return-GS.c)
piglit_add_executable (glsl-1.50-gs-mismatch-prim-type gs-mismatch-prim-type.c)
piglit_add_executable (glsl-1.50-query-gs-prim-types query-gs-prim-types.c)
+piglit_add_executable (glsl-1.50-gs-input-layout-qualifiers
gs-input-layout-qualifiers.c)
+piglit_add_executable (glsl-1.50-gs-output-layout-qualifiers
gs-output-layout-qualifiers.c)
diff --git
a/tests/spec/glsl-1.50/execution/geometry/gs-input-layout-qualifiers.c
b/tests/spec/glsl-1.50/execution/geometry/gs-input-layout-qualifiers.c
new file mode 100644
index 000..0afc4e8
--- /dev/null
+++ b/tests/spec/glsl-1.50/execution/geometry/gs-input-layout-qualifiers.c
@@ -0,0 +1,133 @@
+/**
+ * Copyright © 2013 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.
+ */
+
+/**
+ * Test that geometry shaders only compile with valid input layout qualifiers
+ *
+ * Section 4.3.8.1(Input Layout Qualifiers) of the GLSL 1.50 spec says:
+ * "Geometry shaders allow input layout qualifiers only on the interface
+ * qualifier in, not on an input block, block member, or variable. The layout
+ * qualifier identifiers for geometry shader inputs are
+ * points
+ * lines
+ * lines_adjacency
+ * triangles
+ * triangles_adjacency"
+ */
+
+#include "piglit-util-gl-common.h"
+
+PIGLIT_GL_TEST_CONFIG_BEGIN
+
+ config.supports_gl_compat_version = 32;
+config.supports_gl_core_version = 32;
+
+ config.window_visual = PIGLIT_GL_VISUAL_RGB | PIGLIT_GL_VISUAL_DOUBLE;
+
+PIGLIT_GL_TEST_CONFIG_END
+
+static const char *gstemplate =
+ "#version 150\n"
+ "#define LAYOUT_IN %s\n"
+
