Re: [Piglit] [PATCH] Don't require the same format for mulitisample blits
On Wed, Nov 4, 2015 at 6:45 AM, Neil Robertswrote: > 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 > + * Authors: Anuj Phogat > + * Neil Roberts > */ > > #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)
[Piglit] [PATCH] Don't require the same format for mulitisample blits
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+ * Authors: Anuj Phogat + * Neil Roberts */ #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