Sean, I got ready to push this and ran it against piglit and one of the tests errored out. Looking into it further, it was a bug in the test. I've sent a patch:
http://lists.freedesktop.org/archives/piglit/2015-March/015156.html I'll merge the mesa commit once we get piglit fixed. --Jason On Thu, Mar 12, 2015 at 4:38 PM, Sean Burke <leftmost...@gmail.com> wrote: > Jason, > > No worries. It looks like my mail client munged the patch in some way. > I'm sending it as an attachment in the hopes that it will remain > untouched. > > > Sean Burke > > On Wed, Mar 11, 2015 at 2:53 PM, Jason Ekstrand <ja...@jlekstrand.net> > wrote: > > Sean, > > Sorry it's taken so long for me to get to this, but I went to test/push > this > > today and it doesn't apply against current mesa master. Can you please > > either rebase on master and re-send or give me the SHA1 hash of the > commit > > this one is based on. (Not the SHA1 of this commit but the previous > one). > > --Jason > > > > On Sat, Mar 7, 2015 at 8:34 PM, Sean Burke <leftmost...@gmail.com> > wrote: > >> > >> The memory layout of compatible internal formats may differ in bytes per > >> block, so TexFormat is not a reliable measure of compatibility. For > >> example, > >> GL_RGB8 and GL_RGB8UI are compatible formats, but GL_RGB8 may be laid > out > >> in > >> memory as B8G8R8X8. If GL_RGB8UI has a 3 byte-per-block memory layout, > the > >> existing compatibility check will fail. > >> > >> Additionally, the current check allows any two compressed textures which > >> share > >> block size to be used, whereas the spec gives an explicit table of > >> compatible > >> formats. > >> > >> v2: Use a switch instead of array iteration for block class and show the > >> correct GL error when internal formats are mismatched. > >> v3: Include spec citations for new compatibility checks, rearrange check > >> order to ensure that compressed, view-compatible formats return the > >> correct result, and make style fixes. Original commit message > amended > >> for clarity. > >> v4: Reformatted spec citations. > >> > >> Reviewed-by: Jason Ekstrand <jason.ekstr...@intel.com> > >> --- > >> src/mesa/main/copyimage.c | 151 > >> +++++++++++++++++++++++++++++++++++++++------- > >> 1 file changed, 130 insertions(+), 21 deletions(-) > >> > >> diff --git a/src/mesa/main/copyimage.c b/src/mesa/main/copyimage.c > >> index 455929d..fd22f28 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,124 @@ 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 view-incompatible compressed formats are never compatible. */ > >> + if (_mesa_is_compressed_format(ctx, otherFormat)) { > >> + return false; > >> + } > >> + > >> + /* > >> + * From ARB_copy_image spec: > >> + * Table 4.X.1 (Compatible internal formats for copying between > >> + * compressed and uncompressed internal formats) > >> + * > >> --------------------------------------------------------------------- > >> + * | Texel / | Uncompressed | > >> | > >> + * | Block | internal format | Compressed internal format > >> | > >> + * | size | | > >> | > >> + * > >> --------------------------------------------------------------------- > >> + * | 128-bit | RGBA32UI, | COMPRESSED_RGBA_S3TC_DXT3_EXT, > >> | > >> + * | | RGBA32I, | > >> COMPRESSED_SRGB_ALPHA_S3TC_DXT3_EXT,| > >> + * | | RGBA32F | COMPRESSED_RGBA_S3TC_DXT5_EXT, > >> | > >> + * | | | > >> COMPRESSED_SRGB_ALPHA_S3TC_DXT5_EXT,| > >> + * | | | COMPRESSED_RG_RGTC2, > >> | > >> + * | | | COMPRESSED_SIGNED_RG_RGTC2, > >> | > >> + * | | | COMPRESSED_RGBA_BPTC_UNORM, > >> | > >> + * | | | > >> COMPRESSED_SRGB_ALPHA_BPTC_UNORM, | > >> + * | | | > >> COMPRESSED_RGB_BPTC_SIGNED_FLOAT, | > >> + * | | | > >> COMPRESSED_RGB_BPTC_UNSIGNED_FLOAT | > >> + * > >> --------------------------------------------------------------------- > >> + * | 64-bit | RGBA16F, RG32F, | COMPRESSED_RGB_S3TC_DXT1_EXT, > >> | > >> + * | | RGBA16UI, RG32UI, | COMPRESSED_SRGB_S3TC_DXT1_EXT, > >> | > >> + * | | RGBA16I, RG32I, | COMPRESSED_RGBA_S3TC_DXT1_EXT, > >> | > >> + * | | RGBA16, | > >> COMPRESSED_SRGB_ALPHA_S3TC_DXT1_EXT,| > >> + * | | RGBA16_SNORM | COMPRESSED_RED_RGTC1, > >> | > >> + * | | | COMPRESSED_SIGNED_RED_RGTC1 > >> | > >> + * > >> --------------------------------------------------------------------- > >> + */ > >> + > >> + 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; > >> +} > >> + > >> +static bool > >> +copy_format_compatible(struct gl_context *ctx, > >> + GLenum srcFormat, GLenum dstFormat) > >> +{ > >> + /* > >> + * From ARB_copy_image spec: > >> + * For the purposes of CopyImageSubData, two internal formats > >> + * are considered compatible if any of the following conditions > are > >> + * met: > >> + * * the formats are the same, > >> + * * the formats are considered compatible according to the > >> + * compatibility rules used for texture views as defined in > >> + * section 3.9.X. In particular, if both internal formats are > >> listed > >> + * in the same entry of Table 3.X.2, they are considered > >> compatible, or > >> + * * one format is compressed and the other is uncompressed and > >> + * Table 4.X.1 lists the two formats in the same row. > >> + */ > >> + > >> + if (_mesa_texture_view_compatible_format(ctx, srcFormat, > dstFormat)) { > >> + /* Also checks if formats are equal. */ > >> + 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); > >> + } > >> + > >> + return false; > >> +} > >> + > >> void GLAPIENTRY > >> _mesa_CopyImageSubData(GLuint srcName, GLenum srcTarget, GLint > srcLevel, > >> GLint srcX, GLint srcY, GLint srcZ, > >> @@ -265,7 +389,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 +430,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 +439,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 mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
