[PATCH v2 2/2] winevulkan: Add registry key for manipulating quirks

[Brendan Shanks]

> On Sep 8, 2020, at 6:44 PM, Liam Middlebrook <lmiddlebrook at nvidia.com> wrote:
> 
> Hi Brendan,
> 
> On 9/8/20 1:22 PM, Brendan Shanks wrote:
>> Hi Liam,
>>> On Sep 8, 2020, at 12:46 PM, Liam Middlebrook <lmiddlebrook at nvidia.com> wrote:
>>> 
>>> Tested with Vulkan CTS and WINEVULKAN_QUIRK_IGNORE_EXPLICIT_LAYERS.
>>> 
>>> Signed-off-by: Liam Middlebrook <lmiddlebrook at nvidia.com>
>>> Signed-off-by: Daniel Koch <dkoch at nvidia.com>
>>> ---
>>> v2: fixup comment style
>>> 
>>> dlls/winevulkan/vulkan.c | 17 +++++++++++++++++
>>> 1 file changed, 17 insertions(+)
>>> 
>>> diff --git a/dlls/winevulkan/vulkan.c b/dlls/winevulkan/vulkan.c
>>> index a8beef126bd..19093d47390 100644
>>> --- a/dlls/winevulkan/vulkan.c
>>> +++ b/dlls/winevulkan/vulkan.c
>>> @@ -662,6 +662,7 @@ VkResult WINAPI wine_vkCreateInstance(const VkInstanceCreateInfo *create_info,
>>>     const VkApplicationInfo *app_info;
>>>     struct VkInstance_T *object;
>>>     VkResult res;
>>> +    HKEY key;
>>> 
>>>     TRACE("create_info %p, allocator %p, instance %p\n", create_info, allocator, instance);
>>> 
>>> @@ -679,6 +680,22 @@ 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.
>>> +     */
>> There’s a specially-formatted comment that is used to highlight Wine registry keys, like:
>> /* @@ Wine registry key: HKCU\Software\Wine\Vulkan */
> 
> Ah cool, I was looking through examples in other places and must have missed that. I'll add that in later on, but I figure we may want to hash out the below a bit more first, before I send any follow-up patches.
> 
>> Also, this seems like a good fit to support AppDefaults in the registry.
> 
> Yeah, I think that makes sense to do. Rather than using the process name like is done elsewhere, how would you feel about making use of pApplicationName and pEngineName from VkApplicationInfo?
> 
> The regkey structure could be something like:
>    HKCU\Software\Wine\Vulkan\ApplicationName\%pApplicationName%\Quirks
>    HKCU\Software\Wine\Vulkan\EngineName\%pEngineName%\Quirks
> 

Yeah I like this idea. Some day we’ll need to apply a quirk for “Game.exe” or “Launcher.exe”, and this will come in handy.

I guess in this case you would check them from most to least specific, so application name->engine name->executable name. And then each level would be checked and the first quirks value found gets used?

> Although it'd also be nice to try to account for versions there, but maybe doing that from the registry isn't the right spot. Ideally we shouldn't really need to be adding any more of these application-specific workarounds anyways, but for older poorly-behaving applications they'll always be needed.

Maybe version could be done with another level of keys, like 'Vulkan\ApplicationName\%pApplicationName%\%applicationVersion%\Quirks’. This starts to get complicated though, maybe it should wait until a clear need arises. Plus, a version number check would often need to be ‘less-than some version number’, which is even more complicated.

> Another though I had on this (+CC Georg) was that we should probably invert WINEVULKAN_QUIRK_ADJUST_MAX_IMAGE_COUNT so that the default is zero, possibly rename to add a _DONT_.
> 
> Thinking further on quirk defaults, I wonder if they should be handled as binary, or if trinary is a better fit (unset, forced on, forced off). Right now the quirks bits have us choosing some reasonable default for all applications, and then either using the prospective regkey to enable a quirk, or having winevulkan pick up on some runtime-specific state to determine if a quirk needs to be enabled or not. Although if we're thinking down that route, maybe it would be easiest to have each quirk split to a separate regkey-value, and then track those settings in a structure.

Yeah I think you would want all quirks to default to zero, so if you add new quirks the existing per-app registry values won’t need to change.

Trinary or separate values are tempting, but it adds a lot of complexity vs just a bit mask. To me, I think it’s worth sticking with a single bit mask unless you can really see the use for something more complicated.

Lately I’ve been working on a similar system for per-app overriding the vendor/device ID returned in VkPhysicalDeviceProperties, and hitting some similar issues. These would both be living in vulkan.c, maybe some functions can be shared for all the registry code.


Brendan

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20200909/ff087a3a/attachment-0001.htm>