Document single-level YCbCr image assumption

We currently rely on limiting the mipmap level count of YCbCr images and the presence of sampler YCbCr conversion to disable mipmapping. This is valid, but fragile. The Vulkan 1.2 spec guarantees that when sampler Y'CbCr conversion is used, the image view must have a Y'CbCr format: "If the descriptor refers to a sampler that performs Y'CbCr conversion, the sampler must only be used to sample the image in the same descriptor." "If the image view is to be used with a sampler which supports sampler Y'CbCr conversion, an identically defined object of type VkSamplerYcbcrConversion to that used to create the sampler must be passed to vkCreateImageView in a VkSamplerYcbcrConversionInfo included in the pNext chain of VkImageViewCreateInfo. Conversely, if a VkSamplerYcbcrConversion object is passed to vkCreateImageView, an identically defined VkSamplerYcbcrConversion object must be used when sampling the image." "maxMipLevels is the maximum number of mipmap levels. maxMipLevels must be equal to the number of levels in the complete mipmap chain based on the maxExtent.width, maxExtent.height, and maxExtent.depth, except when one of the following conditions is true, in which case it may instead be 1: * ... * image format is one of those listed in Formats requiring sampler Y′CBCR conversion for VK_IMAGE_ASPECT_COLOR_BIT image views" Bug: b/151263485 Change-Id: Ibb8e3fe34ecda04d6fb41308437fa4c66815f1fb Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/42228 Presubmit-Ready: Nicolas Capens <nicolascapens@google.com> Kokoro-Presubmit: kokoro <noreply+kokoro@google.com> Tested-by: Nicolas Capens <nicolascapens@google.com> Reviewed-by: Alexis Hétu <sugoi@google.com>

Document single-level YCbCr image assumption
77b43d68 · Nicolas Capens · Nicolas Capens · 5c372082 · 77b43d68 · 77b43d68
Commit 77b43d68 authored Mar 12, 2020 by Nicolas Capens Committed by Nicolas Capens Mar 12, 2020
Hide whitespace changes
Inline Side-by-side

Showing with 6 additions and 5 deletions

SpirvShaderSampling.cpp src/Pipeline/SpirvShaderSampling.cpp +2 -1

VkPhysicalDevice.cpp src/Vulkan/VkPhysicalDevice.cpp +4 -4

No files found.
--- a/src/Pipeline/SpirvShaderSampling.cpp
+++ b/src/Pipeline/SpirvShaderSampling.cpp
@@ -268,7 +268,8 @@ sw::MipmapType SpirvShader::convertMipmapMode(const vk::Sampler *sampler)
 	if(sampler->ycbcrConversion)
 	{
-		return MIPMAP_NONE;  // YCbCr images can only have one mipmap level.
+		// TODO(b/151263485): Check image view level count instead.
+		return MIPMAP_NONE;
 	}
 	switch(sampler->mipmapMode)

--- a/src/Vulkan/VkPhysicalDevice.cpp
+++ b/src/Vulkan/VkPhysicalDevice.cpp
@@ -623,7 +623,7 @@ void PhysicalDevice::getFormatProperties(Format format, VkFormatProperties *pFor
 			    VK_FORMAT_FEATURE_STORAGE_IMAGE_ATOMIC_BIT;
 			pFormatProperties->bufferFeatures |=
 			    VK_FORMAT_FEATURE_STORAGE_TEXEL_BUFFER_ATOMIC_BIT;
-			// Fall through
+			// [[fallthrough]]
 		case VK_FORMAT_R8G8B8A8_UNORM:
 		case VK_FORMAT_R8G8B8A8_SNORM:
 		case VK_FORMAT_R8G8B8A8_UINT:
@@ -643,7 +643,7 @@ void PhysicalDevice::getFormatProperties(Format format, VkFormatProperties *pFor
 		case VK_FORMAT_R16G16_SFLOAT:
 			pFormatProperties->optimalTilingFeatures |=
 			    VK_FORMAT_FEATURE_STORAGE_IMAGE_BIT;
-			// Fall through
+			// [[fallthrough]]
 		case VK_FORMAT_A8B8G8R8_UNORM_PACK32:
 		case VK_FORMAT_A8B8G8R8_SNORM_PACK32:
 		case VK_FORMAT_A8B8G8R8_UINT_PACK32:
@@ -678,7 +678,7 @@ void PhysicalDevice::getFormatProperties(Format format, VkFormatProperties *pFor
 		case VK_FORMAT_B10G11R11_UFLOAT_PACK32:
 			pFormatProperties->optimalTilingFeatures |=
 			    VK_FORMAT_FEATURE_COLOR_ATTACHMENT_BLEND_BIT;
-			// Fall through
+			// [[fallthrough]]
 		case VK_FORMAT_R8_UINT:
 		case VK_FORMAT_R8_SINT:
 		case VK_FORMAT_R8G8_UINT:
@@ -898,7 +898,7 @@ void PhysicalDevice::getImageFormatProperties(Format format, VkImageType type, V
 	//  have further restrictions on their limits and capabilities compared to images created with other formats."
 	if(format.isYcbcrFormat())
 	{
-		pImageFormatProperties->maxMipLevels = 1;
+		pImageFormatProperties->maxMipLevels = 1;  // TODO(b/151263485): This is relied on by the sampler to disable mipmapping for Y'CbCr image sampling.
 		pImageFormatProperties->maxArrayLayers = 1;
 		pImageFormatProperties->sampleCounts = VK_SAMPLE_COUNT_1_BIT;
 	}