On Mon, Dec 9, 2013 at 10:02 AM, Paul Berry <stereotype...@gmail.com> wrote:
> On 6 November 2013 17:24, Anuj Phogat <anuj.pho...@gmail.com> wrote: > >> V2: Get rid of redundant projection matrix. >> V3: Draw to a multisample texture and use it later to verify >> the expected color of each sample. >> Use piglit_draw_rect() and get rid of redundant code. >> >> Signed-off-by: Anuj Phogat <anuj.pho...@gmail.com> >> --- >> tests/all.tests | 5 + >> .../arb_sample_shading/execution/CMakeLists.gl.txt | 1 + >> .../execution/builtin-gl-sample-mask.cpp | 227 >> +++++++++++++++++++++ >> 3 files changed, 233 insertions(+) >> create mode 100644 >> tests/spec/arb_sample_shading/execution/builtin-gl-sample-mask.cpp >> >> diff --git a/tests/all.tests b/tests/all.tests >> index 5f66e43..eb9f481 100644 >> --- a/tests/all.tests >> +++ b/tests/all.tests >> @@ -1347,6 +1347,11 @@ for num_samples in TEST_SAMPLE_COUNTS: >> executable = 'arb_sample_shading-{0} -auto'.format(test_name) >> arb_sample_shading[test_name] = PlainExecTest(executable) >> >> +for num_samples in TEST_SAMPLE_COUNTS: >> + test_name = 'builtin-gl-sample-mask {0}'.format(num_samples) >> + executable = 'arb_sample_shading-{0} -auto'.format(test_name) >> + arb_sample_shading[test_name] = PlainExecTest(executable) >> + >> # Group ARB_debug_output >> arb_debug_output = Group() >> spec['ARB_debug_output'] = arb_debug_output >> diff --git a/tests/spec/arb_sample_shading/execution/CMakeLists.gl.txt >> b/tests/spec/arb_sample_shading/execution/CMakeLists.gl.txt >> index 35f2905..d2f1f4a 100644 >> --- a/tests/spec/arb_sample_shading/execution/CMakeLists.gl.txt >> +++ b/tests/spec/arb_sample_shading/execution/CMakeLists.gl.txt >> @@ -13,4 +13,5 @@ link_libraries ( >> piglit_add_executable (arb_sample_shading-api api.c) >> piglit_add_executable (arb_sample_shading-builtin-gl-num-samples >> builtin-gl-num-samples.cpp) >> piglit_add_executable (arb_sample_shading-builtin-gl-sample-id >> builtin-gl-sample-id.cpp) >> +piglit_add_executable (arb_sample_shading-builtin-gl-sample-mask >> builtin-gl-sample-mask.cpp) >> # vim: ft=cmake: >> diff --git >> a/tests/spec/arb_sample_shading/execution/builtin-gl-sample-mask.cpp >> b/tests/spec/arb_sample_shading/execution/builtin-gl-sample-mask.cpp >> new file mode 100644 >> index 0000000..2686377 >> --- /dev/null >> +++ b/tests/spec/arb_sample_shading/execution/builtin-gl-sample-mask.cpp >> @@ -0,0 +1,227 @@ >> +/* >> + * Copyright © 2013 Intel Corporation >> + * >> + * Permission is hereby granted, free of charge, to any person obtaining >> a >> + * copy of this software and associated documentation files (the >> "Software"), >> + * to deal in the Software without restriction, including without >> limitation >> + * the rights to use, copy, modify, merge, publish, distribute, >> sublicense, >> + * and/or sell copies of the Software, and to permit persons to whom the >> + * Software is furnished to do so, subject to the following conditions: >> + * >> + * The above copyright notice and this permission notice (including the >> next >> + * paragraph) shall be included in all copies or substantial portions of >> the >> + * Software. >> + * >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, >> EXPRESS OR >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF >> MERCHANTABILITY, >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT >> SHALL >> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR >> OTHER >> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, >> ARISING >> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER >> DEALINGS >> + * IN THE SOFTWARE. >> + */ >> + >> +/** \file builtin-gl-sample-mask.cpp >> + * This test verifies that supplying a value to gl_SampleMask[] >> + * in fragment shader program works as per ARB_sample_shading >> + * specification. >> + **/ >> + >> +#include "piglit-fbo.h" >> +using namespace piglit_util_fbo; >> + >> +const int pattern_width = 128; const int pattern_height = 128; >> + >> +PIGLIT_GL_TEST_CONFIG_BEGIN >> + >> + config.supports_gl_compat_version = 21; >> > > With my recommended changes to the vertex shader below, you should be able > to also add: > > config.suports_gl_core_version = 31; > > >> + >> + config.window_width = pattern_width; >> + config.window_height = pattern_height; >> + config.window_visual = PIGLIT_GL_VISUAL_DOUBLE | >> PIGLIT_GL_VISUAL_RGBA; >> + >> +PIGLIT_GL_TEST_CONFIG_END >> + >> +static int num_samples; >> +static unsigned prog_0, prog_1; >> +static Fbo multisampled_fbo, multisampled_tex, singlesampled_fbo; >> + >> +static void >> +print_usage_and_exit(char *prog_name) >> +{ >> + printf("Usage: %s <num_samples>\n", prog_name); >> + piglit_report_result(PIGLIT_FAIL); >> +} >> + >> +void >> +compile_shader(void) >> +{ >> + static const char *vert = >> + "#version 130\n" >> + "in vec2 pos;\n" >> + "void main()\n" >> + "{\n" >> + " gl_Position = gl_ModelViewProjectionMatrix *\n" >> + " vec4(pos, 0.0, 1.0);\n" >> + "}\n"; >> > > As with the previous patch, I'd recommend using this vertex shader > instead, since it will work properly with piglit_draw_rect(): > > #version 130 > in vec4 piglit_vertex; > void main() > { > gl_Position = piglit_vertex; > } > > >> + static const char *frag_0 = >> + "#version 130\n" >> + "#extension GL_ARB_sample_shading : enable\n" >> + "uniform int sample_mask;\n" >> > > This uniform isn't used anymore--it should be dropped. > > >> + "out vec4 out_color;\n" >> + "void main()\n" >> + "{\n" >> + /* For 128x128 image size, below formula produces a bit >> + * pattern where no two bits of gl_SampleMask[0] are >> + * correlated. >> + */ >> + " gl_SampleMask[0] = (int(gl_FragCoord.x) * 0x10204081) >> ^\n" >> + " (int(gl_FragCoord.y) * >> 0x01010101);\n" >> + " out_color = vec4(0.0, 1.0, 0.0, 1.0);\n" >> + "}\n"; >> + >> + static const char *frag_template = >> + "#version 130\n" >> + "%s\n" >> + "uniform %s tex;\n" >> + "uniform int samples;\n" >> + "out vec4 out_color;\n" >> + "void main()\n" >> + "{\n" >> + " int i = 0;\n" >> + " bool pass = true;\n" >> + " int mask = (int(gl_FragCoord.x) * 0x10204081) ^\n" >> + " (int(gl_FragCoord.y) * 0x01010101);\n" >> + " vec4 green = vec4(0.0, 1.0, 0.0, 1.0);\n" >> + " vec4 black = vec4(0.0, 0.0, 0.0, 0.0);\n" >> + " do {\n" >> > > Any particular reason not to use a for loop here? (i.e. for (int i = 0; i > < samples; i++)) > I used do-while to include testing of 'samples == 0' case. > > >> + " bool is_sample_mask_set = ((mask >> i) & 0x1) == >> 0x1;\n" >> + " vec4 sample_color =\n" >> + " texelFetch(tex, ivec2(gl_FragCoord.xy)%s);\n" >> + "\n" >> + " if ((is_sample_mask_set && sample_color != green) >> ||\n" >> + " (!is_sample_mask_set && sample_color != black)) >> {\n" >> + " pass = false;\n" >> + " break;\n" >> + " }\n" >> + " i++;\n" >> + " } while (i < samples);\n" >> + "\n" >> + " if (pass == true)\n" >> > > This would be clearer as just "if (pass)" > > >> + " out_color = vec4(0.0, 1.0, 0.0, 1.0);\n" >> + " else\n" >> + " out_color = vec4(1.0, 0.0, 0.0, 1.0);\n" >> + "}\n"; >> + >> + /* Compile program */ >> + prog_0 = glCreateProgram(); >> + GLint vs = piglit_compile_shader_text(GL_VERTEX_SHADER, vert); >> + glAttachShader(prog_0, vs); >> + piglit_check_gl_error(GL_NO_ERROR); >> + GLint fs = piglit_compile_shader_text(GL_FRAGMENT_SHADER, frag_0); >> + glAttachShader(prog_0, fs); >> + glBindAttribLocation(prog_0, 0, "pos"); >> + glLinkProgram(prog_0); >> + if (!piglit_link_check_status(prog_0)) { >> + piglit_report_result(PIGLIT_FAIL); >> + } >> + >> + const char *sampler = num_samples ? "sampler2DMS" : >> "sampler2DRect"; >> + const char *extension = num_samples ? >> + "#extension GL_ARB_texture_multisample : require" : ""; >> + unsigned frag_alloc_len = >> + strlen(frag_template) + strlen(sampler) + >> strlen(extension) + 4; >> + char *frag_1 = (char *) malloc(frag_alloc_len); >> + sprintf(frag_1, frag_template, extension, sampler, >> + num_samples ? ", i" : ""); >> > > I agree with Chris Forbes that this would be better expressed with > asprintf(). > > >> + >> + prog_1 = glCreateProgram(); >> + vs = piglit_compile_shader_text(GL_VERTEX_SHADER, vert); >> + glAttachShader(prog_1, vs); >> + piglit_check_gl_error(GL_NO_ERROR); >> + fs = piglit_compile_shader_text(GL_FRAGMENT_SHADER, frag_1); >> + free(frag_1); >> + glAttachShader(prog_1, fs); >> + glBindAttribLocation(prog_1, 0, "pos"); >> + glLinkProgram(prog_1); >> + if (!piglit_link_check_status(prog_1)) { >> + piglit_report_result(PIGLIT_FAIL); >> + } >> +} >> + >> +void >> +piglit_init(int argc, char **argv) >> +{ >> + if (argc != 2) >> + print_usage_and_exit(argv[0]); >> + >> + /* 1st arg: num_samples */ >> + char *endptr = NULL; >> + num_samples = strtol(argv[1], &endptr, 0); >> + if (endptr != argv[1] + strlen(argv[1])) >> + print_usage_and_exit(argv[0]); >> + >> + piglit_require_extension("GL_ARB_texture_multisample"); >> + piglit_require_extension("GL_ARB_sample_shading"); >> + >> + /* Skip the test if num_samples > GL_MAX_SAMPLES */ >> + GLint max_samples; >> + glGetIntegerv(GL_MAX_SAMPLES, &max_samples); >> + if (num_samples > max_samples) >> + piglit_report_result(PIGLIT_SKIP); >> + >> + piglit_ortho_projection(pattern_width, pattern_height, false); >> > > This won't be necessary with my recommended change to the vertex shader > above. > > >> + singlesampled_fbo.setup(FboConfig(0, >> + pattern_width, >> + pattern_height)); >> + >> + FboConfig msConfig(num_samples, pattern_width, pattern_height); >> + multisampled_fbo.setup(msConfig); >> + msConfig.attach_texture = true; >> + multisampled_tex.setup(msConfig); >> + >> + compile_shader(); >> + if (!piglit_check_gl_error(GL_NO_ERROR)) { >> + piglit_report_result(PIGLIT_FAIL); >> + } >> +} >> + >> +enum piglit_result >> +piglit_display() >> +{ >> + bool pass = true; >> + GLint samples; >> + GLfloat expected[4] = {0.0, 1.0, 0.0, 1.0}; >> + >> + glUseProgram(prog_0); >> + glBindFramebuffer(GL_DRAW_FRAMEBUFFER, multisampled_fbo.handle); >> + glGetIntegerv(GL_SAMPLES, &samples); >> + glClear(GL_COLOR_BUFFER_BIT); >> + glUniform1i(glGetUniformLocation(prog_0, "samples"), samples); >> + piglit_draw_rect(0, 0, pattern_width, pattern_height); >> + >> + glBindFramebuffer(GL_READ_FRAMEBUFFER, multisampled_fbo.handle); >> + glBindFramebuffer(GL_DRAW_FRAMEBUFFER, multisampled_tex.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); >> > > As with the previous patch, it seems unnecessary to render into a > multisampled FBO and then blit into a multisampled texture. You should be > able to render into a multisampled texture directly. > > >> + >> + glBindFramebuffer(GL_DRAW_FRAMEBUFFER, piglit_winsys_fbo); >> + glClear(GL_COLOR_BUFFER_BIT); >> + glUseProgram(prog_1); >> + glUniform1i(glGetUniformLocation(prog_1, "tex"), 0); >> + glUniform1i(glGetUniformLocation(prog_1, "samples"), samples); >> + piglit_draw_rect_tex(0, 0, pattern_width, pattern_height, >> + 0, 0, pattern_width, pattern_height); >> > > As with the previous patch, it's not necessary to use > piglit_draw_rect_tex, since your shader doesn't need a texture coordinate. > Just use piglit_draw_rect(). > > >> + >> + glBindFramebuffer(GL_READ_FRAMEBUFFER, piglit_winsys_fbo); >> + pass = piglit_probe_rect_rgba(0, 0, pattern_width, >> + pattern_width, expected) >> + && pass; >> + piglit_present_results(); >> + return pass ? PIGLIT_PASS : PIGLIT_FAIL; >> +} >> -- >> 1.8.1.4 >> >> > Sorry for the delay in reviewing these, Anuj. Once the above issues are > fixed, you can consider this patch: > > Reviewed-by: Paul Berry <stereotype...@gmail.com> >
_______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit