[Piglit] [PATCH 1/3] arb_internalformat_query2: use CompressedTexImage*D for specific compressed formats
Antia Puentes
apuentes at igalia.com
Tue May 8 12:33:42 UTC 2018
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:
Reviewed-by: Antia Puentes <apuentes at igalia.com>
> 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