Skip to content

anv: Rework YCbCr image aspects

Faith Ekstrand requested to merge gfxstrand/mesa:review/anv-rework-ycbcr into main

The Vulkan 1.2.184 spec says:

"When creating a VkImageView, if sampler Y′CBCR conversion is enabled in the sampler, the aspectMask of a subresourceRange used by the VkImageView must be VK_IMAGE_ASPECT_COLOR_BIT.

When creating a VkImageView, if sampler Y′CBCR conversion is not enabled in the sampler and the image format is multi-planar, the image must have been created with VK_IMAGE_CREATE_MUTABLE_FORMAT_BIT, and the aspectMask of the VkImageView's subresourceRange must be VK_IMAGE_ASPECT_PLANE_0_BIT, VK_IMAGE_ASPECT_PLANE_1_BIT or VK_IMAGE_ASPECT_PLANE_2_BIT."

Previously, for YCbCr images, we were flipping this around. For single- plane views where VK_IMAGE_ASPECT_PLANE_N_BIT would be passed in by the app, we would store VK_IMAGE_ASPECT_COLOR_BIT. For multi-plane views where the client says VK_IMAGE_ASPECT_COLOR_BIT, we would store a all of the planes. (There was also an extra bit of remapping that would compact the planes in the non-existent case of a format with a non- contiguous set of planes.) The idea behind this was that for things like rendering or single-plane sampling, storage, or compute, we want it to look as much like a single-plane image as possible but we wanted the multi-plane case to be the awkward one.

This commit changes it around so that iview->aspects is always exactly the subset of image->vk.aspects represented by the view. This is identical to how aspects work for depth/stencil so it gains us some consistency.

This commit also changes anv_image_view::aspect_mask to aspects to force a full audit of the field. As can be seen, there are only a few uses of this field and they're all mostly fine:

  • A bunch of them are used to check for depth/stencil. That hasn't changed.

  • Most of the checks for color already used ANY_COLOR_BIT, only one needed fixing.

  • There's a check that both src/depth are color for MSAA resolves. However, we don't support MSAA on YCbCr so there's no point in checking for ANY_COLOR_BIT.

There is a hidden usage of planes in anv_descriptor_set_write_image_view that's not as obvious. However, this function simply looks at anv_image_view::n_planes and blindly fills out the descriptor accordingly. As long as image views with a single plane continue to claim n_planes == 1, this will be fine.

This requires a crucible MR: crucible!98 (merged)

Merge request reports