[Piglit] [PATCH] arb_fbo: Test glBlitFramebuffer on depth textures with nonzero miplevels

[Chad Versace]

On 08/15/2012 02:17 PM, Paul Berry wrote:
> On 15 August 2012 12:51, Chad Versace <chad.versace at linux.intel.com
> <mailto:chad.versace at linux.intel.com>> wrote:
> 
>     Extend the framebuffer-blit-levels test to verify that
>     glBlitFramebuffer works when blitting to and from miplevels other than
>     zero for depth textures.
> 
>     CC: Paul Berry <stereotype441 at gmail.com <mailto:stereotype441 at gmail.com>>
>     ---
>      .../framebuffer-blit-levels.c                      | 125 ++++++++++++++++-----
>      1 file changed, 94 insertions(+), 31 deletions(-)
> 
>     diff --git a/tests/spec/arb_framebuffer_object/framebuffer-blit-levels.c
>     b/tests/spec/arb_framebuffer_object/framebuffer-blit-levels.c
>     index 31e0cf2..65bc497 100644
>     --- a/tests/spec/arb_framebuffer_object/framebuffer-blit-levels.c
>     +++ b/tests/spec/arb_framebuffer_object/framebuffer-blit-levels.c
>     @@ -70,6 +70,14 @@ GLuint aux_framebuffer;
>      GLuint test_texture;
>      GLuint aux_texture;
> 
>     +GLenum texture_internal_format;
>     +GLenum texture_format;
>     +GLenum texture_type;
> 
> 
> Is texture_type really necessary?  It looks like we always use GL_FLOAT.
> 
> By the same token, it looks like we always use the same values for
> texture_internal_format and texture_format.
> 
> I won't push you on the subject, though, because I realize that this change
> makes the code more self-explanatory :)

You're right. These variables aren't strictly necessary. But, if we ever extend
the test for more formats, such as for GL_STENCIL_INDEX/GL_BYTE, then these
variables will become useful.

>     +/**
>     + * Generate a block of test data where each pixel has a unique depth value in
>     + * the range [0.0, 1.0).
>     + */
>     +static void
>     +create_test_data_depth(GLfloat *data, unsigned level,
>     +                      unsigned width, unsigned height)
>     +{
>     +       unsigned pixel;
>     +       unsigned num_pixels = width * height;
>     +       double depth_delta = 0.95 / ((SIZE >> level) * (SIZE >> level));
> 
> 
> Consider changing this to the equivalent "double depth_delta = 0.95 / (width *
> height);" just so that it's clearer how we know it won't overflow.

Will do.

>      static void
>      print_usage_and_exit(char *prog_name)
>      {
>             printf("Usage: %s <test_mode>\n"
>                    "  where <test_mode> is one of:\n"
>                    "    draw: test blitting *to* the given texture type\n"
>     -              "    read: test blitting *from* the given texture type\n",
>     +              "    read: test blitting *from* the given texture type\n"
>     +              "  where <format> is one of:\n"
>     +              "    rgba\n"
>     +              "    depth\n",
>                    prog_name);
>             piglit_report_result(PIGLIT_FAIL);
>      }
> 
> 
> all.tests needs to be updated to send in the new command-line parameter.

Oops. Will do.

> My comment about all.tests is the only crucial one.  With that fixed, this patch is:
> 
> Reviewed-by: Paul Berry <stereotype441 at gmail.com <mailto:stereotype441 at gmail.com>>