Re: [Piglit] [PATCH 2/2] multisample/formats: Check for spec-complient integer resolves

2016-05-10 Thread Matt Turner
On Wed, May 4, 2016 at 4:23 PM, Jason Ekstrand  wrote:
> The i965 driver has historically done integer multisample resolves with a
> sort of integer averaging operation.  The spec used to not really say
> anything at all about integer formats when discussing multisampling.  Since
> the tests were written, the spec has been substantially clerified.  The new
> rule is that integer MSAA resolves are supposed to pick a single arbitrary
> sample for each texel rather than trying to combine them in any way.
>
> Signed-off-by: Jason Ekstrand 
> ---
>  tests/spec/ext_framebuffer_multisample/formats.cpp | 196 
> -
>  1 file changed, 155 insertions(+), 41 deletions(-)
>
> diff --git a/tests/spec/ext_framebuffer_multisample/formats.cpp 
> b/tests/spec/ext_framebuffer_multisample/formats.cpp
> index 4b1feba..1024b45 100644
> --- a/tests/spec/ext_framebuffer_multisample/formats.cpp
> +++ b/tests/spec/ext_framebuffer_multisample/formats.cpp
> @@ -70,6 +70,44 @@ ColorGradientSunburst *test_pattern_vec4;
>  ColorGradientSunburst *test_pattern_ivec4;
>  ColorGradientSunburst *test_pattern_uvec4;
>
> +struct int_resolve_check_s {
> +   GLuint prog;
> +   GLint u_resolve;
> +   GLint u_msaa;
> +   GLint u_samples;
> +} int_resolve_check;
> +
> +const char int_resolve_check_vs[] =
> +   "#version 130\n"
> +   "in vec4 piglit_vertex;\n"
> +   "void main() {\n"
> +   "   gl_Position = piglit_vertex;\n"
> +   "}\n";
> +
> +/* The OpenGL ES 3.2 and OpenGL 4.4 specs both state:
> + *
> + * "If the source for- mats are integer types or stencil values, a

formats

> + * single sample’s value is selected for each pixel."

smart quote

> + *
> + * This shader checks exactly that condition, namely that the result of the
> + * resolve is exactly one of the sample values in the original image.
> + */
> +const char int_resolve_check_fs[] =
> +   "#version 130\n"
> +   "#extension GL_ARB_texture_multisample: require\n"
> +   "uniform isampler2DMS msaa;\n"
> +   "uniform isampler2D resolve;\n"
> +   "uniform int samples;\n"
> +   "void main() {\n"
> +   "   gl_FragColor = vec4(1.0, 0.0, 0.0, 1.0);\n"
> +   "   const vec4 green = vec4(0.0, 1.0, 0.0, 1.0);\n"
> +   "   ivec2 pos = ivec2(gl_FragCoord.xy);\n"
> +   "   ivec4 res = texelFetch(resolve, pos, 0);\n"
> +   "   for (int s = 0; s < samples; s++) {\n"
> +   "   if (res == texelFetch(msaa, pos, s))\n"
> +   "   gl_FragColor = green;\n"

Could break here

> +   "   }\n"
> +   "}\n";
>
>  /**
>   * This class encapsulates the code necessary to draw the test pattern
> @@ -80,7 +118,7 @@ ColorGradientSunburst *test_pattern_uvec4;
>  class PatternRenderer
>  {
>  public:
> -   bool try_setup(GLenum internalformat);
> +   bool try_setup(GLenum internalformat, bool is_integer);
> void set_piglit_tolerance();
> void set_color_clamping_mode();
> void draw();
> @@ -140,11 +178,18 @@ public:
>   * incomplete.
>   */
>  bool
> -PatternRenderer::try_setup(GLenum internalformat)
> +PatternRenderer::try_setup(GLenum internalformat, bool is_integer)
>  {
> FboConfig config_downsampled(0, pattern_width, pattern_height);
> config_downsampled.color_internalformat = internalformat;
>
> +   if (is_integer) {
> +   config_downsampled.color_format = GL_RGBA_INTEGER;
> +   config_downsampled.num_rb_attachments = 0;
> +   config_downsampled.num_tex_attachments = 1;
> +   config_downsampled.use_rect = false;
> +   }
> +
> FboConfig config_msaa = config_downsampled;
> config_msaa.num_samples = num_samples;
>
> @@ -497,7 +542,9 @@ test_format(const struct format_desc *format)
>  * supported, we might have received a GL error.  In either
>  * case just skip to the next format.
>  */
> -   bool setup_success = test_renderer.try_setup(format->internalformat);
> +   bool is_integer = (test_sets[test_index].basetype == GL_INT);
> +   bool setup_success = test_renderer.try_setup(format->internalformat,
> +is_integer);
> if (glGetError() != GL_NO_ERROR) {
> printf("Error setting up test renderbuffers\n");
> return PIGLIT_SKIP;
> @@ -511,7 +558,8 @@ test_format(const struct format_desc *format)
>  * This shouldn't fail.
>  */
> setup_success = ref_renderer.try_setup(test_renderer.is_srgb ?
> -  GL_SRGB8_ALPHA8 : GL_RGBA);
> +  GL_SRGB8_ALPHA8 : GL_RGBA,
> +  false);
> if (!piglit_check_gl_error(GL_NO_ERROR)) {
> printf("Error setting up reference renderbuffers\n");
> return PIGLIT_FAIL;
> @@ -521,50 +569,9

[Piglit] [PATCH 2/2] multisample/formats: Check for spec-complient integer resolves

2016-05-04 Thread Jason Ekstrand
The i965 driver has historically done integer multisample resolves with a
sort of integer averaging operation.  The spec used to not really say
anything at all about integer formats when discussing multisampling.  Since
the tests were written, the spec has been substantially clerified.  The new
rule is that integer MSAA resolves are supposed to pick a single arbitrary
sample for each texel rather than trying to combine them in any way.

Signed-off-by: Jason Ekstrand 
---
 tests/spec/ext_framebuffer_multisample/formats.cpp | 196 -
 1 file changed, 155 insertions(+), 41 deletions(-)

diff --git a/tests/spec/ext_framebuffer_multisample/formats.cpp 
b/tests/spec/ext_framebuffer_multisample/formats.cpp
index 4b1feba..1024b45 100644
--- a/tests/spec/ext_framebuffer_multisample/formats.cpp
+++ b/tests/spec/ext_framebuffer_multisample/formats.cpp
@@ -70,6 +70,44 @@ ColorGradientSunburst *test_pattern_vec4;
 ColorGradientSunburst *test_pattern_ivec4;
 ColorGradientSunburst *test_pattern_uvec4;
 
+struct int_resolve_check_s {
+   GLuint prog;
+   GLint u_resolve;
+   GLint u_msaa;
+   GLint u_samples;
+} int_resolve_check;
+
+const char int_resolve_check_vs[] =
+   "#version 130\n"
+   "in vec4 piglit_vertex;\n"
+   "void main() {\n"
+   "   gl_Position = piglit_vertex;\n"
+   "}\n";
+
+/* The OpenGL ES 3.2 and OpenGL 4.4 specs both state:
+ *
+ * "If the source for- mats are integer types or stencil values, a
+ * single sample’s value is selected for each pixel."
+ *
+ * This shader checks exactly that condition, namely that the result of the
+ * resolve is exactly one of the sample values in the original image.
+ */
+const char int_resolve_check_fs[] =
+   "#version 130\n"
+   "#extension GL_ARB_texture_multisample: require\n"
+   "uniform isampler2DMS msaa;\n"
+   "uniform isampler2D resolve;\n"
+   "uniform int samples;\n"
+   "void main() {\n"
+   "   gl_FragColor = vec4(1.0, 0.0, 0.0, 1.0);\n"
+   "   const vec4 green = vec4(0.0, 1.0, 0.0, 1.0);\n"
+   "   ivec2 pos = ivec2(gl_FragCoord.xy);\n"
+   "   ivec4 res = texelFetch(resolve, pos, 0);\n"
+   "   for (int s = 0; s < samples; s++) {\n"
+   "   if (res == texelFetch(msaa, pos, s))\n"
+   "   gl_FragColor = green;\n"
+   "   }\n"
+   "}\n";
 
 /**
  * This class encapsulates the code necessary to draw the test pattern
@@ -80,7 +118,7 @@ ColorGradientSunburst *test_pattern_uvec4;
 class PatternRenderer
 {
 public:
-   bool try_setup(GLenum internalformat);
+   bool try_setup(GLenum internalformat, bool is_integer);
void set_piglit_tolerance();
void set_color_clamping_mode();
void draw();
@@ -140,11 +178,18 @@ public:
  * incomplete.
  */
 bool
-PatternRenderer::try_setup(GLenum internalformat)
+PatternRenderer::try_setup(GLenum internalformat, bool is_integer)
 {
FboConfig config_downsampled(0, pattern_width, pattern_height);
config_downsampled.color_internalformat = internalformat;
 
+   if (is_integer) {
+   config_downsampled.color_format = GL_RGBA_INTEGER;
+   config_downsampled.num_rb_attachments = 0;
+   config_downsampled.num_tex_attachments = 1;
+   config_downsampled.use_rect = false;
+   }
+
FboConfig config_msaa = config_downsampled;
config_msaa.num_samples = num_samples;
 
@@ -497,7 +542,9 @@ test_format(const struct format_desc *format)
 * supported, we might have received a GL error.  In either
 * case just skip to the next format.
 */
-   bool setup_success = test_renderer.try_setup(format->internalformat);
+   bool is_integer = (test_sets[test_index].basetype == GL_INT);
+   bool setup_success = test_renderer.try_setup(format->internalformat,
+is_integer);
if (glGetError() != GL_NO_ERROR) {
printf("Error setting up test renderbuffers\n");
return PIGLIT_SKIP;
@@ -511,7 +558,8 @@ test_format(const struct format_desc *format)
 * This shouldn't fail.
 */
setup_success = ref_renderer.try_setup(test_renderer.is_srgb ?
-  GL_SRGB8_ALPHA8 : GL_RGBA);
+  GL_SRGB8_ALPHA8 : GL_RGBA,
+  false);
if (!piglit_check_gl_error(GL_NO_ERROR)) {
printf("Error setting up reference renderbuffers\n");
return PIGLIT_FAIL;
@@ -521,50 +569,99 @@ test_format(const struct format_desc *format)
return PIGLIT_FAIL;
}
 
-   /* Draw test and reference images, and read them into memory */
-   test_renderer.set_piglit_tolerance();
test_renderer.draw();
float *test_image =
test_renderer.read_image(f