<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Thu, Mar 12, 2015 at 9:24 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div><div>Sean, </div>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: <a href="http://lists.freedesktop.org/archives/piglit/2015-March/015156.html" target="_blank">http://lists.freedesktop.org/archives/piglit/2015-March/015156.html</a> </div>I'll merge the mesa commit once we get piglit fixed. </div>--Jason </div></blockquote><div> </div><div>Pushed. Thanks! </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"> <div class="gmail_quote">On Thu, Mar 12, 2015 at 4:38 PM, Sean Burke <<a href="mailto:leftmostcat@gmail.com" target="_blank">leftmostcat@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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 <div><div> On Wed, Mar 11, 2015 at 2:53 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>> 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 <<a href="mailto:leftmostcat@gmail.com" target="_blank">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. 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 <<a href="mailto:jason.ekstrand@intel.com" target="_blank">jason.ekstrand@intel.com</a>> >> --- >> 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 > > </div></div></blockquote></div> </div> </div></div></blockquote></div> </div></div>