[Mesa-dev] [PATCH] mesa/copyimage: allow width/height to not be multiples of block

[Ian Romanick]

On 11/08/2015 01:50 AM, Ilia Mirkin wrote:
> For compressed textures, the image size is not necessarily a multiple of
> the block size (e.g. the last mip levels). For example the RGTC spec
> calls out this condition as legal, saying to error only when:
> 
>     * <width> is not a multiple of four, and <width> plus <xoffset>
>     * is not equal to TEXTURE_WIDTH;
> 
> While the GL_ARB_copy_image spec does not call this out explicitly, it
> appears that some games rely on this. Also it seems like it should be
> possible to copy in the last miplevels of a compressed texture's
> miptree.

I think this is fixed in the core GL spec.  Section 18.3.2 (Copying
Between Images) of the OpenGL 4.5 Core Profile spec says:

    An INVALID_VALUE error is generated if the dimensions of either
    subregion exceeds the boundaries of the corresponding image object,
    or if the image format is compressed and the dimensions of the
    subregion fail to meet the alignment constraints of the format.

Section 8.7 (Compressed Texture Images) says:

    An INVALID_OPERATION error is generated if any of the following
    conditions occurs:

      • width is not a multiple of four, and width + xoffset is not
        equal to the value of TEXTURE_WIDTH.
      • height is not a multiple of four, and height + yoffset is not
        equal to the value of TEXTURE_HEIGHT.

With the above spec quotations added somewhere (either the commit
message or the code), this patch is

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92860
> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> Cc: mesa-stable at lists.freedesktop.org
> ---
>  src/mesa/main/copyimage.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/src/mesa/main/copyimage.c b/src/mesa/main/copyimage.c
> index f02e842..974de7f 100644
> --- a/src/mesa/main/copyimage.c
> +++ b/src/mesa/main/copyimage.c
> @@ -62,6 +62,8 @@ prepare_target(struct gl_context *ctx, GLuint name, GLenum target,
>                 struct gl_renderbuffer **renderbuffer,
>                 mesa_format *format,
>                 GLenum *internalFormat,
> +               GLuint *width,
> +               GLuint *height,
>                 const char *dbg_prefix)
>  {
>     if (name == 0) {
> @@ -126,6 +128,8 @@ prepare_target(struct gl_context *ctx, GLuint name, GLenum target,
>        *renderbuffer = rb;
>        *format = rb->Format;
>        *internalFormat = rb->InternalFormat;
> +      *width = rb->Width;
> +      *height = rb->Height;
>        *tex_image = NULL;
>     } else {
>        struct gl_texture_object *texObj = _mesa_lookup_texture(ctx, name);
> @@ -194,6 +198,8 @@ prepare_target(struct gl_context *ctx, GLuint name, GLenum target,
>        *renderbuffer = NULL;
>        *format = (*tex_image)->TexFormat;
>        *internalFormat = (*tex_image)->InternalFormat;
> +      *width = (*tex_image)->Width;
> +      *height = (*tex_image)->Height;
>     }
>  
>     return true;
> @@ -423,6 +429,7 @@ _mesa_CopyImageSubData(GLuint srcName, GLenum srcTarget, GLint srcLevel,
>     struct gl_renderbuffer *srcRenderbuffer, *dstRenderbuffer;
>     mesa_format srcFormat, dstFormat;
>     GLenum srcIntFormat, dstIntFormat;
> +   GLuint src_w, src_h, dst_w, dst_h;
>     GLuint src_bw, src_bh, dst_bw, dst_bh;
>     int dstWidth, dstHeight, dstDepth;
>     int i;
> @@ -445,17 +452,18 @@ _mesa_CopyImageSubData(GLuint srcName, GLenum srcTarget, GLint srcLevel,
>  
>     if (!prepare_target(ctx, srcName, srcTarget, srcLevel, srcZ, srcDepth,
>                         &srcTexImage, &srcRenderbuffer, &srcFormat,
> -                       &srcIntFormat, "src"))
> +                       &srcIntFormat, &src_w, &src_h, "src"))
>        return;
>  
>     if (!prepare_target(ctx, dstName, dstTarget, dstLevel, dstZ, srcDepth,
>                         &dstTexImage, &dstRenderbuffer, &dstFormat,
> -                       &dstIntFormat, "dst"))
> +                       &dstIntFormat, &dst_w, &dst_h, "dst"))
>        return;
>  
>     _mesa_get_format_block_size(srcFormat, &src_bw, &src_bh);
>     if ((srcX % src_bw != 0) || (srcY % src_bh != 0) ||
> -       (srcWidth % src_bw != 0) || (srcHeight % src_bh != 0)) {
> +       (srcWidth % src_bw != 0 && (srcX + srcWidth) != src_w) ||
> +       (srcHeight % src_bh != 0 && (srcY + srcHeight) != src_h)) {
>        _mesa_error(ctx, GL_INVALID_VALUE,
>                    "glCopyImageSubData(unaligned src rectangle)");
>        return;
>