Marek Olšák <mar...@gmail.com> writes: > On Wed, Nov 9, 2016 at 8:10 PM, Francisco Jerez <curroje...@riseup.net> wrote: >> Marek Olšák <mar...@gmail.com> writes: >> >>> On Wed, Oct 19, 2016 at 1:36 AM, Francisco Jerez <curroje...@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) >>> >> >> Yeah, the whole block is intentionally indented one tab to the left, >> it's cleaned up in the next commit. >> >>>> >>>> - 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? >>> >> >> Interesting, I guess the test breaks while trying to deallocate the FBO >> From the previous test? Or does it break in some other way? In the >> former case I guess that setting 'draw_fbo = 0' would work around the >> issue, but it would probably also lead to FBO object leaks. > > No idea what's causing the failures, but releasing the FBO should be > really easy at the end of that block. >
Does the attached patch fix the problem for you? I didn't get a full piglit run to work with --process-isolation 0, so I couldn't confirm whether it fixes the issue. > Marek
From c210a937315a2f2eb63926af1cb15f27a4fa3e4c Mon Sep 17 00:00:00 2001 From: Francisco Jerez <curroje...@riseup.net> Date: Wed, 9 Nov 2016 14:36:47 -0800 Subject: [PATCH] shader_runner: Release FBOs at the end of every subtest run. --- tests/shaders/shader_runner.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c index e2d9849..a835781 100644 --- a/tests/shaders/shader_runner.c +++ b/tests/shaders/shader_runner.c @@ -2666,6 +2666,22 @@ setup_ubos(void) } static void +teardown_fbos(void) +{ + if (draw_fbo != 0 && + draw_fbo != piglit_winsys_fbo) + glDeleteFramebuffers(1, &draw_fbo); + + if (read_fbo != 0 && + read_fbo != piglit_winsys_fbo && + read_fbo != draw_fbo) + glDeleteFramebuffers(1, &read_fbo); + + draw_fbo = 0; + read_fbo = 0; +} + +static void teardown_ubos(void) { if (num_uniform_blocks == 0) { @@ -3983,6 +3999,7 @@ piglit_init(int argc, char **argv) /* destroy GL objects? */ teardown_ubos(); teardown_atomics(); + teardown_fbos(); } exit(0); } -- 2.10.1
signature.asc
Description: PGP signature
_______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit