Obtain ImageView state uniquely from its identifier
VkImageView state for sampling routine generation was previously
obtained from the SampledImageDescriptor structure, which assumes that
the mapping from the identifier to this state is unique. This assumption
easily breaks as new members field can be added to the descriptor
structure, without taking into account that the identifier values should
be unique for each unique field value.
This change removes the ImageView state members from the descriptor
structure, and instead we obtain them through the identifier,
guaranteeing a unique mapping.
Note that currently this state is compacted into the identifier's
32-bit integer value itself. Should more state be required than what
fits in 32-bit (like is the case for VkSampler objects), the mapping
between identifiers and their state should be handled through an actual
map container.
Bug: b/180511322
Bug: b/152227757
Change-Id: I1568eb67df29eb29ddcbbe906a90d236078e3080
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/52888
Reviewed-by: Nicolas Capens <nicolascapens@google.com>
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
Commit-Queue: Alexis Hétu <sugoi@google.com>
diff --git a/src/Pipeline/SpirvShader.hpp b/src/Pipeline/SpirvShader.hpp
index bae56c5..69335b8 100644
--- a/src/Pipeline/SpirvShader.hpp
+++ b/src/Pipeline/SpirvShader.hpp
@@ -1295,7 +1295,7 @@
static std::shared_ptr<rr::Routine> emitSamplerRoutine(ImageInstruction instruction, const Sampler &samplerState);
// TODO(b/129523279): Eliminate conversion and use vk::Sampler members directly.
- static sw::FilterType convertFilterMode(const vk::Sampler *sampler, VkImageViewType imageViewType, ImageInstruction instruction);
+ static sw::FilterType convertFilterMode(const vk::Sampler *sampler, VkImageViewType imageViewType, SamplerMethod samplerMethod);
static sw::MipmapType convertMipmapMode(const vk::Sampler *sampler);
static sw::AddressingMode convertAddressingMode(int coordinateIndex, const vk::Sampler *sampler, VkImageViewType imageViewType);
diff --git a/src/Pipeline/SpirvShaderSampling.cpp b/src/Pipeline/SpirvShaderSampling.cpp
index 497397f..ecd2f74 100644
--- a/src/Pipeline/SpirvShaderSampling.cpp
+++ b/src/Pipeline/SpirvShaderSampling.cpp
@@ -42,23 +42,26 @@
vk::Device::SamplingRoutineCache *cache = imageDescriptor->device->getSamplingRoutineCache();
auto createSamplingRoutine = [&](const vk::Device::SamplingRoutineCache::Key &key) {
- auto type = imageDescriptor->type;
+ const vk::Identifier::State imageViewState = vk::Identifier(imageDescriptor->imageViewId).getState();
+
+ auto type = imageViewState.imageViewType;
+ auto samplerMethod = static_cast<SamplerMethod>(instruction.samplerMethod);
Sampler samplerState = {};
samplerState.textureType = type;
- samplerState.textureFormat = imageDescriptor->format;
+ samplerState.textureFormat = imageViewState.format;
samplerState.addressingModeU = convertAddressingMode(0, sampler, type);
samplerState.addressingModeV = convertAddressingMode(1, sampler, type);
samplerState.addressingModeW = convertAddressingMode(2, sampler, type);
samplerState.mipmapFilter = convertMipmapMode(sampler);
- samplerState.swizzle = imageDescriptor->swizzle;
+ samplerState.swizzle = imageViewState.mapping;
samplerState.gatherComponent = instruction.gatherComponent;
if(sampler)
{
- samplerState.textureFilter = convertFilterMode(sampler, type, instruction);
+ samplerState.textureFilter = convertFilterMode(sampler, type, samplerMethod);
samplerState.border = sampler->borderColor;
samplerState.mipmapFilter = convertMipmapMode(sampler);
@@ -197,14 +200,14 @@
return function("sampler");
}
-sw::FilterType SpirvShader::convertFilterMode(const vk::Sampler *sampler, VkImageViewType imageViewType, ImageInstruction instruction)
+sw::FilterType SpirvShader::convertFilterMode(const vk::Sampler *sampler, VkImageViewType imageViewType, SamplerMethod samplerMethod)
{
- if(instruction.samplerMethod == Gather)
+ if(samplerMethod == Gather)
{
return FILTER_GATHER;
}
- if(instruction.samplerMethod == Fetch)
+ if(samplerMethod == Fetch)
{
return FILTER_POINT;
}
@@ -213,7 +216,7 @@
{
if(imageViewType == VK_IMAGE_VIEW_TYPE_2D || imageViewType == VK_IMAGE_VIEW_TYPE_2D_ARRAY)
{
- if(instruction.samplerMethod != Lod) // TODO(b/162926129): Support anisotropic filtering with explicit LOD.
+ if(samplerMethod != Lod) // TODO(b/162926129): Support anisotropic filtering with explicit LOD.
{
return FILTER_ANISOTROPIC;
}
diff --git a/src/Vulkan/VkDescriptorSetLayout.cpp b/src/Vulkan/VkDescriptorSetLayout.cpp
index 242d6d2..ee37ef1 100644
--- a/src/Vulkan/VkDescriptorSetLayout.cpp
+++ b/src/Vulkan/VkDescriptorSetLayout.cpp
@@ -287,11 +287,7 @@
auto update = reinterpret_cast<VkBufferView const *>(src + entry.offset + entry.stride * i);
auto bufferView = vk::Cast(*update);
- sampledImage[i].type = VK_IMAGE_VIEW_TYPE_1D;
sampledImage[i].imageViewId = bufferView->id;
- constexpr VkComponentMapping identityMapping = { VK_COMPONENT_SWIZZLE_R, VK_COMPONENT_SWIZZLE_G, VK_COMPONENT_SWIZZLE_B, VK_COMPONENT_SWIZZLE_A };
- sampledImage[i].swizzle = ResolveComponentMapping(identityMapping, bufferView->getFormat());
- sampledImage[i].format = bufferView->getFormat();
auto numElements = bufferView->getElementCount();
sampledImage[i].width = numElements;
@@ -348,9 +344,6 @@
sampledImage[i].depth = imageView->getDepthOrLayerCount(0);
sampledImage[i].mipLevels = imageView->getSubresourceRange().levelCount;
sampledImage[i].sampleCount = imageView->getSampleCount();
- sampledImage[i].type = imageView->getType();
- sampledImage[i].swizzle = imageView->getComponentMapping();
- sampledImage[i].format = format;
sampledImage[i].device = device;
sampledImage[i].memoryOwner = imageView;
diff --git a/src/Vulkan/VkDescriptorSetLayout.hpp b/src/Vulkan/VkDescriptorSetLayout.hpp
index f1919b1..4880089 100644
--- a/src/Vulkan/VkDescriptorSetLayout.hpp
+++ b/src/Vulkan/VkDescriptorSetLayout.hpp
@@ -40,9 +40,6 @@
vk::Device *device;
uint32_t imageViewId;
- VkImageViewType type;
- VkFormat format;
- VkComponentMapping swizzle;
alignas(16) sw::Texture texture;
int width; // Of base mip-level.
int height;
diff --git a/src/Vulkan/VkFormat.cpp b/src/Vulkan/VkFormat.cpp
index bd412c0..4b7db41 100644
--- a/src/Vulkan/VkFormat.cpp
+++ b/src/Vulkan/VkFormat.cpp
@@ -2396,6 +2396,45 @@
static_assert(pack(VK_FORMAT_ASTC_4x4_SFLOAT_BLOCK_EXT) == 227, "Incorrect VkFormat packed value");
static_assert(pack(VK_FORMAT_ASTC_12x12_SFLOAT_BLOCK_EXT) == 240, "Incorrect VkFormat packed value");
+static constexpr VkFormat unpack(uint8_t format)
+{
+ // 0 - 184 direct mapping
+ if(format >= 0 && format <= 184)
+ {
+ return static_cast<VkFormat>(format);
+ }
+
+ // 185 - 218 -> 10001560xx
+ if(format >= 185 && format <= 218)
+ {
+ return static_cast<VkFormat>(VK_FORMAT_G8B8G8R8_422_UNORM + (format - 185));
+ }
+
+ // 219 - 226 -> 100005400x
+ if(format >= 219 && format <= 226)
+ {
+ return static_cast<VkFormat>(VK_FORMAT_PVRTC1_2BPP_UNORM_BLOCK_IMG + (format - 219));
+ }
+
+ // 227 - 240 -> 10000660xx
+ if(format >= 227 && format <= 240)
+ {
+ return static_cast<VkFormat>(VK_FORMAT_ASTC_4x4_SFLOAT_BLOCK_EXT + (format - 227));
+ }
+
+ return VK_FORMAT_UNDEFINED;
+}
+
+static_assert(unpack(0) == VK_FORMAT_UNDEFINED, "Incorrect VkFormat unpacked value");
+static_assert(unpack(184) == VK_FORMAT_ASTC_12x12_SRGB_BLOCK, "Incorrect VkFormat unpacked value");
+static_assert(unpack(185) == VK_FORMAT_G8B8G8R8_422_UNORM, "Incorrect VkFormat unpacked value");
+static_assert(unpack(218) == VK_FORMAT_G16_B16_R16_3PLANE_444_UNORM, "Incorrect VkFormat unpacked value");
+static_assert(unpack(219) == VK_FORMAT_PVRTC1_2BPP_UNORM_BLOCK_IMG, "Incorrect VkFormat unpacked value");
+static_assert(unpack(226) == VK_FORMAT_PVRTC2_4BPP_SRGB_BLOCK_IMG, "Incorrect VkFormat unpacked value");
+static_assert(unpack(227) == VK_FORMAT_ASTC_4x4_SFLOAT_BLOCK_EXT, "Incorrect VkFormat unpacked value");
+static_assert(unpack(240) == VK_FORMAT_ASTC_12x12_SFLOAT_BLOCK_EXT, "Incorrect VkFormat unpacked value");
+static_assert(unpack(241) == VK_FORMAT_UNDEFINED, "Incorrect VkFormat unpacked value");
+
uint8_t Format::mapTo8bit(VkFormat format)
{
ASSERT(format <= VK_FORMAT_G16_B16_R16_3PLANE_444_UNORM);
@@ -2404,4 +2443,12 @@
return packed;
}
+VkFormat Format::mapFrom8bit(uint8_t format)
+{
+ ASSERT(format <= 240);
+ VkFormat unpacked = unpack(format);
+ ASSERT_MSG(unpacked != VK_FORMAT_UNDEFINED, "Update uint8_t to VkFormat mapping");
+ return unpacked;
+}
+
} // namespace vk
diff --git a/src/Vulkan/VkFormat.hpp b/src/Vulkan/VkFormat.hpp
index 128e38b..6c5e74f 100644
--- a/src/Vulkan/VkFormat.hpp
+++ b/src/Vulkan/VkFormat.hpp
@@ -70,6 +70,7 @@
bool isRGBComponent(int component) const;
static uint8_t mapTo8bit(VkFormat format);
+ static VkFormat mapFrom8bit(uint8_t format);
private:
VkFormat compatibleFormat() const;
diff --git a/src/Vulkan/VkImageView.cpp b/src/Vulkan/VkImageView.cpp
index d6756e9..0aa34f6 100644
--- a/src/Vulkan/VkImageView.cpp
+++ b/src/Vulkan/VkImageView.cpp
@@ -78,21 +78,43 @@
};
}
-Identifier::Identifier(const Image *image, VkImageViewType type, VkFormat fmt, VkComponentMapping mapping)
+Identifier::Identifier(const VkImageViewCreateInfo *pCreateInfo)
{
- imageViewType = type;
- format = Format::mapTo8bit(fmt);
- r = mapping.r;
- g = mapping.g;
- b = mapping.b;
- a = mapping.a;
+ const Image *image = vk::Cast(pCreateInfo->image);
+
+ VkImageSubresourceRange subresource = ResolveRemainingLevelsLayers(pCreateInfo->subresourceRange, image);
+ vk::Format viewFormat = GetImageViewFormat(pCreateInfo).getAspectFormat(subresource.aspectMask);
+ const Image *sampledImage = image->getSampledImage(viewFormat);
+
+ vk::Format samplingFormat = (image == sampledImage) ? viewFormat : sampledImage->getFormat().getAspectFormat(subresource.aspectMask);
+ pack({ pCreateInfo->viewType, samplingFormat, ResolveComponentMapping(pCreateInfo->components, viewFormat) });
}
-Identifier::Identifier(VkFormat fmt)
+Identifier::Identifier(VkFormat bufferFormat)
{
static_assert(vk::VK_IMAGE_VIEW_TYPE_END_RANGE == 6, "VkImageViewType does not allow using 7 to indicate buffer view");
- imageViewType = 7; // Still fits in 3-bit field
- format = Format::mapTo8bit(fmt);
+ constexpr VkComponentMapping identityMapping = { VK_COMPONENT_SWIZZLE_R, VK_COMPONENT_SWIZZLE_G, VK_COMPONENT_SWIZZLE_B, VK_COMPONENT_SWIZZLE_A };
+ pack({ VK_IMAGE_VIEW_TYPE_1D, bufferFormat, ResolveComponentMapping(identityMapping, bufferFormat) });
+}
+
+void Identifier::pack(const State &state)
+{
+ imageViewType = static_cast<uint32_t>(state.imageViewType);
+ format = Format::mapTo8bit(state.format);
+ r = static_cast<uint32_t>(state.mapping.r);
+ g = static_cast<uint32_t>(state.mapping.g);
+ b = static_cast<uint32_t>(state.mapping.b);
+ a = static_cast<uint32_t>(state.mapping.a);
+}
+
+Identifier::State Identifier::getState() const
+{
+ return { static_cast<VkImageViewType>(imageViewType),
+ Format::mapFrom8bit(static_cast<uint8_t>(format)),
+ { static_cast<VkComponentSwizzle>(r),
+ static_cast<VkComponentSwizzle>(g),
+ static_cast<VkComponentSwizzle>(b),
+ static_cast<VkComponentSwizzle>(a) } };
}
ImageView::ImageView(const VkImageViewCreateInfo *pCreateInfo, void *mem, const vk::SamplerYcbcrConversion *ycbcrConversion)
@@ -102,7 +124,7 @@
, components(ResolveComponentMapping(pCreateInfo->components, format))
, subresourceRange(ResolveRemainingLevelsLayers(pCreateInfo->subresourceRange, image))
, ycbcrConversion(ycbcrConversion)
- , id(image, viewType, format.getAspectFormat(subresourceRange.aspectMask), components)
+ , id(pCreateInfo)
{
}
diff --git a/src/Vulkan/VkImageView.hpp b/src/Vulkan/VkImageView.hpp
index 5af89e7..7efab9c 100644
--- a/src/Vulkan/VkImageView.hpp
+++ b/src/Vulkan/VkImageView.hpp
@@ -28,23 +28,38 @@
class SamplerYcbcrConversion;
// Uniquely identifies state used by sampling routine generation.
-// ID space shared by image views and buffer views.
+// Integer ID space shared by image views and buffer views.
union Identifier
{
// Image view identifier
- Identifier(const Image *image, VkImageViewType type, VkFormat format, VkComponentMapping mapping);
+ Identifier(const VkImageViewCreateInfo *pCreateInfo);
// Buffer view identifier
Identifier(VkFormat format);
+ // Copy constructor from existing identifier
+ Identifier(uint32_t fromId)
+ : id(fromId)
+ {}
+
operator uint32_t() const
{
static_assert(sizeof(Identifier) == sizeof(uint32_t), "Identifier must be 32-bit");
return id;
}
- uint32_t id = 0;
+ struct State
+ {
+ VkImageViewType imageViewType;
+ VkFormat format;
+ VkComponentMapping mapping;
+ };
+ State getState() const;
+private:
+ void pack(const State &data);
+
+ // Identifier is a union of this struct and the integer below.
struct
{
uint32_t imageViewType : 3;
@@ -54,6 +69,8 @@
uint32_t b : 3;
uint32_t a : 3;
};
+
+ uint32_t id = 0;
};
class ImageView : public Object<ImageView, VkImageView>
