[Piglit] [PATCH v2 2/2] framebuffer-blit-levels: Test stencil buffers.

[Paul Berry]

On 1 October 2012 14:49, Eric Anholt <eric at anholt.net> wrote:

> Now I remember, I was waiting to send review to go look at the original
> file to see if my review note made sense.  I think there's a little
> cleanup available:
>
> Paul Berry <stereotype441 at gmail.com> writes:
> >  static void
> >  create_test_data(GLfloat *data, GLenum texture_format,
> >                unsigned level, unsigned width, unsigned height)
> >  {
> > -     if (texture_format == GL_RGBA)
> > +     switch (texture_format) {
> > +     case GL_RGBA:
> >               create_test_data_rgba(data, level, width, height);
> > -     else if (texture_format == GL_DEPTH_COMPONENT)
> > +             break;
> > +     case GL_DEPTH_COMPONENT:
> >               create_test_data_depth(data, level, width, height);
> > -     else
> > +             break;
> > +     case GL_DEPTH_STENCIL:
> > +             create_test_data_stencil((GLbyte *) data, level,
> > +                                      width, height);
> > +             break;
> > +     default:
> >               assert(0);
> > +             break;
> > +     }
> >  }
>
> GL_DEPTH_COMPONENT mode is introduced here...
>
> > @@ -180,6 +243,12 @@ piglit_init(int argc, char **argv)
> >               texture_type = GL_FLOAT;
> >               framebuffer_attachment = GL_DEPTH_ATTACHMENT;
> >               blit_mask = GL_DEPTH_BUFFER_BIT;
> > +     } else if (strcmp(argv[2], "stencil") == 0) {
> > +             texture_internal_format = GL_DEPTH_STENCIL;
> > +             texture_format = GL_DEPTH_STENCIL;
> > +             texture_type = GL_UNSIGNED_INT_24_8;
> > +             framebuffer_attachment = GL_DEPTH_STENCIL_ATTACHMENT;
> > +             blit_mask = GL_STENCIL_BUFFER_BIT;
>
> And this is the new internalformat == GL_DEPTH_STENCIL
>
> > @@ -239,7 +308,13 @@ upload_test_data(GLuint texture, unsigned
> data_level,
> >
> >       glBindTexture(GL_TEXTURE_2D, texture);
> >
> > -     create_test_data(data, texture_format, data_level, width, height);
> > +     if (texture_format == GL_DEPTH_STENCIL) {
> > +             create_test_data_depthstencil((GLbyte *) data, data_level,
> > +                                           width, height);
> > +     } else {
> > +             create_test_data(data, texture_format, data_level,
> > +                              width, height);
> > +     }
>
> But here you avoid calling create_test_data for GL_DEPTH_STENCIL.  I
> think you could just adjust the GL_DEPTH_STENCIL case above.
>

Ok, I agree.  But there's a difficulty: with the change you suggest,
create_test_data() will be called by both upload_test_data() (which needs
the data in depth/stencil format) and piglit_display() (which needs just
the stencil data, so it can pass it via test_image() to
piglit_probe_image_stencil()).  I'll submit a v2 patch that calls
create_test_data(GL_DEPTH_STENCIL) in the former case and
create_test_data(GL_STENCIL_INDEX) in the latter case.  Hopefully that will
make the code easier to follow.


>
> Other than that, Reviewed-by: Eric Anholt <eric at anholt.net>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20121003/6c0b0be5/attachment-0001.html>