On 29 August 2012 16:42, Stuart Abercrombie <sabercrom...@chromium.org>wrote:
> This is part of the effort to make version handling more flexible for GLES.
>
The patch has indentation errors that need to be fixed. Piglit uses tabs
for indentation, and is intended to be viewed with a tab width of 8.
> ---
> tests/shaders/shader_runner.c | 67
> +++++++++++++++++++----------------------
> 1 files changed, 31 insertions(+), 36 deletions(-)
>
> diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
> index aa60a62..add8a0c 100644
> --- a/tests/shaders/shader_runner.c
> +++ b/tests/shaders/shader_runner.c
> @@ -63,15 +63,8 @@ unsigned num_fragment_shaders = 0;
> int num_uniform_blocks;
> GLuint *uniform_block_bos;
>
> -/**
> - * List of strings loaded from files
> - *
> - * Some test script sections, such as "[vertex shader file]", can supply
> shader
> - * source code from multiple disk files. This array stores those strings.
> - */
> -char *shader_strings[256];
> -GLsizei shader_string_sizes[256];
> -unsigned num_shader_strings = 0;
> +char *shader_string;
> +GLint shader_string_size;
> const char *vertex_data_start = NULL;
> const char *vertex_data_end = NULL;
> GLuint prog;
> @@ -114,7 +107,8 @@ compile_glsl(GLenum target, bool release_text)
> {
> GLuint shader = piglit_CreateShader(target);
> GLint ok;
> - unsigned i;
> + GLchar** shader_string_ptr;
> + GLint* shader_len_ptr;
>
> switch (target) {
> case GL_VERTEX_SHADER:
> @@ -129,9 +123,11 @@ compile_glsl(GLenum target, bool release_text)
> break;
> }
>
> - piglit_ShaderSource(shader, num_shader_strings,
> - (const GLchar **) shader_strings,
> - shader_string_sizes);
> + shader_string_ptr = &shader_string;
> + shader_len_ptr = &shader_string_size;
> + piglit_ShaderSource(shader, 1,
> + shader_string_ptr,
> + shader_len_ptr);
>
This seems unnecessarily wordy. How about just:
piglit_ShaderSource(shader, 1, &shader_string, &shader_string_size);
Other than those two comments, the patch looks good.
>
> piglit_CompileShader(shader);
>
> @@ -155,8 +151,7 @@ compile_glsl(GLenum target, bool release_text)
> }
>
> if (release_text) {
> - for (i = 0; i < num_shader_strings; i++)
> - free(shader_strings[i]);
> + free(shader_string);
> }
>
> switch (target) {
> @@ -280,10 +275,15 @@ comparison_string(enum comparison cmp)
> void
> load_shader_file(const char *line)
> {
> - GLsizei *const size = &shader_string_sizes[num_shader_strings];
> + GLsizei *const size = &shader_string_size;
> char buf[256];
> char *text;
>
> + if (shader_string) {
> + printf("Multiple shader files in same section: %s\n", line);
> + piglit_report_result(PIGLIT_FAIL);
> + }
> +
> strcpy_to_space(buf, line);
>
> text = piglit_load_text_file(buf, (unsigned *) size);
> @@ -304,8 +304,7 @@ load_shader_file(const char *line)
> piglit_report_result(PIGLIT_FAIL);
> }
>
> - shader_strings[num_shader_strings] = text;
> - num_shader_strings++;
> + shader_string = text;
> }
>
>
> @@ -481,8 +480,7 @@ leave_state(enum states state, const char *line)
> break;
>
> case vertex_shader:
> - shader_string_sizes[0] = line - shader_strings[0];
> - num_shader_strings = 1;
> + shader_string_size = line - shader_string;
> compile_glsl(GL_VERTEX_SHADER, false);
> break;
>
> @@ -492,8 +490,8 @@ leave_state(enum states state, const char *line)
>
> case vertex_program:
> compile_and_bind_program(GL_VERTEX_PROGRAM_ARB,
> - shader_strings[0],
> - line - shader_strings[0]);
> + shader_string,
> + line - shader_string);
> break;
>
> case geometry_shader:
> @@ -503,8 +501,7 @@ leave_state(enum states state, const char *line)
> break;
>
> case fragment_shader:
> - shader_string_sizes[0] = line - shader_strings[0];
> - num_shader_strings = 1;
> + shader_string_size = line - shader_string;
> compile_glsl(GL_FRAGMENT_SHADER, false);
> break;
>
> @@ -514,8 +511,8 @@ leave_state(enum states state, const char *line)
>
> case fragment_program:
> compile_and_bind_program(GL_FRAGMENT_PROGRAM_ARB,
> - shader_strings[0],
> - line - shader_strings[0]);
> + shader_string,
> + line - shader_string);
> break;
>
> case vertex_data:
> @@ -629,24 +626,22 @@ process_test_script(const char *script_name)
> state = requirements;
> } else if (string_match("[vertex shader]", line)) {
> state = vertex_shader;
> - shader_strings[0] = NULL;
> + shader_string = NULL;
> } else if (string_match("[vertex program]", line))
> {
> state = vertex_program;
> - shader_strings[0] = NULL;
> + shader_string = NULL;
> } else if (string_match("[vertex shader file]",
> line)) {
> state = vertex_shader_file;
> - shader_strings[0] = NULL;
> - num_shader_strings = 0;
> + shader_string = NULL;
> } else if (string_match("[fragment shader]",
> line)) {
> state = fragment_shader;
> - shader_strings[0] = NULL;
> + shader_string = NULL;
> } else if (string_match("[fragment program]",
> line)) {
> state = fragment_program;
> - shader_strings[0] = NULL;
> + shader_string = NULL;
> } else if (string_match("[fragment shader file]",
> line)) {
> state = fragment_shader_file;
> - shader_strings[0] = NULL;
> - num_shader_strings = 0;
> + shader_string = NULL;
> } else if (string_match("[vertex data]", line)) {
> state = vertex_data;
> vertex_data_start = NULL;
> @@ -671,8 +666,8 @@ process_test_script(const char *script_name)
> case geometry_program:
> case fragment_shader:
> case fragment_program:
> - if (shader_strings[0] == NULL)
> - shader_strings[0] = (char *) line;
> + if (shader_string == NULL)
> + shader_string = (char *) line;
> break;
>
> case vertex_shader_file:
> --
> 1.7.7.3
>
>
_______________________________________________
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
