Revert "Reland^2 "Fix VkTestUtils's VulkanPreferredFeatures usage"" This reverts commit 0e0d6e799facb0dd637b73f926b1b1ff803c7705. Reason for revert: Still leading to Cuttlefish errors for unclear reasons. Original change's description: > Reland^2 "Fix VkTestUtils's VulkanPreferredFeatures usage" > > This is a reland of commit 733217467faf2a2ad0f5f096019eb6d885540cd8 > * Should be fixed by restoring the SYNC_FD extension check in the tests. > > Original change's description: > > Reland "Fix VkTestUtils's VulkanPreferredFeatures usage" > > > > This is a reland of commit 72009acc508c4ddeee63fc8a60c53a232b907258 > > > > * Fix required including another extension (VK_KHR_EXTERNAL_SEMAPHORE_FD), re-adding protected memory feature query, and exiting backend context creation early if protected is requested but not supported (rather than an SkASSERT which stopped protected tests rather than skipping them on devices that do not support protected memory). > > > > Original change's description: > > > Fix VkTestUtils's VulkanPreferredFeatures usage > > > > > > * This CL fixes extension mismanagement issues and includes updates for clarity and style modernization in the touched areas (verified by confirming that VK_KHR_driver_properties extension is included as expected after previously being inadvertently excluded). > > > > > > The issue: > > > * This is a follow-up to the landed change "Remove redundant or outdated extension usage in Vulkan tests" (https://skia-review.git.corp.google.com/c/skia/+/1174856), which exposed an issue with our usage of VulkanPreferredFeatures where we would pass in only the list of extensions the tests want to enable. Instead, we should be providing the list of all *available* extensions on the device. > > > > > > * This misuse, combined with TestUtils removing extension names that should be redundant (i.e. already checked for in VulkanPreferredFeatures), led to certain key extensions/features no longer being utilized. Notably, this included VK_KHR_driver_properties. This meant driver correctness workarounds could not be enabled, which also (likely artificially) led to perf improvements. > > > > > > Anticipated impact of this fix: > > > * This change is anticipated to trigger perf regressions from the current state. In reality, we should just be returning to baseline performance just before the extension removal CL landed. > > > > > > * Once this lands and we get perf numbers, I plan to determine whether actually not enabling certain extensions would get us performance wins without encountering driver correctness issues. > > > > > > Bug: b/487695539 > > > Change-Id: Idef9ac665485a50b6bd032d5138855f359f7858d > > > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/1181116 > > > Reviewed-by: Greg Daniel <egdaniel@google.com> > > > Commit-Queue: Nicolette Prevost <nicolettep@google.com> > > > > Bug: b/487695539 > > Change-Id: Ibf3bdde8bfdd46ad7018cb72331a72d43012f1ff > > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/1190458 > > Commit-Queue: Nicolette Prevost <nicolettep@google.com> > > Auto-Submit: Nicolette Prevost <nicolettep@google.com> > > Reviewed-by: Greg Daniel <egdaniel@google.com> > > Commit-Queue: Greg Daniel <egdaniel@google.com> > > Bug: b/487695539 > Change-Id: I2758336489b75306f07b663df8270d11d2694ba9 > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/1204597 > Commit-Queue: Nicolette Prevost <nicolettep@google.com> > Reviewed-by: Noelle Scobie <nscobie@google.com> Bug: b/487695539 No-Presubmit: true No-Tree-Checks: true No-Try: true Change-Id: Ifdb03de07c310d53799ef54c3aeb9ce94938ee85 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/1205056 Auto-Submit: Nicolette Prevost <nicolettep@google.com> Commit-Queue: Noelle Scobie <nscobie@google.com> Reviewed-by: Noelle Scobie <nscobie@google.com>
diff --git a/tests/VkHardwareBufferTest.cpp b/tests/VkHardwareBufferTest.cpp index a67da38..4b6be4d 100644 --- a/tests/VkHardwareBufferTest.cpp +++ b/tests/VkHardwareBufferTest.cpp
@@ -557,19 +557,19 @@ ACQUIRE_INST_VK_PROC(DeviceWaitIdle); ACQUIRE_INST_VK_PROC(DestroyDevice); - if (!fExtensions->hasExtension( - VK_ANDROID_EXTERNAL_MEMORY_ANDROID_HARDWARE_BUFFER_EXTENSION_NAME, 2)) { + if (!fExtensions->hasExtension(VK_ANDROID_EXTERNAL_MEMORY_ANDROID_HARDWARE_BUFFER_EXTENSION_NAME, + 2)) { return false; } if (!fExtensions->hasExtension(VK_KHR_SAMPLER_YCBCR_CONVERSION_EXTENSION_NAME, 1)) { return false; } - if (!fExtensions->hasExtension(VK_EXT_QUEUE_FAMILY_FOREIGN_EXTENSION_NAME, 1)) { - return false; - } if (!fExtensions->hasExtension(VK_KHR_EXTERNAL_SEMAPHORE_FD_EXTENSION_NAME, 1)) { return false; } + if (!fExtensions->hasExtension(VK_EXT_QUEUE_FAMILY_FOREIGN_EXTENSION_NAME, 1)) { + // return false; + } ACQUIRE_INST_VK_PROC(GetPhysicalDeviceMemoryProperties2); ACQUIRE_INST_VK_PROC(GetPhysicalDeviceImageFormatProperties2);
diff --git a/tools/gpu/vk/VkTestUtils.cpp b/tools/gpu/vk/VkTestUtils.cpp index f3ef802..ef77045 100644 --- a/tools/gpu/vk/VkTestUtils.cpp +++ b/tools/gpu/vk/VkTestUtils.cpp
@@ -10,8 +10,6 @@ #include "tools/gpu/vk/VkTestMemoryAllocator.h" #include "tools/gpu/vk/VkTestUtils.h" -#include <functional> - #ifdef SK_VULKAN #ifndef SK_GPU_TOOLS_VK_LIBRARY_NAME @@ -74,6 +72,37 @@ // Helper code to set up Vulkan context objects #ifdef SK_ENABLE_VK_LAYERS +const char* kDebugLayerNames[] = { + // single merged layer + "VK_LAYER_KHRONOS_validation", + // not included in standard_validation + //"VK_LAYER_LUNARG_api_dump", + //"VK_LAYER_LUNARG_vktrace", + //"VK_LAYER_LUNARG_screenshot", +}; + +static uint32_t remove_patch_version(uint32_t specVersion) { + return (specVersion >> 12) << 12; +} + +// Returns the index into layers array for the layer we want. Returns -1 if not supported. +static int should_include_debug_layer(const char* layerName, + uint32_t layerCount, VkLayerProperties* layers, + uint32_t version) { + for (uint32_t i = 0; i < layerCount; ++i) { + if (!strcmp(layerName, layers[i].layerName)) { + // Since the layers intercept the vulkan calls and forward them on, we need to make sure + // layer was written against a version that isn't older than the version of Vulkan we're + // using so that it has all the api entry points. + if (version <= remove_patch_version(layers[i].specVersion)) { + return i; + } + return -1; + } + + } + return -1; +} static void print_backtrace() { #if defined(__GLIBC__) @@ -161,8 +190,220 @@ } \ } while (0) +// Returns the index into layers array for the layer we want. Returns -1 if not supported. +static bool should_include_extension(const char* extensionName) { + const char* kValidExtensions[] = { + VK_EXT_DEBUG_UTILS_EXTENSION_NAME, + VK_EXT_DEVICE_FAULT_EXTENSION_NAME, + VK_EXT_LAYER_SETTINGS_EXTENSION_NAME, + VK_KHR_SURFACE_EXTENSION_NAME, + VK_KHR_SWAPCHAIN_EXTENSION_NAME, + // Currently only used by Ganesh: + VK_EXT_IMAGE_DRM_FORMAT_MODIFIER_EXTENSION_NAME, + VK_KHR_IMAGE_FORMAT_LIST_EXTENSION_NAME, // Required for above extension if using 1.1 + VK_EXT_QUEUE_FAMILY_FOREIGN_EXTENSION_NAME, + // Below are all platform specific extensions. The name macros like we use above are + // all defined in platform specific vulkan headers. We currently don't include these + // headers as they are a little bit of a pain (e.g. windows headers requires including + // <windows.h> which causes all sorts of fun annoyances/problems. So instead we are + // just listing the strings these macros are defined to. This really shouldn't cause + // any long term issues as the chances of the strings connected to the name macros + // changing is next to zero. + "VK_KHR_win32_surface", // VK_KHR_WIN32_SURFACE_EXTENSION_NAME + "VK_KHR_xcb_surface", // VK_KHR_XCB_SURFACE_EXTENSION_NAME, + "VK_KHR_android_surface", // VK_KHR_ANDROID_SURFACE_EXTENSION_NAME, + }; + + for (size_t i = 0; i < std::size(kValidExtensions); ++i) { + if (!strcmp(extensionName, kValidExtensions[i])) { + return true; + } + } + return false; +} + +static bool init_instance_extensions_and_layers(PFN_vkGetInstanceProcAddr getInstProc, + uint32_t specVersion, + TArray<VkExtensionProperties>* instanceExtensions, + TArray<VkLayerProperties>* instanceLayers) { + if (getInstProc == nullptr) { + return false; + } + + ACQUIRE_VK_INST_PROC_LOCAL(EnumerateInstanceExtensionProperties, VK_NULL_HANDLE); + ACQUIRE_VK_INST_PROC_LOCAL(EnumerateInstanceLayerProperties, VK_NULL_HANDLE); + + VkResult res; + uint32_t layerCount = 0; +#ifdef SK_ENABLE_VK_LAYERS + // instance layers + res = grVkEnumerateInstanceLayerProperties(&layerCount, nullptr); + if (VK_SUCCESS != res) { + return false; + } + VkLayerProperties* layers = new VkLayerProperties[layerCount]; + res = grVkEnumerateInstanceLayerProperties(&layerCount, layers); + if (VK_SUCCESS != res) { + delete[] layers; + return false; + } + + uint32_t nonPatchVersion = remove_patch_version(specVersion); + for (size_t i = 0; i < std::size(kDebugLayerNames); ++i) { + int idx = should_include_debug_layer(kDebugLayerNames[i], layerCount, layers, + nonPatchVersion); + if (idx != -1) { + instanceLayers->push_back() = layers[idx]; + } + } + delete[] layers; +#endif + + // instance extensions + // via Vulkan implementation and implicitly enabled layers + { + uint32_t extensionCount = 0; + res = grVkEnumerateInstanceExtensionProperties(nullptr, &extensionCount, nullptr); + if (VK_SUCCESS != res) { + return false; + } + VkExtensionProperties* extensions = new VkExtensionProperties[extensionCount]; + res = grVkEnumerateInstanceExtensionProperties(nullptr, &extensionCount, extensions); + if (VK_SUCCESS != res) { + delete[] extensions; + return false; + } + for (uint32_t i = 0; i < extensionCount; ++i) { + if (should_include_extension(extensions[i].extensionName)) { + instanceExtensions->push_back() = extensions[i]; + } + } + delete [] extensions; + } + + // via explicitly enabled layers + layerCount = instanceLayers->size(); + for (uint32_t layerIndex = 0; layerIndex < layerCount; ++layerIndex) { + uint32_t extensionCount = 0; + res = grVkEnumerateInstanceExtensionProperties((*instanceLayers)[layerIndex].layerName, + &extensionCount, nullptr); + if (VK_SUCCESS != res) { + return false; + } + VkExtensionProperties* extensions = new VkExtensionProperties[extensionCount]; + res = grVkEnumerateInstanceExtensionProperties((*instanceLayers)[layerIndex].layerName, + &extensionCount, extensions); + if (VK_SUCCESS != res) { + delete[] extensions; + return false; + } + for (uint32_t i = 0; i < extensionCount; ++i) { + if (should_include_extension(extensions[i].extensionName)) { + instanceExtensions->push_back() = extensions[i]; + } + } + delete[] extensions; + } + + return true; +} + #define GET_PROC_LOCAL(F, inst, device) PFN_vk ## F F = (PFN_vk ## F) getProc("vk" #F, inst, device) +static bool init_device_extensions_and_layers(const skgpu::VulkanGetProc& getProc, + uint32_t specVersion, VkInstance inst, + VkPhysicalDevice physDev, + TArray<VkExtensionProperties>* deviceExtensions, + TArray<VkLayerProperties>* deviceLayers) { + if (getProc == nullptr) { + return false; + } + + GET_PROC_LOCAL(EnumerateDeviceExtensionProperties, inst, VK_NULL_HANDLE); + GET_PROC_LOCAL(EnumerateDeviceLayerProperties, inst, VK_NULL_HANDLE); + + if (!EnumerateDeviceExtensionProperties || + !EnumerateDeviceLayerProperties) { + return false; + } + + VkResult res; + // device layers + uint32_t layerCount = 0; +#ifdef SK_ENABLE_VK_LAYERS + res = EnumerateDeviceLayerProperties(physDev, &layerCount, nullptr); + if (VK_SUCCESS != res) { + return false; + } + VkLayerProperties* layers = new VkLayerProperties[layerCount]; + res = EnumerateDeviceLayerProperties(physDev, &layerCount, layers); + if (VK_SUCCESS != res) { + delete[] layers; + return false; + } + + uint32_t nonPatchVersion = remove_patch_version(specVersion); + for (size_t i = 0; i < std::size(kDebugLayerNames); ++i) { + int idx = should_include_debug_layer(kDebugLayerNames[i], layerCount, layers, + nonPatchVersion); + if (idx != -1) { + deviceLayers->push_back() = layers[idx]; + } + } + delete[] layers; +#endif + + // device extensions + // via Vulkan implementation and implicitly enabled layers + { + uint32_t extensionCount = 0; + res = EnumerateDeviceExtensionProperties(physDev, nullptr, &extensionCount, nullptr); + if (VK_SUCCESS != res) { + return false; + } + VkExtensionProperties* extensions = new VkExtensionProperties[extensionCount]; + res = EnumerateDeviceExtensionProperties(physDev, nullptr, &extensionCount, extensions); + if (VK_SUCCESS != res) { + delete[] extensions; + return false; + } + for (uint32_t i = 0; i < extensionCount; ++i) { + if (should_include_extension(extensions[i].extensionName)) { + deviceExtensions->push_back() = extensions[i]; + } + } + delete[] extensions; + } + + // via explicitly enabled layers + layerCount = deviceLayers->size(); + for (uint32_t layerIndex = 0; layerIndex < layerCount; ++layerIndex) { + uint32_t extensionCount = 0; + res = EnumerateDeviceExtensionProperties(physDev, + (*deviceLayers)[layerIndex].layerName, + &extensionCount, nullptr); + if (VK_SUCCESS != res) { + return false; + } + VkExtensionProperties* extensions = new VkExtensionProperties[extensionCount]; + res = EnumerateDeviceExtensionProperties(physDev, + (*deviceLayers)[layerIndex].layerName, + &extensionCount, extensions); + if (VK_SUCCESS != res) { + delete[] extensions; + return false; + } + for (uint32_t i = 0; i < extensionCount; ++i) { + if (should_include_extension(extensions[i].extensionName)) { + deviceExtensions->push_back() = extensions[i]; + } + } + delete[] extensions; + } + + return true; +} + #define ACQUIRE_VK_INST_PROC_NOCHECK(name, instance) \ PFN_vk##name grVk##name = reinterpret_cast<PFN_vk##name>(getInstProc(instance, "vk" #name)) @@ -217,300 +458,46 @@ grVkDestroyInstance(inst, nullptr); return true; } -namespace { -uint32_t remove_patch_version(uint32_t specVersion) { - return (specVersion >> 12) << 12; -} +static bool setup_features(const skgpu::VulkanGetProc& getProc, + VkInstance inst, + VkPhysicalDevice physDev, + uint32_t physDeviceVersion, + const TArray<VkExtensionProperties>& deviceExtensions, + TestVkFeatures& testVkFeatures, + bool isProtected) { + SkASSERT(physDeviceVersion >= VK_API_VERSION_1_1); -#if defined (SK_ENABLE_VK_LAYERS) -// Checks a list of availableLayers for layers we want to use (kDebugLayerNames). If the desired -// layers are available and compatible with our Vulkan version, append their VkLayerProperties to -// layersToUse. -void append_desired_layers(const VkLayerProperties* availableLayers, - const uint32_t availLayerCount, - const uint32_t majorSpecVersion, - TArray<VkLayerProperties>& layersToUse, - TArray<const char*>& layersToUseNames) { - // Comment out or include different debugging layers by modifying the array below: - const char* kDebugLayerNames[] = { - "VK_LAYER_KHRONOS_validation", - // Not included in standard_validation: - // "VK_LAYER_LUNARG_api_dump", - // "VK_LAYER_LUNARG_vktrace", - // "VK_LAYER_LUNARG_screenshot", - }; + testVkFeatures.deviceFeatures = {}; + testVkFeatures.deviceFeatures.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_FEATURES_2; - // Check for each desired debug layer within the list of available layers. If we encounter a - // match, verify that the layer was authored against a version >= the major Vulkan version we - // are using before appending it to the list of layers to use. This ensures the validation - // layers will have all the expected API entry points in order to intercept Vulkan calls. - for (size_t i = 0; i < std::size(kDebugLayerNames); ++i) { - for (size_t j = 0; j < availLayerCount; ++j) { - if (!strcmp(kDebugLayerNames[i], availableLayers[j].layerName) && - majorSpecVersion <= remove_patch_version(availableLayers[j].specVersion)) { - layersToUse.push_back(availableLayers[i]); - layersToUseNames.push_back(kDebugLayerNames[i]); - } - } - } -} + // Add any features that are needed by tests before calling skiaFeatures->addFeaturesToQuery. -bool append_desired_available_layers(std::function<VkResult(VkLayerProperties*)> QueryLayers, - uint32_t* availLayerCount, - uint32_t majorSpecVersion, - TArray<VkLayerProperties>& layersToEnable, - TArray<const char*>& enabledLayerNames) { - if (QueryLayers(nullptr) != VK_SUCCESS) { - return false; - } - VkLayerProperties* availLayers = new VkLayerProperties[*availLayerCount]; - if (QueryLayers(availLayers) != VK_SUCCESS) { - delete[] availLayers; - return false; - } - - append_desired_layers( - availLayers, *availLayerCount, majorSpecVersion, layersToEnable, enabledLayerNames); - - delete[] availLayers; - return true; -} - -// Given a list of enabled layers and a ptr to the Vulkan driver call to query layer extension -// information, tack on to the list and quantity of available extensions. -bool add_enabled_layer_extensions_to_available_list( - uint32_t* availExtCount, - TArray<VkExtensionProperties>& availExtensions, - const TArray<VkLayerProperties>& layersToEnable, - std::function<VkResult(const char*, uint32_t*, VkExtensionProperties*)> getExtensions) { - // Add extensions from layers we intend to enable to our list of available extensions - for (const auto& layer : layersToEnable) { - uint32_t layerAvailExtCount = 0; - if (getExtensions(layer.layerName, &layerAvailExtCount, nullptr) != VK_SUCCESS) { - return false; - } - - // Pushing default entries that the query call can write over preserves TArray size - // attribute accuracy as opposed to reserve calls which only update capacity (useful for - // for/each loops, not having to separately track + pass around more size values, etc.). - availExtensions.push_back_n(layerAvailExtCount); - if (getExtensions(layer.layerName, - &layerAvailExtCount, - &availExtensions[*availExtCount]) != VK_SUCCESS) { - return false; - } - *availExtCount += layerAvailExtCount; - } - - return true; -} -#endif // SK_ENABLE_VK_LAYERS - -bool should_include_extension(const char* extensionName) { - const char* kExtensionsForTests[] = { - VK_EXT_DEBUG_UTILS_EXTENSION_NAME, - VK_EXT_DEVICE_FAULT_EXTENSION_NAME, - VK_EXT_LAYER_SETTINGS_EXTENSION_NAME, - VK_KHR_EXTERNAL_SEMAPHORE_FD_EXTENSION_NAME, - VK_KHR_SURFACE_EXTENSION_NAME, - VK_KHR_SWAPCHAIN_EXTENSION_NAME, - // Currently only used by Ganesh: - VK_EXT_IMAGE_DRM_FORMAT_MODIFIER_EXTENSION_NAME, - VK_KHR_IMAGE_FORMAT_LIST_EXTENSION_NAME, // Required for above extension if using 1.1 - VK_EXT_QUEUE_FAMILY_FOREIGN_EXTENSION_NAME, - // Below are all platform specific extensions. The name macros like we use above are - // all defined in platform specific vulkan headers. We currently don't include these - // headers as they are a little bit of a pain (e.g. windows headers requires including - // <windows.h> which causes all sorts of fun annoyances/problems. So instead we are - // just listing the strings these macros are defined to. This really shouldn't cause - // any long term issues as the chances of the strings connected to the name macros - // changing is next to zero. - "VK_KHR_win32_surface", // VK_KHR_WIN32_SURFACE_EXTENSION_NAME - "VK_KHR_xcb_surface", // VK_KHR_XCB_SURFACE_EXTENSION_NAME, - "VK_KHR_android_surface", // VK_KHR_ANDROID_SURFACE_EXTENSION_NAME, - }; - - for (size_t i = 0; i < std::size(kExtensionsForTests); i++) { - if (!strcmp(extensionName, kExtensionsForTests[i])) { - return true; - } - } - - return false; -} - -// Checks a list of availExtensions, populating a list of all available extension names. If we see -// an extension we want to enable, add the extension and its name to the relvant enabled -// extensions + extension names lists. -void append_desired_extensions(TArray<VkExtensionProperties>& availableExtensions, - TArray<const char*>& availExtNames, - TArray<VkExtensionProperties>& extensionsToEnable, - std::vector<const char*>& enabledExtNames) { - for (const auto& ext : availableExtensions) { - availExtNames.push_back(ext.extensionName); - if (should_include_extension(ext.extensionName)) { - extensionsToEnable.push_back(ext); - enabledExtNames.push_back(ext.extensionName); - } - } -} - -// Provided a function ptr with which to perform the Vulkan query, populate the quantity and -// extension properties of all available extensions. -bool get_avail_extensions( - std::function<VkResult(const char*, uint32_t*, VkExtensionProperties*)> QueryExtensions, - TArray<VkExtensionProperties>& availExtensions, - uint32_t* availExtCount) { - if (QueryExtensions(nullptr, availExtCount, nullptr) != VK_SUCCESS) { - return false; - } - - // Pushing default entries that the query call can write over preserves TArray size attribute - // accuracy as opposed to reserve calls which only update capacity (useful for for/each loops, - // not having to separately track + pass around more size values, etc.). - availExtensions.push_back_n(*availExtCount); - if (QueryExtensions(nullptr, availExtCount, availExtensions.begin()) != VK_SUCCESS) { - return false; - } - - return true; -} - -// Determine all available instance extensions, layers, and those layers' extensions. From that, -// decide which layers to include. Also decide which instance extensions to enable, making sure to -// allow Skia to add to that list. Returns true upon success; false otherwise. -bool init_instance_extensions_and_layers(PFN_vkGetInstanceProcAddr getInstProc, - uint32_t majorSpecVersion, - TArray<VkExtensionProperties>& availableInstExts, - TArray<const char*>& availInstExtNames, - TArray<VkExtensionProperties>& instExtensionsToEnable, - std::vector<const char*>& enabledInstExtNames, - TArray<VkLayerProperties>& instLayersToEnable, - TArray<const char*>& enabledInstLayerNames, - skgpu::VulkanPreferredFeatures& skiaFeatures) { - // First populate the list of all available extensions - ACQUIRE_VK_INST_PROC_LOCAL(EnumerateInstanceExtensionProperties, VK_NULL_HANDLE); - uint32_t availExtCount = 0; - auto getExtensions = [&](const char* layerName, - uint32_t* extensionCount, - VkExtensionProperties* extensionList) { - return grVkEnumerateInstanceExtensionProperties(layerName, extensionCount, extensionList); - }; - if (!get_avail_extensions(getExtensions, availableInstExts, &availExtCount)) { - return false; - } - -#if defined SK_ENABLE_VK_LAYERS - ACQUIRE_VK_INST_PROC_LOCAL(EnumerateInstanceLayerProperties, VK_NULL_HANDLE); - uint32_t availLayerCount = 0; - - auto getLayers = [&](VkLayerProperties* layersList) { - return grVkEnumerateInstanceLayerProperties(&availLayerCount, layersList); - }; - if (!append_desired_available_layers(getLayers, - &availLayerCount, - majorSpecVersion, - instLayersToEnable, - enabledInstLayerNames)) { - return false; - } - - if (!add_enabled_layer_extensions_to_available_list( - &availExtCount, availableInstExts, instLayersToEnable, getExtensions)) { - return false; - } -#endif - - // Now that we have compiled a list of all available instance extensions, populate a list of - // ones we actually want to enable. - append_desired_extensions( - availableInstExts, availInstExtNames, instExtensionsToEnable, enabledInstExtNames); - - // Allow Skia the chance to add to the list of enabled instance extensions. - skiaFeatures.addToInstanceExtensions( - availableInstExts.data(), availExtCount, enabledInstExtNames); - - return true; -} - -void setup_feature_query(TestVkFeatures& testFeatures, bool isProtected) { - // Note: Any structs chained on to the feature query must stay in scope until vkCreateDevice. - // This is why these structs are located in TestVkFeatures and passed into this function. - - // Initialize physical device feature query struct - testFeatures.deviceFeatures = {}; - testFeatures.deviceFeatures.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_FEATURES_2; - - // Add features that would be useful to enable for testing to the feature query. - void** tailPNext = &testFeatures.deviceFeatures.pNext; + void** tailPNext = &testVkFeatures.deviceFeatures.pNext; // If |isProtected| is given, attach that first - testFeatures.protectedMemoryFeatures = {}; + testVkFeatures.protectedMemoryFeatures = {}; if (isProtected) { - testFeatures.protectedMemoryFeatures.sType = + testVkFeatures.protectedMemoryFeatures.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_PROTECTED_MEMORY_FEATURES; - *tailPNext = &testFeatures.protectedMemoryFeatures; - tailPNext = &testFeatures.protectedMemoryFeatures.pNext; + *tailPNext = &testVkFeatures.protectedMemoryFeatures; + tailPNext = &testVkFeatures.protectedMemoryFeatures.pNext; } + + // Allow Skia's VulkanPreferredFeatures to add any desired features or extensions to the query. + testVkFeatures.skiaFeatures.addFeaturesToQuery( + deviceExtensions.begin(), deviceExtensions.size(), testVkFeatures.deviceFeatures); + + ACQUIRE_VK_PROC_LOCAL(GetPhysicalDeviceFeatures2, inst, VK_NULL_HANDLE); + grVkGetPhysicalDeviceFeatures2(physDev, &testVkFeatures.deviceFeatures); + + // We can manually disable any undesired features here. Currently, we only disable robust + // buffer access which can negatively impact performance on some GPUs. + testVkFeatures.deviceFeatures.features.robustBufferAccess = VK_FALSE; + + return !isProtected || testVkFeatures.protectedMemoryFeatures.protectedMemory; } -bool init_device_extensions_and_layers(const skgpu::VulkanGetProc& getProc, - uint32_t majorSpecVersion, - VkInstance inst, - VkPhysicalDevice physDev, - TArray<VkExtensionProperties>& availableDevExts, - TArray<const char*>& availDevExtNames, - TArray<VkExtensionProperties>& devExtsToEnable, - std::vector<const char*>& enabledDevExtNames, - TArray<VkLayerProperties>& devLayersToEnable, - TArray<const char*>& enabledDevLayerNames, - TestVkFeatures& testFeatures, - bool isProtected) { - // First populate the list of all available extensions - GET_PROC_LOCAL(EnumerateDeviceExtensionProperties, inst, VK_NULL_HANDLE); - uint32_t availExtCount = 0; - auto getExtensions = [&](const char* layerName, - uint32_t* extensionCount, - VkExtensionProperties* extensionList) { - return EnumerateDeviceExtensionProperties( - physDev, layerName, extensionCount, extensionList); - }; - if (!get_avail_extensions(getExtensions, availableDevExts, &availExtCount)) { - return false; - } - -#if defined SK_ENABLE_VK_LAYERS - GET_PROC_LOCAL(EnumerateDeviceLayerProperties, inst, VK_NULL_HANDLE); - uint32_t availLayerCount = 0; - - auto getLayers = [&](VkLayerProperties* layersList) { - return EnumerateDeviceLayerProperties(physDev, &availLayerCount, layersList); - }; - append_desired_available_layers( - getLayers, &availLayerCount, majorSpecVersion, devLayersToEnable, enabledDevLayerNames); - - if (!add_enabled_layer_extensions_to_available_list( - &availExtCount, availableDevExts, devLayersToEnable, getExtensions)) { - return false; - } -#endif - - // Now that we have compiled a list of all available device extensions, populate a list of - // ones we actually want to enable. - append_desired_extensions( - availableDevExts, availDevExtNames, devExtsToEnable, enabledDevExtNames); - - // First, add any features we want for testing to the query. Then allow Skia to add to it. - setup_feature_query(testFeatures, isProtected); - - testFeatures.skiaFeatures.addFeaturesToQuery( - availableDevExts.data(), availExtCount, testFeatures.deviceFeatures); - - return true; -} -} // anonymous namespace - bool CreateVkBackendContext(PFN_vkGetInstanceProcAddr getInstProc, skgpu::VulkanBackendContext* ctx, skgpu::VulkanExtensions* extensions, @@ -519,10 +506,6 @@ uint32_t* presentQueueIndexPtr, const CanPresentFn& canPresent, bool isProtected) { - if (!getInstProc || !testVkFeatures) { - return false; - } - VkResult err; ACQUIRE_VK_INST_PROC_NOCHECK(EnumerateInstanceVersion, VK_NULL_HANDLE); @@ -545,60 +528,61 @@ VkDevice device; VkInstance inst = VK_NULL_HANDLE; - static constexpr VkApplicationInfo appInfo = { VK_STRUCTURE_TYPE_APPLICATION_INFO, - /*pNext=*/nullptr, - "vkTestApp", - /*applicationVersion=*/0, - "vkTestEngine", - /*engineVerison=*/0, - kApiVersion }; + const VkApplicationInfo app_info = { + VK_STRUCTURE_TYPE_APPLICATION_INFO, // sType + nullptr, // pNext + "vktest", // pApplicationName + 0, // applicationVersion + "vktest", // pEngineName + 0, // engineVerison + kApiVersion, // apiVersion + }; // Initialize VulkanPreferredFeatures using whatever API value matches that of the version - // assigned to VkApplicationInfo. + // assigned to VkApplicationInfo. based on API value passed in to VkApplicationInfo (apiVersion) testVkFeatures->skiaFeatures.init(kApiVersion); - // Determine all available instance extensions - which is used by Skia's VulkanPreferredFeatures - // and VulkanExtensions - in addition to which instance extensions we want to enable for - // testing. Skia and/or Vulkan API calls eventually expect to be provided with a list of - // extension/layer *names*, so populate those lists at the same time. - TArray<VkExtensionProperties> availableInstExts; - TArray<const char*> availInstExtNames; - TArray<VkExtensionProperties> instExtsToEnable; - std::vector<const char*> enabledInstExtNames; - // For layers, however, we can get away with only tracking which ones we want to enable - // since Skia doesn't currently parse all available layers in order to make any decisions. - TArray<VkLayerProperties> instLayersToEnable; - TArray<const char*> enabledInstLayerNames; - if (!init_instance_extensions_and_layers(getInstProc, - remove_patch_version(instanceVersion), - availableInstExts, - availInstExtNames, - instExtsToEnable, - enabledInstExtNames, - instLayersToEnable, - enabledInstLayerNames, - testVkFeatures->skiaFeatures)) { + TArray<VkLayerProperties> instanceLayers; + TArray<VkExtensionProperties> instanceExtensions; + + if (!init_instance_extensions_and_layers(getInstProc, instanceVersion, + &instanceExtensions, + &instanceLayers)) { return false; } - // Set up VkInstanceCreateInfo - VkInstanceCreateInfo createInstanceInfo = { VK_STRUCTURE_TYPE_INSTANCE_CREATE_INFO, - /*pNext=*/nullptr, - /*flags=*/0, - &appInfo, - (uint32_t)enabledInstLayerNames.size(), - enabledInstLayerNames.data(), - (uint32_t)enabledInstExtNames.size(), - enabledInstExtNames.data() }; + TArray<const char*> instanceLayerNames; + std::vector<const char*> instanceExtensionNames; + for (int i = 0; i < instanceLayers.size(); ++i) { + instanceLayerNames.push_back(instanceLayers[i].layerName); + } + for (int i = 0; i < instanceExtensions.size(); ++i) { + instanceExtensionNames.push_back(instanceExtensions[i].extensionName); + } + + testVkFeatures->skiaFeatures.addToInstanceExtensions( + instanceExtensions.begin(), instanceExtensions.size(), instanceExtensionNames); + + VkInstanceCreateInfo instance_create = { + VK_STRUCTURE_TYPE_INSTANCE_CREATE_INFO, // sType + nullptr, // pNext + 0, // flags + &app_info, // pApplicationInfo + (uint32_t)instanceLayerNames.size(), // enabledLayerNameCount + instanceLayerNames.begin(), // ppEnabledLayerNames + (uint32_t)instanceExtensionNames.size(), // enabledExtensionNameCount + instanceExtensionNames.data(), // ppEnabledExtensionNames + }; + bool hasDebugExtension = false; *debugMessenger = VK_NULL_HANDLE; #ifdef SK_ENABLE_VK_LAYERS bool hasLayerSettingsExt = false; - for (size_t i = 0; - i < enabledInstExtNames.size() && !hasDebugExtension && !hasLayerSettingsExt; ++i) { - if (!strcmp(enabledInstExtNames[i], VK_EXT_DEBUG_UTILS_EXTENSION_NAME)) { + for (size_t i = 0; i < instanceExtensionNames.size() + && !hasDebugExtension && !hasLayerSettingsExt; ++i) { + if (!strcmp(instanceExtensionNames[i], VK_EXT_DEBUG_UTILS_EXTENSION_NAME)) { hasDebugExtension = true; - } else if (!strcmp(enabledInstExtNames[i], VK_EXT_LAYER_SETTINGS_EXTENSION_NAME)) { + } else if (!strcmp(instanceExtensionNames[i], VK_EXT_LAYER_SETTINGS_EXTENSION_NAME)) { hasLayerSettingsExt = true; } } @@ -638,12 +622,12 @@ layerSettingsCreateInfo.settingCount = static_cast<uint32_t>(std::size(layerSettings)); layerSettingsCreateInfo.pSettings = layerSettings; if (hasDebugExtension && hasLayerSettingsExt) { - createInstanceInfo.pNext = &layerSettingsCreateInfo; + instance_create.pNext = &layerSettingsCreateInfo; } #endif ACQUIRE_VK_INST_PROC(CreateInstance, VK_NULL_HANDLE); - err = grVkCreateInstance(&createInstanceInfo, nullptr, &inst); + err = grVkCreateInstance(&instance_create, nullptr, &inst); if (err < 0) { SkDebugf("vkCreateInstance failed: %d\n", err); return false; @@ -720,61 +704,7 @@ grVkGetPhysicalDeviceProperties(physDev, &physDeviceProperties); uint32_t physDeviceVersion = std::min(physDeviceProperties.apiVersion, kApiVersion); - // Now, for the device rather than the instance, repeat the process of gathering all available - // extensions + their names, determining layers we want to enable for testing + their associated - // available extensions, and determining which extensions we actually want to enable (+ their - // names). - TArray<VkExtensionProperties> availableDevExts; - TArray<const char*> availDevExtNames; - TArray<VkExtensionProperties> devExtsToEnable; - std::vector<const char*> enabledDevExtNames; - TArray<VkLayerProperties> devLayersToEnable; - TArray<const char*> enabledDevLayerNames; - if (!init_device_extensions_and_layers(getProc, - remove_patch_version(physDeviceVersion), - inst, - physDev, - availableDevExts, - availDevExtNames, - devExtsToEnable, - enabledDevExtNames, - devLayersToEnable, - enabledDevLayerNames, - *testVkFeatures, - isProtected)) { - destroy_instance(getInstProc, inst, debugMessenger, hasDebugExtension); - return false; - } - - // Now that we have determined all available instance and device extensions, we can initialize - // VulkanExtensions. - extensions->init(getProc, - inst, - physDev, - (uint32_t)availInstExtNames.size(), - availInstExtNames.data(), - (uint32_t)availDevExtNames.size(), - availDevExtNames.data()); - - // Finally, perform the query. - ACQUIRE_VK_INST_PROC(GetPhysicalDeviceFeatures2, inst); - grVkGetPhysicalDeviceFeatures2(physDev, &testVkFeatures->deviceFeatures); - - if (isProtected && !testVkFeatures->protectedMemoryFeatures.protectedMemory) { - SkDebugf("Device does not support protected memory.\n"); - destroy_instance(getInstProc, inst, debugMessenger, hasDebugExtension); - return false; - } - - // We can manually disable any undesired features here. Currently, we only disable robust - // buffer access which can negatively impact performance on some GPUs. - testVkFeatures->deviceFeatures.features.robustBufferAccess = VK_FALSE; - - // Allow Skia to enable available features - testVkFeatures->skiaFeatures.addFeaturesToEnable(enabledDevExtNames, - testVkFeatures->deviceFeatures); - - // Query for queue property size, then fetch the properties. + // query to get the initial queue props size uint32_t queueCount; grVkGetPhysicalDeviceQueueFamilyProperties(physDev, &queueCount, nullptr); if (!queueCount) { @@ -784,10 +714,12 @@ } SkAutoMalloc queuePropsAlloc(queueCount * sizeof(VkQueueFamilyProperties)); + // now get the actual queue props VkQueueFamilyProperties* queueProps = (VkQueueFamilyProperties*)queuePropsAlloc.get(); + grVkGetPhysicalDeviceQueueFamilyProperties(physDev, &queueCount, queueProps); - // Iterate to find the graphics queue + // iterate to find the graphics queue uint32_t graphicsQueueIndex = queueCount; for (uint32_t i = 0; i < queueCount; i++) { if (queueProps[i].queueFlags & VK_QUEUE_GRAPHICS_BIT) { @@ -801,7 +733,7 @@ return false; } - // Iterate to find the present queue, if needed + // iterate to find the present queue, if needed uint32_t presentQueueIndex = queueCount; if (presentQueueIndexPtr && canPresent) { for (uint32_t i = 0; i < queueCount; i++) { @@ -822,6 +754,53 @@ presentQueueIndex = graphicsQueueIndex; } + TArray<VkLayerProperties> deviceLayers; + TArray<VkExtensionProperties> deviceExtensions; + if (!init_device_extensions_and_layers(getProc, physDeviceVersion, + inst, physDev, + &deviceExtensions, + &deviceLayers)) { + destroy_instance(getInstProc, inst, debugMessenger, hasDebugExtension); + return false; + } + + TArray<const char*> deviceLayerNames; + std::vector<const char*> deviceExtensionNames; + for (int i = 0; i < deviceLayers.size(); ++i) { + deviceLayerNames.push_back(deviceLayers[i].layerName); + } + + for (int i = 0; i < deviceExtensions.size(); ++i) { + deviceExtensionNames.push_back(deviceExtensions[i].extensionName); + } + + // Note: Any struct that setup_features chains must stay in scope until vkCreateDevice. This is + // why these structs are located in TestVkFeatures and passed in this function. + if (!setup_features(getProc, + inst, + physDev, + physDeviceVersion, + deviceExtensions, + *testVkFeatures, + isProtected)) { + destroy_instance(getInstProc, inst, debugMessenger, hasDebugExtension); + return false; + } + + // Enable features and extensions that are desired by Skia. This _adds_ to features and + // extensions already enabled in deviceExtensionNames and features that aren't done by + // skiaFeatures itself. + testVkFeatures->skiaFeatures.addFeaturesToEnable(deviceExtensionNames, + testVkFeatures->deviceFeatures); + + extensions->init(getProc, + inst, + physDev, + (uint32_t)instanceExtensionNames.size(), + instanceExtensionNames.data(), + (uint32_t)deviceExtensionNames.size(), + deviceExtensionNames.data()); + VkDeviceQueueCreateFlags flags = isProtected ? VK_DEVICE_QUEUE_CREATE_PROTECTED_BIT : 0; float queuePriorities[1] = { 0.0 }; // Here we assume no need for swapchain queue @@ -847,16 +826,19 @@ }; uint32_t queueInfoCount = (presentQueueIndex != graphicsQueueIndex) ? 2 : 1; - const VkDeviceCreateInfo deviceInfo = { VK_STRUCTURE_TYPE_DEVICE_CREATE_INFO, - &testVkFeatures->deviceFeatures, - /*flags=*/0, - queueInfoCount, - queueInfo, - (uint32_t) enabledDevLayerNames.size(), - enabledDevLayerNames.data(), - (uint32_t) enabledDevExtNames.size(), - enabledDevExtNames.data(), - /*ppEnabledFeatures*/nullptr }; + const VkDeviceCreateInfo deviceInfo = { + VK_STRUCTURE_TYPE_DEVICE_CREATE_INFO, // sType + &testVkFeatures->deviceFeatures, // pNext + 0, // VkDeviceCreateFlags + queueInfoCount, // queueCreateInfoCount + queueInfo, // pQueueCreateInfos + (uint32_t) deviceLayerNames.size(), // layerCount + deviceLayerNames.begin(), // ppEnabledLayerNames + (uint32_t) deviceExtensionNames.size(), // extensionCount + deviceExtensionNames.data(), // ppEnabledExtensionNames + nullptr, // ppEnabledFeatures + }; + { #if defined(SK_ENABLE_SCOPED_LSAN_SUPPRESSIONS) // skbug.com/40040003
diff --git a/tools/gpu/vk/VkTestUtils.h b/tools/gpu/vk/VkTestUtils.h index 943c585..d9647be 100644 --- a/tools/gpu/vk/VkTestUtils.h +++ b/tools/gpu/vk/VkTestUtils.h
@@ -26,8 +26,8 @@ struct TestVkFeatures { VkPhysicalDeviceFeatures2 deviceFeatures; - // Structs from skiaFeatures and protectedMemoryFeatures may be chained into deviceFeatures - // and therefore must share the same lifetime. + // protectedMemoryFeatures and structs from skiaFeatures may be chained into deviceFeatures, + // so must share the same lifetime. skgpu::VulkanPreferredFeatures skiaFeatures; VkPhysicalDeviceProtectedMemoryFeatures protectedMemoryFeatures; };