[Piglit] [PATCH 1/3] arb_internalformat_query2: use CompressedTexImage*D for specific compressed formats
Alejandro Piñeiro
apinheiro at igalia.com
Tue May 8 13:46:24 UTC 2018
On 08/05/18 14:33, Antia Puentes wrote:
> On 06/05/18 11:42, Alejandro Piñeiro wrote:
>
>> Until right now we were using glTexImage*D for any compressed
>> internalformat. That would only work always if the driver support
>> online compression for specific compressed internalformats.
> s/support/supports
>> As it is not really easy to check which internalformats the driver
>> support online compression, it is safer to use glCompressedTexImage*D
> Again s/support/supports. It sounds better to me rephrasing it as
> "easy to check which internalformats the driver supports online
> compression for"
>
>> for any specific internalformat, keeping glTexImage*D for the generic
>> ones.
>> ---
>> tests/spec/arb_internalformat_query2/common.c | 126
>> ++++++++++++++++++++++++--
>> 1 file changed, 117 insertions(+), 9 deletions(-)
>>
>> diff --git a/tests/spec/arb_internalformat_query2/common.c
>> b/tests/spec/arb_internalformat_query2/common.c
>> index 68f33a4a0..204149936 100644
>> --- a/tests/spec/arb_internalformat_query2/common.c
>> +++ b/tests/spec/arb_internalformat_query2/common.c
>> @@ -186,6 +186,47 @@ test_data_check_supported(const test_data *data,
>> return result;
>> }
>> +
>> +/*
>> + * Spec 4.6, Table 8.14
>> + */
>> +static const GLenum specific_compressed_internalformats[] = {
>> + GL_COMPRESSED_SIGNED_RED_RGTC1,
>> + GL_COMPRESSED_RG_RGTC2,
>> + GL_COMPRESSED_SIGNED_RG_RGTC2,
>> + GL_COMPRESSED_RGBA_BPTC_UNORM,
>> + GL_COMPRESSED_SRGB_ALPHA_BPTC_UNORM,
>> + GL_COMPRESSED_RGB_BPTC_SIGNED_FLOAT,
>> + GL_COMPRESSED_RGB_BPTC_UNSIGNED_FLOAT,
>> + GL_COMPRESSED_RGB8_ETC2,
>> + GL_COMPRESSED_SRGB8_ETC2,
>> + GL_COMPRESSED_RGB8_PUNCHTHROUGH_ALPHA1_ETC2,
>> + GL_COMPRESSED_SRGB8_PUNCHTHROUGH_ALPHA1_ETC2,
>> + GL_COMPRESSED_RGBA8_ETC2_EAC,
>> + GL_COMPRESSED_SRGB8_ALPHA8_ETC2_EAC,
>> + GL_COMPRESSED_R11_EAC,
>> + GL_COMPRESSED_SIGNED_R11_EAC,
>> + GL_COMPRESSED_RG11_EAC,
>> + GL_COMPRESSED_SIGNED_RG11_EAC,
>> +};
> GL_COMPRESSED_RED_RGTC1 is missing from the list.
>
>> +
>> +/*
>> + * Returns if @internalformat is a specific compressed internalformat.
>> + *
>> + * This is needed because specific compressed internalformats that
>> + * doesn't support online compression can't be used with glTexImage*D,
>> + * and glCompressedTexImage*D should be used instead. In general, for
>> + * specific compressed internalformats, it is better to use
>> + * glCompressedTexImage*D
>> + */
>> +static bool
>> +internalformat_is_specific_compressed(const GLenum internalformat)
>> +{
>> + return value_on_set((const GLint*)
>> specific_compressed_internalformats,
>> +
>> ARRAY_SIZE(specific_compressed_internalformats),
>> + (GLint) internalformat);
>> +}
>> +
>> /* Returns if @value is one of the values among @set */
>> bool
>> value_on_set(const GLint *set,
>> @@ -336,8 +377,8 @@ try_basic(const GLenum *targets, unsigned
>> num_targets,
>> }
>> /* Returns a valid format for @internalformat, so it would be
>> possible
>> - * to create a texture using glTexImageXD with that
>> - * format/internalformat combination */
>> + * to create a texture using glTexImageXD/glCompressedTexImageXD with
>> + * that format/internalformat combination */
>> static GLenum
>> format_for_internalformat(const GLenum internalformat)
>> {
>> @@ -432,6 +473,51 @@ create_texture(const GLenum target,
>> int width = 16;
>> int depth = 16;
>> unsigned i;
>> + bool is_specific_compressed =
>> internalformat_is_specific_compressed(internalformat);
>> + int image_size = 0;
>> +
>> + /*
>> + * From OpenGL 4.6 spec, Section 8.5, Texture Image
>> + * Specification:
>> + *
>> + * "An INVALID_ENUM error is generated by
>> + * CompressedTexImage1D if in- ternalformat is one of the
>> + * specific compressed formats. OpenGL defines no specific
>> + * one-dimensional compressed formats, but such formats
>> + * may be pro- vided by extensions."
>> + *
> s/in- ternalformat/internalformat
> s/pro- vided/provided
>
>> + * From OpenGL 4.6 spec, Section 8.7, Compressed Texture
>> + * Images:
>> + *
>> + * "An INVALID_ENUM error is generated if the target
>> + * parameter to any of the CompressedTexImagenD commands
>> + * is TEXTURE_RECTANGLE or PROXY_TEXTURE_RECTANGLE ."
>> + */
>> + if (is_specific_compressed &&
>> + (target == GL_TEXTURE_1D || target ==
>> GL_TEXTURE_RECTANGLE))
>> + return false;
>> +
>> + /*
>> + * From ARB_texture_multisample:
>> + *
>> + * "(2) What commands may be used on multisample textures?
>> + *
>> + * RESOLVED: Multisample textures can be bound for
>> + * rendering and texturing, but they cannot be loaded/read
>> + * with SubImage commands (TexSubImage, CopyTexSubImage,
>> + * GetTexImage), they don't support compressed formats, and
>> + * they don't need TexParameters since they can only be
>> + * fetched with texelFetchMultisample."
>> + */
>> + if (is_specific_compressed &&
>> + (target == GL_TEXTURE_2D_MULTISAMPLE ||
>> + target == GL_TEXTURE_2D_MULTISAMPLE_ARRAY)) {
>> + return false;
>> + }
>> +
>> + if (is_specific_compressed) {
>> + image_size =
>> piglit_compressed_image_size(internalformat, width, height);
>> + }
>
> You may want to extract the common condition (is_specific_compressed)
> and put the remaining code inside an "if (is_specific_compressed)" block.
> Anyway, if you prefer to leave it as it is and avoid another level of
> nesting, that is also fine by me:
I found the current status easier to read. Avoiding another level of
nesting is another advantage. So if you don't mind I will let it as it is.
>
> Reviewed-by: Antia Puentes <apuentes at igalia.com>
Thanks!
>
>> glGenTextures(1, &tex);
>> glBindTexture(target, tex);
>> @@ -444,14 +530,29 @@ create_texture(const GLenum target,
>> case GL_TEXTURE_1D_ARRAY:
>> case GL_TEXTURE_2D:
>> case GL_TEXTURE_RECTANGLE:
>> - glTexImage2D(target, 0, internalformat, width,
>> height, 0,
>> - format, type, NULL);
>> + if (is_specific_compressed) {
>> + glCompressedTexImage2D(target, 0,
>> internalformat,
>> + width, height, 0,
>> + image_size, NULL);
>> + } else {
>> + glTexImage2D(target, 0, internalformat,
>> + width, height, 0,
>> + format, type, NULL);
>> + }
>> break;
>> case GL_TEXTURE_CUBE_MAP:
>> for (i = 0; i < 6; i++) {
>> - glTexImage2D(GL_TEXTURE_CUBE_MAP_POSITIVE_X
>> + i, 0,
>> - internalformat, width, height,
>> 0, format, type,
>> - NULL);
>> + if (is_specific_compressed) {
>> +
>> glCompressedTexImage2D(GL_TEXTURE_CUBE_MAP_POSITIVE_X + i, 0,
>> + internalformat,
>> + width,
>> height, 0,
>> + image_size,
>> NULL);
>> + } else {
>> +
>> glTexImage2D(GL_TEXTURE_CUBE_MAP_POSITIVE_X + i, 0,
>> + internalformat,
>> + width, height, 0,
>> + format, type, NULL);
>> + }
>> }
>> break;
>> @@ -462,8 +563,15 @@ create_texture(const GLenum target,
>> /* fall through */
>> case GL_TEXTURE_2D_ARRAY:
>> case GL_TEXTURE_3D:
>> - glTexImage3D(target, 0, internalformat, width,
>> height, depth, 0,
>> - format, type, NULL);
>> + image_size = image_size * depth;
>> + if (is_specific_compressed) {
>> + glCompressedTexImage3D(target, 0,
>> internalformat,
>> + width, height, depth, 0,
>> + image_size, NULL);
>> + } else {
>> + glTexImage3D(target, 0, internalformat,
>> width, height, depth, 0,
>> + format, type, NULL);
>> + }
>> break;
>> case GL_TEXTURE_2D_MULTISAMPLE:
>> glTexImage2DMultisample(target, 1, internalformat, width,
>> height,
>
>
More information about the Piglit
mailing list