[Mesa-dev] [PATCH rework] mesa: rework Driver.CopyImageSubData() and related code

Jason Ekstrand jason at jlekstrand.net
Thu Aug 27 23:43:55 PDT 2015


On Thu, Aug 27, 2015 at 11:42 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
> From: Brian Paul <brianp at vmware.com>
>
> Previously, core Mesa's _mesa_CopyImageSubData() created temporary textures
> to wrap renderbuffer sources/destinations.  This caused a bit of a mess in
> the Mesa/gallium state tracker because we had to basically undo that
> wrapping.
>
> Instead, change ctx->Driver.CopyImageSubData() to take both gl_renderbuffer
> and gl_texture_image src/dst pointers (one being null, the other non-null)
> so the driver can handle renderbuffer vs. texture as needed.
>
> For the i965 driver, we basically moved the code that wrapped textures
> around renderbuffers from copyimage.c down into the driver.  So that
> approach is still used there as before.
>
> The old code in copyimage.c also made some questionable calls to
> _mesa_BindTexture(), etc. which weren't undone at the end.
>
> v2 (Jason Ekstrand): Rework the intel bits
> ---
>
> TBH, I haven't actually reviewed the rest of the patch yet but I figured
> I'd save Brian the time of guess-and-check implementing it on Intel.  I
> completely reworked the intel bits so we now use the tex/rb directly and
> pun the renderbuffer wrapping all the way down to the meta layer where it
> belongs.

I ran a few quick piglit tests on my Broadwell and it seems to work.
I also sent it off to our CI system and I'll have those results in the
morning.

>  src/mesa/drivers/common/meta.h               |   2 +
>  src/mesa/drivers/common/meta_copy_image.c    | 103 ++++++++-
>  src/mesa/drivers/dri/i965/intel_copy_image.c |  80 +++++--
>  src/mesa/main/copyimage.c                    | 301 +++++++++++++++------------
>  src/mesa/main/dd.h                           |  15 +-
>  5 files changed, 335 insertions(+), 166 deletions(-)
>
> diff --git a/src/mesa/drivers/common/meta.h b/src/mesa/drivers/common/meta.h
> index fe43915..23fa209 100644
> --- a/src/mesa/drivers/common/meta.h
> +++ b/src/mesa/drivers/common/meta.h
> @@ -494,8 +494,10 @@ _mesa_meta_and_swrast_BlitFramebuffer(struct gl_context *ctx,
>  bool
>  _mesa_meta_CopyImageSubData_uncompressed(struct gl_context *ctx,
>                                           struct gl_texture_image *src_tex_image,
> +                                         struct gl_renderbuffer *src_renderbuffer,
>                                           int src_x, int src_y, int src_z,
>                                           struct gl_texture_image *dst_tex_image,
> +                                         struct gl_renderbuffer *dst_renderbuffer,
>                                           int dst_x, int dst_y, int dst_z,
>                                           int src_width, int src_height);
>
> diff --git a/src/mesa/drivers/common/meta_copy_image.c b/src/mesa/drivers/common/meta_copy_image.c
> index 149ed18..33490ee 100644
> --- a/src/mesa/drivers/common/meta_copy_image.c
> +++ b/src/mesa/drivers/common/meta_copy_image.c
> @@ -35,6 +35,46 @@
>  #include "mtypes.h"
>  #include "meta.h"
>
> +/**
> + * Create a texture image that wraps a renderbuffer.
> + */
> +static struct gl_texture_image *
> +wrap_renderbuffer(struct gl_context *ctx, struct gl_renderbuffer *rb)
> +{
> +   GLenum texTarget;
> +   struct gl_texture_object *texObj;
> +   struct gl_texture_image *texImage;
> +
> +   if (rb->NumSamples > 1)
> +      texTarget = GL_TEXTURE_2D_MULTISAMPLE;
> +   else
> +      texTarget = GL_TEXTURE_2D;
> +
> +   /* Texture ID is not significant since it never goes into the hash table */
> +   texObj = ctx->Driver.NewTextureObject(ctx, 0, texTarget);
> +   assert(texObj);
> +   if (!texObj)
> +      return NULL;
> +
> +   texImage = _mesa_get_tex_image(ctx, texObj, texTarget, 0);
> +   assert(texImage);
> +   if (!texImage)
> +      return NULL;
> +
> +   if (!ctx->Driver.BindRenderbufferTexImage(ctx, rb, texImage)) {
> +      _mesa_problem(ctx, "Failed to create texture from renderbuffer");
> +      return NULL;
> +   }
> +
> +   if (ctx->Driver.FinishRenderTexture && !rb->NeedsFinishRenderTexture) {
> +      rb->NeedsFinishRenderTexture = true;
> +      ctx->Driver.FinishRenderTexture(ctx, rb);
> +   }
> +
> +   return texImage;
> +}
> +
> +
>  /* This function makes a texture view without bothering with all of the API
>   * checks.  Most of them are the same for CopyTexSubImage so checking would
>   * be redundant.  The one major difference is that we don't check for
> @@ -112,11 +152,15 @@ make_view(struct gl_context *ctx, struct gl_texture_image *tex_image,
>  bool
>  _mesa_meta_CopyImageSubData_uncompressed(struct gl_context *ctx,
>                                           struct gl_texture_image *src_tex_image,
> +                                         struct gl_renderbuffer *src_renderbuffer,
>                                           int src_x, int src_y, int src_z,
>                                           struct gl_texture_image *dst_tex_image,
> +                                         struct gl_renderbuffer *dst_renderbuffer,
>                                           int dst_x, int dst_y, int dst_z,
>                                           int src_width, int src_height)
>  {
> +   mesa_format src_format, dst_format;
> +   GLint src_internal_format, dst_internal_format;
>     GLuint src_view_texture = 0;
>     struct gl_texture_image *src_view_tex_image;
>     GLuint fbos[2];
> @@ -124,15 +168,37 @@ _mesa_meta_CopyImageSubData_uncompressed(struct gl_context *ctx,
>     GLbitfield mask;
>     GLenum status, attachment;
>
> -   if (_mesa_is_format_compressed(dst_tex_image->TexFormat))
> +   if (src_renderbuffer) {
> +      src_format = src_renderbuffer->Format;
> +      src_internal_format = src_renderbuffer->InternalFormat;
> +   } else {
> +      assert(src_tex_image);
> +      src_format = src_tex_image->TexFormat;
> +      src_internal_format = src_tex_image->InternalFormat;
> +   }
> +
> +   if (dst_renderbuffer) {
> +      dst_format = dst_renderbuffer->Format;
> +      dst_internal_format = dst_renderbuffer->InternalFormat;
> +   } else {
> +      assert(dst_tex_image);
> +      dst_format = dst_tex_image->TexFormat;
> +      dst_internal_format = dst_tex_image->InternalFormat;
> +   }
> +
> +   if (_mesa_is_format_compressed(src_format))
>        return false;
>
> -   if (_mesa_is_format_compressed(src_tex_image->TexFormat))
> +   if (_mesa_is_format_compressed(dst_format))
>        return false;
>
> -   if (src_tex_image->InternalFormat == dst_tex_image->InternalFormat) {
> +   if (src_internal_format == dst_internal_format) {
>        src_view_tex_image = src_tex_image;
>     } else {
> +      if (src_renderbuffer) {
> +         assert(src_tex_image == NULL);
> +         src_tex_image = wrap_renderbuffer(ctx, src_renderbuffer);
> +      }
>        if (!make_view(ctx, src_tex_image, &src_view_tex_image, &src_view_texture,
>                       dst_tex_image->InternalFormat))
>           goto cleanup;
> @@ -145,7 +211,7 @@ _mesa_meta_CopyImageSubData_uncompressed(struct gl_context *ctx,
>     _mesa_BindFramebuffer(GL_READ_FRAMEBUFFER, fbos[0]);
>     _mesa_BindFramebuffer(GL_DRAW_FRAMEBUFFER, fbos[1]);
>
> -   switch (_mesa_get_format_base_format(src_tex_image->TexFormat)) {
> +   switch (_mesa_get_format_base_format(src_format)) {
>     case GL_DEPTH_COMPONENT:
>        attachment = GL_DEPTH_ATTACHMENT;
>        mask = GL_DEPTH_BUFFER_BIT;
> @@ -165,15 +231,32 @@ _mesa_meta_CopyImageSubData_uncompressed(struct gl_context *ctx,
>        _mesa_ReadBuffer(GL_COLOR_ATTACHMENT0);
>     }
>
> -   _mesa_meta_bind_fbo_image(GL_READ_FRAMEBUFFER, attachment,
> -                             src_view_tex_image, src_z);
> +   if (src_view_tex_image) {
> +      /* Prever the tex image because, even if we have a renderbuffer, we may
> +       * have had to wrap it in a texture view.
> +       */
> +      _mesa_meta_bind_fbo_image(GL_READ_FRAMEBUFFER, attachment,
> +                                src_view_tex_image, src_z);
> +   } else {
> +      _mesa_FramebufferRenderbuffer(GL_READ_FRAMEBUFFER,
> +                                    attachment,
> +                                    GL_RENDERBUFFER,
> +                                    src_renderbuffer->Name);
> +   }
>
>     status = _mesa_CheckFramebufferStatus(GL_READ_FRAMEBUFFER);
>     if (status != GL_FRAMEBUFFER_COMPLETE)
>        goto meta_end;
>
> -   _mesa_meta_bind_fbo_image(GL_DRAW_FRAMEBUFFER, attachment,
> -                             dst_tex_image, dst_z);
> +   if (dst_renderbuffer) {
> +      _mesa_FramebufferRenderbuffer(GL_DRAW_FRAMEBUFFER,
> +                                    attachment,
> +                                    GL_RENDERBUFFER,
> +                                    dst_renderbuffer->Name);
> +   } else {
> +      _mesa_meta_bind_fbo_image(GL_DRAW_FRAMEBUFFER, attachment,
> +                                dst_tex_image, dst_z);
> +   }
>
>     status = _mesa_CheckFramebufferStatus(GL_DRAW_FRAMEBUFFER);
>     if (status != GL_FRAMEBUFFER_COMPLETE)
> @@ -205,5 +288,9 @@ meta_end:
>  cleanup:
>     _mesa_DeleteTextures(1, &src_view_texture);
>
> +   /* If we got a renderbuffer source, delete the temporary texture */
> +   if (src_renderbuffer && src_tex_image)
> +      ctx->Driver.DeleteTexture(ctx, src_tex_image->TexObject);
> +
>     return success;
>  }
> diff --git a/src/mesa/drivers/dri/i965/intel_copy_image.c b/src/mesa/drivers/dri/i965/intel_copy_image.c
> index ac2738f..d57651c 100644
> --- a/src/mesa/drivers/dri/i965/intel_copy_image.c
> +++ b/src/mesa/drivers/dri/i965/intel_copy_image.c
> @@ -25,10 +25,12 @@
>   *    Jason Ekstrand <jason.ekstrand at intel.com>
>   */
>
> +#include "intel_fbo.h"
>  #include "intel_tex.h"
>  #include "intel_blit.h"
>  #include "intel_mipmap_tree.h"
>  #include "main/formats.h"
> +#include "main/teximage.h"
>  #include "drivers/common/meta.h"
>
>  static bool
> @@ -196,54 +198,86 @@ copy_image_with_memcpy(struct brw_context *brw,
>     }
>  }
>
> +
>  static void
>  intel_copy_image_sub_data(struct gl_context *ctx,
>                            struct gl_texture_image *src_image,
> +                          struct gl_renderbuffer *src_renderbuffer,
>                            int src_x, int src_y, int src_z,
>                            struct gl_texture_image *dst_image,
> +                          struct gl_renderbuffer *dst_renderbuffer,
>                            int dst_x, int dst_y, int dst_z,
>                            int src_width, int src_height)
>  {
>     struct brw_context *brw = brw_context(ctx);
> -   struct intel_texture_image *intel_src_image = intel_texture_image(src_image);
> -   struct intel_texture_image *intel_dst_image = intel_texture_image(dst_image);
> +   struct intel_mipmap_tree *src_mt, *dst_mt;
> +   unsigned src_level, dst_level;
>
>     if (_mesa_meta_CopyImageSubData_uncompressed(ctx,
> -                                                src_image, src_x, src_y, src_z,
> -                                                dst_image, dst_x, dst_y, dst_z,
> +                                                src_image, src_renderbuffer,
> +                                                src_x, src_y, src_z,
> +                                                dst_image, dst_renderbuffer,
> +                                                dst_x, dst_y, dst_z,
>                                                  src_width, src_height)) {
>        return;
>     }
>
> -   if (intel_src_image->mt->num_samples > 0 ||
> -       intel_dst_image->mt->num_samples > 0) {
> +   if (src_image) {
> +      src_mt = intel_texture_image(src_image)->mt;
> +   } else {
> +      assert(src_renderbuffer);
> +      src_mt = intel_renderbuffer(src_renderbuffer)->mt;
> +      src_image = src_renderbuffer->TexImage;
> +   }
> +
> +   if (dst_image) {
> +      dst_mt = intel_texture_image(dst_image)->mt;
> +   } else {
> +      assert(dst_renderbuffer);
> +      dst_mt = intel_renderbuffer(dst_renderbuffer)->mt;
> +      src_image = src_renderbuffer->TexImage;
> +   }
> +
> +   if (src_mt->num_samples > 0 || dst_mt->num_samples > 0) {
>        _mesa_problem(ctx, "Failed to copy multisampled texture with meta path\n");
>        return;
>     }
>
> -   /* Cube maps actually have different images per face */
> -   if (src_image->TexObject->Target == GL_TEXTURE_CUBE_MAP)
> -      src_z = src_image->Face;
> -   if (dst_image->TexObject->Target == GL_TEXTURE_CUBE_MAP)
> -      dst_z = dst_image->Face;
> +   if (src_image) {
> +      src_level = src_image->Level + src_image->TexObject->MinLevel;
> +
> +      /* Cube maps actually have different images per face */
> +      if (src_image->TexObject->Target == GL_TEXTURE_CUBE_MAP)
> +         src_z = src_image->Face;
> +   } else {
> +      src_level = 0;
> +   }
> +
> +   if (dst_image) {
> +      dst_level = dst_image->Level + dst_image->TexObject->MinLevel;
> +
> +      /* Cube maps actually have different images per face */
> +      if (dst_image->TexObject->Target == GL_TEXTURE_CUBE_MAP)
> +         dst_z = dst_image->Face;
> +   } else {
> +      dst_level = 0;
> +   }
>
>     /* We are now going to try and copy the texture using the blitter.  If
>      * that fails, we will fall back mapping the texture and using memcpy.
>      * In either case, we need to do a full resolve.
>      */
> -   intel_miptree_all_slices_resolve_hiz(brw, intel_src_image->mt);
> -   intel_miptree_all_slices_resolve_depth(brw, intel_src_image->mt);
> -   intel_miptree_resolve_color(brw, intel_src_image->mt);
> +   intel_miptree_all_slices_resolve_hiz(brw, src_mt);
> +   intel_miptree_all_slices_resolve_depth(brw, src_mt);
> +   intel_miptree_resolve_color(brw, src_mt);
>
> -   intel_miptree_all_slices_resolve_hiz(brw, intel_dst_image->mt);
> -   intel_miptree_all_slices_resolve_depth(brw, intel_dst_image->mt);
> -   intel_miptree_resolve_color(brw, intel_dst_image->mt);
> +   intel_miptree_all_slices_resolve_hiz(brw, dst_mt);
> +   intel_miptree_all_slices_resolve_depth(brw, dst_mt);
> +   intel_miptree_resolve_color(brw, dst_mt);
>
> -   unsigned src_level = src_image->Level + src_image->TexObject->MinLevel;
> -   unsigned dst_level = dst_image->Level + dst_image->TexObject->MinLevel;
> -   if (copy_image_with_blitter(brw, intel_src_image->mt, src_level,
> +   if (copy_image_with_blitter(brw, src_mt, src_level,
>                                 src_x, src_y, src_z,
> -                               intel_dst_image->mt, dst_level,
> +                               dst_mt, dst_level,
>                                 dst_x, dst_y, dst_z,
>                                 src_width, src_height))
>        return;
> @@ -251,9 +285,9 @@ intel_copy_image_sub_data(struct gl_context *ctx,
>     /* This is a worst-case scenario software fallback that maps the two
>      * textures and does a memcpy between them.
>      */
> -   copy_image_with_memcpy(brw, intel_src_image->mt, src_level,
> +   copy_image_with_memcpy(brw, src_mt, src_level,
>                            src_x, src_y, src_z,
> -                          intel_dst_image->mt, dst_level,
> +                          dst_mt, dst_level,
>                            dst_x, dst_y, dst_z,
>                            src_width, src_height);
>  }
> diff --git a/src/mesa/main/copyimage.c b/src/mesa/main/copyimage.c
> index 05bc50d..f02e842 100644
> --- a/src/mesa/main/copyimage.c
> +++ b/src/mesa/main/copyimage.c
> @@ -41,22 +41,27 @@ enum mesa_block_class {
>  };
>
>  /**
> - * Prepare the source or destination resource, including:
> - * - Error checking
> - * - Creating texture wrappers for renderbuffers
> + * Prepare the source or destination resource.  This involves error
> + * checking and returning the relevant gl_texture_image or gl_renderbuffer.
> + * Note that one of the resulting tex_image or renderbuffer pointers will be
> + * NULL and the other will be non-null.
> + *
>   * \param name  the texture or renderbuffer name
> - * \param target  GL_TEXTURE target or GL_RENDERBUFFER.  For the later, will
> - *                be changed to a compatible GL_TEXTURE target.
> + * \param target  One of GL_TEXTURE_x target or GL_RENDERBUFFER
>   * \param level  mipmap level
> - * \param tex_obj  returns a pointer to a texture object
> + * \param z  src or dest Z
> + * \param depth  number of slices/faces/layers to copy
>   * \param tex_image  returns a pointer to a texture image
> - * \param tmp_tex  returns temporary texture object name
> + * \param renderbuffer  returns a pointer to a renderbuffer
>   * \return true if success, false if error
>   */
>  static bool
> -prepare_target(struct gl_context *ctx, GLuint name, GLenum *target, int level,
> -               struct gl_texture_object **tex_obj,
> -               struct gl_texture_image **tex_image, GLuint *tmp_tex,
> +prepare_target(struct gl_context *ctx, GLuint name, GLenum target,
> +               int level, int z, int depth,
> +               struct gl_texture_image **tex_image,
> +               struct gl_renderbuffer **renderbuffer,
> +               mesa_format *format,
> +               GLenum *internalFormat,
>                 const char *dbg_prefix)
>  {
>     if (name == 0) {
> @@ -72,7 +77,7 @@ prepare_target(struct gl_context *ctx, GLuint name, GLenum *target, int level,
>      *   - is TEXTURE_BUFFER, or
>      *   - is one of the cubemap face selectors described in table 3.17,
>      */
> -   switch (*target) {
> +   switch (target) {
>     case GL_RENDERBUFFER:
>        /* Not a texture target, but valid */
>     case GL_TEXTURE_1D:
> @@ -93,12 +98,13 @@ prepare_target(struct gl_context *ctx, GLuint name, GLenum *target, int level,
>     default:
>        _mesa_error(ctx, GL_INVALID_ENUM,
>                    "glCopyImageSubData(%sTarget = %s)", dbg_prefix,
> -                  _mesa_enum_to_string(*target));
> +                  _mesa_enum_to_string(target));
>        return false;
>     }
>
> -   if (*target == GL_RENDERBUFFER) {
> +   if (target == GL_RENDERBUFFER) {
>        struct gl_renderbuffer *rb = _mesa_lookup_renderbuffer(ctx, name);
> +
>        if (!rb) {
>           _mesa_error(ctx, GL_INVALID_VALUE,
>                       "glCopyImageSubData(%sName = %u)", dbg_prefix, name);
> @@ -117,49 +123,38 @@ prepare_target(struct gl_context *ctx, GLuint name, GLenum *target, int level,
>           return false;
>        }
>
> -      if (rb->NumSamples > 1)
> -         *target = GL_TEXTURE_2D_MULTISAMPLE;
> -      else
> -         *target = GL_TEXTURE_2D;
> -
> -      *tmp_tex = 0;
> -      _mesa_GenTextures(1, tmp_tex);
> -      if (*tmp_tex == 0)
> -         return false; /* Error already set by GenTextures */
> -
> -      _mesa_BindTexture(*target, *tmp_tex);
> -      *tex_obj = _mesa_lookup_texture(ctx, *tmp_tex);
> -      *tex_image = _mesa_get_tex_image(ctx, *tex_obj, *target, 0);
> -
> -      if (!ctx->Driver.BindRenderbufferTexImage(ctx, rb, *tex_image)) {
> -         _mesa_problem(ctx, "Failed to create texture from renderbuffer");
> -         return false;
> -      }
> -
> -      if (ctx->Driver.FinishRenderTexture && !rb->NeedsFinishRenderTexture) {
> -         rb->NeedsFinishRenderTexture = true;
> -         ctx->Driver.FinishRenderTexture(ctx, rb);
> -      }
> +      *renderbuffer = rb;
> +      *format = rb->Format;
> +      *internalFormat = rb->InternalFormat;
> +      *tex_image = NULL;
>     } else {
> -      *tex_obj = _mesa_lookup_texture(ctx, name);
> -      if (!*tex_obj) {
> +      struct gl_texture_object *texObj = _mesa_lookup_texture(ctx, name);
> +
> +      if (!texObj) {
>           _mesa_error(ctx, GL_INVALID_VALUE,
>                       "glCopyImageSubData(%sName = %u)", dbg_prefix, name);
>           return false;
>        }
>
> -      _mesa_test_texobj_completeness(ctx, *tex_obj);
> -      if (!(*tex_obj)->_BaseComplete ||
> -          (level != 0 && !(*tex_obj)->_MipmapComplete)) {
> +      _mesa_test_texobj_completeness(ctx, texObj);
> +      if (!texObj->_BaseComplete ||
> +          (level != 0 && !texObj->_MipmapComplete)) {
>           _mesa_error(ctx, GL_INVALID_OPERATION,
>                       "glCopyImageSubData(%sName incomplete)", dbg_prefix);
>           return false;
>        }
>
> -      if ((*tex_obj)->Target != *target) {
> -         _mesa_error(ctx, GL_INVALID_ENUM,
> +      /* Note that target will not be a cube face name */
> +      if (texObj->Target != target) {
> +         /*
> +          * From GL_ARB_copy_image specification:
> +          * "INVALID_VALUE is generated if either <srcName> or <dstName> does
> +          * not correspond to a valid renderbuffer or texture object according
> +          * to the corresponding target parameter."
> +          */
> +         _mesa_error(ctx, GL_INVALID_VALUE,
>                       "glCopyImageSubData(%sTarget = %s)", dbg_prefix,
> -                     _mesa_enum_to_string(*target));
> +                     _mesa_enum_to_string(target));
>           return false;
>        }
>
> @@ -169,12 +164,36 @@ prepare_target(struct gl_context *ctx, GLuint name, GLenum *target, int level,
>           return false;
>        }
>
> -      *tex_image = _mesa_select_tex_image(*tex_obj, *target, level);
> +      if (target == GL_TEXTURE_CUBE_MAP) {
> +         int i;
> +
> +         assert(z < MAX_FACES);  /* should have been caught earlier */
> +
> +         /* make sure all the cube faces are present */
> +         for (i = 0; i < depth; i++) {
> +            if (!texObj->Image[z+i][level]) {
> +               /* missing cube face */
> +               _mesa_error(ctx, GL_INVALID_OPERATION,
> +                           "glCopyImageSubData(missing cube face)");
> +               return false;
> +            }
> +         }
> +
> +         *tex_image = texObj->Image[z][level];
> +      }
> +      else {
> +         *tex_image = _mesa_select_tex_image(texObj, target, level);
> +      }
> +
>        if (!*tex_image) {
>           _mesa_error(ctx, GL_INVALID_VALUE,
>                       "glCopyImageSubData(%sLevel = %u)", dbg_prefix, level);
>           return false;
>        }
> +
> +      *renderbuffer = NULL;
> +      *format = (*tex_image)->TexFormat;
> +      *internalFormat = (*tex_image)->InternalFormat;
>     }
>
>     return true;
> @@ -188,10 +207,14 @@ prepare_target(struct gl_context *ctx, GLuint name, GLenum *target, int level,
>   */
>  static bool
>  check_region_bounds(struct gl_context *ctx,
> +                    GLenum target,
>                      const struct gl_texture_image *tex_image,
> +                    const struct gl_renderbuffer *renderbuffer,
>                      int x, int y, int z, int width, int height, int depth,
>                      const char *dbg_prefix)
>  {
> +   int surfWidth, surfHeight, surfDepth;
> +
>     if (width < 0 || height < 0 || depth < 0) {
>        _mesa_error(ctx, GL_INVALID_VALUE,
>                    "glCopyImageSubData(%sWidth, %sHeight, or %sDepth is negative)",
> @@ -207,7 +230,14 @@ check_region_bounds(struct gl_context *ctx,
>     }
>
>     /* Check X direction */
> -   if (x + width > tex_image->Width) {
> +   if (target == GL_RENDERBUFFER) {
> +      surfWidth = renderbuffer->Width;
> +   }
> +   else {
> +      surfWidth = tex_image->Width;
> +   }
> +
> +   if (x + width > surfWidth) {
>        _mesa_error(ctx, GL_INVALID_VALUE,
>                    "glCopyImageSubData(%sX or %sWidth exceeds image bounds)",
>                    dbg_prefix, dbg_prefix);
> @@ -215,66 +245,49 @@ check_region_bounds(struct gl_context *ctx,
>     }
>
>     /* Check Y direction */
> -   switch (tex_image->TexObject->Target) {
> +   switch (target) {
> +   case GL_RENDERBUFFER:
> +      surfHeight = renderbuffer->Height;
> +      break;
>     case GL_TEXTURE_1D:
>     case GL_TEXTURE_1D_ARRAY:
> -      if (y != 0 || height != 1) {
> -         _mesa_error(ctx, GL_INVALID_VALUE,
> -                     "glCopyImageSubData(%sY or %sHeight exceeds image bounds)",
> -                     dbg_prefix, dbg_prefix);
> -         return false;
> -      }
> +      surfHeight = 1;
>        break;
>     default:
> -      if (y + height > tex_image->Height) {
> -         _mesa_error(ctx, GL_INVALID_VALUE,
> -                     "glCopyImageSubData(%sY or %sHeight exceeds image bounds)",
> -                     dbg_prefix, dbg_prefix);
> -         return false;
> -      }
> -      break;
> +      surfHeight = tex_image->Height;
> +   }
> +
> +   if (y + height > surfHeight) {
> +      _mesa_error(ctx, GL_INVALID_VALUE,
> +                  "glCopyImageSubData(%sY or %sHeight exceeds image bounds)",
> +                  dbg_prefix, dbg_prefix);
> +      return false;
>     }
>
>     /* Check Z direction */
> -   switch (tex_image->TexObject->Target) {
> +   switch (target) {
> +   case GL_RENDERBUFFER:
>     case GL_TEXTURE_1D:
>     case GL_TEXTURE_2D:
>     case GL_TEXTURE_2D_MULTISAMPLE:
>     case GL_TEXTURE_RECTANGLE:
> -      if (z != 0 || depth != 1) {
> -         _mesa_error(ctx, GL_INVALID_VALUE,
> -                     "glCopyImageSubData(%sZ or %sDepth exceeds image bounds)",
> -                     dbg_prefix, dbg_prefix);
> -         return false;
> -      }
> +      surfDepth = 1;
>        break;
>     case GL_TEXTURE_CUBE_MAP:
> -      if (z < 0 || z + depth > 6) {
> -         _mesa_error(ctx, GL_INVALID_VALUE,
> -                     "glCopyImageSubData(%sZ or %sDepth exceeds image bounds)",
> -                     dbg_prefix, dbg_prefix);
> -         return false;
> -      }
> +      surfDepth = 6;
>        break;
>     case GL_TEXTURE_1D_ARRAY:
> -      if (z < 0 || z + depth > tex_image->Height) {
> -         _mesa_error(ctx, GL_INVALID_VALUE,
> -                     "glCopyImageSubData(%sZ or %sDepth exceeds image bounds)",
> -                     dbg_prefix, dbg_prefix);
> -         return false;
> -      }
> -      break;
> -   case GL_TEXTURE_CUBE_MAP_ARRAY:
> -   case GL_TEXTURE_2D_ARRAY:
> -   case GL_TEXTURE_2D_MULTISAMPLE_ARRAY:
> -   case GL_TEXTURE_3D:
> -      if (z < 0 || z + depth > tex_image->Depth) {
> -         _mesa_error(ctx, GL_INVALID_VALUE,
> -                     "glCopyImageSubData(%sZ or %sDepth exceeds image bounds)",
> -                     dbg_prefix, dbg_prefix);
> -         return false;
> -      }
> +      surfDepth = tex_image->Height;
>        break;
> +   default:
> +      surfDepth = tex_image->Depth;
> +   }
> +
> +   if (z < 0 || z + depth > surfDepth) {
> +      _mesa_error(ctx, GL_INVALID_VALUE,
> +                  "glCopyImageSubData(%sZ or %sDepth exceeds image bounds)",
> +                  dbg_prefix, dbg_prefix);
> +      return false;
>     }
>
>     return true;
> @@ -406,10 +419,12 @@ _mesa_CopyImageSubData(GLuint srcName, GLenum srcTarget, GLint srcLevel,
>                         GLsizei srcWidth, GLsizei srcHeight, GLsizei srcDepth)
>  {
>     GET_CURRENT_CONTEXT(ctx);
> -   GLuint tmpTexNames[2] = { 0, 0 };
> -   struct gl_texture_object *srcTexObj, *dstTexObj;
>     struct gl_texture_image *srcTexImage, *dstTexImage;
> +   struct gl_renderbuffer *srcRenderbuffer, *dstRenderbuffer;
> +   mesa_format srcFormat, dstFormat;
> +   GLenum srcIntFormat, dstIntFormat;
>     GLuint src_bw, src_bh, dst_bw, dst_bh;
> +   int dstWidth, dstHeight, dstDepth;
>     int i;
>
>     if (MESA_VERBOSE & VERBOSE_API)
> @@ -420,7 +435,7 @@ _mesa_CopyImageSubData(GLuint srcName, GLenum srcTarget, GLint srcLevel,
>                    srcX, srcY, srcZ,
>                    dstName, _mesa_enum_to_string(dstTarget), dstLevel,
>                    dstX, dstY, dstZ,
> -                  srcWidth, srcHeight, srcWidth);
> +                  srcWidth, srcHeight, srcDepth);
>
>     if (!ctx->Extensions.ARB_copy_image) {
>        _mesa_error(ctx, GL_INVALID_OPERATION,
> @@ -428,67 +443,93 @@ _mesa_CopyImageSubData(GLuint srcName, GLenum srcTarget, GLint srcLevel,
>        return;
>     }
>
> -   if (!prepare_target(ctx, srcName, &srcTarget, srcLevel,
> -                       &srcTexObj, &srcTexImage, &tmpTexNames[0], "src"))
> -      goto cleanup;
> +   if (!prepare_target(ctx, srcName, srcTarget, srcLevel, srcZ, srcDepth,
> +                       &srcTexImage, &srcRenderbuffer, &srcFormat,
> +                       &srcIntFormat, "src"))
> +      return;
>
> -   if (!prepare_target(ctx, dstName, &dstTarget, dstLevel,
> -                       &dstTexObj, &dstTexImage, &tmpTexNames[1], "dst"))
> -      goto cleanup;
> +   if (!prepare_target(ctx, dstName, dstTarget, dstLevel, dstZ, srcDepth,
> +                       &dstTexImage, &dstRenderbuffer, &dstFormat,
> +                       &dstIntFormat, "dst"))
> +      return;
>
> -   _mesa_get_format_block_size(srcTexImage->TexFormat, &src_bw, &src_bh);
> +   _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)) {
>        _mesa_error(ctx, GL_INVALID_VALUE,
>                    "glCopyImageSubData(unaligned src rectangle)");
> -      goto cleanup;
> +      return;
>     }
>
> -   _mesa_get_format_block_size(dstTexImage->TexFormat, &dst_bw, &dst_bh);
> +   _mesa_get_format_block_size(dstFormat, &dst_bw, &dst_bh);
>     if ((dstX % dst_bw != 0) || (dstY % dst_bh != 0)) {
>        _mesa_error(ctx, GL_INVALID_VALUE,
>                    "glCopyImageSubData(unaligned dst rectangle)");
> -      goto cleanup;
> +      return;
>     }
>
> -   if (!check_region_bounds(ctx, srcTexImage, srcX, srcY, srcZ,
> -                            srcWidth, srcHeight, srcDepth, "src"))
> -      goto cleanup;
> +   /* From the GL_ARB_copy_image spec:
> +    *
> +    * "The dimensions are always specified in texels, even for compressed
> +    * texture formats. But it should be noted that if only one of the
> +    * source and destination textures is compressed then the number of
> +    * texels touched in the compressed image will be a factor of the
> +    * block size larger than in the uncompressed image."
> +    *
> +    * So, if copying from compressed to uncompressed, the dest region is
> +    * shrunk by the src block size factor.  If copying from uncompressed
> +    * to compressed, the dest region is grown by the dest block size factor.
> +    * Note that we're passed the _source_ width, height, depth and those
> +    * dimensions are never changed.
> +    */
> +   dstWidth = srcWidth * dst_bw / src_bw;
> +   dstHeight = srcHeight * dst_bh / src_bh;
> +   dstDepth = srcDepth;
> +
> +   if (!check_region_bounds(ctx, srcTarget, srcTexImage, srcRenderbuffer,
> +                            srcX, srcY, srcZ, srcWidth, srcHeight, srcDepth,
> +                            "src"))
> +      return;
>
> -   if (!check_region_bounds(ctx, dstTexImage, dstX, dstY, dstZ,
> -                            (srcWidth / src_bw) * dst_bw,
> -                            (srcHeight / src_bh) * dst_bh, srcDepth, "dst"))
> -      goto cleanup;
> +   if (!check_region_bounds(ctx, dstTarget, dstTexImage, dstRenderbuffer,
> +                            dstX, dstY, dstZ, dstWidth, dstHeight, dstDepth,
> +                            "dst"))
> +      return;
>
> -   if (!copy_format_compatible(ctx, srcTexImage->InternalFormat,
> -                               dstTexImage->InternalFormat)) {
> +   if (!copy_format_compatible(ctx, srcIntFormat, dstIntFormat)) {
>        _mesa_error(ctx, GL_INVALID_OPERATION,
>                    "glCopyImageSubData(internalFormat mismatch)");
> -      goto cleanup;
> +      return;
>     }
>
> +   /* loop over 2D slices/faces/layers */
>     for (i = 0; i < srcDepth; ++i) {
> -      int srcNewZ, dstNewZ;
> -
> -      if (srcTexObj->Target == GL_TEXTURE_CUBE_MAP) {
> -         srcTexImage = srcTexObj->Image[i + srcZ][srcLevel];
> -         srcNewZ = 0;
> -      } else {
> -         srcNewZ = srcZ + i;
> +      int newSrcZ = srcZ + i;
> +      int newDstZ = dstZ + i;
> +
> +      if (srcTexImage &&
> +          srcTexImage->TexObject->Target == GL_TEXTURE_CUBE_MAP) {
> +         /* need to update srcTexImage pointer for the cube face */
> +         assert(srcZ + i < MAX_FACES);
> +         srcTexImage = srcTexImage->TexObject->Image[srcZ + i][srcLevel];
> +         assert(srcTexImage);
> +         newSrcZ = 0;
>        }
>
> -      if (dstTexObj->Target == GL_TEXTURE_CUBE_MAP) {
> -         dstTexImage = dstTexObj->Image[i + dstZ][dstLevel];
> -         dstNewZ = 0;
> -      } else {
> -         dstNewZ = dstZ + i;
> +      if (dstTexImage &&
> +          dstTexImage->TexObject->Target == GL_TEXTURE_CUBE_MAP) {
> +         /* need to update dstTexImage pointer for the cube face */
> +         assert(dstZ + i < MAX_FACES);
> +         dstTexImage = dstTexImage->TexObject->Image[dstZ + i][dstLevel];
> +         assert(dstTexImage);
> +         newDstZ = 0;
>        }
>
> -      ctx->Driver.CopyImageSubData(ctx, srcTexImage, srcX, srcY, srcNewZ,
> -                                   dstTexImage, dstX, dstY, dstNewZ,
> +      ctx->Driver.CopyImageSubData(ctx,
> +                                   srcTexImage, srcRenderbuffer,
> +                                   srcX, srcY, newSrcZ,
> +                                   dstTexImage, dstRenderbuffer,
> +                                   dstX, dstY, newDstZ,
>                                     srcWidth, srcHeight);
>     }
> -
> -cleanup:
> -   _mesa_DeleteTextures(2, tmpTexNames);
>  }
> diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h
> index 87eb63e..2c746fc 100644
> --- a/src/mesa/main/dd.h
> +++ b/src/mesa/main/dd.h
> @@ -269,20 +269,25 @@ struct dd_function_table {
>                             struct gl_renderbuffer *rb,
>                             GLint x, GLint y,
>                             GLsizei width, GLsizei height);
> -
>     /**
>      * Called by glCopyImageSubData().
>      *
> -    * This function should copy one 2-D slice from srcTexImage to
> -    * dstTexImage.  If one of the textures is 3-D or is a 1-D or 2-D array
> +    * This function should copy one 2-D slice from src_teximage or
> +    * src_renderbuffer to dst_teximage or dst_renderbuffer.  Either the
> +    * teximage or renderbuffer pointer will be non-null to indicate which
> +    * is the real src/dst.
> +    *
> +    * If one of the textures is 3-D or is a 1-D or 2-D array
>      * texture, this function will be called multiple times: once for each
>      * slice.  If one of the textures is a cube map, this function will be
>      * called once for each face to be copied.
>      */
>     void (*CopyImageSubData)(struct gl_context *ctx,
> -                            struct gl_texture_image *src_image,
> +                            struct gl_texture_image *src_teximage,
> +                            struct gl_renderbuffer *src_renderbuffer,
>                              int src_x, int src_y, int src_z,
> -                            struct gl_texture_image *dstTexImage,
> +                            struct gl_texture_image *dst_teximage,
> +                            struct gl_renderbuffer *dst_renderbuffer,
>                              int dst_x, int dst_y, int dst_z,
>                              int src_width, int src_height);
>
> --
> 2.5.0.400.gff86faf
>


More information about the mesa-dev mailing list