Expose VK_EXT_image_robustness support
The feature is currently always enabled, so we ignore the creation flag
for now.
Bug: b/159329067
Tests: dEQP-VK.robustness.image_robustness.*
Change-Id: I5385196afd93e3deeaec5713f301af4fed4ef15e
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/46828
Presubmit-Ready: Nicolas Capens <nicolascapens@google.com>
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Alexis Hétu <sugoi@google.com>
diff --git a/src/Pipeline/SamplerCore.cpp b/src/Pipeline/SamplerCore.cpp
index 1a84042..1c9124b 100644
--- a/src/Pipeline/SamplerCore.cpp
+++ b/src/Pipeline/SamplerCore.cpp
@@ -2257,7 +2257,6 @@
xyz0 = Min(Max(xyz, Int4(0)), maxXYZ);
// VK_EXT_image_robustness requires checking for out-of-bounds accesses.
- // TODO(b/159329067): Claim VK_EXT_image_robustness
// TODO(b/162327166): Only perform bounds checks when VK_EXT_image_robustness is enabled.
// If the above clamping altered the result, the access is out-of-bounds.
// In that case set the coordinate to -1 to perform texel replacement later.
diff --git a/src/Pipeline/SpirvShader.cpp b/src/Pipeline/SpirvShader.cpp
index afcdbac..0374aaa 100644
--- a/src/Pipeline/SpirvShader.cpp
+++ b/src/Pipeline/SpirvShader.cpp
@@ -1511,7 +1511,6 @@
case spv::StorageClassImage:
// VK_EXT_image_robustness requires nullifying out-of-bounds accesses.
- // TODO(b/159329067): Claim VK_EXT_image_robustness
// TODO(b/162327166): Only perform bounds checks when VK_EXT_image_robustness is enabled.
return OutOfBoundsBehavior::Nullify;
diff --git a/src/Pipeline/SpirvShaderImage.cpp b/src/Pipeline/SpirvShaderImage.cpp
index e291fe8..4ce3532 100644
--- a/src/Pipeline/SpirvShaderImage.cpp
+++ b/src/Pipeline/SpirvShaderImage.cpp
@@ -648,14 +648,7 @@
auto &dst = state->createIntermediate(insn.resultId(), resultType.componentCount);
- // "The value returned by a read of an invalid texel is undefined,
- // unless that read operation is from a buffer resource and the robustBufferAccess feature is enabled."
- // TODO: Don't always assume a buffer resource.
- //
- // While we could be using OutOfBoundsBehavior::RobustBufferAccess for read operations from buffer resources,
- // emulating the glsl function loadImage() requires that this function returns 0 when used with out of bounds
- // coordinates, so we have to use OutOfBoundsBehavior::Nullify in that case.
- // TODO(b/159329067): Claim VK_EXT_image_robustness
+ // VK_EXT_image_robustness requires replacing out-of-bounds access with zero.
// TODO(b/162327166): Only perform bounds checks when VK_EXT_image_robustness is enabled.
auto robustness = OutOfBoundsBehavior::Nullify;
@@ -1178,12 +1171,9 @@
break;
}
- // SPIR-V 1.4: "If the coordinates are outside the image, the memory location that is accessed is undefined."
-
- // Emulating the glsl function imageStore() requires that this function is noop when used with out of bounds
- // coordinates, so we have to use OutOfBoundsBehavior::Nullify in that case.
- // TODO(b/159329067): Claim VK_EXT_image_robustness
- // TODO(b/162327166): Only perform bounds checks when VK_EXT_image_robustness is enabled.
+ // "The integer texel coordinates are validated according to the same rules as for texel input coordinate
+ // validation. If the texel fails integer texel coordinate validation, then the write has no effect."
+ // - https://www.khronos.org/registry/vulkan/specs/1.2/html/chap16.html#textures-output-coordinate-validation
auto robustness = OutOfBoundsBehavior::Nullify;
auto texelPtr = GetTexelAddress(state, imageBase, imageSizeInBytes, coordinate, imageType, binding, texelSize, 0, false, robustness);
@@ -1251,7 +1241,6 @@
auto imageSizeInBytes = *Pointer<Int>(binding + OFFSET(vk::StorageImageDescriptor, sizeInBytes));
// VK_EXT_image_robustness requires checking for out-of-bounds accesses.
- // TODO(b/159329067): Claim VK_EXT_image_robustness
// TODO(b/162327166): Only perform bounds checks when VK_EXT_image_robustness is enabled.
auto robustness = OutOfBoundsBehavior::Nullify;
diff --git a/src/Pipeline/SpirvShaderSampling.cpp b/src/Pipeline/SpirvShaderSampling.cpp
index e9203ab..07e4d39 100644
--- a/src/Pipeline/SpirvShaderSampling.cpp
+++ b/src/Pipeline/SpirvShaderSampling.cpp
@@ -82,7 +82,6 @@
{
// OpImageFetch does not take a sampler descriptor, but for VK_EXT_image_robustness
// requires replacing invalid texels with zero.
- // TODO(b/159329067): Claim VK_EXT_image_robustness
// TODO(b/162327166): Only perform bounds checks when VK_EXT_image_robustness is enabled.
samplerState.border = VK_BORDER_COLOR_FLOAT_TRANSPARENT_BLACK;
}
@@ -366,7 +365,6 @@
// VK_EXT_image_robustness requires nullifying out-of-bounds accesses.
// ADDRESSING_BORDER causes texel replacement to be performed.
- // TODO(b/159329067): Claim VK_EXT_image_robustness
// TODO(b/162327166): Only perform bounds checks when VK_EXT_image_robustness is enabled.
return ADDRESSING_BORDER;
}
diff --git a/src/Vulkan/VkPhysicalDevice.cpp b/src/Vulkan/VkPhysicalDevice.cpp
index 9518a95..39b56e2 100644
--- a/src/Vulkan/VkPhysicalDevice.cpp
+++ b/src/Vulkan/VkPhysicalDevice.cpp
@@ -191,6 +191,11 @@
features->provokingVertexLast = VK_TRUE;
}
+void PhysicalDevice::getFeatures(VkPhysicalDeviceImageRobustnessFeaturesEXT *features) const
+{
+ features->robustImageAccess = VK_TRUE;
+}
+
VkSampleCountFlags PhysicalDevice::getSampleCounts() const
{
return VK_SAMPLE_COUNT_1_BIT | VK_SAMPLE_COUNT_4_BIT;
diff --git a/src/Vulkan/VkPhysicalDevice.hpp b/src/Vulkan/VkPhysicalDevice.hpp
index ea74d16..fbb9719 100644
--- a/src/Vulkan/VkPhysicalDevice.hpp
+++ b/src/Vulkan/VkPhysicalDevice.hpp
@@ -45,6 +45,7 @@
void getFeatures(VkPhysicalDeviceLineRasterizationFeaturesEXT *features) const;
void getFeatures(VkPhysicalDeviceSeparateDepthStencilLayoutsFeaturesKHR *features) const;
void getFeatures(VkPhysicalDeviceProvokingVertexFeaturesEXT *features) const;
+ void getFeatures(VkPhysicalDeviceImageRobustnessFeaturesEXT *features) const;
bool hasFeatures(const VkPhysicalDeviceFeatures &requestedFeatures) const;
const VkPhysicalDeviceProperties &getProperties() const;
diff --git a/src/Vulkan/libVulkan.cpp b/src/Vulkan/libVulkan.cpp
index 036dc33..acec24b 100644
--- a/src/Vulkan/libVulkan.cpp
+++ b/src/Vulkan/libVulkan.cpp
@@ -100,19 +100,6 @@
}
#endif // __ANDROID__ && ENABLE_BUILD_VERSION_OUTPUT
-bool HasExtensionProperty(const char *extensionName, const VkExtensionProperties *extensionProperties, uint32_t extensionPropertiesCount)
-{
- for(uint32_t j = 0; j < extensionPropertiesCount; ++j)
- {
- if(strcmp(extensionName, extensionProperties[j].extensionName) == 0)
- {
- return true;
- }
- }
-
- return false;
-}
-
// setReactorDefaultConfig() sets the default configuration for Vulkan's use of
// Reactor.
void setReactorDefaultConfig()
@@ -362,6 +349,7 @@
{ VK_EXT_LINE_RASTERIZATION_EXTENSION_NAME, VK_EXT_LINE_RASTERIZATION_SPEC_VERSION },
// The following extension is used by ANGLE to emulate blitting the stencil buffer
{ VK_EXT_SHADER_STENCIL_EXPORT_EXTENSION_NAME, VK_EXT_SHADER_STENCIL_EXPORT_SPEC_VERSION },
+ { VK_EXT_IMAGE_ROBUSTNESS_EXTENSION_NAME, VK_EXT_IMAGE_ROBUSTNESS_SPEC_VERSION },
#ifndef __ANDROID__
// We fully support the KHR_swapchain v70 additions, so just track the spec version.
{ VK_KHR_SWAPCHAIN_EXTENSION_NAME, VK_KHR_SWAPCHAIN_SPEC_VERSION },
@@ -393,6 +381,29 @@
{ VK_GOOGLE_SAMPLER_FILTERING_PRECISION_EXTENSION_NAME, VK_GOOGLE_SAMPLER_FILTERING_PRECISION_SPEC_VERSION },
};
+static bool hasExtension(const char *extensionName, const VkExtensionProperties *extensionProperties, uint32_t extensionPropertiesCount)
+{
+ for(uint32_t i = 0; i < extensionPropertiesCount; i++)
+ {
+ if(strcmp(extensionName, extensionProperties[i].extensionName) == 0)
+ {
+ return true;
+ }
+ }
+
+ return false;
+}
+
+static bool hasInstanceExtension(const char *extensionName)
+{
+ return hasExtension(extensionName, instanceExtensionProperties, sizeof(instanceExtensionProperties) / sizeof(instanceExtensionProperties[0]));
+}
+
+static bool hasDeviceExtension(const char *extensionName)
+{
+ return hasExtension(extensionName, deviceExtensionProperties, sizeof(deviceExtensionProperties) / sizeof(deviceExtensionProperties[0]));
+}
+
VKAPI_ATTR VkResult VKAPI_CALL vkCreateInstance(const VkInstanceCreateInfo *pCreateInfo, const VkAllocationCallbacks *pAllocator, VkInstance *pInstance)
{
TRACE("(const VkInstanceCreateInfo* pCreateInfo = %p, const VkAllocationCallbacks* pAllocator = %p, VkInstance* pInstance = %p)",
@@ -411,10 +422,9 @@
UNIMPLEMENTED("b/148240133: pCreateInfo->enabledLayerCount != 0"); // FIXME(b/148240133)
}
- uint32_t extensionPropertiesCount = sizeof(instanceExtensionProperties) / sizeof(instanceExtensionProperties[0]);
for(uint32_t i = 0; i < pCreateInfo->enabledExtensionCount; ++i)
{
- if(!HasExtensionProperty(pCreateInfo->ppEnabledExtensionNames[i], instanceExtensionProperties, extensionPropertiesCount))
+ if(!hasInstanceExtension(pCreateInfo->ppEnabledExtensionNames[i]))
{
return VK_ERROR_EXTENSION_NOT_PRESENT;
}
@@ -670,10 +680,9 @@
UNSUPPORTED("pCreateInfo->enabledLayerCount != 0");
}
- uint32_t extensionPropertiesCount = sizeof(deviceExtensionProperties) / sizeof(deviceExtensionProperties[0]);
for(uint32_t i = 0; i < pCreateInfo->enabledExtensionCount; ++i)
{
- if(!HasExtensionProperty(pCreateInfo->ppEnabledExtensionNames[i], deviceExtensionProperties, extensionPropertiesCount))
+ if(!hasDeviceExtension(pCreateInfo->ppEnabledExtensionNames[i]))
{
return VK_ERROR_EXTENSION_NOT_PRESENT;
}
@@ -804,6 +813,20 @@
(void)provokingVertexFeatures->provokingVertexLast;
}
break;
+ case VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_IMAGE_ROBUSTNESS_FEATURES_EXT:
+ {
+ const VkPhysicalDeviceImageRobustnessFeaturesEXT *imageRobustnessFeatures = reinterpret_cast<const VkPhysicalDeviceImageRobustnessFeaturesEXT *>(extensionCreateInfo);
+
+ // We currently always provide robust image accesses. When the feature is disabled, results are
+ // undefined (for images with Dim != Buffer), so providing robustness is also acceptable.
+ // TODO(b/159329067): Only provide robustness when requested.
+ (void)imageRobustnessFeatures->robustImageAccess;
+ }
+ break;
+ // For unsupported structures, check that we don't expose the corresponding extension string:
+ case VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_ROBUSTNESS_2_FEATURES_EXT:
+ ASSERT(!hasDeviceExtension(VK_EXT_ROBUSTNESS_2_EXTENSION_NAME));
+ break;
default:
// "the [driver] must skip over, without processing (other than reading the sType and pNext members) any structures in the chain with sType values not defined by [supported extenions]"
LOG_TRAP("pCreateInfo->pNext sType = %s", vk::Stringify(extensionCreateInfo->sType).c_str());
@@ -2936,21 +2959,27 @@
vk::Cast(physicalDevice)->getFeatures(features);
}
break;
+ case VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_IMAGE_ROBUSTNESS_FEATURES_EXT:
+ {
+ auto features = reinterpret_cast<VkPhysicalDeviceImageRobustnessFeaturesEXT *>(extensionFeatures);
+ vk::Cast(physicalDevice)->getFeatures(features);
+ }
+ break;
+ // For unsupported structures, check that we don't expose the corresponding extension string:
case VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_CONDITIONAL_RENDERING_FEATURES_EXT:
- ASSERT(!HasExtensionProperty(VK_EXT_CONDITIONAL_RENDERING_EXTENSION_NAME, deviceExtensionProperties,
- sizeof(deviceExtensionProperties) / sizeof(deviceExtensionProperties[0])));
+ ASSERT(!hasDeviceExtension(VK_EXT_CONDITIONAL_RENDERING_EXTENSION_NAME));
break;
case VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_SCALAR_BLOCK_LAYOUT_FEATURES_EXT:
- ASSERT(!HasExtensionProperty(VK_EXT_SCALAR_BLOCK_LAYOUT_EXTENSION_NAME, deviceExtensionProperties,
- sizeof(deviceExtensionProperties) / sizeof(deviceExtensionProperties[0])));
+ ASSERT(!hasDeviceExtension(VK_EXT_SCALAR_BLOCK_LAYOUT_EXTENSION_NAME));
break;
case VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_TIMELINE_SEMAPHORE_FEATURES_KHR:
- ASSERT(!HasExtensionProperty(VK_KHR_TIMELINE_SEMAPHORE_EXTENSION_NAME, deviceExtensionProperties,
- sizeof(deviceExtensionProperties) / sizeof(deviceExtensionProperties[0])));
+ ASSERT(!hasDeviceExtension(VK_KHR_TIMELINE_SEMAPHORE_EXTENSION_NAME));
break;
case VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_PERFORMANCE_QUERY_FEATURES_KHR:
- ASSERT(!HasExtensionProperty(VK_KHR_PERFORMANCE_QUERY_EXTENSION_NAME, deviceExtensionProperties,
- sizeof(deviceExtensionProperties) / sizeof(deviceExtensionProperties[0])));
+ ASSERT(!hasDeviceExtension(VK_KHR_PERFORMANCE_QUERY_EXTENSION_NAME));
+ break;
+ case VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_ROBUSTNESS_2_FEATURES_EXT:
+ ASSERT(!hasDeviceExtension(VK_EXT_ROBUSTNESS_2_EXTENSION_NAME));
break;
default:
LOG_TRAP("pFeatures->pNext sType = %s", vk::Stringify(extensionFeatures->sType).c_str());
@@ -3014,8 +3043,7 @@
break;
case VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_SAMPLE_LOCATIONS_PROPERTIES_EXT:
// Explicitly ignored, since VK_EXT_sample_locations is not supported
- ASSERT(!HasExtensionProperty(VK_EXT_SAMPLE_LOCATIONS_EXTENSION_NAME, deviceExtensionProperties,
- sizeof(deviceExtensionProperties) / sizeof(deviceExtensionProperties[0])));
+ ASSERT(!hasDeviceExtension(VK_EXT_SAMPLE_LOCATIONS_EXTENSION_NAME));
break;
case VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_EXTERNAL_MEMORY_HOST_PROPERTIES_EXT:
{
@@ -3091,15 +3119,13 @@
case VK_STRUCTURE_TYPE_IMAGE_FORMAT_LIST_CREATE_INFO_KHR:
{
// Explicitly ignored, since VK_KHR_image_format_list is not supported
- ASSERT(!HasExtensionProperty(VK_KHR_IMAGE_FORMAT_LIST_EXTENSION_NAME, deviceExtensionProperties,
- sizeof(deviceExtensionProperties) / sizeof(deviceExtensionProperties[0])));
+ ASSERT(!hasDeviceExtension(VK_KHR_IMAGE_FORMAT_LIST_EXTENSION_NAME));
}
break;
case VK_STRUCTURE_TYPE_IMAGE_STENCIL_USAGE_CREATE_INFO_EXT:
{
// Explicitly ignored, since VK_EXT_separate_stencil_usage is not supported
- ASSERT(!HasExtensionProperty(VK_EXT_SEPARATE_STENCIL_USAGE_EXTENSION_NAME, deviceExtensionProperties,
- sizeof(deviceExtensionProperties) / sizeof(deviceExtensionProperties[0])));
+ ASSERT(!hasDeviceExtension(VK_EXT_SEPARATE_STENCIL_USAGE_EXTENSION_NAME));
}
break;
case VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_EXTERNAL_IMAGE_FORMAT_INFO:
@@ -3111,8 +3137,7 @@
case VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_IMAGE_DRM_FORMAT_MODIFIER_INFO_EXT:
{
// Explicitly ignored, since VK_EXT_image_drm_format_modifier is not supported
- ASSERT(!HasExtensionProperty(VK_EXT_IMAGE_DRM_FORMAT_MODIFIER_EXTENSION_NAME, deviceExtensionProperties,
- sizeof(deviceExtensionProperties) / sizeof(deviceExtensionProperties[0])));
+ ASSERT(!hasDeviceExtension(VK_EXT_IMAGE_DRM_FORMAT_MODIFIER_EXTENSION_NAME));
}
break;
default:
@@ -3144,8 +3169,7 @@
case VK_STRUCTURE_TYPE_TEXTURE_LOD_GATHER_FORMAT_PROPERTIES_AMD:
{
// Explicitly ignored, since VK_AMD_texture_gather_bias_lod is not supported
- ASSERT(!HasExtensionProperty(VK_AMD_TEXTURE_GATHER_BIAS_LOD_EXTENSION_NAME, deviceExtensionProperties,
- sizeof(deviceExtensionProperties) / sizeof(deviceExtensionProperties[0])));
+ ASSERT(!hasDeviceExtension(VK_AMD_TEXTURE_GATHER_BIAS_LOD_EXTENSION_NAME));
}
break;
default:
