On Mar 7, 2015 10:57 AM, "Sean Burke" <<a href="mailto:leftmostcat@gmail.com">leftmostcat@gmail.com</a>> wrote: > > The memory layout of compatible internal formats may differ in bytes per > block, so TexFormat is not a reliable measure of compatibility. Additionally, > the current check allows compressed textures of the same block size to be used, > which is in violation of the spec. Could you please be more specific about exactly when the old code fails. > v2: Use a switch instead of array iteration for block class and show the > correct GL error when internal formats are mismatched. > --- > src/mesa/main/copyimage.c | 112 +++++++++++++++++++++++++++++++++++++--------- > 1 file changed, 91 insertions(+), 21 deletions(-) > > diff --git a/src/mesa/main/copyimage.c b/src/mesa/main/copyimage.c > index 455929d..eaedaa6 100644 > --- a/src/mesa/main/copyimage.c > +++ b/src/mesa/main/copyimage.c > @@ -33,6 +33,12 @@ > #include "texobj.h" > #include "fbobject.h" > #include "textureview.h" > +#include "glformats.h" > + > +enum mesa_block_class { > + BLOCK_CLASS_128_BITS, > + BLOCK_CLASS_64_BITS > +}; > > static bool > prepare_target(struct gl_context *ctx, GLuint name, GLenum *target, int level, > @@ -253,6 +259,85 @@ check_region_bounds(struct gl_context *ctx, > struct gl_texture_image *tex_image, > return true; > } > > +static bool > +compressed_format_compatible(struct gl_context *ctx, > + GLenum compressedFormat, GLenum otherFormat) > +{ > + enum mesa_block_class compressedClass, otherClass; > + > + /* Two different compressed formats are never compatible. */ > + if (_mesa_is_compressed_format(ctx, otherFormat)) { > + return false; > + } > + > + switch (compressedFormat) { > + case GL_COMPRESSED_RGBA_S3TC_DXT3_EXT: > + case GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT3_EXT: > + case GL_COMPRESSED_RGBA_S3TC_DXT5_EXT: > + case GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT5_EXT: > + case GL_COMPRESSED_RG_RGTC2: > + case GL_COMPRESSED_SIGNED_RG_RGTC2: > + case GL_COMPRESSED_RGBA_BPTC_UNORM: > + case GL_COMPRESSED_SRGB_ALPHA_BPTC_UNORM: > + case GL_COMPRESSED_RGB_BPTC_SIGNED_FLOAT: > + case GL_COMPRESSED_RGB_BPTC_UNSIGNED_FLOAT: > + compressedClass = BLOCK_CLASS_128_BITS; > + break; > + case GL_COMPRESSED_RGB_S3TC_DXT1_EXT: > + case GL_COMPRESSED_SRGB_S3TC_DXT1_EXT: > + case GL_COMPRESSED_RGBA_S3TC_DXT1_EXT: > + case GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT1_EXT: > + case GL_COMPRESSED_RED_RGTC1: > + case GL_COMPRESSED_SIGNED_RED_RGTC1: > + compressedClass = BLOCK_CLASS_64_BITS; > + break; > + default: > + return false; > + } > + > + switch (otherFormat) { > + case GL_RGBA32UI: > + case GL_RGBA32I: > + case GL_RGBA32F: > + otherClass = BLOCK_CLASS_128_BITS; > + break; > + case GL_RGBA16F: > + case GL_RG32F: > + case GL_RGBA16UI: > + case GL_RG32UI: > + case GL_RGBA16I: > + case GL_RG32I: > + case GL_RGBA16: > + case GL_RGBA16_SNORM: > + otherClass = BLOCK_CLASS_64_BITS; > + break; > + default: > + return false; > + } > + > + return compressedClass == otherClass; > +} For compressed formats this should be the same as doing a bits-per-block check. Why do we need all this? > +static bool > +copy_format_compatible(struct gl_context *ctx, > + GLenum srcFormat, GLenum dstFormat) > +{ > + if (srcFormat == dstFormat) { > + return true; > + } > + else if (_mesa_is_compressed_format(ctx, srcFormat)) { > + return compressed_format_compatible(ctx, srcFormat, dstFormat); > + } > + else if (_mesa_is_compressed_format(ctx, dstFormat)) { > + return compressed_format_compatible(ctx, dstFormat, srcFormat); > + } > + else { > + return _mesa_texture_view_compatible_format(ctx, > + srcFormat, > + dstFormat); > + } > +} > + > void GLAPIENTRY > _mesa_CopyImageSubData(GLuint srcName, GLenum srcTarget, GLint srcLevel, > GLint srcX, GLint srcY, GLint srcZ, > @@ -265,7 +350,7 @@ _mesa_CopyImageSubData(GLuint srcName, GLenum > srcTarget, GLint srcLevel, > struct gl_texture_object *srcTexObj, *dstTexObj; > struct gl_texture_image *srcTexImage, *dstTexImage; > GLuint src_bw, src_bh, dst_bw, dst_bh; > - int i, srcNewZ, dstNewZ, Bpt; > + int i, srcNewZ, dstNewZ; > > if (MESA_VERBOSE & VERBOSE_API) > _mesa_debug(ctx, "glCopyImageSubData(%u, %s, %d, %d, %d, %d, " > @@ -306,15 +391,6 @@ _mesa_CopyImageSubData(GLuint srcName, GLenum > srcTarget, GLint srcLevel, > goto cleanup; > } > > - /* Very simple sanity check. This is sufficient if one of the textures > - * is compressed. */ > - Bpt = _mesa_get_format_bytes(srcTexImage->TexFormat); > - if (_mesa_get_format_bytes(dstTexImage->TexFormat) != Bpt) { > - _mesa_error(ctx, GL_INVALID_VALUE, > - "glCopyImageSubData(internalFormat mismatch)"); > - goto cleanup; > - } > - > if (!check_region_bounds(ctx, srcTexImage, srcX, srcY, srcZ, > srcWidth, srcHeight, srcDepth, "src")) > goto cleanup; > @@ -324,17 +400,11 @@ _mesa_CopyImageSubData(GLuint srcName, GLenum > srcTarget, GLint srcLevel, > (srcHeight / src_bh) * dst_bh, srcDepth, "dst")) > goto cleanup; > > - if (_mesa_is_format_compressed(srcTexImage->TexFormat)) { > - /* XXX: Technically, we should probaby do some more specific checking > - * here. However, this should be sufficient for all compressed > - * formats that mesa supports since it is a direct memory copy. > - */ > - } else if (_mesa_is_format_compressed(dstTexImage->TexFormat)) { > - } else if (_mesa_texture_view_compatible_format(ctx, > - srcTexImage->InternalFormat, > - > dstTexImage->InternalFormat)) { > - } else { > - return; /* Error logged by _mesa_texture_view_compatible_format */ > + if (!copy_format_compatible(ctx, srcTexImage->InternalFormat, > + dstTexImage->InternalFormat)) { > + _mesa_error(ctx, GL_INVALID_OPERATION, > + "glCopyImageSubData(internalFormat mismatch)"); > + goto cleanup; > } > > for (i = 0; i < srcDepth; ++i) { > -- > 2.1.0 > _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a>