[Piglit] [PATCH 2/2] teximage-errors: Test the combinations of depth and depth-stencil formats

[Anuj Phogat]

On Wed, Mar 12, 2014 at 6:24 AM, Brian Paul <brianp at vmware.com> wrote:

> On 03/11/2014 06:51 PM, Anuj Phogat wrote:
>
>> This patch is to verify a bug fix in mesa commit 079bff5. This commit
>> allowed GL_DEPTH_COMPONENT and GL_DEPTH_STENCIL combinations in
>> glTexImage{123}D functions.
>>
>> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
>> ---
>>   tests/texturing/teximage-errors.c | 73 ++++++++++++++++++++++++++++++
>> ++++++++-
>>   1 file changed, 72 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/texturing/teximage-errors.c
>> b/tests/texturing/teximage-errors.c
>> index 39b7e9b..c01c5e0 100644
>> --- a/tests/texturing/teximage-errors.c
>> +++ b/tests/texturing/teximage-errors.c
>> @@ -36,7 +36,37 @@ PIGLIT_GL_TEST_CONFIG_BEGIN
>>
>>   PIGLIT_GL_TEST_CONFIG_END
>>
>> +struct format_desc {
>> +   GLenum internalformat;
>> +   GLenum format;
>> +   GLenum type;
>> +};
>>
>> +static const struct format_desc formats_allowed[] = {
>> +   {GL_DEPTH_COMPONENT16, GL_DEPTH_STENCIL, GL_UNSIGNED_INT_24_8},
>> +   {GL_DEPTH_COMPONENT24, GL_DEPTH_STENCIL, GL_UNSIGNED_INT_24_8},
>> +   {GL_DEPTH_COMPONENT32F, GL_DEPTH_STENCIL, GL_UNSIGNED_INT_24_8},
>> +
>> +   {GL_DEPTH_COMPONENT16, GL_DEPTH_COMPONENT, GL_FLOAT},
>> +   {GL_DEPTH_COMPONENT24, GL_DEPTH_COMPONENT, GL_FLOAT},
>> +   {GL_DEPTH_COMPONENT32F, GL_DEPTH_COMPONENT, GL_FLOAT},
>> +
>> +   {GL_DEPTH24_STENCIL8, GL_DEPTH_COMPONENT, GL_FLOAT},
>> +   {GL_DEPTH32F_STENCIL8, GL_DEPTH_COMPONENT, GL_FLOAT},
>>
>
> So for that combination, do the texture's stencil values get set to zero?

 From OpenGL 3.3 spec, page 140:
    "If the base internal format is DEPTH_STENCIL and format is not DEPTH_-
     STENCIL, then the values of the stencil index texture components are
undefined."

>
>
>  +   {GL_DEPTH24_STENCIL8, GL_DEPTH_STENCIL, GL_UNSIGNED_INT_24_8},
>> +   {GL_DEPTH32F_STENCIL8, GL_DEPTH_STENCIL, GL_UNSIGNED_INT_24_8}};
>> +
>> +static const struct format_desc formats_not_allowed[] = {
>> +   {GL_DEPTH_COMPONENT16, GL_STENCIL_INDEX, GL_INT},
>> +   {GL_DEPTH_COMPONENT24, GL_STENCIL_INDEX, GL_INT},
>> +   {GL_DEPTH_COMPONENT32F, GL_STENCIL_INDEX, GL_INT},
>> +
>> +   {GL_DEPTH24_STENCIL8, GL_STENCIL_INDEX, GL_INT},
>> +   {GL_DEPTH32F_STENCIL8, GL_STENCIL_INDEX, GL_INT},
>> +
>> +   {GL_RGBA8, GL_DEPTH_COMPONENT, GL_FLOAT},
>> +   {GL_RGBA8, GL_DEPTH_STENCIL, GL_UNSIGNED_INT_24_8}};
>>
>>   /** Test target params to glTexImage functions */
>>   static GLboolean
>> @@ -167,7 +197,34 @@ test_pos_and_sizes(void)
>>      return GL_TRUE;
>>   }
>>
>> -
>> +/* Test the combinations of depth formats in glTexImage{123}D() */
>> +static GLboolean
>>
>
> bool
>
>
>
>  +test_depth_formats(const struct format_desc *test, GLenum expected_error,
>> +                   GLint n_tests)
>> +{
>> +   int i;
>> +   bool result = true;
>> +
>> +   for(i = 0; i < n_tests; i++) {
>>
>
> We usually put a space after "for" and "if"
>
>
>
>  +      if((test[i].internalformat == GL_DEPTH_COMPONENT32F ||
>> +          test[i].internalformat == GL_DEPTH32F_STENCIL8) &&
>> +          !piglit_is_extension_supported("GL_ARB_depth_buffer_float"))
>> +         continue;
>> +
>> +      glTexImage1D(GL_TEXTURE_1D, 0, test[i].internalformat, 16, 0,
>> +                   test[i].format, test[i].type, NULL);
>> +      result = piglit_check_gl_error(expected_error) && result;
>> +
>> +      glTexImage2D(GL_TEXTURE_2D, 0, test[i].internalformat, 16, 16,
>> +                   0, test[i].format, test[i].type, NULL);
>> +      result = piglit_check_gl_error(expected_error) && result;
>> +
>> +      glTexImage3D(GL_TEXTURE_2D_ARRAY, 0, test[i].internalformat,
>> +                   16, 16, 16, 0, test[i].format, test[i].type, NULL);
>>
>
> Do we need to check if the texture array extension is available?

Yes. I'll add a check before  glTexImage3D() call.

>
>
>  +      result = piglit_check_gl_error(expected_error) && result;
>> +   }
>> +   return result;
>> +}
>>   enum piglit_result
>>   piglit_display(void)
>>   {
>> @@ -175,6 +232,20 @@ piglit_display(void)
>>      pass = test_targets() && pass;
>>      pass = test_pos_and_sizes() && pass;
>>
>> +  /*  From OpenGL 3.3 spec, page 141:
>> +   *    "Textures with a base internal format of DEPTH_COMPONENT or
>> +   *     DEPTH_STENCIL require either depth component data or
>> depth/stencil
>> +   *     component data. Textures with other base internal formats
>> require
>> +   *     RGBA component data. The error INVALID_OPERATION is generated if
>> +   *     one of the base internal format and format is DEPTH_COMPONENT or
>> +   *     DEPTH_STENCIL, and the other is neither of these values."
>> +   */
>> +   pass = test_depth_formats(formats_allowed, GL_NO_ERROR,
>> +                             ARRAY_SIZE(formats_allowed))
>> +          && pass;
>> +   pass = test_depth_formats(formats_not_allowed, GL_INVALID_OPERATION,
>> +                             ARRAY_SIZE(formats_not_allowed))
>> +          && pass;
>>      return pass ? PIGLIT_PASS: PIGLIT_FAIL;
>>   }
>>
>>
>>
> Looks OK otherwise.
>
 After fixing your comments, should I consider this r-b you?

>
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20140312/bf7745a8/attachment.html>