On 08/15/2012 02:17 PM, Paul Berry wrote:
> On 15 August 2012 12:51, Chad Versace <chad.vers...@linux.intel.com
> <mailto:chad.vers...@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 <stereotype...@gmail.com <mailto:stereotype...@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 <stereotype...@gmail.com
> <mailto:stereotype...@gmail.com>>
_______________________________________________
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
