On Mon, 5 Dec 2011 17:06:00 +0100, Vincent Lejeune <v...@ovi.com> wrote:
Nice start! I love that this just takes a chunk of the spec and tests
it. Just a few little comments.
One is that the all.tests line to get your new testcase executed is
missing. Since this test doesn't do any drawing, it should be a
concurrent test. Unfortunately our helper functions are pretty dumb so
far, so you'll want something like:
arb_uniform_buffer_object['standard_layout'] =
concurrent_test('arb_uniform_buffer_object_standard_layout')
because if you just use add_concurrent_test(), it will end up in the
summary file as:
ARB_uniform_buffer_object/arb_uniform_buffer_object_standard_layout
which is quite ugly, though we have many examples of that sort of ugly
around. The way we write entries in all.tests today is awful, and I
want something better. I just haven't figured out what that is exactly.
> +#define Elements(x) (sizeof(x)/sizeof(*(x)))
There's the usual linux kernel-named ARRAY_SIZE macro already
available.
> +static const struct result {
> + const char* name;
> + GLint offset;
> + GLint size;
tab indenting.
> +} expected_result[] =
> + {
> + { "a", 0, 1 },
> + { "b", 8, 1 },
> + { "c", 16, 1 },
> + { "f.d", 32, 1},
> + { "f.e", 40, 1},
> + { "g", 48, 1},
> + { "h", 64, 2},
> + { "i", 96, 1},
> + { "o[0].j", 128,1},
> + { "o[0].k", 144, 1},
> + { "o[0].l", 160, 2},
> + { "o[0].m", 192, 1},
> + { "o[0].n", 208, 2},
> + { "o[1].j", 304,1},
whitespace
> + { "o[1].k", 320, 1},
> + { "o[1].l", 336, 2},
> + { "o[1].m", 368, 1},
> + { "o[1].n", 384, 2},
> + };
> +static const char frag_shader_text[] =
> + "#version 130\n"
> + "#extension GL_ARB_uniform_buffer_object : enable \n"
> + "layout(std140) uniform test_ubo { \n"
> + "float a;\n"
> + "vec2 b;\n"
> + "vec3 c;\n"
> + "struct {\n"
> + "int d;\n"
> + "bvec2 e;\n"
> + "} f;\n"
> + "float g;\n"
> + "float h[2];\n"
> + "mat2x3 i;\n"
> + "struct {\n"
> + "uvec3 j;\n"
> + "vec2 k;\n"
> + "float l[2];\n"
> + "vec2 m;\n"
> + "mat3 n[2];\n"
> + "} o[2];"
> + "};\n"
> + "void main() {}";
Could we get some indenting in the shader code?
> +static void
> +init(void)
> +{
> + GLuint fs;
> +
> + piglit_require_GLSL_version(120);
I think you mean 130.
> +static bool
> +validate_offset_and_size()
> +{
> + unsigned i,index;
> + GLint offset,size;
s/,/, /
> + for(i = 0; i < Elements(expected_result); i++) {
> + const char* temp_name = expected_result[i].name;
s/char* /char */
> + glGetUniformIndices(prog, 1, &temp_name, &index);
> + if (index == GL_INVALID_INDEX) {
> + printf("%s not found\n",temp_name);
> + return 0;
> + }
> + glGetActiveUniformsiv(prog, 1, &index, GL_UNIFORM_OFFSET,
> &offset);
> + if (offset != expected_result[i].offset) {
> + printf("For %s : offset is %d, should be
> %d\n",temp_name,
> + offset,expected_result[i].offset);
> + return 0;
> + }
> + glGetActiveUniformsiv(prog, 1, &index, GL_UNIFORM_SIZE, &size);
> + if (size != expected_result[i].size) {
> + printf("For %s : size is %d, should be %d\n",temp_name,
> + size,expected_result[i].size);
> + return 0;
> + }
> + }
> + return 1;
> +
> +}
Let's use true and false for bool return values :)
pgpJRzWn2YS0e.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
