Marek Olšák <[email protected]> writes: > On Wed, Oct 19, 2016 at 1:36 AM, Francisco Jerez <[email protected]> > 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.
> Marek
signature.asc
Description: PGP signature
_______________________________________________ Piglit mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/piglit
