From 05e9e7767d7081861bc5d0838a18a5f9067366e6 Mon Sep 17 00:00:00 2001 From: Jason Ekstrand Date: Sat, 12 Feb 2022 14:40:46 -0600 Subject: [PATCH] vulkan: Rename vk_image_view::format to view_format When I originally added vk_image_view, I was overly clever when it came to the format field. I decided to make it only contain the bits of the format contained in the selected aspects. However, this is confusing (not generally a good thing) and it's also not always what you want. The Vulkan 1.3.204 spec says: "When using an image view of a depth/stencil image to populate a descriptor set (e.g. for sampling in the shader, or for use as an input attachment), the aspectMask must only include one bit, which selects whether the image view is used for depth reads (i.e. using a floating-point sampler or input attachment in the shader) or stencil reads (i.e. using an unsigned integer sampler or input attachment in the shader). When an image view of a depth/stencil image is used as a depth/stencil framebuffer attachment, the aspectMask is ignored and both depth and stencil image subresources are used." So, while the restricted format makes sense for texturing, it doesn't for when the image is being used as an attachment. What we probably actually want is both versions of the format. We'll call the one given by the VkImageViewCreateInfo vk_image_view::format and the restricted one vk_image_view::view_format. This is just the first commit which switches format to view_format so the compiler will make sure we get them all. The next commit will re-add vk_image_view::format but this time unmodified. Reviewed-by: Lionel Landwerlin Part-of: --- src/broadcom/vulkan/v3dv_image.c | 6 +++--- src/broadcom/vulkan/v3dvx_cmd_buffer.c | 4 ++-- src/broadcom/vulkan/v3dvx_image.c | 2 +- src/intel/vulkan/anv_image.c | 6 +++--- src/intel/vulkan/anv_pass.c | 6 +++--- src/vulkan/runtime/vk_image.c | 6 +++--- src/vulkan/runtime/vk_image.h | 2 +- 7 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/broadcom/vulkan/v3dv_image.c b/src/broadcom/vulkan/v3dv_image.c index 4afc6437b45..37e3a5ee7f5 100644 --- a/src/broadcom/vulkan/v3dv_image.c +++ b/src/broadcom/vulkan/v3dv_image.c @@ -536,13 +536,13 @@ v3dv_CreateImageView(VkDevice _device, image_view_swizzle); } - iview->vk.format = format; + iview->vk.view_format = format; iview->format = v3dv_X(device, get_format)(format); assert(iview->format && iview->format->supported); - if (vk_format_is_depth_or_stencil(iview->vk.format)) { + if (vk_format_is_depth_or_stencil(iview->vk.view_format)) { iview->internal_type = - v3dv_X(device, get_internal_depth_type)(iview->vk.format); + v3dv_X(device, get_internal_depth_type)(iview->vk.view_format); } else { v3dv_X(device, get_internal_type_bpp_for_output_format) (iview->format->rt_type, &iview->internal_type, &iview->internal_bpp); diff --git a/src/broadcom/vulkan/v3dvx_cmd_buffer.c b/src/broadcom/vulkan/v3dvx_cmd_buffer.c index c879493d389..42344c0f5c8 100644 --- a/src/broadcom/vulkan/v3dvx_cmd_buffer.c +++ b/src/broadcom/vulkan/v3dvx_cmd_buffer.c @@ -2327,9 +2327,9 @@ v3dX(cmd_buffer_render_pass_setup_render_target)(struct v3dv_cmd_buffer *cmd_buf *rt_bpp = iview->internal_bpp; *rt_type = iview->internal_type; - if (vk_format_is_int(iview->vk.format)) + if (vk_format_is_int(iview->vk.view_format)) *rt_clamp = V3D_RENDER_TARGET_CLAMP_INT; - else if (vk_format_is_srgb(iview->vk.format)) + else if (vk_format_is_srgb(iview->vk.view_format)) *rt_clamp = V3D_RENDER_TARGET_CLAMP_NORM; else *rt_clamp = V3D_RENDER_TARGET_CLAMP_NONE; diff --git a/src/broadcom/vulkan/v3dvx_image.c b/src/broadcom/vulkan/v3dvx_image.c index 06deb25aba7..1aa2947e92f 100644 --- a/src/broadcom/vulkan/v3dvx_image.c +++ b/src/broadcom/vulkan/v3dvx_image.c @@ -132,7 +132,7 @@ pack_texture_shader_state_helper(struct v3dv_device *device, tex.array_stride_64_byte_aligned = image->cube_map_stride / 64; - tex.srgb = vk_format_is_srgb(image_view->vk.format); + tex.srgb = vk_format_is_srgb(image_view->vk.view_format); /* At this point we don't have the job. That's the reason the first * parameter is NULL, to avoid a crash when cl_pack_emit_reloc tries to diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c index 755adf2f2dd..087aa8de60f 100644 --- a/src/intel/vulkan/anv_image.c +++ b/src/intel/vulkan/anv_image.c @@ -2577,8 +2577,8 @@ anv_CreateImageView(VkDevice _device, /* Format is undefined, this can happen when using external formats. Set * view format from the passed conversion info. */ - if (iview->vk.format == VK_FORMAT_UNDEFINED && conv_format) - iview->vk.format = conv_format->vk_format; + if (iview->vk.view_format == VK_FORMAT_UNDEFINED && conv_format) + iview->vk.view_format = conv_format->vk_format; /* Now go through the underlying image selected planes and map them to * planes in the image view. @@ -2589,7 +2589,7 @@ anv_CreateImageView(VkDevice _device, const uint32_t vplane = anv_aspect_to_plane(iview->vk.aspects, 1UL << iaspect_bit); struct anv_format_plane format; - format = anv_get_format_plane(&device->info, iview->vk.format, + format = anv_get_format_plane(&device->info, iview->vk.view_format, vplane, image->vk.tiling); iview->planes[vplane].image_plane = iplane; diff --git a/src/intel/vulkan/anv_pass.c b/src/intel/vulkan/anv_pass.c index a4a59b52cec..166aef43dd8 100644 --- a/src/intel/vulkan/anv_pass.c +++ b/src/intel/vulkan/anv_pass.c @@ -596,7 +596,7 @@ anv_dynamic_pass_init_full(struct anv_dynamic_render_pass *dyn_render_pass, ANV_FROM_HANDLE(anv_image_view, iview, info->pColorAttachments[att].imageView); pass->attachments[att] = (struct anv_render_pass_attachment) { - .format = iview->vk.format, + .format = iview->vk.view_format, .samples = iview->vk.image->samples, .usage = VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT, }; @@ -636,7 +636,7 @@ anv_dynamic_pass_init_full(struct anv_dynamic_render_pass *dyn_render_pass, ANV_FROM_HANDLE(anv_image_view, iview, d_or_s_att->imageView); pass->attachments[ds_idx] = (struct anv_render_pass_attachment) { - .format = iview->vk.format, + .format = iview->vk.view_format, .samples = iview->vk.image->samples, .usage = VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT, }; @@ -670,7 +670,7 @@ anv_dynamic_pass_init_full(struct anv_dynamic_render_pass *dyn_render_pass, ANV_FROM_HANDLE(anv_image_view, iview, fsr_attachment->imageView); pass->attachments[fsr_idx] = (struct anv_render_pass_attachment) { - .format = iview->vk.format, + .format = iview->vk.view_format, .samples = iview->vk.image->samples, .usage = VK_IMAGE_USAGE_FRAGMENT_SHADING_RATE_ATTACHMENT_BIT_KHR, }; diff --git a/src/vulkan/runtime/vk_image.c b/src/vulkan/runtime/vk_image.c index 2dccaf4bf05..68525fb0ba1 100644 --- a/src/vulkan/runtime/vk_image.c +++ b/src/vulkan/runtime/vk_image.c @@ -366,11 +366,11 @@ vk_image_view_init(struct vk_device *device, * conversion." */ if (image_view->aspects == VK_IMAGE_ASPECT_STENCIL_BIT) { - image_view->format = vk_format_stencil_only(pCreateInfo->format); + image_view->view_format = vk_format_stencil_only(pCreateInfo->format); } else if (image_view->aspects == VK_IMAGE_ASPECT_DEPTH_BIT) { - image_view->format = vk_format_depth_only(pCreateInfo->format); + image_view->view_format = vk_format_depth_only(pCreateInfo->format); } else { - image_view->format = pCreateInfo->format; + image_view->view_format = pCreateInfo->format; } image_view->swizzle = (VkComponentMapping) { diff --git a/src/vulkan/runtime/vk_image.h b/src/vulkan/runtime/vk_image.h index e636edb1352..b668bbb9615 100644 --- a/src/vulkan/runtime/vk_image.h +++ b/src/vulkan/runtime/vk_image.h @@ -158,7 +158,7 @@ struct vk_image_view { * plane of the multi-planar format. In this case, the format will be * the plane-compatible format requested by the client. */ - VkFormat format; + VkFormat view_format; /* Component mapping, aka swizzle *