[Piglit] [PATCH 14/16] msaa: Verify accuracy of sRGB MSAA resolves.

Marek Olšák maraeo at gmail.com
Mon Dec 9 10:13:50 PST 2013


On Fri, Jun 15, 2012 at 5:32 PM, Paul Berry <stereotype441 at gmail.com> wrote:
> From the GL spec, version 4.2, section 4.1.11 (Additional Multisample
> Fragment Operations):
>
>     If a framebuffer object is not bound, after all operations have
>     been completed on the multisample buffer, the sample values for
>     each color in the multisample buffer are combined to produce a
>     single color value, and that value is written into the
>     corresponding color buffers selected by DrawBuffer or
>     DrawBuffers. An implementation may defer the writing of the color
>     buffers until a later time, but the state of the framebuffer must
>     behave as if the color buffers were updated as each fragment was
>     processed. The method of combination is not specified. If the
>     framebuffer contains sRGB values, then it is recommended that the
>     an average of sample values is computed in a linearized space, as
>     for blending (see section 4.1.7).

I think this is wrong. Using a quote from the spec that explicitly
states "If a framebuffer object is not bound ..." and then fixing a
test with it that only tests the case where the framebuffer object IS
bound... it's a contradiction.

Also, if the spec recommends a certain behavior but doesn't require
it, failing to comply with the behavior should report WARN and not
FAIL.

Anyway, according to the GL 4.4 spec, most of our piglit sRGB
glBlitFramebuffer tests are wrong.

Marek

>
> This patch adds a new "srgb" mode to the MSAA accuracy test, which
> verifies that the formula used by the implementation to blend sRGB
> samples matches the GL spec's recommendations.  When an "srgb"
> accuracy test is requested, the test is modified to do the following
> things:
>
> - Render using a buffer format of SRGB8_ALPHA8 instead of RGBA.
>
> - When manually downsampling the reference image, enable
>   FRAMEBUFFER_SRGB, so that the data output by the manual downsampler
>   gets converted from linear color space into sRGB color space.  This
>   ensures that the reference image is generated according to the
>   spec's recommendations.
>
> - Convert pixels from sRGB color space to linear color space before
>   comparing the test image and the reference image.  This ensures that
>   the RMS error is computed in linear color space, so that the RMS
>   error computed for sRGB mode is comparable with the RMS error
>   computed for linear RGBA color.
> ---
>  tests/all.tests                                    |    2 +-
>  .../spec/ext_framebuffer_multisample/accuracy.cpp  |    3 +
>  tests/spec/ext_framebuffer_multisample/common.cpp  |   55 ++++++++++++++++---
>  tests/spec/ext_framebuffer_multisample/common.h    |    7 ++-
>  4 files changed, 55 insertions(+), 12 deletions(-)
>
> diff --git a/tests/all.tests b/tests/all.tests
> index 2a636f3..755eace 100644
> --- a/tests/all.tests
> +++ b/tests/all.tests
> @@ -1333,7 +1333,7 @@ ext_framebuffer_multisample['renderbuffer-samples'] = concurrent_test('ext_frame
>  ext_framebuffer_multisample['samples'] = concurrent_test('ext_framebuffer_multisample-samples')
>
>  for num_samples in MSAA_SAMPLE_COUNTS:
> -        for test_type in ('color', 'stencil_draw', 'stencil_resolve',
> +        for test_type in ('color', 'srgb', 'stencil_draw', 'stencil_resolve',
>                            'depth_draw', 'depth_resolve'):
>                  for options in power_set(('small', 'depthstencil')):
>                          test_name = ' '.join(['accuracy', str(num_samples), test_type]
> diff --git a/tests/spec/ext_framebuffer_multisample/accuracy.cpp b/tests/spec/ext_framebuffer_multisample/accuracy.cpp
> index 1b7ac1c..0462db0 100644
> --- a/tests/spec/ext_framebuffer_multisample/accuracy.cpp
> +++ b/tests/spec/ext_framebuffer_multisample/accuracy.cpp
> @@ -56,6 +56,7 @@ print_usage_and_exit(char *prog_name)
>         printf("Usage: %s <num_samples> <test_type> [options]\n"
>                "  where <test_type> is one of:\n"
>                "    color: test downsampling of color buffer\n"
> +              "    srgb: test downsampling of srgb color buffer\n"
>                "    stencil_draw: test drawing using MSAA stencil buffer\n"
>                "    stencil_resolve: test resolve of MSAA stencil buffer\n"
>                "    depth_draw: test drawing using MSAA depth buffer\n"
> @@ -105,6 +106,8 @@ piglit_init(int argc, char **argv)
>         test_type_enum test_type;
>         if (strcmp(argv[2], "color") == 0) {
>                 test_type = TEST_TYPE_COLOR;
> +       } else if (strcmp(argv[2], "srgb") == 0) {
> +               test_type = TEST_TYPE_SRGB;
>         } else if (strcmp(argv[2], "stencil_draw") == 0) {
>                 test_type = TEST_TYPE_STENCIL_DRAW;
>         } else if (strcmp(argv[2], "stencil_resolve") == 0) {
> diff --git a/tests/spec/ext_framebuffer_multisample/common.cpp b/tests/spec/ext_framebuffer_multisample/common.cpp
> index e1060a8..c211363 100644
> --- a/tests/spec/ext_framebuffer_multisample/common.cpp
> +++ b/tests/spec/ext_framebuffer_multisample/common.cpp
> @@ -342,7 +342,8 @@ DownsampleProg::compile(int supersample_factor)
>  }
>
>  void
> -DownsampleProg::run(const Fbo *src_fbo, int dest_width, int dest_height)
> +DownsampleProg::run(const Fbo *src_fbo, int dest_width, int dest_height,
> +                   bool srgb)
>  {
>         float w = dest_width;
>         float h = dest_height;
> @@ -363,7 +364,15 @@ DownsampleProg::run(const Fbo *src_fbo, int dest_width, int dest_height)
>         glBufferData(GL_ARRAY_BUFFER, sizeof(vertex_data), vertex_data,
>                      GL_STREAM_DRAW);
>
> +       if (srgb) {
> +               /* If we're testing sRGB color, instruct OpenGL to
> +                * convert the output of the fragment shader from
> +                * linear color space to sRGB color space.
> +                */
> +               glEnable(GL_FRAMEBUFFER_SRGB);
> +       }
>         glDrawElements(GL_TRIANGLES, 6, GL_UNSIGNED_INT, (void *) 0);
> +       glDisable(GL_FRAMEBUFFER_SRGB);
>  }
>
>  void
> @@ -1156,11 +1165,12 @@ Stats::is_better_than(double rms_error_threshold)
>  }
>
>  Test::Test(TestPattern *pattern, ManifestProgram *manifest_program,
> -          bool test_resolve, GLbitfield blit_type)
> +          bool test_resolve, GLbitfield blit_type, bool srgb)
>         : pattern(pattern),
>           manifest_program(manifest_program),
>           test_resolve(test_resolve),
> -         blit_type(blit_type)
> +         blit_type(blit_type),
> +         srgb(srgb)
>  {
>  }
>
> @@ -1176,6 +1186,8 @@ Test::init(int num_samples, bool small, bool combine_depth_stencil,
>         FboConfig test_fbo_config(0,
>                                   small ? 16 : pattern_width,
>                                   small ? 16 : pattern_height);
> +       if (srgb)
> +               test_fbo_config.color_internalformat = GL_SRGB8_ALPHA8;
>         test_fbo_config.combine_depth_stencil = combine_depth_stencil;
>         test_fbo.setup(test_fbo_config);
>
> @@ -1235,7 +1247,7 @@ Test::downsample_color(int downsampled_width, int downsampled_height)
>         downsample_fbo.set_viewport();
>         downsample_prog.run(&supersample_fbo,
>                             downsample_fbo.config.width,
> -                           downsample_fbo.config.height);
> +                           downsample_fbo.config.height, srgb);
>  }
>
>  /**
> @@ -1371,6 +1383,20 @@ Test::draw_reference_image()
>  }
>
>  /**
> + * Convert from sRGB color space to linear color space, using the
> + * formula from the GL 3.0 spec, section 4.1.8 (sRGB Texture Color
> + * Conversion).
> + */
> +float
> +decode_srgb(float x)
> +{
> +       if (x <= 0.0405)
> +               return x / 12.92;
> +       else
> +               return pow((x + 0.055) / 1.055, 2.4);
> +}
> +
> +/**
>   * Measure the accuracy of MSAA downsampling.  Pixels that are fully
>   * on or off in the reference image are required to be fully on or off
>   * in the test image.  Pixels that are not fully on or off in the
> @@ -1403,6 +1429,14 @@ Test::measure_accuracy()
>                                 int pixel_pos = 4*(y*pattern_width + x) + c;
>                                 float ref = reference_data[pixel_pos];
>                                 float test = test_data[pixel_pos];
> +                               /* When testing sRGB, compare pixels
> +                                * linearly so that the measured error
> +                                * is comparable to the non-sRGB case.
> +                                */
> +                               if (srgb && c < 3) {
> +                                       ref = decode_srgb(ref);
> +                                       test = decode_srgb(test);
> +                               }
>                                 if (ref <= 0.0)
>                                         unlit_stats.record(test - ref);
>                                 else if (ref >= 1.0)
> @@ -1464,29 +1498,32 @@ create_test(test_type_enum test_type, int n_samples, bool small,
>         Test *test = NULL;
>         switch (test_type) {
>         case TEST_TYPE_COLOR:
> -               test = new Test(new Triangles(), NULL, false, 0);
> +               test = new Test(new Triangles(), NULL, false, 0, false);
> +               break;
> +       case TEST_TYPE_SRGB:
> +               test = new Test(new Triangles(), NULL, false, 0, true);
>                 break;
>         case TEST_TYPE_STENCIL_DRAW:
>                 test = new Test(new StencilSunburst(),
>                                 new ManifestStencil(),
> -                               false, 0);
> +                               false, 0, false);
>                 break;
>         case TEST_TYPE_STENCIL_RESOLVE:
>                 test = new Test(new StencilSunburst(),
>                                 new ManifestStencil(),
>                                 true,
> -                               GL_STENCIL_BUFFER_BIT);
> +                               GL_STENCIL_BUFFER_BIT, false);
>                 break;
>         case TEST_TYPE_DEPTH_DRAW:
>                 test = new Test(new DepthSunburst(),
>                                 new ManifestDepth(),
> -                               false, 0);
> +                               false, 0, false);
>                 break;
>         case TEST_TYPE_DEPTH_RESOLVE:
>                 test = new Test(new DepthSunburst(),
>                                 new ManifestDepth(),
>                                 true,
> -                               GL_DEPTH_BUFFER_BIT);
> +                               GL_DEPTH_BUFFER_BIT, false);
>                 break;
>         default:
>                 printf("Unrecognized test type\n");
> diff --git a/tests/spec/ext_framebuffer_multisample/common.h b/tests/spec/ext_framebuffer_multisample/common.h
> index e7cc9b7..a248dbc 100644
> --- a/tests/spec/ext_framebuffer_multisample/common.h
> +++ b/tests/spec/ext_framebuffer_multisample/common.h
> @@ -31,6 +31,7 @@
>
>  enum test_type_enum {
>         TEST_TYPE_COLOR,
> +       TEST_TYPE_SRGB,
>         TEST_TYPE_STENCIL_DRAW,
>         TEST_TYPE_STENCIL_RESOLVE,
>         TEST_TYPE_DEPTH_DRAW,
> @@ -139,7 +140,8 @@ class DownsampleProg
>  {
>  public:
>         void compile(int supersample_factor);
> -       void run(const Fbo *src_fbo, int dest_width, int dest_height);
> +       void run(const Fbo *src_fbo, int dest_width, int dest_height,
> +                bool srgb);
>
>  private:
>         GLint prog;
> @@ -424,7 +426,7 @@ class Test
>  {
>  public:
>         Test(TestPattern *pattern, ManifestProgram *manifest_program,
> -            bool test_resolve, GLbitfield blit_type);
> +            bool test_resolve, GLbitfield blit_type, bool srgb);
>         void init(int num_samples, bool small, bool combine_depth_stencil,
>                   int pattern_width, int pattern_height,
>                   int supersample_factor);
> @@ -497,6 +499,7 @@ private:
>         int pattern_width;
>         int pattern_height;
>         int supersample_factor;
> +       bool srgb;
>         DownsampleProg downsample_prog;
>  };
>
> --
> 1.7.7.6
>
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit


More information about the Piglit mailing list