[Mesa-dev] [PATCH] anv: Sanitize Image extents and offsets

Jason Ekstrand jason at jlekstrand.net
Thu Mar 24 17:17:36 UTC 2016


On Thu, Mar 24, 2016 at 9:27 AM, Nanley Chery <nanleychery at gmail.com> wrote:

> From: Nanley Chery <nanley.g.chery at intel.com>
>
> Prepare Image extents and offsets for internal consumption by assigning
> the default values implicitly defined by the spec. Fixes textures on
> several Vulkan demos in which the VkImageCopy depth is set to zero when
> copying a 2D image.
>
> Signed-off-by: Nanley Chery <nanley.g.chery at intel.com>
> ---
>  src/intel/vulkan/anv_image.c        | 56
> ++++++++++++++++++++++++-------------
>  src/intel/vulkan/anv_meta_copy.c    | 54
> ++++++++++++++++++++++++-----------
>  src/intel/vulkan/anv_meta_resolve.c | 41 ++++++++++++++++++++-------
>  src/intel/vulkan/anv_private.h      |  7 +++++
>  4 files changed, 113 insertions(+), 45 deletions(-)
>
> diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c
> index 143a084..88db1fc 100644
> --- a/src/intel/vulkan/anv_image.c
> +++ b/src/intel/vulkan/anv_image.c
> @@ -97,6 +97,38 @@ get_surface(struct anv_image *image, VkImageAspectFlags
> aspect)
>     }
>  }
>
> +struct VkExtent3D
> +anv_prep_image_extent(const VkImageType imageType,
> +                      const struct VkExtent3D *imageExtent)
> +{
> +   switch (imageType) {
> +   case VK_IMAGE_TYPE_1D:
> +      return (VkExtent3D) { imageExtent->width, 1, 1 };
> +   case VK_IMAGE_TYPE_2D:
> +      return (VkExtent3D) { imageExtent->width, imageExtent->height, 1 };
> +   case VK_IMAGE_TYPE_3D:
> +      return *imageExtent;
> +   default:
> +      unreachable("invalid image type");
> +   }
> +}
> +
> +struct VkOffset3D
> +anv_prep_image_offset(const VkImageType imageType,
> +                      const struct VkOffset3D *imageOffset)
> +{
> +   switch (imageType) {
> +   case VK_IMAGE_TYPE_1D:
> +      return (VkOffset3D) { imageOffset->x, 0, 0 };
> +   case VK_IMAGE_TYPE_2D:
> +      return (VkOffset3D) { imageOffset->x, imageOffset->y, 0 };
> +   case VK_IMAGE_TYPE_3D:
> +      return *imageOffset;
> +   default:
> +      unreachable("invalid image type");
>

Two primary comments.  First, I don't really like "prep".  Maybe
"anv_sanatize_offset".  Second, I think this is a good candidate for an
inline function.  It's used a lot of places and the compiler may be able to
optimize things a bit.

Also, I don't think we really need to pass in a pointer.  For a struct as
small as VkOffset3D it's probably going to be more efficient to just pass
the struct.

Other than that, I really like it.


> +   }
> +}
> +
>  /**
>   * Initialize the anv_image::*_surface selected by \a aspect. Then update
> the
>   * image's memory requirements (that is, the image's size and alignment).
> @@ -124,30 +156,16 @@ make_surface(const struct anv_device *dev,
>
>     struct anv_surface *anv_surf = get_surface(image, aspect);
>
> -   VkExtent3D extent;
> -   switch (vk_info->imageType) {
> -   case VK_IMAGE_TYPE_1D:
> -      extent = (VkExtent3D) { vk_info->extent.width, 1, 1 };
> -      break;
> -   case VK_IMAGE_TYPE_2D:
> -      extent = (VkExtent3D) { vk_info->extent.width,
> vk_info->extent.height, 1 };
> -      break;
> -   case VK_IMAGE_TYPE_3D:
> -      extent = vk_info->extent;
> -      break;
> -   default:
> -      unreachable("invalid image type");
> -   }
> -
> -   image->extent = extent;
> +   image->extent = anv_prep_image_extent(vk_info->imageType,
> +                                         &vk_info->extent);
>
>     ok = isl_surf_init(&dev->isl_dev, &anv_surf->isl,
>        .dim = vk_to_isl_surf_dim[vk_info->imageType],
>        .format = anv_get_isl_format(vk_info->format, aspect,
>                                     vk_info->tiling, NULL),
> -      .width = extent.width,
> -      .height = extent.height,
> -      .depth = extent.depth,
> +      .width = image->extent.width,
> +      .height = image->extent.height,
> +      .depth = image->extent.depth,
>        .levels = vk_info->mipLevels,
>        .array_len = vk_info->arrayLayers,
>        .samples = vk_info->samples,
> diff --git a/src/intel/vulkan/anv_meta_copy.c
> b/src/intel/vulkan/anv_meta_copy.c
> index 1a2bfd6..16a802b 100644
> --- a/src/intel/vulkan/anv_meta_copy.c
> +++ b/src/intel/vulkan/anv_meta_copy.c
> @@ -28,16 +28,16 @@
>   * if Image is uncompressed or compressed, respectively.
>   */
>  static struct VkExtent3D
> -meta_region_extent_el(const VkFormat format,
> +meta_region_extent_el(const struct anv_image *image,
>                        const struct VkExtent3D *extent)
>  {
>     const struct isl_format_layout *isl_layout =
> -      anv_format_for_vk_format(format)->isl_layout;
> -   return (VkExtent3D) {
> +      anv_format_for_vk_format(image->vk_format)->isl_layout;
> +   return anv_prep_image_extent(image->type, &(VkExtent3D) {
>        .width  = DIV_ROUND_UP(extent->width , isl_layout->bw),
>        .height = DIV_ROUND_UP(extent->height, isl_layout->bh),
>        .depth  = DIV_ROUND_UP(extent->depth , isl_layout->bd),
> -   };
> +   });
>  }
>
>  /* Returns the user-provided VkBufferImageCopy::imageOffset in units of
> @@ -49,11 +49,11 @@ meta_region_offset_el(const struct anv_image *image,
>                        const struct VkOffset3D *offset)
>  {
>     const struct isl_format_layout *isl_layout = image->format->isl_layout;
> -   return (VkOffset3D) {
> +   return anv_prep_image_offset(image->type, &(VkOffset3D) {
>        .x = offset->x / isl_layout->bw,
>        .y = offset->y / isl_layout->bh,
>        .z = offset->z / isl_layout->bd,
> -   };
> +   });
>  }
>
>  static struct anv_meta_blit2d_surf
> @@ -115,17 +115,29 @@ meta_copy_buffer_to_image(struct anv_cmd_buffer
> *cmd_buffer,
>
>     for (unsigned r = 0; r < regionCount; r++) {
>
> -      /* Start creating blit rect */
> +
> +      /**
> +       * From the Vulkan 1.0.6 spec: 18.3 Copying Data Between Images
> +       *    extent is the size in texels of the source image to copy in
> width,
> +       *    height and depth. 1D images use only x and width. 2D images
> use x, y,
> +       *    width and height. 3D images use x, y, z, width, height and
> depth.
> +       *
> +       *
> +       * Also, convert the offsets and extent from units of texels to
> units of
> +       * blocks - which is the highest resolution accessible in this
> command.
> +       */
>        const VkOffset3D img_offset_el =
>           meta_region_offset_el(image, &pRegions[r].imageOffset);
>        const VkExtent3D bufferExtent = {
>           .width = pRegions[r].bufferRowLength,
>           .height = pRegions[r].bufferImageHeight,
>        };
> +
> +      /* Start creating blit rect */
>        const VkExtent3D buf_extent_el =
> -         meta_region_extent_el(image->vk_format, &bufferExtent);
> +         meta_region_extent_el(image, &bufferExtent);
>        const VkExtent3D img_extent_el =
> -         meta_region_extent_el(image->vk_format,
> &pRegions[r].imageExtent);
> +         meta_region_extent_el(image, &pRegions[r].imageExtent);
>        struct anv_meta_blit2d_rect rect = {
>           .width = MAX2(buf_extent_el.width, img_extent_el.width),
>           .height = MAX2(buf_extent_el.height, img_extent_el.height),
> @@ -152,7 +164,7 @@ meta_copy_buffer_to_image(struct anv_cmd_buffer
> *cmd_buffer,
>        uint32_t *y_offset = forward ? &rect.dst_y : &rect.src_y;
>
>        /* Loop through each 3D or array slice */
> -      unsigned num_slices_3d = pRegions[r].imageExtent.depth;
> +      unsigned num_slices_3d = img_extent_el.depth;
>        unsigned num_slices_array = pRegions[r].imageSubresource.layerCount;
>        unsigned slice_3d = 0;
>        unsigned slice_array = 0;
> @@ -163,7 +175,7 @@ meta_copy_buffer_to_image(struct anv_cmd_buffer
> *cmd_buffer,
>                                      pRegions[r].imageSubresource.mipLevel,
>
>  pRegions[r].imageSubresource.baseArrayLayer
>                                         + slice_array,
> -                                    pRegions[r].imageOffset.z + slice_3d,
> +                                    img_offset_el.z + slice_3d,
>                                      x_offset,
>                                      y_offset);
>           *x_offset += img_offset_el.x;
> @@ -259,20 +271,30 @@ void anv_CmdCopyImage(
>        struct anv_meta_blit2d_surf b_dst =
>           blit_surf_for_image(dest_image, dst_isl_surf);
>
> -      /* Start creating blit rect */
> +      /**
> +       * From the Vulkan 1.0.6 spec: 18.4 Copying Data Between Buffers
> and Images
> +       *    imageExtent is the size in texels of the image to copy in
> width, height
> +       *    and depth. 1D images use only x and width. 2D images use x,
> y, width
> +       *    and height. 3D images use x, y, z, width, height and depth.
> +       *
> +       * Also, convert the offsets and extent from units of texels to
> units of
> +       * blocks - which is the highest resolution accessible in this
> command.
> +       */
>        const VkOffset3D dst_offset_el =
>           meta_region_offset_el(dest_image, &pRegions[r].dstOffset);
>        const VkOffset3D src_offset_el =
>           meta_region_offset_el(src_image, &pRegions[r].srcOffset);
>        const VkExtent3D img_extent_el =
> -         meta_region_extent_el(src_image->vk_format, &pRegions[r].extent);
> +         meta_region_extent_el(src_image, &pRegions[r].extent);
> +
> +      /* Start creating blit rect */
>        struct anv_meta_blit2d_rect rect = {
>           .width = img_extent_el.width,
>           .height = img_extent_el.height,
>        };
>
>        /* Loop through each 3D or array slice */
> -      unsigned num_slices_3d = pRegions[r].extent.depth;
> +      unsigned num_slices_3d = img_extent_el.depth;
>        unsigned num_slices_array = pRegions[r].dstSubresource.layerCount;
>        unsigned slice_3d = 0;
>        unsigned slice_array = 0;
> @@ -283,14 +305,14 @@ void anv_CmdCopyImage(
>                                      pRegions[r].dstSubresource.mipLevel,
>
>  pRegions[r].dstSubresource.baseArrayLayer
>                                         + slice_array,
> -                                    pRegions[r].dstOffset.z + slice_3d,
> +                                    dst_offset_el.z + slice_3d,
>                                      &rect.dst_x,
>                                      &rect.dst_y);
>           isl_surf_get_image_offset_el(src_isl_surf,
>                                      pRegions[r].srcSubresource.mipLevel,
>
>  pRegions[r].srcSubresource.baseArrayLayer
>                                         + slice_array,
> -                                    pRegions[r].srcOffset.z + slice_3d,
> +                                    src_offset_el.z + slice_3d,
>                                      &rect.src_x,
>                                      &rect.src_y);
>           rect.dst_x += dst_offset_el.x;
> diff --git a/src/intel/vulkan/anv_meta_resolve.c
> b/src/intel/vulkan/anv_meta_resolve.c
> index 19fb3ad..914494b 100644
> --- a/src/intel/vulkan/anv_meta_resolve.c
> +++ b/src/intel/vulkan/anv_meta_resolve.c
> @@ -719,6 +719,27 @@ void anv_CmdResolveImage(
>           anv_meta_get_iview_layer(dest_image, &region->dstSubresource,
>                                    &region->dstOffset);
>
> +      /**
> +       * From Vulkan 1.0.6 spec: 18.6 Resolving Multisample Images
> +       *
> +       *    extent is the size in texels of the source image to resolve
> in width,
> +       *    height and depth. 1D images use only x and width. 2D images
> use x, y,
> +       *    width and height. 3D images use x, y, z, width, height and
> depth.
> +       *
> +       *    srcOffset and dstOffset select the initial x, y, and z
> offsets in
> +       *    texels of the sub-regions of the source and destination image
> data.
> +       *    extent is the size in texels of the source image to resolve
> in width,
> +       *    height and depth. 1D images use only x and width. 2D images
> use x, y,
> +       *    width and height. 3D images use x, y, z, width, height and
> depth.
> +       */
> +      const struct VkExtent3D extent =
> +         anv_prep_image_extent(src_image->type, &region->extent);
> +      const struct VkOffset3D srcOffset =
> +         anv_prep_image_offset(src_image->type, &region->srcOffset);
> +      const struct VkOffset3D dstOffset =
> +         anv_prep_image_offset(dest_image->type, &region->dstOffset);
> +
> +
>        for (uint32_t layer = 0; layer < region->srcSubresource.layerCount;
>             ++layer) {
>
> @@ -780,12 +801,12 @@ void anv_CmdResolveImage(
>                 .framebuffer = fb_h,
>                 .renderArea = {
>                    .offset = {
> -                     region->dstOffset.x,
> -                     region->dstOffset.y,
> +                     dstOffset.x,
> +                     dstOffset.y,
>                    },
>                    .extent = {
> -                     region->extent.width,
> -                     region->extent.height,
> +                     extent.width,
> +                     extent.height,
>                    }
>                 },
>                 .clearValueCount = 0,
> @@ -796,17 +817,17 @@ void anv_CmdResolveImage(
>           emit_resolve(cmd_buffer,
>               &src_iview,
>               &(VkOffset2D) {
> -               .x = region->srcOffset.x,
> -               .y = region->srcOffset.y,
> +               .x = srcOffset.x,
> +               .y = srcOffset.y,
>               },
>               &dest_iview,
>               &(VkOffset2D) {
> -               .x = region->dstOffset.x,
> -               .y = region->dstOffset.y,
> +               .x = dstOffset.x,
> +               .y = dstOffset.y,
>               },
>               &(VkExtent2D) {
> -               .width = region->extent.width,
> -               .height = region->extent.height,
> +               .width = extent.width,
> +               .height = extent.height,
>               });
>
>           ANV_CALL(CmdEndRenderPass)(cmd_buffer_h);
> diff --git a/src/intel/vulkan/anv_private.h
> b/src/intel/vulkan/anv_private.h
> index 03e8767..44b4091 100644
> --- a/src/intel/vulkan/anv_private.h
> +++ b/src/intel/vulkan/anv_private.h
> @@ -1670,6 +1670,13 @@ struct anv_buffer_view {
>  const struct anv_format *
>  anv_format_for_descriptor_type(VkDescriptorType type);
>
> +struct VkExtent3D
> +anv_prep_image_extent(const VkImageType imageType,
> +                      const struct VkExtent3D *imageExtent);
> +struct VkOffset3D
> +anv_prep_image_offset(const VkImageType imageType,
> +                      const struct VkOffset3D *imageOffset);
> +
>  void anv_fill_buffer_surface_state(struct anv_device *device,
>                                     struct anv_state state,
>                                     enum isl_format format,
> --
> 2.7.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160324/43b1f841/attachment-0001.html>


More information about the mesa-dev mailing list