On 1 October 2012 14:49, Eric Anholt <e...@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 <stereotype...@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 <e...@anholt.net> > >
_______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit