[Mesa-dev] [PATCH v2] i965: Allow intel_try_pbo_upload for 3D and array textures

Jason Ekstrand jason at jlekstrand.net
Wed Jan 7 20:28:52 PST 2015


On Mon, Dec 22, 2014 at 4:32 PM, Neil Roberts <neil at linux.intel.com> wrote:

> I just realised I made regular cube map textures stop working via the
> blit path with this patch. Here is a v2 which just adds
> GL_TEXTURE_CUBE_MAP to the switch in intel_try_pbo_upload. I've tested
> that it still works with a hacky tweak to the piglit test case.
>
> ------- >8 --------------- (use git am --scissors to automatically chop
> here)
>
> intel_try_pbo_upload now iterates over each slice of the uploaded data and
> and does a separate blit for each image. This copies in some fiddly details
> store_texsubimage in order to handle the image stride correctly for 1D
> array
> textures.
> ---
>  src/mesa/drivers/dri/i965/intel_tex.h          |   4 +-
>  src/mesa/drivers/dri/i965/intel_tex_image.c    | 116
> ++++++++++++++++++-------
>  src/mesa/drivers/dri/i965/intel_tex_subimage.c |   7 +-
>  3 files changed, 88 insertions(+), 39 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_tex.h
> b/src/mesa/drivers/dri/i965/intel_tex.h
> index dfe1b03..f941d26 100644
> --- a/src/mesa/drivers/dri/i965/intel_tex.h
> +++ b/src/mesa/drivers/dri/i965/intel_tex.h
> @@ -71,8 +71,8 @@ intel_texsubimage_tiled_memcpy(struct gl_context *ctx,
>  bool
>  intel_try_pbo_upload(struct gl_context *ctx,
>                       struct gl_texture_image *image,
> -                     GLint xoffset, GLint yoffset,
> -                     GLsizei width, GLsizei height,
> +                     GLint xoffset, GLint yoffset, GLint zoffset,
> +                     GLsizei width, GLsizei height, GLsizei depth,
>                       const struct gl_pixelstore_attrib *unpack,
>                       GLenum format, GLenum type, const void *pixels,
>                       bool for_glTexImage);
> diff --git a/src/mesa/drivers/dri/i965/intel_tex_image.c
> b/src/mesa/drivers/dri/i965/intel_tex_image.c
> index b1b49b9..6b8b953 100644
> --- a/src/mesa/drivers/dri/i965/intel_tex_image.c
> +++ b/src/mesa/drivers/dri/i965/intel_tex_image.c
> @@ -88,8 +88,8 @@ intel_miptree_create_for_teximage(struct brw_context
> *brw,
>  bool
>  intel_try_pbo_upload(struct gl_context *ctx,
>                       struct gl_texture_image *image,
> -                     GLint xoffset, GLint yoffset,
> -                     GLsizei width, GLsizei height,
> +                     GLint xoffset, GLint yoffset, GLint zoffset,
> +                     GLsizei width, GLsizei height, GLsizei depth,
>                       const struct gl_pixelstore_attrib *unpack,
>                       GLenum format, GLenum type, const void *pixels,
>                       bool for_glTexImage)
> @@ -99,6 +99,7 @@ intel_try_pbo_upload(struct gl_context *ctx,
>     struct intel_buffer_object *pbo =
> intel_buffer_object(unpack->BufferObj);
>     GLuint src_offset;
>     drm_intel_bo *src_buffer;
> +   int i;
>
>     if (!_mesa_is_bufferobj(unpack->BufferObj))
>        return false;
> @@ -126,47 +127,96 @@ intel_try_pbo_upload(struct gl_context *ctx,
>        return false;
>     }
>
> -   if (image->TexObject->Target == GL_TEXTURE_1D_ARRAY ||
> -       image->TexObject->Target == GL_TEXTURE_2D_ARRAY) {
> -      DBG("%s: no support for array textures\n", __FUNCTION__);
> -      return false;
> -   }
> -
>     int src_stride =
>        _mesa_image_row_stride(unpack, width, format, type);
> +   int dims, num_slices = 1, slice_offset = 0;
> +   int src_image_stride = 0;
> +
> +   switch (image->TexObject->Target) {
> +   case GL_TEXTURE_2D:
> +   case GL_TEXTURE_RECTANGLE:
> +   case GL_TEXTURE_CUBE_MAP:
> +      /* one image slice, nothing special needs to be done */
> +      dims = 2;
> +      src_image_stride = src_stride * height;
>

Any reason you're not using _mesa_image_image_stride here?


> +      break;
> +   case GL_TEXTURE_1D:
> +      dims = 1;
> +      src_image_stride = src_stride * height;
>

or here?


> +      break;
> +   case GL_TEXTURE_1D_ARRAY:
> +      num_slices = height;
> +      slice_offset = yoffset;
> +      height = 1;
> +      yoffset = 0;
> +      dims = 2;
> +      src_image_stride = src_stride;
>

or here?


> +      break;
> +   case GL_TEXTURE_2D_ARRAY:
> +      num_slices = depth;
> +      slice_offset = zoffset;
> +      depth = 1;
> +      zoffset = 0;
> +      dims = 3;
> +      src_image_stride = _mesa_image_image_stride(unpack, width, height,
> +                                                  format, type);
> +      break;
> +   case GL_TEXTURE_3D:
> +      num_slices = depth;
> +      slice_offset = zoffset;
> +      dims = 3;
> +      src_image_stride = _mesa_image_image_stride(unpack, width, height,
> +                                                  format, type);
> +      break;
> +   case GL_TEXTURE_CUBE_MAP_ARRAY:
> +      num_slices = depth;
> +      slice_offset = zoffset;
> +      dims = 3;
> +      src_image_stride = _mesa_image_image_stride(unpack, width, height,
> +                                                  format, type);
> +      break;
> +   default:
> +      return false;
> +   }
>

In general, I find this switch statement a big awkward.  You could just
have dims passed in and slice_offset and num_slices seem to be the same as
zoffset and depth.  The only exception here being 1-D array textures which
are a bit silly and you would have to move the yoffset and height into
zoffset and depth, but I think I'd prefer that to the giant switch.


>
>     /* note: potential 64-bit ptr to 32-bit int cast */
>     src_offset = (GLuint) (unsigned long) pixels;
> -   src_offset += _mesa_image_offset(2,
> +   src_offset += _mesa_image_offset(dims,
>                                      unpack,
>                                      width, height,
>                                      format, type,
>                                      0, 0, 0 /* img/row/column */);
> -   src_buffer = intel_bufferobj_buffer(brw, pbo,
> -                                       src_offset, src_stride * height);
>
> -   struct intel_mipmap_tree *pbo_mt =
> -      intel_miptree_create_for_bo(brw,
> -                                  src_buffer,
> -                                  intelImage->mt->format,
> -                                  src_offset,
> -                                  width, height,
> -                                  src_stride);
> -   if (!pbo_mt)
> -      return false;
> +   for (i = 0; i < num_slices; i++) {
> +      src_buffer = intel_bufferobj_buffer(brw, pbo,
> +                                          src_offset, src_stride *
> height);
> +
> +      struct intel_mipmap_tree *pbo_mt =
> +         intel_miptree_create_for_bo(brw,
> +                                     src_buffer,
> +                                     intelImage->mt->format,
> +                                     src_offset,
> +                                     width, height,
> +                                     src_stride);
> +      if (!pbo_mt)
> +         return false;
> +
> +      if (!intel_miptree_blit(brw,
> +                              pbo_mt, 0, 0,
> +                              0, 0, false,
> +                              intelImage->mt, image->Level,
> +                              slice_offset + i + image->Face,
> +                              xoffset, yoffset, false,
> +                              width, height, GL_COPY)) {
> +         DBG("%s: blit failed\n", __FUNCTION__);
> +         intel_miptree_release(&pbo_mt);
> +         return false;
> +      }
>
> -   if (!intel_miptree_blit(brw,
> -                           pbo_mt, 0, 0,
> -                           0, 0, false,
> -                           intelImage->mt, image->Level, image->Face,
> -                           xoffset, yoffset, false,
> -                           width, height, GL_COPY)) {
> -      DBG("%s: blit failed\n", __FUNCTION__);
>        intel_miptree_release(&pbo_mt);
> -      return false;
> -   }
>
> -   intel_miptree_release(&pbo_mt);
> +      src_offset += src_image_stride;
> +   }
>
>     DBG("%s: success\n", __FUNCTION__);
>     return true;
> @@ -199,11 +249,11 @@ intelTexImage(struct gl_context * ctx,
>
>     /* Attempt to use the blitter for PBO image uploads.
>      */
> -   if (dims <= 2 &&
> -       intel_try_pbo_upload(ctx, texImage,
> -                            0, 0, /* x,y offsets */
> +   if (intel_try_pbo_upload(ctx, texImage,
> +                            0, 0, 0, /* x,y,z offsets */
>                              texImage->Width,
>                              texImage->Height,
> +                            texImage->Depth,
>                              unpack, format, type, pixels,
>                              true /*for_glTexImage*/)) {
>        return;
> diff --git a/src/mesa/drivers/dri/i965/intel_tex_subimage.c
> b/src/mesa/drivers/dri/i965/intel_tex_subimage.c
> index dde1591..692e3ec 100644
> --- a/src/mesa/drivers/dri/i965/intel_tex_subimage.c
> +++ b/src/mesa/drivers/dri/i965/intel_tex_subimage.c
> @@ -693,10 +693,9 @@ intelTexSubImage(struct gl_context * ctx,
>
>     /* Attempt to use the blitter for PBO image uploads.
>      */
> -   if (dims <= 2 &&
> -       intel_try_pbo_upload(ctx, texImage,
> -                            xoffset, yoffset,
> -                            width, height,
> +   if (intel_try_pbo_upload(ctx, texImage,
> +                            xoffset, yoffset, zoffset,
> +                            width, height, depth,
>                              packing, format, type, pixels,
>                              false /*for_glTexImage*/)) {
>        return;
> --
> 1.9.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150107/ca845d47/attachment-0001.html>


More information about the mesa-dev mailing list