On 1 October 2012 14:49, Eric Anholt <span dir="ltr"><<a href="mailto:eric@anholt.net" target="_blank">eric@anholt.net</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Now I remember, I was waiting to send review to go look at the original<br>
file to see if my review note made sense. I think there's a little<br>
cleanup available:<br>
<div class="im"><br>
Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> writes:<br>
> static void<br>
> create_test_data(GLfloat *data, GLenum texture_format,<br>
> unsigned level, unsigned width, unsigned height)<br>
> {<br>
> - if (texture_format == GL_RGBA)<br>
> + switch (texture_format) {<br>
> + case GL_RGBA:<br>
> create_test_data_rgba(data, level, width, height);<br>
> - else if (texture_format == GL_DEPTH_COMPONENT)<br>
> + break;<br>
> + case GL_DEPTH_COMPONENT:<br>
> create_test_data_depth(data, level, width, height);<br>
> - else<br>
> + break;<br>
> + case GL_DEPTH_STENCIL:<br>
> + create_test_data_stencil((GLbyte *) data, level,<br>
> + width, height);<br>
> + break;<br>
> + default:<br>
> assert(0);<br>
> + break;<br>
> + }<br>
> }<br>
<br>
</div>GL_DEPTH_COMPONENT mode is introduced here...<br>
<div class="im"><br>
> @@ -180,6 +243,12 @@ piglit_init(int argc, char **argv)<br>
> texture_type = GL_FLOAT;<br>
> framebuffer_attachment = GL_DEPTH_ATTACHMENT;<br>
> blit_mask = GL_DEPTH_BUFFER_BIT;<br>
> + } else if (strcmp(argv[2], "stencil") == 0) {<br>
> + texture_internal_format = GL_DEPTH_STENCIL;<br>
> + texture_format = GL_DEPTH_STENCIL;<br>
> + texture_type = GL_UNSIGNED_INT_24_8;<br>
> + framebuffer_attachment = GL_DEPTH_STENCIL_ATTACHMENT;<br>
> + blit_mask = GL_STENCIL_BUFFER_BIT;<br>
<br>
</div>And this is the new internalformat == GL_DEPTH_STENCIL<br>
<div class="im"><br>
> @@ -239,7 +308,13 @@ upload_test_data(GLuint texture, unsigned data_level,<br>
><br>
> glBindTexture(GL_TEXTURE_2D, texture);<br>
><br>
> - create_test_data(data, texture_format, data_level, width, height);<br>
> + if (texture_format == GL_DEPTH_STENCIL) {<br>
> + create_test_data_depthstencil((GLbyte *) data, data_level,<br>
> + width, height);<br>
> + } else {<br>
> + create_test_data(data, texture_format, data_level,<br>
> + width, height);<br>
> + }<br>
<br>
</div>But here you avoid calling create_test_data for GL_DEPTH_STENCIL. I<br>
think you could just adjust the GL_DEPTH_STENCIL case above.<br></blockquote><div><br>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.<br>
</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Other than that, Reviewed-by: Eric Anholt <<a href="mailto:eric@anholt.net">eric@anholt.net</a>><br>
<br>
</blockquote></div><br>