On 15 August 2012 16:47, Chad Versace <[email protected]> wrote:
> On 08/15/2012 02:17 PM, Paul Berry wrote: > > On 15 August 2012 12:51, Chad Versace <[email protected] > > <mailto:[email protected]>> 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 <[email protected] <mailto: > [email protected]>> > > --- > > .../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. > Good point. BTW, I have started investigating what it will take to fix this test on Mesa/i965, and it looks like the code is going to have sharp corners for stencil buffers. So extending the test for GL_STENCIL_INDEX/GL_BYTE is definitely worth doing. In fact, I'll probably do it in the next few days if you don't. > > > +/** > > + * 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 <[email protected] <mailto: > [email protected]>> >
_______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
