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