[Piglit] [PATCH 2/3] arb_texture_view: Test valid and invalid formats
Ian Romanick
idr at freedesktop.org
Thu Oct 17 20:42:57 CEST 2013
On 10/17/2013 07:29 AM, Brian Paul wrote:
> On 10/16/2013 05:37 PM, Jon Ashburn wrote:
>> When calling glTextureView() only certain formats are valid based on
>> the format
>> of the original texture. This tests valid/invalid combinations.
>>
>> Tested on Nvidia Quadro 600, all tests pass.
>> ---
>> tests/all.tests | 1 +
>> tests/spec/arb_texture_view/CMakeLists.gl.txt | 1 +
>> tests/spec/arb_texture_view/common.c | 48 ++++
>> tests/spec/arb_texture_view/formats.c | 309
>> ++++++++++++++++++++++++++
>> 4 files changed, 359 insertions(+)
>> create mode 100644 tests/spec/arb_texture_view/common.c
>> create mode 100644 tests/spec/arb_texture_view/formats.c
>>
>> diff --git a/tests/all.tests b/tests/all.tests
>> index 2bb6aed..4d089b4 100644
>> --- a/tests/all.tests
>> +++ b/tests/all.tests
>> @@ -1473,6 +1473,7 @@ arb_texture_view = Group()
>> spec['ARB_texture_view'] = arb_texture_view
>> arb_texture_view['immutable_levels'] =
>> concurrent_test('arb_texture_view-texture-immutable-levels')
>> arb_texture_view['params'] = concurrent_test('arb_texture_view-params')
>> +arb_texture_view['formats'] =
>> concurrent_test('arb_texture_view-formats')
>>
>> tdfx_texture_compression_fxt1 = Group()
>> spec['3DFX_texture_compression_FXT1'] = tdfx_texture_compression_fxt1
>> diff --git a/tests/spec/arb_texture_view/CMakeLists.gl.txt
>> b/tests/spec/arb_texture_view/CMakeLists.gl.txt
>> index 4f2b1ef..44b6a64 100644
>> --- a/tests/spec/arb_texture_view/CMakeLists.gl.txt
>> +++ b/tests/spec/arb_texture_view/CMakeLists.gl.txt
>> @@ -11,5 +11,6 @@ link_libraries(
>>
>> piglit_add_executable(arb_texture_view-texture-immutable-levels
>> texture-immutable-levels.c)
>> piglit_add_executable(arb_texture_view-params params.c)
>> +piglit_add_executable(arb_texture_view-formats formats.c common.c)
>>
>> # vim: ft=cmake:
>> diff --git a/tests/spec/arb_texture_view/common.c
>> b/tests/spec/arb_texture_view/common.c
>> new file mode 100644
>> index 0000000..c5f5a23
>> --- /dev/null
>> +++ b/tests/spec/arb_texture_view/common.c
>> @@ -0,0 +1,48 @@
>> +/*
>> + * Copyright © 2013 LunarG, Inc.
>> + *
>> + * Permission is hereby granted, free of charge, to any person
>> obtaining a
>> + * copy of this software and associated documentation files (the
>> "Software"),
>> + * to deal in the Software without restriction, including without
>> limitation
>> + * the rights to use, copy, modify, merge, publish, distribute,
>> sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including
>> the next
>> + * paragraph) shall be included in all copies or substantial portions
>> of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
>> SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES
>> OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>> ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>> OTHER DEALINGS
>> + * IN THE SOFTWARE.
>> + *
>> + * Author: Jon Ashburn <jon at lunarg.com>
>> + */
>> +
>> +#include "GL/gl.h"
>> +#include <stdarg.h>
>> +
>> +
>
> I think this function could use a comment to indicate what it does.
>
>> +void update_valid_arrays(GLenum *valid, GLenum *invalid,
>> + unsigned int numInvalid, unsigned int
>> numValid, ... )
>
> Too much indentation for the second line there.
>
>> +{
>> + va_list args;
>> + GLenum val;
>> + unsigned int i,j;
>> +
>> + va_start(args, numValid);
>> + for (i = 0; i < numValid; i++) {
>> + val = va_arg(args, GLenum);
>> + valid[i] = val;
>> + /* remove the valid enum from the invalid array */
>> + for (j= 0; j < numInvalid; j++) {
>> + if (invalid[j] == val)
>> + invalid[j] = 0;
>> + }
>> + }
>> + va_end(args);
>> +}
>> diff --git a/tests/spec/arb_texture_view/formats.c
>> b/tests/spec/arb_texture_view/formats.c
>> new file mode 100644
>> index 0000000..96c3a3e
>> --- /dev/null
>> +++ b/tests/spec/arb_texture_view/formats.c
>> @@ -0,0 +1,309 @@
>> +/*
>> + * Copyright © 2013 LunarG, Inc.
>> + *
>> + * Permission is hereby granted, free of charge, to any person
>> obtaining a
>> + * copy of this software and associated documentation files (the
>> "Software"),
>> + * to deal in the Software without restriction, including without
>> limitation
>> + * the rights to use, copy, modify, merge, publish, distribute,
>> sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including
>> the next
>> + * paragraph) shall be included in all copies or substantial portions
>> of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
>> SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES
>> OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>> ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>> OTHER DEALINGS
>> + * IN THE SOFTWARE.
>> + *
>> + * Author: Jon Ashburn <jon at lunarg.com>
>> + */
>> +
>> +/**
>> + * \file
>> + * This (arb_texture_view-formats) tests valid and invalid new
>> TextureView
>> + * formats based on the original textures format.
>> + *
>> + * Section 8.18 (Texture Views) of OpenGL 4.3 Core says:
>> + * "The two textures’ internal formats must be compatible
>> according to
>> + * table 8.21 if the internal format exists in that table. The
>> internal
>> + * formats must be identical if not in that table."
>> + *
>> + */
>> +
>> +#include "piglit-util-gl-common.h"
>> +
>> +PIGLIT_GL_TEST_CONFIG_BEGIN
>> +
>> + config.supports_gl_compat_version = 10;
>> + config.supports_gl_core_version = 31;
>> +
>> + config.window_visual = PIGLIT_GL_VISUAL_RGBA |
>> PIGLIT_GL_VISUAL_DOUBLE;
>> +
>> +PIGLIT_GL_TEST_CONFIG_END
>> +
>> +static const char *TestName = "arb_texture_view-formats";
>> +void update_valid_arrays(GLenum *valid, GLenum *invalid,
>> + unsigned int numInvalid, unsigned int
>> numValid, ... );
>
> Maybe that declaration should be put in a common.h header file?
>
>
>> +
>> +/**
>> + * Do error-check tests for texture formats
>> + */
>> +static bool
>> +test_format_errors(GLenum format_class)
>> +{
>> + GLint width = 16, height = 16;
>> + const GLsizei levels = 5;
>> + GLenum target = GL_TEXTURE_CUBE_MAP;
>> + GLuint tex;
>> + enum piglit_result pass = true;
>
> I think that should be bool to match the function return type.
>
>
>> + GLenum legalFormats[17];
>
> Why 17? Seems like a magic number that should be a #define.
>
>
>> + unsigned int numFormats, i;
>> + GLenum illegalFormats[] = {
>
> I think this array could be static const. const at least.
It can't. The call to update_valid_arrays later modifies it.
>> + /* skip compressed sized formats */
>> + /* 128 bit */
>> + GL_RGBA32F,
>> + GL_RGBA32UI,
>> + GL_RGBA32I,
>> + /* 96 bit */
>> + GL_RGB32F,
>> + GL_RGB32UI,
>> + GL_RGB32I,
>> + /* 64 bit */
>> + GL_RGBA16F,
>> + GL_RG32F,
>> + GL_RGBA16UI,
>> + GL_RG32UI,
>> + GL_RGBA16I,
>> + GL_RG32I,
>> + GL_RGBA16,
>> + GL_RGBA16_SNORM,
>> + /* 48 bit */
>> + GL_RGB16,
>> + GL_RGB16_SNORM,
>> + GL_RGB16F,
>> + GL_RGB16UI,
>> + GL_RGB16I,
>> + /* 32 bits */
>> + GL_RG16F,
>> + GL_R11F_G11F_B10F,
>> + GL_R32F,
>> + GL_RGB10_A2UI,
>> + GL_RGBA8UI,
>> + GL_RG16UI,
>> + GL_R32UI,
>> + GL_RGBA8I,
>> + GL_RG16I,
>> + GL_R32I,
>> + GL_RGB10_A2,
>> + GL_RGBA8,
>> + GL_RG16,
>> + GL_RGBA8_SNORM,
>> + GL_RG16_SNORM,
>> + GL_SRGB8_ALPHA8,
>> + GL_RGB9_E5,
>> + /* 24 bits */
>> + GL_RGB8,
>> + GL_RGB8_SNORM,
>> + GL_SRGB8,
>> + GL_RGB8UI,
>> + GL_RGB8I,
>> + /* 16 bits */
>> + GL_R16F,
>> + GL_RG8UI,
>> + GL_R16UI,
>> + GL_RG8I,
>> + GL_R16I,
>> + GL_RG8,
>> + GL_R16,
>> + GL_RG8_SNORM,
>> + GL_R16_SNORM,
>> + /* 8 bits */
>> + GL_R8UI,
>> + GL_R8I,
>> + GL_R8,
>> + GL_R8_SNORM,
>> + /* a sampling of unsized formats */
>> + GL_ALPHA,
>> + GL_LUMINANCE,
>> + GL_LUMINANCE_ALPHA,
>> + GL_INTENSITY,
>> + GL_RGB,
>> + GL_RGBA,
>> + GL_DEPTH_COMPONENT,
>> + GL_COMPRESSED_ALPHA,
>> + GL_COMPRESSED_LUMINANCE_ALPHA,
>> + GL_COMPRESSED_LUMINANCE,
>> + GL_COMPRESSED_INTENSITY,
>> + GL_COMPRESSED_RGB,
>> + GL_COMPRESSED_RGBA,
>> + GL_COMPRESSED_RGBA,
>> + GL_COMPRESSED_SRGB,
>> + GL_COMPRESSED_SRGB_ALPHA,
>> + GL_COMPRESSED_SLUMINANCE,
>> + GL_COMPRESSED_SLUMINANCE_ALPHA,
>> + };
>> +
>> + /* skip compressed internal formats for now including extensions */
>> + assert(format_class == GL_VIEW_CLASS_128_BITS ||
>> + format_class == GL_VIEW_CLASS_96_BITS ||
>> + format_class == GL_VIEW_CLASS_64_BITS ||
>> + format_class == GL_VIEW_CLASS_48_BITS ||
>> + format_class == GL_VIEW_CLASS_32_BITS ||
>> + format_class == GL_VIEW_CLASS_24_BITS ||
>> + format_class == GL_VIEW_CLASS_16_BITS ||
>> + format_class == GL_VIEW_CLASS_8_BITS);
>
> I think you could remove this assertion and just add a default switch
> case below to catch invalid values.
I think he copied this from /your/ texture-storage test. :)
>> +
>> + glGenTextures(1, &tex); /* orig tex */
>> + glBindTexture(target, tex);
>> +
>> + switch (format_class) {
>> + case GL_VIEW_CLASS_128_BITS:
>> + glTexStorage2D(target, levels, GL_RGBA32F, width, height);
>> + numFormats = 3;
>> + update_valid_arrays(legalFormats, illegalFormats,
>> + ARRAY_SIZE(illegalFormats), numFormats,
>> + GL_RGBA32F, GL_RGBA32UI, GL_RGBA32I);
>
> The indentation here and below is a bit off.
>
>
>> + break;
>> + case GL_VIEW_CLASS_96_BITS:
>> + glTexStorage2D(target, levels, GL_RGB32F, width, height);
>> + numFormats = 3;
>> + update_valid_arrays(legalFormats, illegalFormats,
>> + ARRAY_SIZE(illegalFormats), numFormats,
>> + GL_RGB32F, GL_RGB32UI, GL_RGB32I);
>> + break;
>> + case GL_VIEW_CLASS_64_BITS:
>> + glTexStorage2D(target, levels, GL_RGBA16F, width, height);
>> + numFormats = 8;
>> + update_valid_arrays(legalFormats, illegalFormats,
>> + ARRAY_SIZE(illegalFormats), numFormats,
>> + GL_RGBA16F, GL_RG32F, GL_RGBA16UI,
>> GL_RG32UI,
>> + GL_RGBA16I, GL_RG32I, GL_RGBA16,
>> GL_RGBA16_SNORM);
>> + break;
>> + case GL_VIEW_CLASS_48_BITS:
>> + glTexStorage2D(target, levels, GL_RGB16, width, height);
>> + numFormats = 5;
>> + update_valid_arrays(legalFormats, illegalFormats,
>> + ARRAY_SIZE(illegalFormats), numFormats,
>> + GL_RGB16, GL_RGB16_SNORM, GL_RGB16F, GL_RGB16UI,
>> GL_RGB16I);
>> + break;
>> + case GL_VIEW_CLASS_32_BITS:
>> + glTexStorage2D(target, levels, GL_RG16F, width, height);
>> + numFormats = 17;
>> + update_valid_arrays(legalFormats, illegalFormats,
>> + ARRAY_SIZE(illegalFormats), numFormats,
>> + GL_RG16F, GL_R11F_G11F_B10F, GL_R32F, GL_RGB10_A2UI,
>> + GL_RGBA8UI, GL_RG16UI, GL_R32UI, GL_RGBA8I,
>> GL_RG16I,
>> + GL_R32I, GL_RGB10_A2, GL_RGBA8, GL_RG16,
>> GL_RGBA8_SNORM,
>> + GL_RG16_SNORM, GL_SRGB8_ALPHA8, GL_RGB9_E5);
>> + break;
>> + case GL_VIEW_CLASS_24_BITS:
>> + glTexStorage2D(target, levels, GL_RGB8, width, height);
>> + numFormats = 5;
>> + update_valid_arrays(legalFormats, illegalFormats,
>> + ARRAY_SIZE(illegalFormats), numFormats,
>> + GL_RGB8, GL_RGB8_SNORM, GL_SRGB8, GL_RGB8UI,
>> GL_RGB8I);
>> + break;
>> + case GL_VIEW_CLASS_16_BITS:
>> + glTexStorage2D(target, levels, GL_R16F, width, height);
>> + numFormats = 9;
>> + update_valid_arrays(legalFormats, illegalFormats,
>> + ARRAY_SIZE(illegalFormats), numFormats,
>> + GL_R16F, GL_RG8UI, GL_R16UI, GL_RG8I, GL_R16I,
>> GL_RG8,
>> + GL_R16, GL_RG8_SNORM, GL_R16_SNORM);
>> + break;
>> + case GL_VIEW_CLASS_8_BITS:
>> + glTexStorage2D(target, levels, GL_R8I, width, height);
>> + numFormats = 4;
>> + update_valid_arrays(legalFormats, illegalFormats,
>> + ARRAY_SIZE(illegalFormats), numFormats,
>> + GL_R8UI, GL_R8I, GL_R8, GL_R8_SNORM);
>> + break;
>> + }
>> +
>> + if (!piglit_check_gl_error(GL_NO_ERROR)) {
>> + printf("%s:%s Found gl errors prior to testing glTextureView\n",
>> + TestName, __func__);
>> + pass = PIGLIT_FAIL;
>
> pass = false;
>
>
>> + goto err_out;
>> + }
>> +
>> + /* ensure TextureView of legal formats works */
>> + for (i = 0; i < numFormats; i++) {
>> + GLenum format;
>> + GLuint new_tex, layers=6;
>> + format = legalFormats[i];
>> + glGenTextures(1, &new_tex);
>> + glTextureView(new_tex, target, tex, format, 0, levels, 0,
>> layers);
>> + glDeleteTextures(1, &new_tex);
>> + if (!piglit_check_gl_error(GL_NO_ERROR)) {
>> + pass = false;
>> + goto err_out;
>> + }
>> + }
>> +
>> + /* ensure TextureView of illegal formats returns an error */
>> + for (i = 0; i < ARRAY_SIZE(illegalFormats); i++) {
>> + GLenum format;
>> + GLuint newTex, layers=6;
>> + format = illegalFormats[i];
>> + if (format == 0)
>> + continue;
>> + glGenTextures(1, &newTex);
>> + glTextureView(newTex, target, tex, format, 0, levels, 0,
>> layers);
>> + if (!piglit_check_gl_error(GL_INVALID_OPERATION)) {
>> + glDeleteTextures(1, &newTex);
>> + pass = false;
>> + goto err_out;
>> + }
>> + glDeleteTextures(1, &newTex);
>> + }
>> +
>> + err_out:
>> + glDeleteTextures(1, &tex);
>> +
>> + return pass;
>> +}
>> +
>> +
>> +#define X(f, n) \
>
> 'n' seems like a number. Maybe 'test' to better indicate a string?
I think 'n' is short for name?
>> + do { \
>> + const bool subtest_pass = (f); \
>> + piglit_report_subtest_result(subtest_pass \
>> + ? PIGLIT_PASS : PIGLIT_FAIL, \
>> + (n)); \
>> + pass = pass && subtest_pass; \
>> + } while (0)
>> +
>> +enum piglit_result
>> +piglit_display(void)
>> +{
>> + bool pass = true;
>> +
>> + X(test_format_errors(GL_VIEW_CLASS_128_BITS), "Format 128 bits
>> validity");
>> + X(test_format_errors(GL_VIEW_CLASS_96_BITS), "Format 96 bits
>> validity");
>> + X(test_format_errors(GL_VIEW_CLASS_64_BITS), "Format 64 bits
>> validity");
>> + X(test_format_errors(GL_VIEW_CLASS_48_BITS), "Format 48 bits
>> validity");
>> + X(test_format_errors(GL_VIEW_CLASS_32_BITS), "Format 32 bits
>> validity");
>> + X(test_format_errors(GL_VIEW_CLASS_24_BITS), "Format 24 bits
>> validity");
>> + X(test_format_errors(GL_VIEW_CLASS_16_BITS), "Format 16 bits
>> validity");
>> + X(test_format_errors(GL_VIEW_CLASS_8_BITS), "Format 8 bits
>> validity");
>> +#undef X
>> + pass = piglit_check_gl_error(GL_NO_ERROR) && pass;
>> + return pass ? PIGLIT_PASS : PIGLIT_FAIL;
>> +}
>> +
>> +
>> +void
>> +piglit_init(int argc, char **argv)
>> +{
>> + piglit_require_extension("GL_ARB_texture_storage");
>> + piglit_require_extension("GL_ARB_texture_view");
>> + piglit_require_extension("GL_EXT_texture_integer");
>> + piglit_require_extension("GL_ARB_texture_float");
>> + piglit_require_extension("GL_ARB_texture_cube_map");
>> +}
More information about the Piglit
mailing list