[PATCH 1/5] winevulkan: Invert MAX_IMAGE_COUNT quirk to default to zero
Signed-off-by: Liam Middlebrook <lmiddlebrook(a)nvidia.com> Signed-off-by: Daniel Koch <dkoch(a)nvidia.com> --- dlls/winevulkan/vulkan.c | 4 +--- dlls/winevulkan/vulkan_private.h | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/dlls/winevulkan/vulkan.c b/dlls/winevulkan/vulkan.c index f730c04923a..a0dafbca46b 100644 --- a/dlls/winevulkan/vulkan.c +++ b/dlls/winevulkan/vulkan.c @@ -729,8 +729,6 @@ VkResult WINAPI wine_vkCreateInstance(const VkInstanceCreateInfo *create_info, object->quirks |= WINEVULKAN_QUIRK_GET_DEVICE_PROC_ADDR; } - object->quirks |= WINEVULKAN_QUIRK_ADJUST_MAX_IMAGE_COUNT; - *instance = object; TRACE("Created instance %p (native instance %p).\n", object, object->instance); return VK_SUCCESS; @@ -1580,7 +1578,7 @@ static inline void adjust_max_image_count(VkPhysicalDevice phys_dev, VkSurfaceCa * https://vulkan.gpuinfo.org/displayreport.php?id=9122#surface * https://vulkan.gpuinfo.org/displayreport.php?id=9121#surface */ - if ((phys_dev->instance->quirks & WINEVULKAN_QUIRK_ADJUST_MAX_IMAGE_COUNT) && !capabilities->maxImageCount) + if (!(phys_dev->instance->quirks & WINEVULKAN_QUIRK_DONT_ADJUST_MAX_IMAGE_COUNT) && !capabilities->maxImageCount) { capabilities->maxImageCount = max(capabilities->minImageCount, 16); } diff --git a/dlls/winevulkan/vulkan_private.h b/dlls/winevulkan/vulkan_private.h index 4bcc4de440d..00698aff4bb 100644 --- a/dlls/winevulkan/vulkan_private.h +++ b/dlls/winevulkan/vulkan_private.h @@ -38,7 +38,7 @@ #define VULKAN_ICD_MAGIC_VALUE 0x01CDC0DE #define WINEVULKAN_QUIRK_GET_DEVICE_PROC_ADDR 0x00000001 -#define WINEVULKAN_QUIRK_ADJUST_MAX_IMAGE_COUNT 0x00000002 +#define WINEVULKAN_QUIRK_DONT_ADJUST_MAX_IMAGE_COUNT 0x00000002 #define WINEVULKAN_QUIRK_IGNORE_EXPLICIT_LAYERS 0x00000004 struct vulkan_func -- 2.17.1
Tested with Vulkan CTS and WINEVULKAN_QUIRK_IGNORE_EXPLICIT_LAYERS. Signed-off-by: Liam Middlebrook <lmiddlebrook(a)nvidia.com> Signed-off-by: Daniel Koch <dkoch(a)nvidia.com> --- dlls/winevulkan/vulkan.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/dlls/winevulkan/vulkan.c b/dlls/winevulkan/vulkan.c index a0dafbca46b..57b0e65152a 100644 --- a/dlls/winevulkan/vulkan.c +++ b/dlls/winevulkan/vulkan.c @@ -655,6 +655,28 @@ fail: return res; } + +static void wine_vk_process_quirks(const VkApplicationInfo *pApplicationInfo, struct VkInstance_T *object) +{ + HKEY globalKey; + + /* Load Global Quirks + * @@ Wine registry key: HKCU\Software\Wine\Vulkan + */ + if (RegOpenKeyA(HKEY_CURRENT_USER, "Software\\Wine\\Vulkan", &globalKey) == 0) + { + DWORD type, value, size; + size = sizeof(value); + if (RegQueryValueExA(globalKey, "Quirks", NULL, &type, (LPBYTE)&value, &size) == 0 + && type == REG_DWORD) + { + object->quirks = value; + TRACE("Loaded Quirks value %x\n", value); + } + RegCloseKey(globalKey); + } +} + VkResult WINAPI wine_vkCreateInstance(const VkInstanceCreateInfo *create_info, const VkAllocationCallbacks *allocator, VkInstance *instance) { @@ -679,6 +701,11 @@ VkResult WINAPI wine_vkCreateInstance(const VkInstanceCreateInfo *create_info, } object->base.loader_magic = VULKAN_ICD_MAGIC_VALUE; + /* Load optional WineVulkan quirks bits from registry, see vulkan_private.h + * for a list of quirks. + */ + wine_vk_process_quirks(create_info->pApplicationInfo, object); + res = wine_vk_instance_convert_create_info(create_info, &create_info_host, object); if (res != VK_SUCCESS) { -- 2.17.1
Refactors existing handling of the Quirks regkey to allow for easier addition of future keys. Signed-off-by: Liam Middlebrook <lmiddlebrook(a)nvidia.com> Signed-off-by: Daniel Koch <dkoch(a)nvidia.com> --- dlls/winevulkan/vulkan.c | 36 ++++++++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/dlls/winevulkan/vulkan.c b/dlls/winevulkan/vulkan.c index 57b0e65152a..a9a341209f8 100644 --- a/dlls/winevulkan/vulkan.c +++ b/dlls/winevulkan/vulkan.c @@ -658,22 +658,46 @@ fail: static void wine_vk_process_quirks(const VkApplicationInfo *pApplicationInfo, struct VkInstance_T *object) { - HKEY globalKey; + uint8_t validKeysMask = 0; + int keyIndex = 0; + HKEY keys[1]; + int i; - /* Load Global Quirks - * @@ Wine registry key: HKCU\Software\Wine\Vulkan + memset(&keys, 0, sizeof(keys)); + + /* Match regkey settings in the following order, breaking early if settings + * are found: + * global defaults + * @@ Wine registry key: HKCU\Software\Wine\Vulkan */ - if (RegOpenKeyA(HKEY_CURRENT_USER, "Software\\Wine\\Vulkan", &globalKey) == 0) + if (RegOpenKeyA(HKEY_CURRENT_USER, "Software\\Wine\\Vulkan", keys + keyIndex++) == 0) + validKeysMask |= (1 << (keyIndex - 1)); + + /* Load Global Quirks */ + for (i = 0; i < ARRAY_SIZE(keys); i++) { DWORD type, value, size; size = sizeof(value); - if (RegQueryValueExA(globalKey, "Quirks", NULL, &type, (LPBYTE)&value, &size) == 0 + + if ((validKeysMask & (1 << i)) == 0) + continue; + + if (RegQueryValueExA(keys[i], "Quirks", NULL, &type, (LPBYTE)&value, &size) == 0 && type == REG_DWORD) { object->quirks = value; TRACE("Loaded Quirks value %x\n", value); + break; } - RegCloseKey(globalKey); + } + + for (i = 0; i < ARRAY_SIZE(keys); i++) + { + if ((validKeysMask & (1 << i)) == 0) + continue; + + validKeysMask &= ~(1 << i); + RegCloseKey(keys[i]); } } -- 2.17.1
Add support for regkeys to set Quirks bits depending on VkInstance's pApplicationInfo->pApplicationName and pApplicationInfo->pEngineName properties. Signed-off-by: Liam Middlebrook <lmiddlebrook(a)nvidia.com> Signed-off-by: Daniel Koch <dkoch(a)nvidia.com> --- dlls/winevulkan/vulkan.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/dlls/winevulkan/vulkan.c b/dlls/winevulkan/vulkan.c index a9a341209f8..e78ca1eda62 100644 --- a/dlls/winevulkan/vulkan.c +++ b/dlls/winevulkan/vulkan.c @@ -655,22 +655,44 @@ fail: return res; } +static LSTATUS wine_vk_open_regkey_with_suffix(HKEY *key, const char *path_prefix, const char *path_suffix, BOOL allow_null_suffix) +{ + char buf[MAX_PATH]; + + if (!allow_null_suffix && !path_suffix) + return ERROR_INVALID_PARAMETER; + + lstrcpynA(buf, path_prefix, MAX_PATH); + if (path_suffix) + { + strncat(buf, path_suffix, MAX_PATH - strlen(buf)); + } + return RegOpenKeyA(HKEY_CURRENT_USER, buf, key); +} static void wine_vk_process_quirks(const VkApplicationInfo *pApplicationInfo, struct VkInstance_T *object) { uint8_t validKeysMask = 0; int keyIndex = 0; - HKEY keys[1]; + HKEY keys[3]; int i; memset(&keys, 0, sizeof(keys)); /* Match regkey settings in the following order, breaking early if settings * are found: + * pApplicationInfo->pApplicationName + * @@ Wine registry key: HKCU\Software\Wine\Vulkan\pApplicationName\<pApplicationName> + * pApplicationInfo->pEngineName + * @@ Wine registry key: HKCU\Software\Wine\Vulkan\pEngineName\<pEngineName> * global defaults * @@ Wine registry key: HKCU\Software\Wine\Vulkan */ - if (RegOpenKeyA(HKEY_CURRENT_USER, "Software\\Wine\\Vulkan", keys + keyIndex++) == 0) + if (wine_vk_open_regkey_with_suffix(keys + keyIndex++, "Software\\Wine\\Vulkan\\pApplicationName\\", pApplicationInfo->pApplicationName, FALSE) == 0) + validKeysMask |= (1 << (keyIndex - 1)); + if (wine_vk_open_regkey_with_suffix(keys + keyIndex++, "Software\\Wine\\Vulkan\\pEngineName\\", pApplicationInfo->pEngineName, FALSE) == 0) + validKeysMask |= (1 << (keyIndex - 1)); + if (wine_vk_open_regkey_with_suffix(keys + keyIndex++, "Software\\Wine\\Vulkan", NULL, TRUE) == 0) validKeysMask |= (1 << (keyIndex - 1)); /* Load Global Quirks */ -- 2.17.1
Add support for regkeys to set Quirks bits depending the executable name of the currently running program. Signed-off-by: Liam Middlebrook <lmiddlebrook(a)nvidia.com> Signed-off-by: Daniel Koch <dkoch(a)nvidia.com> --- dlls/winevulkan/vulkan.c | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/dlls/winevulkan/vulkan.c b/dlls/winevulkan/vulkan.c index e78ca1eda62..a77664a7e5f 100644 --- a/dlls/winevulkan/vulkan.c +++ b/dlls/winevulkan/vulkan.c @@ -670,21 +670,51 @@ static LSTATUS wine_vk_open_regkey_with_suffix(HKEY *key, const char *path_prefi return RegOpenKeyA(HKEY_CURRENT_USER, buf, key); } +static uint32_t wine_vk_get_exe_name(char *name, uint32_t length) +{ + char buf[MAX_PATH]; + uint32_t buf_len; + char *p, *exe_path; + + buf_len = GetModuleFileNameA(0, buf, MAX_PATH); + if (!buf_len || buf_len == MAX_PATH) + return 0; + + exe_path = buf; + if ((p = strrchr(exe_path, '/'))) + exe_path = p + 1; + if ((p = strrchr(exe_path, '\\'))) + exe_path = p + 1; + + buf_len = strlen(exe_path) + 1; + if (buf_len > length) + return 0; + + lstrcpynA(name, exe_path, length); + return length; +} + static void wine_vk_process_quirks(const VkApplicationInfo *pApplicationInfo, struct VkInstance_T *object) { + char exe_name[MAX_PATH]; uint8_t validKeysMask = 0; int keyIndex = 0; - HKEY keys[3]; + HKEY keys[4]; int i; memset(&keys, 0, sizeof(keys)); + if (!wine_vk_get_exe_name(exe_name, MAX_PATH)) + exe_name[0] = '\0'; + /* Match regkey settings in the following order, breaking early if settings * are found: * pApplicationInfo->pApplicationName * @@ Wine registry key: HKCU\Software\Wine\Vulkan\pApplicationName\<pApplicationName> * pApplicationInfo->pEngineName * @@ Wine registry key: HKCU\Software\Wine\Vulkan\pEngineName\<pEngineName> + * executable name + * @@ Wine registry key: HKCU\Software\Wine\Vulkan\exeName\app.exe * global defaults * @@ Wine registry key: HKCU\Software\Wine\Vulkan */ @@ -692,6 +722,8 @@ static void wine_vk_process_quirks(const VkApplicationInfo *pApplicationInfo, st validKeysMask |= (1 << (keyIndex - 1)); if (wine_vk_open_regkey_with_suffix(keys + keyIndex++, "Software\\Wine\\Vulkan\\pEngineName\\", pApplicationInfo->pEngineName, FALSE) == 0) validKeysMask |= (1 << (keyIndex - 1)); + if (wine_vk_open_regkey_with_suffix(keys + keyIndex++, "Software\\Wine\\Vulkan\\exeName\\", exe_name, FALSE) == 0) + validKeysMask |= (1 << (keyIndex - 1)); if (wine_vk_open_regkey_with_suffix(keys + keyIndex++, "Software\\Wine\\Vulkan", NULL, TRUE) == 0) validKeysMask |= (1 << (keyIndex - 1)); -- 2.17.1
Liam Middlebrook <lmiddlebrook(a)nvidia.com> writes:
+ if (!wine_vk_get_exe_name(exe_name, MAX_PATH)) + exe_name[0] = '\0'; + /* Match regkey settings in the following order, breaking early if settings * are found: * pApplicationInfo->pApplicationName * @@ Wine registry key: HKCU\Software\Wine\Vulkan\pApplicationName\<pApplicationName> * pApplicationInfo->pEngineName * @@ Wine registry key: HKCU\Software\Wine\Vulkan\pEngineName\<pEngineName> + * executable name + * @@ Wine registry key: HKCU\Software\Wine\Vulkan\exeName\app.exe * global defaults * @@ Wine registry key: HKCU\Software\Wine\Vulkan
Do we really need to invent a new mechanism for this? All other modules use HKCU\Software\Wine\AppDefaults\<exename> for that sort of thing. -- Alexandre Julliard julliard(a)winehq.org
On 9/15/20 12:52 AM, Alexandre Julliard wrote:
Liam Middlebrook <lmiddlebrook(a)nvidia.com> writes:
+ if (!wine_vk_get_exe_name(exe_name, MAX_PATH)) + exe_name[0] = '\0'; + /* Match regkey settings in the following order, breaking early if settings * are found: * pApplicationInfo->pApplicationName * @@ Wine registry key: HKCU\Software\Wine\Vulkan\pApplicationName\<pApplicationName> * pApplicationInfo->pEngineName * @@ Wine registry key: HKCU\Software\Wine\Vulkan\pEngineName\<pEngineName> + * executable name + * @@ Wine registry key: HKCU\Software\Wine\Vulkan\exeName\app.exe * global defaults * @@ Wine registry key: HKCU\Software\Wine\Vulkan
Do we really need to invent a new mechanism for this? All other modules use HKCU\Software\Wine\AppDefaults\<exename> for that sort of thing.
For the executable name matching. I'm fine with using the existing HKCU\Software\Wine\AppDefaults\<exename>\ pattern that other modules are using. I had picked the format above to be consistent with this newer syntax I came up with for VkApplicationInfo, but I agree that consistency with the rest of WINE seems like a better fit overall. I'll change that to be consistent with the other AppDefaults regkeys in WINE. Looking at the other APIs which have AppDefaults keys available, it doesn't appear that any of these APIs have a mechanism which serves quite the same purpose that Vulkan does with VkApplicationInfo (namely the application providing information about itself to the implementation). I guess there could be an argument that this is generically possible by keying off of values from VS_VERSIONINFO, but I don't think it's quite the same as that's a mechanism of Windows rather than the APIs themselves. The Vulkan specification notes the following about VkInstanceCreateInfo.pApplicationInfo [1]:
this information helps implementations recognize behavior inherent to classes of applications
I think it makes sense moving forward, as we see more applications adopting use of Vulkan, to utilize the mechanisms the API affords implementations for identifying a running application. Additionally, this information is already being used within WineVulkan to match the idTech engine [2]. There are also a few different Vulkan driver implementations [3], [4], [5], which use this for enabling per-application/per-engine settings. While writing this out I also found an issue with patch 3/5 where I was incorrectly assuming that pApplicationInfo would always be a valid pointer we could read from. I'll fix that locally for now, but wait to send out v2 of the series until we bottom out on the discussion here. Thanks, Liam Middlebrook [1] - https://www.khronos.org/registry/vulkan/specs/1.2-extensions/html/vkspec.htm... [2] - https://source.winehq.org/git/wine.git/blob/HEAD:/dlls/winevulkan/vulkan.c#l... [3] - https://github.com/GPUOpen-Drivers/xgl/blob/dev/icd/api/app_profile.cpp#L100 [4] - https://gitlab.freedesktop.org/mesa/mesa/-/blob/master/src/amd/vulkan/radv_d... [5] - https://gitlab.freedesktop.org/mesa/mesa/-/blob/master/src/intel/vulkan/anv_...
On Tue, 15 Sep 2020 at 16:13, Liam Middlebrook <lmiddlebrook(a)nvidia.com> wrote:
Additionally, this information is already being used within WineVulkan to match the idTech engine [2]. There are also a few different Vulkan driver implementations [3], [4], [5], which use this for enabling per-application/per-engine settings.
Incidentally, I absolutely *hate* drivers that do this kind of thing, particularly the overly broad variants like the "vkd3d" example in [4] above, or the simply *broken* variants like [6]/[7] where the default configuration is broken to work around an application bug in a non-default configuration. Back on-topic though—and I think what Alexandre was referring to—do we really think there's a case in practice for users to apply quirks based on e.g. the internal engine name, instead of simply the executable name? [6] https://bugs.winehq.org/show_bug.cgi?id=48818 [7] https://bugs.freedesktop.org/show_bug.cgi?id=110462
On Sep 15, 2020, at 5:33 AM, Henri Verbeet <hverbeet(a)gmail.com> wrote:
On Tue, 15 Sep 2020 at 16:13, Liam Middlebrook <lmiddlebrook(a)nvidia.com> wrote:
Additionally, this information is already being used within WineVulkan to match the idTech engine [2]. There are also a few different Vulkan driver implementations [3], [4], [5], which use this for enabling per-application/per-engine settings.
Incidentally, I absolutely *hate* drivers that do this kind of thing, particularly the overly broad variants like the "vkd3d" example in [4] above, or the simply *broken* variants like [6]/[7] where the default configuration is broken to work around an application bug in a non-default configuration.
Back on-topic though—and I think what Alexandre was referring to—do we really think there's a case in practice for users to apply quirks based on e.g. the internal engine name, instead of simply the executable name?
I don’t know of a real-world example where this has been necessary, but one theoretical case is that an app with a generic name like “Game.exe” or “Launcher.exe” (but a unique application/engine name) needs a quirk or override applied. Maybe this is too remote a possibility to add it preemptively though. Also Liam, in case it’s decided to go ahead with this, since pApplicationInfo and pEngineName are UTF-8, should they be converted to UTF-16 and Unicode registry functions be used? Brendan
participants (4)
-
Alexandre Julliard -
Brendan Shanks -
Henri Verbeet -
Liam Middlebrook