[Piglit] [PATCH] varying-packing: add arrays of arrays support
V3: rebase and test double types
V2: renamed variables from 'inner' that should have been 'outer'
---
tests/all.py | 4 +-
.../glsl-1.10/execution/varying-packing/simple.c | 137 +++--
2 files changed, 100 insertions(+), 41 deletions(-)
diff --git a/tests/all.py b/tests/all.py
index 18124b7..85b622f 100755
--- a/tests/all.py
+++ b/tests/all.py
@@ -1290,7 +1290,7 @@ with profile.group_manager(
'ivec3', 'ivec4', 'uvec2', 'uvec3', 'uvec4', 'mat2', 'mat3',
'mat4', 'mat2x3', 'mat2x4', 'mat3x2', 'mat3x4', 'mat4x2',
'mat4x3']:
-for arrayspec in ['array', 'separate']:
+for arrayspec in ['array', 'separate', 'arrays_of_arrays']:
g(['varying-packing-simple', type_, arrayspec],
'simple {} {}'.format(type_, arrayspec))
@@ -2062,7 +2062,7 @@ with profile.group_manager(
for type in ['double', 'dvec2', 'dvec3', 'dvec4', 'dmat2', 'dmat3',
'dmat4', 'dmat2x3', 'dmat2x4', 'dmat3x2', 'dmat3x4',
'dmat4x2', 'dmat4x3']:
-for arrayspec in ['array', 'separate']:
+for arrayspec in ['array', 'separate', 'arrays_of_arrays']:
g(['varying-packing-simple', type, arrayspec],
'simple {0} {1}'.format(type, arrayspec))
diff --git a/tests/spec/glsl-1.10/execution/varying-packing/simple.c
b/tests/spec/glsl-1.10/execution/varying-packing/simple.c
index e9935c7..2b1c010 100644
--- a/tests/spec/glsl-1.10/execution/varying-packing/simple.c
+++ b/tests/spec/glsl-1.10/execution/varying-packing/simple.c
@@ -100,6 +100,8 @@
static void
parse_args(int argc, char *argv[], struct piglit_gl_test_config *config);
+static const int outer_dim_size = 2;
+
PIGLIT_GL_TEST_CONFIG_BEGIN
config.window_visual = PIGLIT_GL_VISUAL_RGB | PIGLIT_GL_VISUAL_DOUBLE;
@@ -118,6 +120,13 @@ enum base_type
BASE_TYPE_DOUBLE,
};
+enum test_array_type
+{
+ SEPARATE,
+ ARRAY,
+ ARRAYS_OF_ARRAYS,
+};
+
static const char *
get_base_type_name(enum base_type t)
{
@@ -219,7 +228,8 @@ const struct type_desc *all_types[] = {
struct varying_desc
{
const struct type_desc *type;
- unsigned array_elems;
+ unsigned one_dim_array_elems;
+ unsigned two_dim_array_elems;
};
/**
@@ -228,12 +238,12 @@ struct varying_desc
*/
static GLint
get_shader(bool is_vs, unsigned glsl_version, int num_varyings,
- struct varying_desc *varyings)
+ struct varying_desc *varyings, enum test_array_type array_type)
{
GLuint shader;
char *full_text = malloc(1000 * 100 + num_varyings);
char *text = full_text;
- unsigned i, j, k, l;
+ unsigned i, j, k, l, m;
const char *varying_keyword;
unsigned offset = 0;
GLenum shader_type = is_vs ? GL_VERTEX_SHADER : GL_FRAGMENT_SHADER;
@@ -249,6 +259,8 @@ get_shader(bool is_vs, unsigned glsl_version, int
num_varyings,
}
text += sprintf(text, "#version %u\n", glsl_version);
+ if (array_type == ARRAYS_OF_ARRAYS)
+ text += sprintf(text, "#extension GL_ARB_arrays_of_arrays:
enable\n");
for (i = 0; i < num_varyings; ++i) {
const char *opt_flat_keyword = "";
if (!fp64 && varyings[i].type->base == BASE_TYPE_DOUBLE) {
@@ -257,11 +269,17 @@ get_shader(bool is_vs, unsigned glsl_version, int
num_varyings,
}
if (varyings[i].type->base != BASE_TYPE_FLOAT)
opt_flat_keyword = "flat ";
- if (varyings[i].array_elems != 0) {
+ if (varyings[i].two_dim_array_elems != 0) {
+ text += sprintf(text, "%s%s %s var%03u[%u][%u];\n",
+ opt_flat_keyword, varying_keyword,
+ varyings[i].type->name, i,
+ outer_dim_size,
+ varyings[i].two_dim_array_elems);
+ } else if (varyings[i].one_dim_array_elems != 0) {
text += sprintf(text, "%s%s %s var%03u[%u];\n",
opt_flat_keyword, varying_keyword,
varyings[i].type->name, i,
- varyings[i].array_elems);
+ varyings[i].one_dim_array_elems);
} else {
text += sprintf(text, "%s%s %s var%03u;\n",
opt_flat_keyword, varying_keyword,
@@ -282,31 +300,42 @@ get_shader(bool is_vs, unsigned glsl_version, int
num_varyings,
else
text += sprintf(text, " bool failed = false;\n");
for (i = 0; i < num_varyings; ++i) {
- unsigned array_loop_bound = varyings[i].array_elems;
+ unsigned array_loop_bound;
+ unsigned
Re: [Piglit] [PATCH 1/2] arb_arrays_of_arrays: change input compile tests to fragment shaders
On Sat, Apr 25, 2015 at 4:39 AM, Timothy Arceri wrote: > On Sat, 2015-04-25 at 02:02 -0700, Matt Turner wrote: >> On Tue, Mar 31, 2015 at 10:01 PM, Timothy Arceri >> wrote: >> > Although the existing tests are valid until link time, >> > it doesn't make much sense to test this way. >> > --- >> > .../compiler/input-array-array-var.frag | 15 >> > +++ >> > .../compiler/input-array-array-var.vert | 15 >> > --- >> > .../compiler/input-array-var-array.frag | 15 >> > +++ >> > .../compiler/input-array-var-array.vert | 15 >> > --- >> > .../compiler/input-var-array-array.frag | 15 >> > +++ >> > .../compiler/input-var-array-array.vert | 15 >> > --- >> > 6 files changed, 45 insertions(+), 45 deletions(-) >> > create mode 100644 >> > tests/spec/arb_arrays_of_arrays/compiler/input-array-array-var.frag >> > delete mode 100644 >> > tests/spec/arb_arrays_of_arrays/compiler/input-array-array-var.vert >> > create mode 100644 >> > tests/spec/arb_arrays_of_arrays/compiler/input-array-var-array.frag >> > delete mode 100644 >> > tests/spec/arb_arrays_of_arrays/compiler/input-array-var-array.vert >> > create mode 100644 >> > tests/spec/arb_arrays_of_arrays/compiler/input-var-array-array.frag >> > delete mode 100644 >> > tests/spec/arb_arrays_of_arrays/compiler/input-var-array-array.vert >> >> I'm not sure I understand the rationale. Could you explain? > > Sure. These are compile tests to see if the implementation allows a > shader input to be an array of arrays, however a vertex can't have input > varyings (a mistake from copy and pasting when I created the tests). > > According to the spec having an input in a vertex shader is only a link > time error so technically these tests are still valid as vertex shaders > because we are just checking compile time validation but it makes sense > to just fix this and change them to fragment shaders instead. Ahh, that makes sense. > I can add something like above to the commit message before committing > if you like. Sure, that'd be great. Thanks Timothy. Reviewed-by: Matt Turner ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH 1/2] arb_arrays_of_arrays: change input compile tests to fragment shaders
On Sat, 2015-04-25 at 02:02 -0700, Matt Turner wrote: > On Tue, Mar 31, 2015 at 10:01 PM, Timothy Arceri > wrote: > > Although the existing tests are valid until link time, > > it doesn't make much sense to test this way. > > --- > > .../compiler/input-array-array-var.frag | 15 > > +++ > > .../compiler/input-array-array-var.vert | 15 > > --- > > .../compiler/input-array-var-array.frag | 15 > > +++ > > .../compiler/input-array-var-array.vert | 15 > > --- > > .../compiler/input-var-array-array.frag | 15 > > +++ > > .../compiler/input-var-array-array.vert | 15 > > --- > > 6 files changed, 45 insertions(+), 45 deletions(-) > > create mode 100644 > > tests/spec/arb_arrays_of_arrays/compiler/input-array-array-var.frag > > delete mode 100644 > > tests/spec/arb_arrays_of_arrays/compiler/input-array-array-var.vert > > create mode 100644 > > tests/spec/arb_arrays_of_arrays/compiler/input-array-var-array.frag > > delete mode 100644 > > tests/spec/arb_arrays_of_arrays/compiler/input-array-var-array.vert > > create mode 100644 > > tests/spec/arb_arrays_of_arrays/compiler/input-var-array-array.frag > > delete mode 100644 > > tests/spec/arb_arrays_of_arrays/compiler/input-var-array-array.vert > > I'm not sure I understand the rationale. Could you explain? Sure. These are compile tests to see if the implementation allows a shader input to be an array of arrays, however a vertex can't have input varyings (a mistake from copy and pasting when I created the tests). According to the spec having an input in a vertex shader is only a link time error so technically these tests are still valid as vertex shaders because we are just checking compile time validation but it makes sense to just fix this and change them to fragment shaders instead. I can add something like above to the commit message before committing if you like. Thanks for taking a look at the tests. ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH 14/14] khr_debug/debug-push-pop-group: fix test
On Sat, 2015-04-25 at 12:44 +0200, Marek Olšák wrote:
> If the spec and the example in the spec don't agree, it's a spec bug, isn't
> it?
>
> Marek
I should have been a bit clearer in my initial reply.
They do not disagree, it just doesn't explicitly say the message is
added to the log, but neither does DebugMessageInsert().
It seems implied that because you are passing a message to these
functions that it will end up in the log.
I'm assuming the reason PopDebugGroup() explicitly states that a message
is added to the log is because this function does not accept a message
as a parameter, it reuses the one from PushDebugGroup().
>
> On Sat, Apr 25, 2015 at 6:59 AM, Timothy Arceri wrote:
> > On Mon, 2015-04-13 at 20:28 +0200, Marek Olšák wrote:
> >> From: Daniel Kurtz
> >>
> >> AFAICT from [0], only PopDebugGroup() adds a message to the log, not
> >> PushDebugGroup().
> >>
> >> [0] https://www.opengl.org/registry/specs/KHR/debug.txt
> >>
> >> Thus, there should only be three messages in test_push_pop_debug_group:
> >> (1) DebugMessageInsert() -> TestMessage1
> >> (2) PopDebugGroup() -> TestMessage2
> >> (3) DebugMessageInsert() -> TestMessage4
> >
> > Although Daniel is correct that the text describing PushDebugGroup does
> > not explicitly say that PushDebugGroup adds a message to the log. I
> > recall basing the piglit test on the example given in the spec which
> > shows the PushDebugGroup does add a message to the log.
> >
> > So this change is incorrect.
> >
> > Example from the spec below:
> >
> > Scenario 1: skip a section of the code
> > // Setup of the default active debug group: Filter everything in
> > glDebugMessageControl(GL_DONT_CARE, GL_DONT_CARE, GL_DONT_CARE, 0,
> > NULL, GL_TRUE);
> >
> > // Generate a debug marker debug output message
> > glDebugMessageInsert(
> > GL_DEBUG_SOURCE_APPLICATION,
> > GL_DEBUG_TYPE_MARKER, 100,
> > GL_DEBUG_SEVERITY_NOTIFICATION,
> > -1, "Message 1");
> >
> > // Push debug group 1
> > glPushDebugGroup(
> > GL_DEBUG_SOURCE_APPLICATION,
> > 1,
> > -1, "Message 2");
> >
> > // Setup of the debug group 1: Filter everything out
> > glDebugMessageControl(GL_DONT_CARE, GL_DONT_CARE, GL_DONT_CARE, 0,
> > NULL, GL_FALSE);
> >
> > // This message won't appear in the debug output log of
> > glDebugMessageInsert(
> > GL_DEBUG_SOURCE_APPLICATION,
> > GL_DEBUG_TYPE_MARKER, 100,
> > GL_DEBUG_SEVERITY_NOTIFICATION,
> > -1, "Message 3");
> >
> > // Pop debug group 1, restore the volume control of the default
> > debug group.
> > glPopDebugGroup();
> >
> > // Generate a debug marker debug output message
> > glDebugMessageInsert(
> > GL_DEBUG_SOURCE_APPLICATION,
> > GL_DEBUG_TYPE_MARKER, 100,
> > GL_DEBUG_SEVERITY_NOTIFICATION,
> > -1, "Message 5");
> >
> > // Expected debug output from the GL implementation
> > // Message 1
> > // Message 2
> > // Message 2
> > // Message 5
> >
> >
> >
> >
> >>
> >> Signed-off-by: Daniel Kurtz
> >> ---
> >> tests/spec/khr_debug/debug-push-pop-group.c | 14 +++---
> >> 1 file changed, 7 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/tests/spec/khr_debug/debug-push-pop-group.c
> >> b/tests/spec/khr_debug/debug-push-pop-group.c
> >> index 8fa4474..105bdc4 100644
> >> --- a/tests/spec/khr_debug/debug-push-pop-group.c
> >> +++ b/tests/spec/khr_debug/debug-push-pop-group.c
> >> @@ -268,8 +268,8 @@ static bool test_push_pop_debug_group()
> >>lengths,
> >>messageLog);
> >>
> >> - if (count != 4) {
> >> - fprintf(stderr, "The message log should contain 4 messages
> >> not %i messages\n", count);
> >> + if (count != 3) {
> >> + fprintf(stderr, "The message log should contain 3 messages
> >> not %i messages\n", count);
> >> nextMessage = 0;
> >> for (i = 0; i < count; i++) {
> >> fprintf(stderr, "%s\n", messageLog+nextMessage);
> >> @@ -279,14 +279,14 @@ static bool test_push_pop_debug_group()
> >> }
> >>
> >> if (pass) {
> >> - /* the third message should contain TestMessage2 from
> >> PopDebugGroup() */
> >> - nextMessage = lengths[0] + lengths[1];
> >> + /* the second message should contain TestMessage2 from
> >> PopDebugGroup() */
> >> + nextMessage = lengths[0];
> >> if (strstr(messageLog+nextMessage, TestMessage2) == NULL) {
> >> fprintf(stderr, "Expected: %s Message: %s\n",
> >> TestMessage2, messageLog+nextMessage);
> >> pass = false;
> >> }
> >>
> >> - /* double check that TestMessage3 didnt sneak into the log */
> >> + /* double check that TestMessage3 didn't sneak into the log
> >> */
> >>
Re: [Piglit] [PATCH 14/14] khr_debug/debug-push-pop-group: fix test
If the spec and the example in the spec don't agree, it's a spec bug, isn't it?
Marek
On Sat, Apr 25, 2015 at 6:59 AM, Timothy Arceri wrote:
> On Mon, 2015-04-13 at 20:28 +0200, Marek Olšák wrote:
>> From: Daniel Kurtz
>>
>> AFAICT from [0], only PopDebugGroup() adds a message to the log, not
>> PushDebugGroup().
>>
>> [0] https://www.opengl.org/registry/specs/KHR/debug.txt
>>
>> Thus, there should only be three messages in test_push_pop_debug_group:
>> (1) DebugMessageInsert() -> TestMessage1
>> (2) PopDebugGroup() -> TestMessage2
>> (3) DebugMessageInsert() -> TestMessage4
>
> Although Daniel is correct that the text describing PushDebugGroup does
> not explicitly say that PushDebugGroup adds a message to the log. I
> recall basing the piglit test on the example given in the spec which
> shows the PushDebugGroup does add a message to the log.
>
> So this change is incorrect.
>
> Example from the spec below:
>
> Scenario 1: skip a section of the code
> // Setup of the default active debug group: Filter everything in
> glDebugMessageControl(GL_DONT_CARE, GL_DONT_CARE, GL_DONT_CARE, 0,
> NULL, GL_TRUE);
>
> // Generate a debug marker debug output message
> glDebugMessageInsert(
> GL_DEBUG_SOURCE_APPLICATION,
> GL_DEBUG_TYPE_MARKER, 100,
> GL_DEBUG_SEVERITY_NOTIFICATION,
> -1, "Message 1");
>
> // Push debug group 1
> glPushDebugGroup(
> GL_DEBUG_SOURCE_APPLICATION,
> 1,
> -1, "Message 2");
>
> // Setup of the debug group 1: Filter everything out
> glDebugMessageControl(GL_DONT_CARE, GL_DONT_CARE, GL_DONT_CARE, 0,
> NULL, GL_FALSE);
>
> // This message won't appear in the debug output log of
> glDebugMessageInsert(
> GL_DEBUG_SOURCE_APPLICATION,
> GL_DEBUG_TYPE_MARKER, 100,
> GL_DEBUG_SEVERITY_NOTIFICATION,
> -1, "Message 3");
>
> // Pop debug group 1, restore the volume control of the default
> debug group.
> glPopDebugGroup();
>
> // Generate a debug marker debug output message
> glDebugMessageInsert(
> GL_DEBUG_SOURCE_APPLICATION,
> GL_DEBUG_TYPE_MARKER, 100,
> GL_DEBUG_SEVERITY_NOTIFICATION,
> -1, "Message 5");
>
> // Expected debug output from the GL implementation
> // Message 1
> // Message 2
> // Message 2
> // Message 5
>
>
>
>
>>
>> Signed-off-by: Daniel Kurtz
>> ---
>> tests/spec/khr_debug/debug-push-pop-group.c | 14 +++---
>> 1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/tests/spec/khr_debug/debug-push-pop-group.c
>> b/tests/spec/khr_debug/debug-push-pop-group.c
>> index 8fa4474..105bdc4 100644
>> --- a/tests/spec/khr_debug/debug-push-pop-group.c
>> +++ b/tests/spec/khr_debug/debug-push-pop-group.c
>> @@ -268,8 +268,8 @@ static bool test_push_pop_debug_group()
>>lengths,
>>messageLog);
>>
>> - if (count != 4) {
>> - fprintf(stderr, "The message log should contain 4 messages not
>> %i messages\n", count);
>> + if (count != 3) {
>> + fprintf(stderr, "The message log should contain 3 messages not
>> %i messages\n", count);
>> nextMessage = 0;
>> for (i = 0; i < count; i++) {
>> fprintf(stderr, "%s\n", messageLog+nextMessage);
>> @@ -279,14 +279,14 @@ static bool test_push_pop_debug_group()
>> }
>>
>> if (pass) {
>> - /* the third message should contain TestMessage2 from
>> PopDebugGroup() */
>> - nextMessage = lengths[0] + lengths[1];
>> + /* the second message should contain TestMessage2 from
>> PopDebugGroup() */
>> + nextMessage = lengths[0];
>> if (strstr(messageLog+nextMessage, TestMessage2) == NULL) {
>> fprintf(stderr, "Expected: %s Message: %s\n",
>> TestMessage2, messageLog+nextMessage);
>> pass = false;
>> }
>>
>> - /* double check that TestMessage3 didnt sneak into the log */
>> + /* double check that TestMessage3 didn't sneak into the log */
>> nextMessage = 0;
>> for (i = 0; i < count; i++) {
>> if (strstr(messageLog+nextMessage, TestMessage3) !=
>> NULL) {
>> @@ -297,8 +297,8 @@ static bool test_push_pop_debug_group()
>> nextMessage += lengths[i];
>> }
>>
>> - /* the forth message should contain TestMessage4 */
>> - nextMessage = lengths[0] + lengths[1] + lengths[2];
>> + /* the third message should contain TestMessage4 */
>> + nextMessage = lengths[0] + lengths[1];
>> if (strstr(messageLog+nextMessage, TestMessage4) == NULL) {
>> fprintf(stderr, "Expected: %s Message: %s\n",
>> TestMessage4, messageLog+nextMessage);
>> pass = false;
>
>
_
Re: [Piglit] [PATCH 1/2] arb_arrays_of_arrays: change input compile tests to fragment shaders
On Tue, Mar 31, 2015 at 10:01 PM, Timothy Arceri wrote: > Although the existing tests are valid until link time, > it doesn't make much sense to test this way. > --- > .../compiler/input-array-array-var.frag | 15 > +++ > .../compiler/input-array-array-var.vert | 15 > --- > .../compiler/input-array-var-array.frag | 15 > +++ > .../compiler/input-array-var-array.vert | 15 > --- > .../compiler/input-var-array-array.frag | 15 > +++ > .../compiler/input-var-array-array.vert | 15 > --- > 6 files changed, 45 insertions(+), 45 deletions(-) > create mode 100644 > tests/spec/arb_arrays_of_arrays/compiler/input-array-array-var.frag > delete mode 100644 > tests/spec/arb_arrays_of_arrays/compiler/input-array-array-var.vert > create mode 100644 > tests/spec/arb_arrays_of_arrays/compiler/input-array-var-array.frag > delete mode 100644 > tests/spec/arb_arrays_of_arrays/compiler/input-array-var-array.vert > create mode 100644 > tests/spec/arb_arrays_of_arrays/compiler/input-var-array-array.frag > delete mode 100644 > tests/spec/arb_arrays_of_arrays/compiler/input-var-array-array.vert I'm not sure I understand the rationale. Could you explain? ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH V3 2/2] arb_arrays_of_arrays: compile test all basic types
On Tue, Mar 31, 2015 at 10:44 PM, Timothy Arceri wrote: > V3: Tell piglit that GL_ARB_arrays_of_arrays is required > > V2: fix glsl-1.20-basic-types.vert previously it contained the > glsl-4.20-basic-types.frag test > > Test results: > > Nvidia GeForce 840M - NVIDIA 346.47: pass > --- I haven't confirmed that each list is precisely what those GLSL versions offer, but I like the tests. Have a Reviewed-by: Matt Turner ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit
