[Piglit] [PATCH 06/18] shader_runner: Refactor handling of fb commands.

[Marek Olšák]

On Wed, Oct 19, 2016 at 1:36 AM, Francisco Jerez <currojerez at riseup.net> wrote:
> This refactors the implementation of the various "fb" commands to be
> part of a single 'if (parse_str(line, "fb ", ...)) {}' block in order
> to make code-sharing easier among fb subcommands.  Will be more useful
> when we start introducing additional fb subcommands.
> ---
>  tests/shaders/shader_runner.c | 67 ++++++++++++++++++++++++++-----------------
>  1 file changed, 40 insertions(+), 27 deletions(-)
>
> diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
> index ab2b907..4a2c807 100644
> --- a/tests/shaders/shader_runner.c
> +++ b/tests/shaders/shader_runner.c
> @@ -140,7 +140,7 @@ static bool prog_in_use = false;
>  static bool sso_in_use = false;
>  static GLchar *prog_err_info = NULL;
>  static GLuint vao = 0;
> -static GLuint fbo = 0;
> +static GLuint draw_fbo = 0;
>  static GLint render_width, render_height;
>
>  static bool report_subtests = false;
> @@ -2959,13 +2959,16 @@ piglit_display(void)
>                         do_enable_disable(rest, true);
>                 } else if (sscanf(line, "depthfunc %31s", s) == 1) {
>                         glDepthFunc(piglit_get_gl_enum_from_name(s));
> -               } else if (sscanf(line, "fb tex 2d %d", &tex) == 1) {
> -                       GLenum status;
> +               } else if (parse_str(line, "fb ", &rest)) {
> +               GLuint fbo = 0;

Wrong indentation. (it can lead to buggy code)

>
> -                       if (fbo == 0) {
> -                               glGenFramebuffers(1, &fbo);
> -                               glBindFramebuffer(GL_FRAMEBUFFER, fbo);
> -                       }
> +               if (parse_str(rest, "tex 2d ", &rest)) {
> +                       glGenFramebuffers(1, &fbo);
> +                       glBindFramebuffer(GL_FRAMEBUFFER, fbo);
> +
> +                       REQUIRE(parse_int(rest, &tex, &rest),
> +                               "Framebuffer binding command not "
> +                               "understood at: %s\n", rest);
>
>                         glFramebufferTexture2D(GL_FRAMEBUFFER,
>                                                GL_COLOR_ATTACHMENT0,
> @@ -2976,21 +2979,12 @@ piglit_display(void)
>                                 piglit_report_result(PIGLIT_FAIL);
>                         }
>
> -                       status = glCheckFramebufferStatus(GL_FRAMEBUFFER);
> -                       if (status != GL_FRAMEBUFFER_COMPLETE) {
> -                               fprintf(stderr, "incomplete fbo (status 0x%x)\n", status);
> -                               piglit_report_result(PIGLIT_FAIL);
> -                       }
> -
> -                       render_width = get_texture_binding(tex)->width;
> -                       render_height = get_texture_binding(tex)->height;
> -               } else if (sscanf(line, "fb tex layered %d", &tex) == 1) {
> -                       GLenum status;
> +                       w = get_texture_binding(tex)->width;
> +                       h = get_texture_binding(tex)->height;
>
> -                       if (fbo == 0) {
> -                               glGenFramebuffers(1, &fbo);
> -                               glBindFramebuffer(GL_FRAMEBUFFER, fbo);
> -                       }
> +               } else if (sscanf(rest, "tex layered %d", &tex) == 1) {
> +                       glGenFramebuffers(1, &fbo);
> +                       glBindFramebuffer(GL_FRAMEBUFFER, fbo);
>
>                         glFramebufferTexture(GL_FRAMEBUFFER,
>                                              GL_COLOR_ATTACHMENT0,
> @@ -3000,11 +2994,31 @@ piglit_display(void)
>                                 piglit_report_result(PIGLIT_FAIL);
>                         }
>
> -                       status = glCheckFramebufferStatus(GL_FRAMEBUFFER);
> -                       if (status != GL_FRAMEBUFFER_COMPLETE) {
> -                               fprintf(stderr, "incomplete fbo (status 0x%x)\n", status);
> -                               piglit_report_result(PIGLIT_FAIL);
> -                       }
> +                       w = get_texture_binding(tex)->width;
> +                       h = get_texture_binding(tex)->height;
> +
> +               } else {
> +                       fprintf(stderr, "Unknown fb bind subcommand "
> +                               "\"%s\"\n", rest);
> +                       piglit_report_result(PIGLIT_FAIL);
> +               }
> +
> +               const GLenum status = glCheckFramebufferStatus(GL_FRAMEBUFFER);
> +               if (status != GL_FRAMEBUFFER_COMPLETE) {
> +                       fprintf(stderr, "incomplete fbo (status 0x%x)\n", status);
> +                       piglit_report_result(PIGLIT_FAIL);
> +               }
> +
> +               render_width = w;
> +               render_height = h;
> +
> +               /* Delete the previous draw FB in case
> +                * it's no longer reachable.
> +                */
> +               if (draw_fbo != 0)
> +                       glDeleteFramebuffers(1, &draw_fbo);
> +
> +               draw_fbo = fbo;
>
>                 } else if (parse_str(line, "frustum", &rest)) {
>                         parse_floats(rest, c, 6, NULL);
> @@ -3622,7 +3636,6 @@ piglit_init(int argc, char **argv)
>                         sso_in_use = false;
>                         prog_err_info = NULL;
>                         vao = 0;
> -                       fbo = 0;

This breaks some piglit tests with --process-isolation 0. Adding
"draw_fbo = 0" here fixes that. Is that the right fix?

Marek