On Wed, Nov 4, 2015 at 6:45 AM, Neil Roberts <n...@linux.intel.com> wrote: > Previously the GL spec required that whenever glBlitFramebuffer is > used with either buffer being multisampled, the internal formats must > match. However the GL 4.4 spec was later changed to remove this > restriction. In the section entitled “Changes in the released > Specification of July 22, 2013” it says: > > “Relax BlitFramebuffer in section 18.3.1 so that format conversion can > take place during multisample blits, since drivers already allow this > and some apps depend on it.” > > If most drivers already allowed this in earlier versions I think it's > safe to assume that this is a spec bug and it should also be allowed > in all versions. > > This patch modifies the existing test that checks for > GL_INVALID_OPERATION when blitting between different formats so that > it instead checks that the blit actually succeeded. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92706 > --- > > I tested this on NVidia's binary driver with an old GeForce 9400 GT > and it passes. It also passes on HSW and SKL with Mesa with the small > patch series that I'm about to post to mesa-dev. > > .../blit-mismatched-formats.cpp | 177 > ++++++++++++++++++--- > 1 file changed, 159 insertions(+), 18 deletions(-) > > diff --git > a/tests/spec/ext_framebuffer_multisample/blit-mismatched-formats.cpp > b/tests/spec/ext_framebuffer_multisample/blit-mismatched-formats.cpp > index d2a7be9..dfbf912 100644 > --- a/tests/spec/ext_framebuffer_multisample/blit-mismatched-formats.cpp > +++ b/tests/spec/ext_framebuffer_multisample/blit-mismatched-formats.cpp > @@ -1,5 +1,5 @@ > /* > - * Copyright © 2012 Intel Corporation > + * Copyright © 2012, 2015 Intel Corporation > * > * Permission is hereby granted, free of charge, to any person obtaining a > * copy of this software and associated documentation files (the "Software"), > @@ -24,14 +24,22 @@ > /** > * @file blit-mismatched-formats.cpp > * > - * This test verifies if glBlitFramebuffer() throws GL_INVALID_OPERATION with > - * non-matching formats for multisample framebuffer objects. > + * This test verifies that calling glBlitFramebuffer to blit between > + * two multisampled framebuffers works even if they have different > + * formats. Note that originally the GL spec required that blitting > + * between differing formats should report a GL_INVALID_OPERATION > + * error. However in practice most drivers allowed it anyway and in > + * the GL 4.4 spec the restriction was removed. I think it can be > + * considered a mistake in the spec that this was not the case > + * originally so this test assumes that it should be possible in any > + * version. > * > - * We initialize two FBOs with minimum supported sample count and different > - * buffer formats, do blitting operation between them and then query the gl > - * error. > + * We initialize two FBOs with minimum supported sample count and > + * different buffer formats, do blitting operation between them and > + * verify the expected results. > * > - * Author: Anuj Phogat <anuj.pho...@gmail.com> > + * Authors: Anuj Phogat <anuj.pho...@gmail.com> > + * Neil Roberts <n...@linux.intel.com> > */ > > #include "piglit-test-pattern.h" > @@ -50,24 +58,70 @@ PIGLIT_GL_TEST_CONFIG_BEGIN > PIGLIT_GL_TEST_CONFIG_END > > const int pattern_width = 256; const int pattern_height = 256; > -Fbo src_fbo, dst_fbo; > +Fbo src_fbo, dst_fbo, ss_fbo; > +ColorGradientSunburst *test_pattern; > +GLfloat *reference_image; > > -static GLenum color_formats[] = { > - GL_RED, > - GL_RG, > - GL_RGB, > - GL_ALPHA}; > +enum component { > + COMPONENT_RED = (1 << 0), > + COMPONENT_GREEN = (1 << 1), > + COMPONENT_BLUE = (1 << 2), > + COMPONENT_ALPHA = (1 << 3), > +}; > + > +struct color_format { > + GLenum name; > + GLbitfield components; > +}; > + > +static struct color_format > +color_formats[] = { > + { GL_ALPHA, COMPONENT_ALPHA }, > + { GL_RED, COMPONENT_RED }, > + { GL_RG, COMPONENT_RED | COMPONENT_GREEN }, > + { GL_RGB, COMPONENT_RED | COMPONENT_GREEN | COMPONENT_BLUE } > +}; > + > +static GLfloat * > +generate_expected_image(const GLfloat *ref_image, > + int pattern_width, int pattern_height, > + GLbitfield components) > +{ > + GLfloat *expected_image = (GLfloat *) malloc(pattern_width * > + pattern_height * 4 * > + sizeof(GLfloat)); > + GLfloat *dst = expected_image; > + const GLfloat *src = ref_image; > + int i; > + > + for (i = 0; i < pattern_width * pattern_height; i++) { > + dst[0] = (components & COMPONENT_RED) ? src[0] : 0.0f; > + dst[1] = (components & COMPONENT_GREEN) ? src[1] : 0.0f; > + dst[2] = (components & COMPONENT_BLUE) ? src[2] : 0.0f; > + dst[3] = (components & COMPONENT_ALPHA) ? src[3] : 1.0f; > + src += 4; > + dst += 4; > + } > + > + return expected_image; > +} > > enum piglit_result > piglit_display() > { > bool pass = true; > + GLfloat *expected_image; > GLuint i; > > FboConfig config_ms(1 , pattern_width, pattern_height); > > for(i = 0; i < ARRAY_SIZE(color_formats); i++) { > - config_ms.color_internalformat = color_formats[i]; > + expected_image = > + generate_expected_image(reference_image, > + pattern_width, pattern_height, > + color_formats[i].components); > + > + config_ms.color_internalformat = color_formats[i].name; > src_fbo.setup(config_ms); > > if (!piglit_check_gl_error(GL_NO_ERROR)) { > @@ -75,18 +129,73 @@ piglit_display() > piglit_report_result(PIGLIT_FAIL); > } > > + glBindFramebuffer(GL_FRAMEBUFFER, src_fbo.handle); > + test_pattern->draw(TestPattern::no_projection); > + > /* Blit multisample-to-multisample with non-matching formats > */ > glBindFramebuffer(GL_READ_FRAMEBUFFER, src_fbo.handle); > glBindFramebuffer(GL_DRAW_FRAMEBUFFER, dst_fbo.handle); > + > + glClear(GL_COLOR_BUFFER_BIT); > + > + glBlitFramebuffer(0, 0, pattern_width, pattern_height, > + 0, 0, pattern_width, pattern_height, > + GL_COLOR_BUFFER_BIT, GL_NEAREST); > + > + /* Blitting between different formats shouldn't > + * generate an error. > + */ > + pass = piglit_check_gl_error(GL_NO_ERROR) && pass; > + > + /* Downsample the blitted buffer so we can read the > + * results > + */ > + glBindFramebuffer(GL_READ_FRAMEBUFFER, dst_fbo.handle); > + glBindFramebuffer(GL_DRAW_FRAMEBUFFER, ss_fbo.handle); > + glClear(GL_COLOR_BUFFER_BIT); > + glBlitFramebuffer(0, 0, pattern_width, pattern_height, > + 0, 0, pattern_width, pattern_height, > + GL_COLOR_BUFFER_BIT, GL_NEAREST); > + > + pass = piglit_check_gl_error(GL_NO_ERROR) && pass; > + > + glBindFramebuffer(GL_FRAMEBUFFER, ss_fbo.handle); I think it's more readable to use: glBindFramebuffer(GL_READ_FRAMEBUFFER, ss_fbo.handle); > + pass = piglit_probe_image_rgba(0, 0, > + pattern_width, pattern_height, > + expected_image) && pass; > + > + /* Also try a downsample blit with mismatched formats > + */ > + glBindFramebuffer(GL_READ_FRAMEBUFFER, src_fbo.handle); > + glBindFramebuffer(GL_DRAW_FRAMEBUFFER, ss_fbo.handle); > + glClear(GL_COLOR_BUFFER_BIT); > glBlitFramebuffer(0, 0, pattern_width, pattern_height, > 0, 0, pattern_width, pattern_height, > GL_COLOR_BUFFER_BIT, GL_NEAREST); > > - /* Blitting between different formats is not allowed for > - * multisample frambuffers. Here GL_INVALID_OPERATION is an > - * expected gl error. > + pass = piglit_check_gl_error(GL_NO_ERROR) && pass; > + > + glBindFramebuffer(GL_FRAMEBUFFER, ss_fbo.handle); > + pass = piglit_probe_image_rgba(0, 0, > + pattern_width, pattern_height, > + expected_image) && pass; > + > + /* Blit the result to the window system buffer so that > + * something will be displayed in a non-automatic > + * test. > */ > - pass = piglit_check_gl_error(GL_INVALID_OPERATION) && pass; > + glBindFramebuffer(GL_READ_FRAMEBUFFER, ss_fbo.handle); > + glBindFramebuffer(GL_DRAW_FRAMEBUFFER, 0); > + glClear(GL_COLOR_BUFFER_BIT); > + glBlitFramebuffer(0, 0, pattern_width, pattern_height, > + 0, 0, pattern_width, pattern_height, > + GL_COLOR_BUFFER_BIT, GL_NEAREST); > + > + pass = piglit_check_gl_error(GL_NO_ERROR) && pass; > + > + piglit_present_results(); > + > + free(expected_image); > } > > return (pass ? PIGLIT_PASS : PIGLIT_FAIL); > @@ -110,8 +219,40 @@ piglit_init(int argc, char **argv) > pattern_width, > pattern_height)); > > + /* Single-sampled FBO used so that we can call glReadPixels to > + * examine the results. > + */ > + ss_fbo.setup(FboConfig(0 /* sample count */, > + pattern_width, > + pattern_height)); > + > if (!piglit_check_gl_error(GL_NO_ERROR)) { > printf("Error setting up frame buffer objects\n"); > piglit_report_result(PIGLIT_FAIL); > } > + > + test_pattern = new ColorGradientSunburst(GL_FLOAT); > + test_pattern->compile(); > + > + glBindFramebuffer(GL_FRAMEBUFFER, src_fbo.handle); > + test_pattern->draw(TestPattern::no_projection); > + > + /* Generate a reference image by downsampling between matching > + * formats. > + */ > + glBindFramebuffer(GL_READ_FRAMEBUFFER, src_fbo.handle); > + glBindFramebuffer(GL_DRAW_FRAMEBUFFER, ss_fbo.handle); > + glBlitFramebuffer(0, 0, pattern_width, pattern_height, > + 0, 0, pattern_width, pattern_height, > + GL_COLOR_BUFFER_BIT, GL_NEAREST); > + if (!piglit_check_gl_error(GL_NO_ERROR)) > + piglit_report_result(PIGLIT_FAIL); > + > + reference_image = (GLfloat *) malloc(pattern_width * > + pattern_height * 4 * > + sizeof(GLfloat)); > + > + glBindFramebuffer(GL_FRAMEBUFFER, ss_fbo.handle); > + glReadPixels(0, 0, pattern_width, pattern_height, > + GL_RGBA, GL_FLOAT, reference_image); > } > -- > 1.9.3 > > _______________________________________________ > Piglit mailing list > Piglit@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/piglit
Patch is: Reviewed-by: Anuj Phogat <anuj.pho...@gmail.com> Maybe you can wait for few days to hear any concerns from other drivers. _______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit