commit 383901609736409e9af99df975a199a10fddb768
author Nicolas Capens <capn@google.com> Fri Feb 25 16:29:43 2022 -0500
committer Nicolas Capens <nicolascapens@google.com> Tue Mar 01 19:28:02 2022 +0000
tree fd1258f1b95e687281192a0e9c2225ae077640b4
parent 6e9eafd3463912183cfcac9c2b26bd61ab57a43b

Fix image subresource offset query

Previously the VkSubresourceLayout::offset included the offset within
the device memory allocation bound to the image. This change removes
that offset, and explicitly adds it for the getTexelPointer() method
which was relying on this previous behavior.

Also, for images with disjoint planes the subresource offset is relative
to the plane containing that subresource, so getSubresourceOffset() now
leaves out the aspect offset for these images to avoid counting it
twice.

Bug: b/208505033
Change-Id: If20f37a00ae24ef244940878e60fd1331b95edcc
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/63488
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Jason Macnak <natsu@google.com>

src/Vulkan/VkImage.cpp

diff --git a/src/Vulkan/VkImage.cpp b/src/Vulkan/VkImage.cpp
index fbd109c..a19f16b 100644
--- a/src/Vulkan/VkImage.cpp
+++ b/src/Vulkan/VkImage.cpp
@@ -725,7 +725,8 @@
 void *Image::getTexelPointer(const VkOffset3D &offset, const VkImageSubresource &subresource) const
 {
 	VkImageAspectFlagBits aspect = static_cast<VkImageAspectFlagBits>(subresource.aspectMask);
-	return deviceMemory->getOffsetPointer(texelOffsetBytesInStorage(offset, subresource) +
+	return deviceMemory->getOffsetPointer(getMemoryOffset(aspect) +
+	                                      texelOffsetBytesInStorage(offset, subresource) +
 	                                      getSubresourceOffset(aspect, subresource.mipLevel, subresource.arrayLayer));
 }
 
@@ -912,13 +913,18 @@
 	return reinterpret_cast<uint8_t *>(deviceMemory->getOffsetPointer(deviceMemory->getCommittedMemoryInBytes() + 1));
 }
 
-VkDeviceSize Image::getAspectOffset(VkImageAspectFlagBits aspect) const
+VkDeviceSize Image::getMemoryOffset(VkImageAspectFlagBits aspect) const
 {
 	if(deviceMemory && deviceMemory->hasExternalImageProperties())
 	{
 		return deviceMemory->externalImageMemoryOffset(aspect);
 	}
 
+	return memoryOffset;
+}
+
+VkDeviceSize Image::getAspectOffset(VkImageAspectFlagBits aspect) const
+{
 	switch(format)
 	{
 	case VK_FORMAT_D16_UNORM_S8_UINT:
@@ -927,26 +933,26 @@
 		if(aspect == VK_IMAGE_ASPECT_STENCIL_BIT)
 		{
 			// Offset by depth buffer to get to stencil buffer
-			return memoryOffset + getStorageSize(VK_IMAGE_ASPECT_DEPTH_BIT);
+			return getStorageSize(VK_IMAGE_ASPECT_DEPTH_BIT);
 		}
 		break;
 
 	case VK_FORMAT_G8_B8_R8_3PLANE_420_UNORM:
 		if(aspect == VK_IMAGE_ASPECT_PLANE_2_BIT)
 		{
-			return memoryOffset + getStorageSize(VK_IMAGE_ASPECT_PLANE_1_BIT) + getStorageSize(VK_IMAGE_ASPECT_PLANE_0_BIT);
+			return getStorageSize(VK_IMAGE_ASPECT_PLANE_1_BIT) + getStorageSize(VK_IMAGE_ASPECT_PLANE_0_BIT);
 		}
 		// Fall through to 2PLANE case:
 	case VK_FORMAT_G8_B8R8_2PLANE_420_UNORM:
 		if(aspect == VK_IMAGE_ASPECT_PLANE_1_BIT)
 		{
-			return memoryOffset + getStorageSize(VK_IMAGE_ASPECT_PLANE_0_BIT);
+			return getStorageSize(VK_IMAGE_ASPECT_PLANE_0_BIT);
 		}
 		else
 		{
 			ASSERT(aspect == VK_IMAGE_ASPECT_PLANE_0_BIT);
 
-			return memoryOffset;
+			return 0;
 		}
 		break;
 
@@ -954,18 +960,22 @@
 		break;
 	}
 
-	return memoryOffset;
+	return 0;
 }
 
 VkDeviceSize Image::getSubresourceOffset(VkImageAspectFlagBits aspect, uint32_t mipLevel, uint32_t layer) const
 {
-	VkDeviceSize mipmapOffset = getAspectOffset(aspect);
+	// "If the image is disjoint, then the offset is relative to the base address of the plane.
+	//  If the image is non-disjoint, then the offset is relative to the base address of the image."
+	bool disjoint = flags & VK_IMAGE_CREATE_DISJOINT_BIT;
+	VkDeviceSize offset = !disjoint ? getAspectOffset(aspect) : 0;
+
 	for(uint32_t i = 0; i < mipLevel; i++)
 	{
-		mipmapOffset += getMultiSampledLevelSize(aspect, i);
+		offset += getMultiSampledLevelSize(aspect, i);
 	}
 
-	return layer * getLayerOffset(aspect, mipLevel) + mipmapOffset;
+	return offset + layer * getLayerOffset(aspect, mipLevel);
 }
 
 VkDeviceSize Image::getMipLevelSize(VkImageAspectFlagBits aspect, uint32_t mipLevel) const

src/Vulkan/VkImage.hpp

diff --git a/src/Vulkan/VkImage.hpp b/src/Vulkan/VkImage.hpp
index e163e3c..118d656 100644
--- a/src/Vulkan/VkImage.hpp
+++ b/src/Vulkan/VkImage.hpp
@@ -125,6 +125,7 @@
 	VkDeviceSize getStorageSize(VkImageAspectFlags flags) const;
 	VkDeviceSize getMultiSampledLevelSize(VkImageAspectFlagBits aspect, uint32_t mipLevel) const;
 	VkDeviceSize getLayerOffset(VkImageAspectFlagBits aspect, uint32_t mipLevel) const;
+	VkDeviceSize getMemoryOffset(VkImageAspectFlagBits aspect) const;
 	VkDeviceSize getAspectOffset(VkImageAspectFlagBits aspect) const;
 	VkDeviceSize getSubresourceOffset(VkImageAspectFlagBits aspect, uint32_t mipLevel, uint32_t layer) const;
 	VkDeviceSize texelOffsetBytesInStorage(const VkOffset3D &offset, const VkImageSubresource &subresource) const;