[Piglit] [PATCH] khr_texture_compression_astc: Enable subtest reports

Dylan Baker baker.dylan.c at gmail.com
Mon Nov 30 17:04:54 PST 2015


On Mon, Nov 30, 2015 at 04:34:40PM -0800, Nanley Chery wrote:
> On Mon, Nov 30, 2015 at 03:50:04PM -0500, Ilia Mirkin wrote:
> > On Wed, Oct 28, 2015 at 7:55 PM, Nanley Chery <nanleychery at gmail.com> wrote:
> > > From: Nanley Chery <nanley.g.chery at intel.com>
> > >
> > > Enable Piglit to report the result of each individual subtest
> > > for the array and miptree ASTC tests. Modify the miptree test
> > > to only run one subset of ASTC formats at a time.
> > >
> > > v2. Modify miptree test to only check the given subtest.
> > >     Use the -subtest option to run a specific subtest.
> > >
> > > Signed-off-by: Nanley Chery <nanley.g.chery at intel.com>
> > > ---
> > >  tests/all.py                                       | 12 ++-
> > >  .../khr_compressed_astc-miptree.c                  | 90 +++++++++-------------
> > >  2 files changed, 45 insertions(+), 57 deletions(-)
> > >
> > > diff --git a/tests/all.py b/tests/all.py
> > > index 82e6311..c63cf15 100644
> > > --- a/tests/all.py
> > > +++ b/tests/all.py
> > > @@ -4182,12 +4182,16 @@ with profile.group_manager(
> > >           PiglitGLTest,
> > >           grouptools.join('spec', 'khr_texture_compression_astc')) as g:
> > >      g(['arb_texture_compression-invalid-formats', 'astc'], 'invalid formats')
> > > -    g(['khr_compressed_astc-array_gl'], 'array-gl')
> > > -    g(['khr_compressed_astc-array_gles3'], 'array-gles')
> > >      g(['khr_compressed_astc-basic_gl'], 'basic-gl')
> > >      g(['khr_compressed_astc-basic_gles2'], 'basic-gles')
> > > -    g(['khr_compressed_astc-miptree_gl'], 'miptree-gl')
> > > -    g(['khr_compressed_astc-miptree_gles2'], 'miptree-gles')
> > > +
> > > +    for subtest in ('odd', 'even'):
> > > +        g(['khr_compressed_astc-array_gl', '-subtest', subtest])
> > > +        g(['khr_compressed_astc-array_gles3', '-subtest', subtest])
> > > +
> > > +    for subtest in ('ldr', 'srgb', 'hdr'):
> > > +        g(['khr_compressed_astc-miptree_gl', '-subtest', subtest])
> > > +        g(['khr_compressed_astc-miptree_gles2', '-subtest', subtest])
> > 
> > Won't it auto-run all the subtests on its own? It did that for me, I'm
> > pretty sure.
> > 
> 
> It does auto-run all of them by default, however it is useful to know
> which specific case is causing the test to fail.

Alternatively you could use piglit_report_subtest_result when the test
runs all subtests.

> 
> > >
> > >  with profile.group_manager(
> > >           PiglitGLTest,
> > > diff --git a/tests/spec/khr_texture_compression_astc/khr_compressed_astc-miptree.c b/tests/spec/khr_texture_compression_astc/khr_compressed_astc-miptree.c
> > > index 20f2415..20d1c79 100644
> > > --- a/tests/spec/khr_texture_compression_astc/khr_compressed_astc-miptree.c
> > > +++ b/tests/spec/khr_texture_compression_astc/khr_compressed_astc-miptree.c
> > > @@ -222,10 +222,9 @@ bool draw_compare_levels(bool check_error, bool check_srgb,
> > >  enum piglit_result
> > >  test_miptrees(void* input_type)
> > >  {
> > > -       int subtest =  0;
> > > -       enum test_type * type = (enum test_type*) input_type;
> > > -       bool is_srgb_test = *type == TEST_TYPE_SRGB;
> > > -       bool is_hdr_test  = *type == TEST_TYPE_HDR;
> > > +       const enum test_type subtest = *(enum test_type*) input_type;
> > > +       const bool is_srgb_test = subtest == TEST_TYPE_SRGB;
> > > +       const bool is_hdr_test  = subtest == TEST_TYPE_HDR;
> > >
> > >         static const char * tests[3] = {"hdr", "ldrl", "ldrs"};
> > >         static const char * block_dim_str[14] = {
> > > @@ -245,62 +244,47 @@ test_miptrees(void* input_type)
> > >                 "12x12"
> > >         };
> > >
> > > -       bool has_hdr = piglit_is_extension_supported(
> > > -                       "GL_KHR_texture_compression_astc_hdr");
> > > -
> > > -       /* If testing sRGB mode, fast-forward to the srgb test. */
> > > -       if (is_srgb_test) {
> > > -               subtest =  TEST_TYPE_SRGB;
> > > -       } else {
> > > -               /* Skip if on an HDR system not running the HDR test
> > > -                * or if on an LDR system running the HDR test.
> > > -                */
> > > -               if (has_hdr != is_hdr_test)
> > > -                       return PIGLIT_SKIP;
> > > +       if (!is_srgb_test)
> > >                 piglit_require_extension("GL_EXT_texture_sRGB_decode");
> > 
> > Which we sadly don't expose in GLES. (Although isn't the functionality
> > part of GLES3?)
> > 
> 
> Yes, we do no expose this in GLES. I'm not sure if it's part of GLES3.
> I think someone should implement this in mesa. I started to do so, but
> received some higher priority tasks.
> 
> > >
> > > -       }
> > > -
> > >         GLint pixel_offset_loc = glGetUniformLocation(prog, "pixel_offset");
> > >         GLint level_pixel_size_loc = glGetUniformLocation(prog,
> > >                                                         "level_pixel_size");
> > >
> > > -       /* Test each submode */
> > > -       for (; subtest < ARRAY_SIZE(tests); ++subtest) {
> > > -
> > > -               /*  Check for error color if an LDR-only sys reading an HDR
> > > -                *  texture. No need to draw a reference mipmap in this case.
> > > -                */
> > > -               int check_error = !has_hdr && subtest == TEST_TYPE_HDR;
> > > -               int block_dims = 0;
> > > -               for (; block_dims < ARRAY_SIZE(block_dim_str); ++block_dims) {
> > > -
> > > -                       /* Texture objects. */
> > > -                       GLuint tex_compressed;
> > > -                       GLuint tex_decompressed;
> > > -
> > > -                       /* Load texture for current submode and block size */
> > > -                       load_texture("compressed", tests[subtest],
> > > -                                       block_dim_str[block_dims],
> > > -                                       &tex_compressed);
> > > -                       if (!check_error) {
> > > -                               load_texture("decompressed", tests[subtest],
> > > -                                               block_dim_str[block_dims],
> > > -                                               &tex_decompressed);
> > > -                       }
> > > +       /*  Check for error color if an LDR-only sys reading an HDR
> > > +        *  texture. No need to draw a reference mipmap in this case.
> > > +        */
> > > +       const bool has_hdr = piglit_is_extension_supported(
> > > +               "GL_KHR_texture_compression_astc_hdr");
> > > +       const int check_error = is_hdr_test && !has_hdr;
> > 
> > bool
> > 
> 
> Will fix.
> 
> > > +       int block_dims = 0;
> > > +       for (; block_dims < ARRAY_SIZE(block_dim_str); ++block_dims) {
> > 
> > The traditional way of writing this is
> > 
> > int block_dims;
> > 
> > for (block_dims = 0; block_dims < ... )
> > 
> 
> Will update.
> 
> > > +
> > > +               /* Texture objects. */
> > > +               GLuint tex_compressed;
> > > +               GLuint tex_decompressed;
> > 
> > Initialize these to 0, otherwise you might accidentally delete a random texture.
> > 
> 
> Will update.
> 
> > > +
> > > +               /* Load texture for current submode and block size */
> > > +               load_texture("compressed", tests[subtest],
> > > +                       block_dim_str[block_dims],
> > > +                       &tex_compressed);
> > > +               if (!check_error) {
> > > +                       load_texture("decompressed", tests[subtest],
> > > +                       block_dim_str[block_dims],
> > 
> > Here and elsewhere, this needs to be indented to the (, or past it...
> > otherwise it's unreadable.
> > 
> 
> Isn't this the coding style of Piglit? See this snippet in HACKING:
> "* Indent with 8-column tabs"
> 
> -Nanley
> 
> > > +                       &tex_decompressed);
> > > +               }
> > >
> > > -                       /* Draw and compare each level of the two textures */
> > > -                       glClear(GL_COLOR_BUFFER_BIT);
> > > -                       if (!draw_compare_levels(check_error, is_srgb_test,
> > > -                                               level_pixel_size_loc,
> > > -                                               pixel_offset_loc,
> > > -                                               tex_compressed,
> > > -                                               tex_decompressed)) {
> > > -                               piglit_loge("Mode %s Block %s.",
> > > -                                       tests[subtest],
> > > -                                       block_dim_str[block_dims]);
> > > -                               return PIGLIT_FAIL;
> > > -                       }
> > > +               /* Draw and compare each level of the two textures */
> > > +               glClear(GL_COLOR_BUFFER_BIT);
> > > +               if (!draw_compare_levels(check_error, is_srgb_test,
> > > +                       level_pixel_size_loc,
> > > +                       pixel_offset_loc,
> > > +                       tex_compressed,
> > > +                       tex_decompressed)) {
> > > +                       piglit_loge("Mode %s Block %s.",
> > > +                       tests[subtest],
> > > +                       block_dim_str[block_dims]);
> > > +                       return PIGLIT_FAIL;
> > >                 }
> > >         }
> > >         return PIGLIT_PASS;
> > > --
> > > 2.6.2
> > >
> > > _______________________________________________
> > > Piglit mailing list
> > > Piglit at lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/piglit
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20151130/c1f90c18/attachment-0001.sig>


More information about the Piglit mailing list