[Mesa-dev] [PATCH v4] mesa: improve ARB_copy_image internal format compat check
Jason Ekstrand
jason at jlekstrand.net
Thu Mar 12 22:03:49 PDT 2015
On Thu, Mar 12, 2015 at 9:24 PM, Jason Ekstrand <jason at jlekstrand.net>
wrote:
> 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
>
Pushed. Thanks!
>
> On Thu, Mar 12, 2015 at 4:38 PM, Sean Burke <leftmostcat at 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 <jason at 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 <leftmostcat at 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.ekstrand at 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
>> >
>> >
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150312/ecfa9ad1/attachment-0001.html>
More information about the mesa-dev
mailing list