On Thu, Mar 24, 2016 at 9:27 AM, Nanley Chery <nanleych...@gmail.com> wrote:
> From: Nanley Chery <nanley.g.ch...@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.ch...@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, ®ion->dstSubresource,
> ®ion->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, ®ion->extent);
> + const struct VkOffset3D srcOffset =
> + anv_prep_image_offset(src_image->type, ®ion->srcOffset);
> + const struct VkOffset3D dstOffset =
> + anv_prep_image_offset(dest_image->type, ®ion->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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
