<div dir="ltr">On 5 January 2013 01:07, Chris Forbes <<a href="mailto:chrisf@ijw.co.nz" target="_blank">chrisf@ijw.co.nz</a>> wrote: <div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> This tests FBO setup with various combinations of multisample textures and `classic` multisample renderbuffers, and for each, checks: - That the renderbuffers or textures can be created - Completeness status If the configuration is expected to work, additionally: - Actual sample count >= requested sample count - Sample positions meet spec requirements (all obtainable, and in [0,1] Covers the fixedsamplelocations and sample count consistency requirements in the spec. Signed-off-by: Chris Forbes <<a href="mailto:chrisf@ijw.co.nz" target="_blank">chrisf@ijw.co.nz</a>> </blockquote><div> </div><div>I'm not really comfortable with the fact that this test only covers 4x MSAA. We need to test a variety of sample counts since the sample positions are going to be different for different sample counts. I would recommend modeling this on the tests in ext_framebuffer_multisample--these tests take a sample count as an argment, and skip if the requested sample count is larger than the implementation's value of GL_MAX_SAMPLES. In all.tests, we generate an instance of each of these tests for all power-of-two sample counts from 2 to 32. (Note: Marek's patch "all.tests: test 6x MSAA" also adds the non-power of two sample count of 6). </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> --- tests/all.tests | 1 + .../spec/arb_texture_multisample/CMakeLists.gl.txt | 1 + .../spec/arb_texture_multisample/fb-completeness.c | 242 +++++++++++++++++++++ 3 files changed, 244 insertions(+) create mode 100644 tests/spec/arb_texture_multisample/fb-completeness.c diff --git a/tests/all.tests b/tests/all.tests index acaf1ad..379964f 100644 --- a/tests/all.tests +++ b/tests/all.tests @@ -844,6 +844,7 @@ add_plain_test(arb_point_sprite, 'point-sprite') arb_texture_multisample = Group() spec['ARB_texture_multisample'] = arb_texture_multisample add_concurrent_test(arb_texture_multisample, 'arb_texture_multisample-minmax') +add_concurrent_test(arb_texture_multisample, 'arb_texture_multisample-fb-completeness') # Group AMD_shader_stencil_export spec['AMD_shader_stencil_export'] = Group() diff --git a/tests/spec/arb_texture_multisample/CMakeLists.gl.txt b/tests/spec/arb_texture_multisample/CMakeLists.gl.txt index 90dae9e..d793256 100644 --- a/tests/spec/arb_texture_multisample/CMakeLists.gl.txt +++ b/tests/spec/arb_texture_multisample/CMakeLists.gl.txt @@ -11,5 +11,6 @@ link_libraries ( ) piglit_add_executable (arb_texture_multisample-minmax minmax.c) +piglit_add_executable (arb_texture_multisample-fb-completeness fb-completeness.c) # vim: ft=cmake: diff --git a/tests/spec/arb_texture_multisample/fb-completeness.c b/tests/spec/arb_texture_multisample/fb-completeness.c new file mode 100644 index 0000000..44f9786 --- /dev/null +++ b/tests/spec/arb_texture_multisample/fb-completeness.c @@ -0,0 +1,242 @@ +#include "piglit-util-gl-common.h" + +PIGLIT_GL_TEST_CONFIG_BEGIN + + config.supports_gl_compat_version = 30; + + config.window_width = 10; + config.window_height = 10; + config.window_visual = PIGLIT_GL_VISUAL_RGB; + +PIGLIT_GL_TEST_CONFIG_END + +enum piglit_result +piglit_display(void) +{ + return PIGLIT_FAIL; +} + +struct attachment_info +{ + GLenum target; + GLenum attachment; + int samples; + int width; + int height; </blockquote><div> </div><div>It looks liike width and height are 64 in all test variants. I would recommend putting them in a single place rather than in every single attachment_info struct. </div> <div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> + bool fixedsamplelocations; + GLuint format; </blockquote><div> </div><div>It would be nice to have a comment here explaining that if format is 0, it will be automatically chosen based on attachment. That was a stumbling block for me in understanding this code. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> +}; + +struct test_info +{ + char const *name; + int expected; + struct attachment_info attachments[4]; +}; + +struct test_info tests[] = { + { "single_msaa_color", GL_FRAMEBUFFER_COMPLETE, + { { GL_TEXTURE_2D_MULTISAMPLE, GL_COLOR_ATTACHMENT0, 4, 64, 64, GL_TRUE }, + { 0 }, + } + }, + { "msaa_mrt_color", GL_FRAMEBUFFER_COMPLETE, + { { GL_TEXTURE_2D_MULTISAMPLE, GL_COLOR_ATTACHMENT0, 4, 64, 64, GL_TRUE }, + { GL_TEXTURE_2D_MULTISAMPLE, GL_COLOR_ATTACHMENT1, 4, 64, 64, GL_TRUE }, + { 0 }, + } + }, + { "msaa_mixed_texture_and_renderbuffer", GL_FRAMEBUFFER_COMPLETE, + { { GL_TEXTURE_2D_MULTISAMPLE, GL_COLOR_ATTACHMENT0, 4, 64, 64, GL_TRUE }, + { GL_RENDERBUFFER, GL_COLOR_ATTACHMENT0, 4, 64, 64, GL_TRUE }, + { 0 }, + } + }, + { "mixed_msaa_and_plain", GL_FRAMEBUFFER_INCOMPLETE_MULTISAMPLE, + { { GL_TEXTURE_2D_MULTISAMPLE, GL_COLOR_ATTACHMENT0, 4, 64, 64, GL_TRUE }, + { GL_RENDERBUFFER, GL_COLOR_ATTACHMENT1, 0, 64, 64, GL_TRUE }, + { 0 }, + } + }, + { "msaa_mrt_color_nofixed", GL_FRAMEBUFFER_COMPLETE, + { { GL_TEXTURE_2D_MULTISAMPLE, GL_COLOR_ATTACHMENT0, 4, 64, 64, GL_FALSE }, + { GL_TEXTURE_2D_MULTISAMPLE, GL_COLOR_ATTACHMENT1, 4, 64, 64, GL_FALSE }, + { 0 }, + } + }, + { "mix_fixedmode", GL_FRAMEBUFFER_INCOMPLETE_MULTISAMPLE, + { { GL_TEXTURE_2D_MULTISAMPLE, GL_COLOR_ATTACHMENT0, 4, 64, 64, GL_TRUE }, + { GL_TEXTURE_2D_MULTISAMPLE, GL_COLOR_ATTACHMENT1, 4, 64, 64, GL_FALSE }, + { 0 }, + } + }, + { "mix_fixedmode_with_renderbuffer", GL_FRAMEBUFFER_INCOMPLETE_MULTISAMPLE, + { { GL_TEXTURE_2D_MULTISAMPLE, GL_COLOR_ATTACHMENT0, 4, 64, 64, GL_FALSE }, + { GL_RENDERBUFFER, GL_COLOR_ATTACHMENT1, 4, 64, 64, GL_TRUE }, + { 0 }, + } + }, + { "msaa_depth", GL_FRAMEBUFFER_COMPLETE, + { { GL_TEXTURE_2D_MULTISAMPLE, GL_DEPTH_ATTACHMENT, 4, 64, 64, GL_TRUE }, + { 0 }, + } + }, + { "msaa_depth_stencil", GL_FRAMEBUFFER_COMPLETE, + { { GL_TEXTURE_2D_MULTISAMPLE, GL_DEPTH_ATTACHMENT, 4, 64, 64, GL_TRUE, + GL_DEPTH_STENCIL + }, + { 0 }, + } + }, + { "msaa_classic_stencil", GL_FRAMEBUFFER_COMPLETE, + { { GL_TEXTURE_2D_MULTISAMPLE, GL_COLOR_ATTACHMENT0, 4, 64, 64, GL_TRUE }, + { GL_RENDERBUFFER, GL_STENCIL_ATTACHMENT, 4, 64, 64, GL_TRUE }, + { 0 }, + } + }, + { "msaa_stencil", GL_FRAMEBUFFER_COMPLETE, + { { GL_TEXTURE_2D_MULTISAMPLE, GL_COLOR_ATTACHMENT0, 4, 64, 64, GL_TRUE }, + { GL_TEXTURE_2D_MULTISAMPLE, GL_STENCIL_ATTACHMENT, 4, 64, 64, GL_TRUE }, + { 0 }, + } + }, + { 0 }, +}; </blockquote><div> </div><div>Our preferred way of handling situations like this, where a test has multiple variants, is to select the variant using a command line argument, and then have all.tests invoke the test separately using each possible variant. That way, the piglit framework can track failures at the granularity of the variants, rather than just considering the entire test to pass or fail. See for example b/tests/spec/ext_transform_feedback/change-size.c. </div><div>I'm not comfortable with the fact that this test only tests a sample count of 4 (and occasionally 0). At least one piece of hardware I know of (Ivy Bridge) </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> + +static GLuint +choose_format(struct attachment_info *att) +{ + if (att->format) + return att->format; + + switch(att->attachment) { + case GL_DEPTH_ATTACHMENT: + return GL_DEPTH_COMPONENT; + case GL_STENCIL_ATTACHMENT: + return GL_STENCIL_INDEX; + default: + return GL_RGBA; + } +} + +static enum piglit_result +check_sample_positions(int expected_sample_count) +{ + GLint samples; + int i; + + glGetIntegerv(GL_SAMPLES, &samples); + if (!piglit_check_gl_error(GL_NO_ERROR)) + return PIGLIT_FAIL; + + if (samples < expected_sample_count) { + printf("Expected sample count at least %d, got %d\n", + expected_sample_count, samples); + return PIGLIT_FAIL; + } + + for (i = 0; i < samples; i++) { + float sample_pos[2]; + + glGetMultisamplefv(GL_SAMPLE_POSITION, i, sample_pos); + + if (!piglit_check_gl_error(GL_NO_ERROR)) + return PIGLIT_FAIL; + + printf("Sample %d position %2.2f %2.2f\n", + i, sample_pos[0], sample_pos[1] ); + + if (sample_pos[0] < 0 || sample_pos[0] > 1 || + sample_pos[1] < 0 || sample_pos[1] > 1) { + printf("Sample %d out of range\n", i ); + return PIGLIT_FAIL; + } + } + + return PIGLIT_PASS; +} + +static enum piglit_result +exec_test(struct test_info *info) +{ + GLuint fb, tex, rb; + GLint result; + struct attachment_info *att; + + glGenFramebuffers(1, &fb); + glBindFramebuffer(GL_FRAMEBUFFER, fb); + + printf("Testing fbo completeness for config '%s'\n", info->name); + + for (att=info->attachments; att->target; att++) { + printf(" Att target=%x att=%x samples=%d dims=%d,%d fixed=%d\n", + att->target, att->attachment, att->samples, + att->width, att->height, att->fixedsamplelocations); + + switch (att->target) { + case GL_TEXTURE_2D_MULTISAMPLE: + glGenTextures(1, &tex); + glBindTexture(GL_TEXTURE_2D_MULTISAMPLE, tex); + glTexImage2DMultisample(GL_TEXTURE_2D_MULTISAMPLE, + att->samples, choose_format(att), + att->width, att->height, + att->fixedsamplelocations); + + if (!piglit_check_gl_error(GL_NO_ERROR)) + return PIGLIT_FAIL; + + glFramebufferTexture2D(GL_FRAMEBUFFER, att->attachment, + att->target, tex, 0); + break; + + case GL_RENDERBUFFER: + /* RENDERBUFFER has fixedsamplelocations implicitly */ + assert(att->fixedsamplelocations); + glGenRenderbuffers(1, &rb); + glBindRenderbuffer(GL_RENDERBUFFER, rb); + if (att->samples == 0) { + /* non-MSAA renderbuffer */ + glRenderbufferStorage(GL_RENDERBUFFER, choose_format(att), + att->width, att->height); + } + else { + glRenderbufferStorageMultisample(GL_RENDERBUFFER, + att->samples, choose_format(att), + att->width, att->height); + } + + glFramebufferRenderbuffer(GL_FRAMEBUFFER, + att->attachment, att->target, rb); + + if (!piglit_check_gl_error(GL_NO_ERROR)) + return PIGLIT_FAIL; + break; + + default: + assert(!"Unsupported target"); + } + } + + result = glCheckFramebufferStatus(GL_FRAMEBUFFER); + printf("glCheckFramebufferStatus: expected %d, got %d\n", + info->expected, result); + if (result != info->expected) + return PIGLIT_FAIL; + + if (result == GL_FRAMEBUFFER_COMPLETE) + return check_sample_positions(info->attachments->samples); + + return PIGLIT_PASS; +} + +void +piglit_init(int argc, char **argv) +{ + struct test_info *info; + enum piglit_result result = PIGLIT_PASS; + + for (info = tests; info->name; info++) + piglit_merge_result(&result, exec_test(info)); + + piglit_report_result(result); +} -- 1.8.1 _______________________________________________ Piglit mailing list <a href="mailto:Piglit@lists.freedesktop.org" target="_blank">Piglit@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/piglit" target="_blank">http://lists.freedesktop.org/mailman/listinfo/piglit</a> </blockquote></div> </div></div>