Vulkan: Properly handle Device extension dependencies
The checks for dependencies of Device extensions were incomplete, makes
sure the transitive dependencies of the extensions we care about are all
known so they can participate in the dependency check.
Also removes a workaround for surprising Vulkan behavior with instance
extensions getting promoted as device functions. This should be handled
correctly now as DeviceExt contains the physical device extensions as
well.
Bug: dawn:457
Change-Id: I4b79729d809c9edfedcb075a0e6aa5b4dd473ab3
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/22942
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
diff --git a/src/dawn_native/vulkan/BackendVk.cpp b/src/dawn_native/vulkan/BackendVk.cpp
index c682b72..0a68231 100644
--- a/src/dawn_native/vulkan/BackendVk.cpp
+++ b/src/dawn_native/vulkan/BackendVk.cpp
@@ -210,22 +210,18 @@
// cases are removed.
InstanceExtSet extensionsToRequest = mGlobalInfo.extensions;
- // TODO(cwallez@chromium.org): don't request extensions that have been promoted to Vulkan
- // 1.1. This can only happen when we correctly detect and handle VkPhysicalDevice instance
- // extensions that are promoted to be "device" extensions in the core Vulkan. If we don't
- // do this there is a crash because a Vulkan 1.1 loader instance will not emulate the call
- // on a Vulkan 1.0 ICD (and call nullptr).
- // See https://github.com/KhronosGroup/Vulkan-Loader/issues/412.
-
if (!GetInstance()->IsBackendValidationEnabled()) {
extensionsToRequest.Set(InstanceExt::DebugReport, false);
}
-
usedKnobs.extensions = extensionsToRequest;
std::vector<const char*> extensionNames;
for (uint32_t ext : IterateBitSet(extensionsToRequest.extensionBitSet)) {
- extensionNames.push_back(GetInstanceExtInfo(static_cast<InstanceExt>(ext)).name);
+ const InstanceExtInfo& info = GetInstanceExtInfo(static_cast<InstanceExt>(ext));
+
+ if (info.versionPromoted > mGlobalInfo.apiVersion) {
+ extensionNames.push_back(info.name);
+ }
}
VkApplicationInfo appInfo;
diff --git a/src/dawn_native/vulkan/VulkanExtensions.cpp b/src/dawn_native/vulkan/VulkanExtensions.cpp
index 353f087..9015a6d 100644
--- a/src/dawn_native/vulkan/VulkanExtensions.cpp
+++ b/src/dawn_native/vulkan/VulkanExtensions.cpp
@@ -136,11 +136,23 @@
static constexpr size_t kDeviceExtCount = static_cast<size_t>(DeviceExt::EnumCount);
static constexpr std::array<DeviceExtInfo, kDeviceExtCount> sDeviceExtInfos{{
//
+ {DeviceExt::BindMemory2, "VK_KHR_bind_memory2", VulkanVersion_1_1},
{DeviceExt::Maintenance1, "VK_KHR_maintenance1", VulkanVersion_1_1},
+ {DeviceExt::StorageBufferStorageClass, "VK_KHR_storage_buffer_storage_class",
+ VulkanVersion_1_1},
+ {DeviceExt::GetPhysicalDeviceProperties2, "VK_KHR_get_physical_device_properties2",
+ VulkanVersion_1_1},
+ {DeviceExt::GetMemoryRequirements2, "VK_KHR_get_memory_requirements2", VulkanVersion_1_1},
+ {DeviceExt::ExternalMemoryCapabilities, "VK_KHR_external_memory_capabilities",
+ VulkanVersion_1_1},
+ {DeviceExt::ExternalSemaphoreCapabilities, "VK_KHR_external_semaphore_capabilities",
+ VulkanVersion_1_1},
{DeviceExt::ExternalMemory, "VK_KHR_external_memory", VulkanVersion_1_1},
{DeviceExt::ExternalSemaphore, "VK_KHR_external_semaphore", VulkanVersion_1_1},
{DeviceExt::_16BitStorage, "VK_KHR_16bit_storage", VulkanVersion_1_1},
+ {DeviceExt::SamplerYCbCrConversion, "VK_KHR_sampler_ycbcr_conversion", VulkanVersion_1_1},
+ {DeviceExt::ImageFormatList, "VK_KHR_image_format_list", VulkanVersion_1_2},
{DeviceExt::ShaderFloat16Int8, "VK_KHR_shader_float16_int8", VulkanVersion_1_2},
{DeviceExt::ExternalMemoryFD, "VK_KHR_external_memory_fd", NeverPromoted},
@@ -195,6 +207,32 @@
bool hasDependencies = false;
switch (ext) {
+ // Happy extensions don't need anybody else!
+ case DeviceExt::BindMemory2:
+ case DeviceExt::GetMemoryRequirements2:
+ case DeviceExt::Maintenance1:
+ case DeviceExt::ImageFormatList:
+ case DeviceExt::StorageBufferStorageClass:
+ hasDependencies = true;
+ break;
+
+ // Physical device extensions technically don't require the instance to support
+ // them but VulkanFunctions only loads the function pointers if the instance
+ // advertises the extension. So if we didn't have this check, we'd risk a calling
+ // a nullptr.
+ case DeviceExt::GetPhysicalDeviceProperties2:
+ hasDependencies = instanceExts.Has(InstanceExt::GetPhysicalDeviceProperties2);
+ break;
+ case DeviceExt::ExternalMemoryCapabilities:
+ hasDependencies = instanceExts.Has(InstanceExt::ExternalMemoryCapabilities) &&
+ HasDep(DeviceExt::GetPhysicalDeviceProperties2);
+ break;
+ case DeviceExt::ExternalSemaphoreCapabilities:
+ hasDependencies =
+ instanceExts.Has(InstanceExt::ExternalSemaphoreCapabilities) &&
+ HasDep(DeviceExt::GetPhysicalDeviceProperties2);
+ break;
+
case DeviceExt::DebugMarker:
// TODO(cwallez@chromium.org): VK_KHR_debug_report is deprecated, switch to
// using VK_KHR_debug_utils instead.
@@ -202,51 +240,33 @@
break;
case DeviceExt::ImageDrmFormatModifier:
- // TODO(cwallez@chromium.org): ImageDrmFormatModifier actually requires:
- // - VK_KHR_bind_memory2
- // - VK_KHR_image_format_list
- // - VK_KHR_sampler_ycbcr_conversion
- //
- // Also switch to using DeviceExt::GetPhysicalDeviceProperties2 when we decouple
- // Instance / Device physical device extensions.
- hasDependencies = instanceExts.Has(InstanceExt::GetPhysicalDeviceProperties2);
+ hasDependencies = HasDep(DeviceExt::BindMemory2) &&
+ HasDep(DeviceExt::GetPhysicalDeviceProperties2) &&
+ HasDep(DeviceExt::ImageFormatList) &&
+ HasDep(DeviceExt::SamplerYCbCrConversion);
break;
case DeviceExt::Swapchain:
hasDependencies = instanceExts.Has(InstanceExt::Surface);
break;
- case DeviceExt::Maintenance1:
- hasDependencies = true;
+ case DeviceExt::SamplerYCbCrConversion:
+ hasDependencies = HasDep(DeviceExt::Maintenance1) &&
+ HasDep(DeviceExt::BindMemory2) &&
+ HasDep(DeviceExt::GetMemoryRequirements2) &&
+ HasDep(DeviceExt::GetPhysicalDeviceProperties2);
break;
case DeviceExt::ShaderFloat16Int8:
- // TODO(cwallez@chromium.org): switch to using
- // DeviceExt::GetPhysicalDeviceProperties2 when we decouple Instance / Device
- // physical device extensions.
- hasDependencies = instanceExts.Has(InstanceExt::GetPhysicalDeviceProperties2);
+ hasDependencies = HasDep(DeviceExt::GetPhysicalDeviceProperties2);
break;
case DeviceExt::ExternalMemory:
- // TODO(cwallez@chromium.org): switch to using
- // DeviceExt::ExternalMemoryCapabilities when we decouple Instance / Device
- // physical device extensions.
- hasDependencies = instanceExts.Has(InstanceExt::ExternalMemoryCapabilities);
+ hasDependencies = HasDep(DeviceExt::ExternalMemoryCapabilities);
break;
case DeviceExt::ExternalSemaphore:
- // TODO(cwallez@chromium.org): switch to using
- // DeviceExt::ExternalSemaphoreCapabilities when we decouple Instance / Device
- // physical device extensions.
- hasDependencies = instanceExts.Has(InstanceExt::ExternalSemaphoreCapabilities);
- break;
-
- case DeviceExt::_16BitStorage:
- // TODO(cwallez@chromium.org): switch to using
- // DeviceExt::GetPhysicalDeviceProperties2 when we decouple Instance / Device
- // physical device extensions.
- // Also depends on VK_KHR_storage_buffer_storage_class
- hasDependencies = instanceExts.Has(InstanceExt::GetPhysicalDeviceProperties2);
+ hasDependencies = HasDep(DeviceExt::ExternalSemaphoreCapabilities);
break;
case DeviceExt::ExternalMemoryFD:
@@ -263,6 +283,11 @@
hasDependencies = HasDep(DeviceExt::ExternalSemaphore);
break;
+ case DeviceExt::_16BitStorage:
+ hasDependencies = HasDep(DeviceExt::GetPhysicalDeviceProperties2) &&
+ HasDep(DeviceExt::StorageBufferStorageClass);
+ break;
+
default:
UNREACHABLE();
break;
diff --git a/src/dawn_native/vulkan/VulkanExtensions.h b/src/dawn_native/vulkan/VulkanExtensions.h
index 184df0b..5c59bc8 100644
--- a/src/dawn_native/vulkan/VulkanExtensions.h
+++ b/src/dawn_native/vulkan/VulkanExtensions.h
@@ -74,12 +74,20 @@
// inside EnsureDependencies)
enum class DeviceExt {
// Promoted to 1.1
+ BindMemory2,
Maintenance1,
+ StorageBufferStorageClass,
+ GetPhysicalDeviceProperties2,
+ GetMemoryRequirements2,
+ ExternalMemoryCapabilities,
+ ExternalSemaphoreCapabilities,
ExternalMemory,
ExternalSemaphore,
_16BitStorage,
+ SamplerYCbCrConversion,
// Promoted to 1.2
+ ImageFormatList,
ShaderFloat16Int8,
// External* extensions
