[Piglit] [PATCH v3] arb_texture_barrier: add an arb_texture_barrier test
Alejandro Piñeiro
apinheiro at igalia.com
Fri Jul 8 07:11:52 UTC 2016
On 07/07/16 15:59, Andres Gomez wrote:
> On Wed, 2016-06-29 at 11:36 +0200, Alejandro Piñeiro wrote:
>> Allow to configure:
>> * texture resolution
>> * number of blend passes
>> * number of draw passes (calls to DrawRangeElements)
>> * granularity (triangle count) of the square being draw
>> * number of textures
>>
>> This commits adds to all.py execute the test with a combination of
>> all this parameters.
>>
>> v2: reduce GLSL requirement to 330 (Nicolai Hähnle, Ilia Mirkin)
>> v3: reduce GLSL requirement to 130, clean up resources
>>
>> Acked-by: Marek Olšák <marek.olsak at amd.com>
>> ---
>>
>> I reduced the GLSL requirement to 130 in order to be able to execute
>> this test on gen6. The only reason v2 needed 330 (so arb_gpu_shader5)
>> was because the shader was accessing the sampler array on a loop. Now
>> we mimick that functionality when creating the final shader based on a
>> template (basically unrolling the loop).
>>
>> tests/all.py | 13 +
>> tests/spec/CMakeLists.txt | 1 +
>> tests/spec/arb_texture_barrier/CMakeLists.gl.txt | 13 +
>> tests/spec/arb_texture_barrier/CMakeLists.txt | 1 +
>> .../arb_texture_barrier/blending-in-shader-arb.c | 525 +++++++++++++++++++++
>> 5 files changed, 553 insertions(+)
>> create mode 100644 tests/spec/arb_texture_barrier/CMakeLists.gl.txt
>> create mode 100644 tests/spec/arb_texture_barrier/CMakeLists.txt
>> create mode 100644 tests/spec/arb_texture_barrier/blending-in-shader-arb.c
>>
>> diff --git a/tests/all.py b/tests/all.py
>> index 8af76a1..a343312 100644
>> --- a/tests/all.py
>> +++ b/tests/all.py
>> @@ -4712,6 +4712,19 @@ with profile.group_manager(
>> g(['object-namespace-pollution', operation, object_type],
>> '{} with {}'.format(object_type, operation))
>>
>> +# Group ARB_texture_barrier
>> +with profile.group_manager(
>> + PiglitGLTest,
>> + grouptools.join('spec', 'arb_texture_barrier')) as g:
>> + for resolution in ['32', '512']:
>> + for blend_passes in ['1', '42']:
>> + for num_textures in ['1', '8']:
>> + for granularity in ['8', '64', '128']:
>> + for draw_passes in ['1', '8']:
>> + g(['arb_texture_barrier-blending-in-shader', resolution,
>> + blend_passes, num_textures, granularity, draw_passes])
>> +
>> +
>> # Group ARB_invalidate_subdata
>> with profile.group_manager(
>> PiglitGLTest,
>> diff --git a/tests/spec/CMakeLists.txt b/tests/spec/CMakeLists.txt
>> index 6f818ae..29693f6 100644
>> --- a/tests/spec/CMakeLists.txt
>> +++ b/tests/spec/CMakeLists.txt
>> @@ -147,3 +147,4 @@ add_subdirectory (arb_shader_draw_parameters)
>> add_subdirectory (arb_indirect_parameters)
>> add_subdirectory (arb_query_buffer_object)
>> add_subdirectory (ext_window_rectangles)
>> +add_subdirectory (arb_texture_barrier)
>> diff --git a/tests/spec/arb_texture_barrier/CMakeLists.gl.txt b/tests/spec/arb_texture_barrier/CMakeLists.gl.txt
>> new file mode 100644
>> index 0000000..30578e8
>> --- /dev/null
>> +++ b/tests/spec/arb_texture_barrier/CMakeLists.gl.txt
>> @@ -0,0 +1,13 @@
>> +include_directories(
>> + ${GLEXT_INCLUDE_DIR}
>> + ${OPENGL_INCLUDE_PATH}
>> +)
>> +
>> +link_libraries (
>> + piglitutil_${piglit_target_api}
>> + ${OPENGL_gl_LIBRARY}
>> +)
>> +
>> +piglit_add_executable (arb_texture_barrier-blending-in-shader blending-in-shader-arb.c)
>> +
>> +# vim: ft=cmake:
>> diff --git a/tests/spec/arb_texture_barrier/CMakeLists.txt b/tests/spec/arb_texture_barrier/CMakeLists.txt
>> new file mode 100644
>> index 0000000..144a306
>> --- /dev/null
>> +++ b/tests/spec/arb_texture_barrier/CMakeLists.txt
>> @@ -0,0 +1 @@
>> +piglit_include_target_api()
>> diff --git a/tests/spec/arb_texture_barrier/blending-in-shader-arb.c b/tests/spec/arb_texture_barrier/blending-in-shader-arb.c
>> new file mode 100644
>> index 0000000..8f49b30
>> --- /dev/null
>> +++ b/tests/spec/arb_texture_barrier/blending-in-shader-arb.c
>> @@ -0,0 +1,525 @@
>> +/*
>> + * Copyright © 2016 Intel Corporation
>> + *
>> + * Based on blending-in-shader.c, by Marek Olšák
>> + *
>> + * 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 blending-in-shader-arb.c
>> + *
>> + * Test programmable blending with GL_ARB_texture_barrier.
>> + *
>> + * This test is equivalent to blending-in-shader.c, but:
>> + *
>> + * * Tests GL_ARB_texture_barrier instead of GL_NV_texture_barrier,
>> + * that are totally equivalent.
>> + *
>> + * * Uses GL_ARB_framebuffer_object instead of
>> + * GL_EXT_framebuffer_object. Those are slightly different, and
>> + * several drivers doesn't support the EXT one.
>> + *
>> + * * This tests switches to use a integer texture, because that would
> an --------------------------------^
>
>> + * allow us to do actual vs reference comparisons without the need
>> + * of a tolerance.
>> + *
>> + * * Allows to parametrize several aspects: resolution, number of
>> + * blending passes, number of drawing passes, square granularity
>> + * (triangle count) and number of textures.
>> + */
>> +
>> +#include "piglit-util-gl.h"
>> +
>> +PIGLIT_GL_TEST_CONFIG_BEGIN
>> +
>> + config.supports_gl_compat_version = 31;
>> + config.supports_gl_core_version = 31;
>> + config.window_visual = PIGLIT_GL_VISUAL_DOUBLE | PIGLIT_GL_VISUAL_RGBA;
>> +
>> +PIGLIT_GL_TEST_CONFIG_END
>> +
>> +#define BUFFER_OFFSET(i) ((char *)NULL + (i))
>> +
>> +/* Texture stuff */
>> +static GLuint fbo, prog;
>> +static GLuint *tex;
>> +static GLuint *texloc;
>> +
>> +static GLuint **tex_data;
>> +static GLuint **reference_data;
>> +
>> +/* vertex data */
>> +typedef struct {
>> + float data[4];
>> +}Vertex;
> Space ^
>
>> +
>> +unsigned int total_indices;
>> +GLuint buf = 0;
>> +GLuint vao = 0;
>> +GLuint buf_index = 0;
>> +
>> +#define MAX_NUM_TEXTURES 8
>> +/* Command line parameters */
>> +static int width = 0;
>> +static int height = 0;
>> +static int num_textures = 0;
>> +static int blend_passes = 0;
>> +static int granularity = 0;
>> +static int draw_passes = 0;
>> +
>> +static const char *vs_text = {
>> + "#version 130\n"
>> + "in vec4 piglit_vertex;"
>> + "void main() {\n"
>> + " gl_Position = piglit_vertex;\n"
>> + "}\n"
>> +};
>> +
>> +static const char *texel_fetch_template = {
>> + " color[%i] = texelFetch(fb[%i], ivec2(gl_FragCoord.xy), 0);\n"
>> +};
>> +
>> +static const char *fs_text_template = {
>> + "#version 130\n"
>> + "#define NUM_TEXTURES %i\n"
>> + "uniform usampler2D fb[NUM_TEXTURES];\n"
>> + "out uvec4 color[NUM_TEXTURES];\n"
>> + "void main() {\n"
>> + "%s%s%s%s%s%s%s%s"
> The usage of the MAX_NUM_TEXTURES constant define is defeated by using
> 8 "%s" here.
>
> I would suggest to use a "%s" and concatenate the needed string with a
> loop to MAX_NUM_TEXTURES. See below ...
>
>> + " for (int t = 0; t < NUM_TEXTURES; t++){\n"
>> + " color[t]++;\n"
>> + " }\n"
>> + "}\n"
>> +};
>> +
>> +static void
>> +allocate_data()
>> +{
>> + unsigned int t;
>> +
>> + tex_data = malloc(sizeof(GLuint *) * num_textures);
>> + reference_data = malloc(sizeof(GLuint *) * num_textures);
>> +
>> + for (t = 0; t < num_textures; t++) {
>> + tex_data[t] = malloc(sizeof(GLuint) * width * height);
>> + reference_data[t] = malloc(sizeof(GLuint) * width * height);
>> + }
>> +}
>> +
>> +static bool
>> +compare_with_reference()
>> +{
>> + bool outcome = true;
>> + unsigned int t;
>> + GLuint *actual;
>> +
>> + actual = malloc(sizeof(GLuint) * width * height);
>> +
>> + for (t = 0; t < num_textures; t++) {
>> + unsigned int count = 0;
>> + unsigned int i;
>> +
>> + glActiveTexture(GL_TEXTURE0 + t);
>> + glGetTexImage(GL_TEXTURE_2D, 0, GL_RED_INTEGER, GL_UNSIGNED_INT, actual);
>> +
>> + for (i = 0; i < width * height; i++) {
>> + if (reference_data[t][i] != actual[i]) {
>> + count++;
>> + outcome = false;
>> + }
>> + }
>> +
>> + if (count > 0) {
>> + fprintf(stderr, "Error on texture %i: %i texels out of "
>> + "%i are different\n", t, count, width * height);
> You probably want to output with %u for unsigned int.
>
>> + }
>> + }
>> +
>> + free(actual);
>> +
>> + return outcome;
>> +}
>> +
>> +static void
>> +initialize_data()
>> +{
>> + unsigned int i,j;
>> + unsigned int t;
>> +
>> + allocate_data();
>> +
>> + for (t = 0; t < num_textures; t++) {
>> + for (i = 0; i < width * height; i++) {
>> + tex_data[t][i] = reference_data[t][i] = rand();
>> + for (j = 0; j < blend_passes; j++)
>> + reference_data[t][i] = reference_data[t][i] + 1;
>> + }
>> +
>> + glActiveTexture(GL_TEXTURE0 + t);
>> + glTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, width, height, GL_RED_INTEGER,
>> + GL_UNSIGNED_INT, tex_data[t]);
>> + glFramebufferTexture(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0 + t, tex[t], 0);
>> + }
>> + assert(glCheckFramebufferStatus(GL_FRAMEBUFFER) == GL_FRAMEBUFFER_COMPLETE);
>> +}
>> +
>> +static void
>> +interpolate_square (const Vertex square[4],
> Extra space ---------^
>
>> + unsigned int granularity,
>> + Vertex *result)
>> +{
>> + unsigned int x;
>> + unsigned int y;
>> + unsigned int b = granularity - 1;
>> + unsigned int k = 0;
>> +
>> + for (y = 0; y < granularity; y++) {
>> + for (x = 0; x < granularity; x++) {
>> + result[k].data[0] = (square[0].data[0] * (b - x) + square[2].data[0] * x)/b;
>> + result[k].data[1] = (square[0].data[1] * (b - y) + square[1].data[1] * y)/b;
>> + result[k].data[2] = 0;
>> + result[k].data[3] = 1;
>> +
>> + k++;
>> + }
>> + }
>> +}
>> +
>> +/*
>> + * Tesselates @square, returning a set of vertex information and
> @square_pos -----^^^^^^^
>
>> + * indices compatible to draw with glDrawElements at (@tesselated_pos,
>> + * @tesselated_texcoord, @indices). @granularity is the number of
> ?? ---^^^^^^^^^^^^^^^^^^^^
>
>> + * vertexes per side. So the minimum should be 2.
>> + *
>> + * Is assumed that the vertex at @square are in the order
> Ditto.
>
>> + * (bottom-left, top-left, top-right, bottom-right)
>> + *
>> + * Probably not the most optimal code to do this, but it works
>> + *
>> + * Returns: newly allocated memory at @tesselated_pos,
>> + * @tesselated_texcoord and @indices. @numIndices returns the number
> ?? ---^^^^^^^^^^^^^^^^^^^^---------------^^^^^^^^^^^
>
>> + * of indices at @indices
>> + */
>> +static void
>> +util_tesselate_square (const Vertex square_pos[4],
> Extra space ------------^
>
>> + unsigned int granularity,
>> + Vertex **tesselated_pos,
>> + unsigned int **indices)
>> +{
>> + unsigned int x;
>> + unsigned int y;
>> + unsigned int k;
>> + Vertex *result_pos;
>> + unsigned int *result_indices;
>> +
>> + if (granularity < 2) {
>> + fprintf(stderr, "Granularity should be equal or greater to 2"
>> + " in order to tesselate a square. Setting it to 2\n");
>> + granularity = 2;
>> + }
>> +
>> + result_pos = (Vertex*) malloc(sizeof(Vertex) * granularity * granularity);
>> + total_indices = (granularity - 1) * (granularity - 1) * 6;
>> + result_indices = (unsigned int*) malloc (sizeof(unsigned int) * total_indices);
> Extra space --------------------------------------^
>
>> +
>> + interpolate_square (square_pos, granularity, result_pos);
> Extra space -----------------^
>
>> +
>> + k = 0;
>> + for (y = 0; y < granularity - 1; y++) {
>> + for (x = 0; x < granularity - 1; x++) {
>> + result_indices[k++] = y * granularity + x;
>> + result_indices[k++] = y * granularity + x + 1;
>> + result_indices[k++] = (y + 1) * granularity + x;
>> +
>> + result_indices[k++] = y * granularity + x + 1;
>> + result_indices[k++] = (y + 1) * granularity + x +1;
>> + result_indices[k++] = (y + 1) * granularity + x;
>> + }
>> + }
>> +
>> + *tesselated_pos = result_pos;
>> + *indices = result_indices;
>> +}
>> +
>> +static GLvoid
>> +initialize_vertex_data(float x, float y, float w, float h)
>> +{
>> + Vertex verts[4];
>> + Vertex *tesselated_pos;
>> + unsigned int *indices;
>> +
>> + verts[0].data[0] = x;
>> + verts[0].data[1] = y;
>> + verts[1].data[0] = x;
>> + verts[1].data[1] = y + h;
>> + verts[2].data[0] = x + w;
>> + verts[2].data[1] = y + h;
>> + verts[3].data[0] = x + w;
>> + verts[3].data[1] = y;
>> +
>> + util_tesselate_square (verts, granularity,
>>
> Extra space --------------------^
>
> + &tesselated_pos,
>> + &indices);
>> +
>> + glGenVertexArrays(1, &vao);
>> + glBindVertexArray(vao);
>> +
>> + glGenBuffers(1, &buf);
>> + glBindBuffer(GL_ARRAY_BUFFER, buf);
>> +
>> + glBufferData(GL_ARRAY_BUFFER,
>> + (sizeof(GLfloat) * 4 * granularity * granularity),
>> + tesselated_pos,
>> + GL_STATIC_DRAW);
>> +
>> + glVertexAttribPointer(PIGLIT_ATTRIB_POS, 4, GL_FLOAT,
>> + GL_FALSE, 0,
>> + 0);
>> + glEnableVertexAttribArray(PIGLIT_ATTRIB_POS);
>> +
>> + glGenBuffers(1, &buf_index);
>> + glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, buf_index);
>> + glBufferData(GL_ELEMENT_ARRAY_BUFFER, sizeof(GLuint) * total_indices,
>> + indices, GL_STATIC_DRAW);
>> +
>> + free(tesselated_pos);
>> + free(indices);
>> +}
>> +
>> +static void
>> +clean_resources()
>> +{
>> + unsigned int t;
>> +
>> + glDeleteTextures(num_textures, tex);
>> + glDeleteProgram(prog);
>> + glDeleteFramebuffers(1, &fbo);
>> + glDeleteVertexArrays(1, &vao);
>> + glDeleteBuffers(1, &buf);
>> + glDeleteBuffers(1, &buf_index);
>> +
>> + for (t = 0; t < num_textures; t++) {
>> + free(tex_data[t]);
>> + free(reference_data[t]);
>> + }
>> +
>> + free(tex_data);
>> + free(reference_data);
>> + free(texloc);
>> + free(tex);
>> +}
>> +
>> +static unsigned int
>> +min(unsigned int value1, unsigned int value2)
> piglit-util.h defines MIN2()
>
>> +{
>> + if (value1 < value2)
>> + return value1;
>> + else
>> + return value2;
>> +}
>> +
>> +static GLvoid
>> +draw_rect_tex()
>> +{
>> + unsigned int basic_count = (total_indices * 3)/(3 * draw_passes);
> 3/3 == 1
>
>> + unsigned int first = 0;
>> +
>> + while (first < total_indices) {
>> + unsigned int count = min(total_indices - first, basic_count);
>> +
>> + glDrawRangeElements(GL_TRIANGLES, first, first + count, count,
>> + GL_UNSIGNED_INT, BUFFER_OFFSET(first * sizeof(GLuint)));
>> + first += count;
>> + }
>> +}
>> +
>> +enum piglit_result
>> +piglit_display(void)
>> +{
>> + unsigned int i;
>> + bool outcome = false;
>> +
>> + glViewport(0, 0, width, height);
>> +
>> + for (i = 0; i < blend_passes; i++) {
>> + if (i != 0)
>> + glTextureBarrier();
>> + draw_rect_tex();
>> + }
>> +
>> + outcome = compare_with_reference() ? PIGLIT_PASS : PIGLIT_FAIL;
>> +
>> + clean_resources();
>> +
>> + return outcome;
>> +}
>> +
>> +static void
>> +initialize_textures()
>> +{
>> + unsigned int t;
>> +
>> + tex = malloc(sizeof(GLuint) * num_textures);
>> + glGenTextures(num_textures, tex);
>> + for (t = 0; t < num_textures; t++) {
>> + glActiveTexture(GL_TEXTURE0 + t);
>> + glBindTexture(GL_TEXTURE_2D, tex[t]);
>> + glTexStorage2D(GL_TEXTURE_2D, 1, GL_R32UI, width, height);
>> + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
>> + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
>> + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
>> + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
>> + }
>> +}
>> +
>> +static void
>> +initialize_fbo()
>> +{
>> + unsigned int t;
>> + GLenum draw_buffers[MAX_NUM_TEXTURES];
>> +
>> + glGenFramebuffers(1, &fbo);
>> + glBindFramebuffer(GL_FRAMEBUFFER, fbo);
>> + for (t = 0; t < num_textures; t++) {
>> + draw_buffers[t] = GL_COLOR_ATTACHMENT0 + t;
>> + }
>> + glDrawBuffers(num_textures, draw_buffers);
>> +}
>> +
>> +static void
>> +initialize_program()
>> +{
>> + char *fetch_string[MAX_NUM_TEXTURES];
>> + char *fs_text;
>> + int result;
>> + unsigned int t;
>> + char name[] = "fb[0]";
>> +
>> + for (t = 0; t < num_textures; t++) {
>> + result = asprintf(&fetch_string[t], texel_fetch_template, t, t);
>> + if (result == -1)
> Shouldn't we spit a meaningful error message?
>
>> + piglit_report_result(PIGLIT_FAIL);
>> + }
>> + for (t = num_textures; t < MAX_NUM_TEXTURES; t++) {
>> + fetch_string[t] = "";
>> + }
>> +
>> + result = asprintf(&fs_text, fs_text_template, num_textures,
>> + fetch_string[0], fetch_string[1], fetch_string[2], fetch_string[3],
>> + fetch_string[4], fetch_string[5], fetch_string[6], fetch_string[7]);
>> + if (result == -1)
> As explained at the top, in the FS template, this defeats the usage of
> the MAX_NUM_TEXTURES constant define.
>
> I would rather get rid of the fetch_string array and just construct a
> single string by concatenating templates in the first "for" above. The
> second "for" is not needed either.
>
>> + piglit_report_result(PIGLIT_FAIL);
>> +
>> + prog = piglit_build_simple_program(vs_text, fs_text);
>> + piglit_check_gl_error(GL_NO_ERROR);
>> +
>> + texloc = malloc(sizeof(GLuint) * num_textures);
>> + glUseProgram(prog);
>> + for (t = 0; t < num_textures; t++) {
>> + name[3] = '0' + t;
> Same here. This works for MAX_NUM_TEXTURES < 10. It would be smarter
> just to construct the string here with asprintf.
>
>> + texloc[t] = glGetUniformLocation(prog, name);
>> + if (texloc[t] == -1)
>> + fprintf(stderr, "Error getting uniform %s\n", name);
> Shouldn't we piglit_report_result(PIGLIT_FAIL) ?
>
>> + glUniform1i(texloc[t], t);
>> + }
>> +
>> + free(fs_text);
>> + for (t = 0; t < num_textures; t++) {
>> + free(fetch_string[t]);
>> + }
>> +}
>> +
>> +static void
>> +print_usage()
>> +{
>> + printf( "Usage: arb_texture_barrier-blending-in-shader ");
> Extra space ------^
>
>> + printf("<resolution> <blend_passes> <num_textures> <granularity> <common piglit args>\n");
>> + printf("\tresolution valid range is [1, 1024]\n");
>> + printf("\tblend_passes valid range is [1,42]\n");
> Space missing --------------------------------------^
>
>> + printf("\tnum_textures valid range is [1, 8]\n");
> MAX_NUM_TEXTURES ? ----------------------------------^
>
>> + printf("\tgranularity (the number of vertices per side) valid range is [2, 256]\n");
>> + printf("\tdraw_passes valid range is [1, 10]\n");
>> +}
> Blank line missing.
>
>> +static void
>> +parse_args(int argc, char **argv)
>> +{
>> + if (argc != 6) {
>> + print_usage();
>> + piglit_report_result(PIGLIT_FAIL);
>> + }
>> +
>> + width = height = atoi(argv[1]);
>> + if (width < 1 || width > 1024) {
>> + fprintf(stderr, "Wrong value for resolution: %s\n", argv[1]);
>> + print_usage();
>> + piglit_report_result(PIGLIT_FAIL);
>> + }
>> +
>> + blend_passes = atoi(argv[2]);
>> + if (blend_passes < 1 || blend_passes > 42) {
>> + fprintf(stderr, "Wrong value for blend_passes: %s\n", argv[2]);
>> + print_usage();
>> + piglit_report_result(PIGLIT_FAIL);
>> + }
>> +
>> + num_textures = atoi(argv[3]);
>> + if (num_textures < 1 || num_textures > MAX_NUM_TEXTURES) {
>> + fprintf(stderr, "Wrong value for num_textures: %s\n", argv[3]);
>> + print_usage();
>> + piglit_report_result(PIGLIT_FAIL);
>> + }
>> +
>> + granularity = atoi(argv[4]);
>> + if (granularity < 2 || granularity > 256) {
>> + fprintf(stderr, "Wrong value for granularity: %s\n", argv[4]);
>> + print_usage();
>> + piglit_report_result(PIGLIT_FAIL);
>> + }
>> +
>> + draw_passes = atoi(argv[5]);
>> + if (granularity < 1 || draw_passes > 10) {
>> + fprintf(stderr, "Wrong value for draw_passes: %s\n", argv[5]);
>> + print_usage();
>> + piglit_report_result(PIGLIT_FAIL);
>> + }
>> +
>> + fprintf(stdout, "Executing test with parameters (resolution, blend_passes,"
>> + " num_textures, granularity, draw_passes)=(%i, %i, %i, %i, %i)\n",
>> + width, blend_passes, num_textures, granularity, draw_passes);
>> +}
>> +
>> +void
>> +piglit_init(int argc, char **argv)
>> +{
>> + piglit_require_extension("GL_ARB_framebuffer_object");
>> + piglit_require_extension("GL_ARB_texture_barrier");
>> + piglit_require_GLSL_version(130);
>> +
>> + parse_args(argc, argv);
>> +
>> + srand(0);
>> +
>> + initialize_program();
>> + initialize_textures();
>> + initialize_fbo();
>> + initialize_data();
>> + initialize_vertex_data(-1, -1, 2, 2);
>> +}
> In addition, the patch has a mix of tabs and spaces indentation. I
> believe we use tabs in piglit.
Probably it was caused by a C&P from the nv_texture_barrier test. Will
unify it.
>
> Most of the comments are small nitpicks that can be ignored.
But most of them are easy to solve so I will do that. Thanks for the
careful review.
>
> Specially, the ones regarding MAX_NUM_TEXTURES. If the value is not
> meant to change, it really doesn't matter to change the code logic but
> it is kind of weird to use a define and then having code with hard-
> coded values instead of having into account that constant. Any future
> change in the define (for debugging, for example) would involve also
> quite a rework in the code to be able to apply the change.
Although this involves a little more work, you are right. I will try to
implement your suggestions.
>
> This is:
> Reviewed-by: Andres Gomez <agomez at igalia.com>
>
More information about the Piglit
mailing list