From d3aeff861e34f3733af1408dfce235c0d182c1ca Mon Sep 17 00:00:00 2001 From: Nukem Date: Fri, 10 Jan 2025 16:13:16 -0500 Subject: [PATCH] libretro: Restore Vulkan MSAA video option --- Common/GPU/Vulkan/VulkanContext.cpp | 4 -- libretro/LibretroVulkanContext.cpp | 2 +- libretro/libretro.cpp | 6 -- libretro/libretro_core_options.h | 6 +- libretro/libretro_vulkan.cpp | 98 +++++++++++++++++++---------- 5 files changed, 67 insertions(+), 49 deletions(-) diff --git a/Common/GPU/Vulkan/VulkanContext.cpp b/Common/GPU/Vulkan/VulkanContext.cpp index 18e246b779..0436ad6b4e 100644 --- a/Common/GPU/Vulkan/VulkanContext.cpp +++ b/Common/GPU/Vulkan/VulkanContext.cpp @@ -174,13 +174,9 @@ VkResult VulkanContext::CreateInstance(const CreateInfo &info) { } } - // Temporary hack for libretro. For some reason, when we try to load the functions from this extension, - // we get null pointers when running libretro. Quite strange. -#if !defined(__LIBRETRO__) if (EnableInstanceExtension(VK_KHR_GET_PHYSICAL_DEVICE_PROPERTIES_2_EXTENSION_NAME, VK_API_VERSION_1_1)) { extensionsLookup_.KHR_get_physical_device_properties2 = true; } -#endif if (EnableInstanceExtension(VK_EXT_SWAPCHAIN_COLOR_SPACE_EXTENSION_NAME, 0)) { extensionsLookup_.EXT_swapchain_colorspace = true; diff --git a/libretro/LibretroVulkanContext.cpp b/libretro/LibretroVulkanContext.cpp index e4c5fbe544..25737c370b 100644 --- a/libretro/LibretroVulkanContext.cpp +++ b/libretro/LibretroVulkanContext.cpp @@ -83,7 +83,7 @@ static const VkApplicationInfo *GetApplicationInfo(void) { app_info.applicationVersion = Version(PPSSPP_GIT_VERSION).ToInteger(); app_info.pEngineName = "PPSSPP"; app_info.engineVersion = 2; - app_info.apiVersion = VK_API_VERSION_1_0; + app_info.apiVersion = VK_API_VERSION_1_3; return &app_info; } diff --git a/libretro/libretro.cpp b/libretro/libretro.cpp index 8764abe66e..3b03c33a0d 100644 --- a/libretro/libretro.cpp +++ b/libretro/libretro.cpp @@ -700,7 +700,6 @@ static void check_variables(CoreParameter &coreParam) g_Config.iInternalResolution = 1; } -#if 0 // see issue #16786 var.key = "ppsspp_mulitsample_level"; if (environ_cb(RETRO_ENVIRONMENT_GET_VARIABLE, &var) && var.value) { @@ -715,7 +714,6 @@ static void check_variables(CoreParameter &coreParam) else if (!strcmp(var.value, "x8")) g_Config.iMultiSampleLevel = 3; } -#endif var.key = "ppsspp_cropto16x9"; if (environ_cb(RETRO_ENVIRONMENT_GET_VARIABLE, &var) && var.value) @@ -1143,7 +1141,6 @@ static void check_variables(CoreParameter &coreParam) gpu->NotifyDisplayResized(); } -#if 0 // see issue #16786 if (g_Config.iMultiSampleLevel != iMultiSampleLevel_prev && PSP_IsInited()) { if (gpu) @@ -1151,7 +1148,6 @@ static void check_variables(CoreParameter &coreParam) gpu->NotifyRenderResized(); } } -#endif if (updateAvInfo) { @@ -1488,10 +1484,8 @@ bool retro_load_game(const struct retro_game_info *game) // Show/hide 'MSAA' and 'Texture Shader' options, Vulkan only option_display.visible = (g_Config.iGPUBackend == (int)GPUBackend::VULKAN); -#if 0 // see issue #16786 option_display.key = "ppsspp_mulitsample_level"; environ_cb(RETRO_ENVIRONMENT_SET_CORE_OPTIONS_DISPLAY, &option_display); -#endif option_display.key = "ppsspp_texture_shader"; environ_cb(RETRO_ENVIRONMENT_SET_CORE_OPTIONS_DISPLAY, &option_display); diff --git a/libretro/libretro_core_options.h b/libretro/libretro_core_options.h index f0ac57e3b0..7feb948206 100644 --- a/libretro/libretro_core_options.h +++ b/libretro/libretro_core_options.h @@ -326,12 +326,11 @@ struct retro_core_option_v2_definition option_defs_us[] = { }, "480x272" }, -#if 0 // see issue #16786 { "ppsspp_mulitsample_level", - "MSAA Antialiasing (Vulkan Only)", - NULL, + "MSAA Antialiasing", NULL, + "Vulkan only. Core restart required.", NULL, "video", { @@ -343,7 +342,6 @@ struct retro_core_option_v2_definition option_defs_us[] = { }, "Disabled" }, -#endif { "ppsspp_cropto16x9", "Crop to 16x9", diff --git a/libretro/libretro_vulkan.cpp b/libretro/libretro_vulkan.cpp index cdd4b56221..f505ffd1ce 100644 --- a/libretro/libretro_vulkan.cpp +++ b/libretro/libretro_vulkan.cpp @@ -1,11 +1,3 @@ -// Debugging notes -// The crash happens when we try to call vkGetPhysicalDeviceProperties2KHR which seems to be null. -// -// Apparently we don't manage to specify the extensions we want. Still something reports that this one -// is present? -// Failed to load : vkGetPhysicalDeviceProperties2KHR -// Failed to load : vkGetPhysicalDeviceFeatures2KHR - #include #include #include @@ -80,41 +72,79 @@ static VKAPI_ATTR VkResult VKAPI_CALL vkCreateInstance_libretro(const VkInstance } static void add_name_unique(std::vector &list, const char *value) { - for (const char *name : list) - if (!strcmp(value, name)) - return; + for (const char *name : list) { + if (!strcmp(value, name)) + return; + } - list.push_back(value); + list.push_back(value); } + static VKAPI_ATTR VkResult VKAPI_CALL vkCreateDevice_libretro(VkPhysicalDevice physicalDevice, const VkDeviceCreateInfo *pCreateInfo, const VkAllocationCallbacks *pAllocator, VkDevice *pDevice) { - VkDeviceCreateInfo info = *pCreateInfo; - std::vector EnabledLayerNames(info.ppEnabledLayerNames, info.ppEnabledLayerNames + info.enabledLayerCount); - std::vector EnabledExtensionNames(info.ppEnabledExtensionNames, info.ppEnabledExtensionNames + info.enabledExtensionCount); - VkPhysicalDeviceFeatures EnabledFeatures = *info.pEnabledFeatures; + VkDeviceCreateInfo newInfo = *pCreateInfo; - for (unsigned i = 0; i < vk_init_info.num_required_device_layers; i++) - add_name_unique(EnabledLayerNames, vk_init_info.required_device_layers[i]); + // Add our custom layers + std::vector enabledLayerNames(pCreateInfo->ppEnabledLayerNames, pCreateInfo->ppEnabledLayerNames + pCreateInfo->enabledLayerCount); - for (unsigned i = 0; i < vk_init_info.num_required_device_extensions; i++) - add_name_unique(EnabledExtensionNames, vk_init_info.required_device_extensions[i]); + for (uint32_t i = 0; i < vk_init_info.num_required_device_layers; i++) { + add_name_unique(enabledLayerNames, vk_init_info.required_device_layers[i]); + } - for (unsigned i = 0; i < sizeof(VkPhysicalDeviceFeatures) / sizeof(VkBool32); i++) { - if (((VkBool32 *)vk_init_info.required_features)[i]) - ((VkBool32 *)&EnabledFeatures)[i] = VK_TRUE; - } + newInfo.enabledLayerCount = (uint32_t)enabledLayerNames.size(); + newInfo.ppEnabledLayerNames = newInfo.enabledLayerCount ? enabledLayerNames.data() : nullptr; - for (auto extension_name : EnabledExtensionNames) { - if (!strcmp(extension_name, VK_KHR_DEDICATED_ALLOCATION_EXTENSION_NAME)) - DEDICATED_ALLOCATION = true; - } + // Add our custom extensions + std::vector enabledExtensionNames(pCreateInfo->ppEnabledExtensionNames, pCreateInfo->ppEnabledExtensionNames + pCreateInfo->enabledExtensionCount); - info.enabledLayerCount = (uint32_t)EnabledLayerNames.size(); - info.ppEnabledLayerNames = info.enabledLayerCount ? EnabledLayerNames.data() : nullptr; - info.enabledExtensionCount = (uint32_t)EnabledExtensionNames.size(); - info.ppEnabledExtensionNames = info.enabledExtensionCount ? EnabledExtensionNames.data() : nullptr; - info.pEnabledFeatures = &EnabledFeatures; + for (uint32_t i = 0; i < vk_init_info.num_required_device_extensions; i++) { + add_name_unique(enabledExtensionNames, vk_init_info.required_device_extensions[i]); + } - return vkCreateDevice_org(physicalDevice, &info, pAllocator, pDevice); + for (const char *extensionName : enabledExtensionNames) { + if (!strcmp(extensionName, VK_KHR_DEDICATED_ALLOCATION_EXTENSION_NAME)) + DEDICATED_ALLOCATION = true; + } + + newInfo.enabledExtensionCount = (uint32_t)enabledExtensionNames.size(); + newInfo.ppEnabledExtensionNames = newInfo.enabledExtensionCount ? enabledExtensionNames.data() : nullptr; + + // Then check for VkPhysicalDeviceFeatures2 chaining or pEnabledFeatures to enable required features. Note that when both + // structs are present Features2 takes precedence. vkCreateDevice parameters don't give us a simple way to detect + // VK_KHR_get_physical_device_properties2 usage so we'll always try both paths. + std::unordered_map originalFeaturePointers; + VkPhysicalDeviceFeatures placeholderEnabledFeatures{}; + + for (const VkBaseOutStructure *next = (const VkBaseOutStructure *)pCreateInfo->pNext; next != nullptr;) { + if (next->sType == VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_FEATURES_2) { + VkPhysicalDeviceFeatures *enabledFeatures = &((VkPhysicalDeviceFeatures2 *)next)->features; + originalFeaturePointers.try_emplace(enabledFeatures, *enabledFeatures); + } + + next = (const VkBaseOutStructure *)next->pNext; + } + + if (newInfo.pEnabledFeatures) { + placeholderEnabledFeatures = *newInfo.pEnabledFeatures; + } + + newInfo.pEnabledFeatures = &placeholderEnabledFeatures; + originalFeaturePointers.try_emplace((VkPhysicalDeviceFeatures *)newInfo.pEnabledFeatures, *newInfo.pEnabledFeatures); + + for (const auto& pair : originalFeaturePointers) { + for (uint32_t i = 0; i < sizeof(VkPhysicalDeviceFeatures) / sizeof(VkBool32); i++) { + if (((VkBool32 *)vk_init_info.required_features)[i]) + ((VkBool32 *)pair.first)[i] = VK_TRUE; + } + } + + VkResult res = vkCreateDevice_org(physicalDevice, &newInfo, pAllocator, pDevice); + + // The above code potentially modifies application memory. Restore it to avoid unexpected side effects. + for (const auto& pair : originalFeaturePointers) { + *pair.first = pair.second; + } + + return res; } static VKAPI_ATTR VkResult VKAPI_CALL vkCreateLibretroSurfaceKHR(VkInstance instance, const void *pCreateInfo, const VkAllocationCallbacks *pAllocator, VkSurfaceKHR *pSurface) {