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>