[PATCH v10 [rebased] 2/4] vulkan-1/tests: Add tests for VK_KHR_external_memory_win32.

[Derek Lesho]

On 7/1/21 3:59 PM, Zebediah Figura (she/her) wrote:
>> +    argc = winetest_get_mainargs(&argv);
>> +    if (argc > 3 && !strcmp(argv[2], "resource"))
>> +    {
>> +        sscanf(argv[6], "%p", &handle);
>> +
>> +        import_handle_info.sType = 
>> VK_STRUCTURE_TYPE_IMPORT_MEMORY_WIN32_HANDLE_INFO_KHR;
>> +        import_handle_info.pNext = &dedicated_alloc_info;
>> +        import_handle_info.handleType = strcmp(argv[5], "kmt") ?
>> +            VK_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_WIN32_BIT_KHR :
>> + VK_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_WIN32_KMT_BIT_KHR;
>> +        import_handle_info.handle = handle;
>> +        import_handle_info.name = NULL;
>> +
>> +        alloc_info.pNext = &import_handle_info;
>> +
>> +        vr = vkAllocateMemory(vk_device, &alloc_info, NULL, 
>> &vk_memory);
>> +        ok(vr == VK_SUCCESS, "vkAllocateMemory failed, VkResult 
>> %d.\n", vr);
>> +
>> +        vkFreeMemory(vk_device, vk_memory, NULL);
>> +        vkDestroyBuffer(vk_device, vk_buffer, NULL);
>> +        vkDestroyDevice(vk_device, NULL);
>
> Any reason not to include the by-name tests here?
>
> For that matter, you could add a helper function to test both 
> in-process and cross-process import.

I don't think a helper function for this would be very helpful, the only 
part you could truly abstract away would be filling import_handle_info 
and import_handle_info, which isn't too much boilerplate to duplicate.
>
>> +static void test_unique_device(uint8_t driver_uuid[VK_UUID_SIZE], 
>> uint8_t device_uuid[VK_UUID_SIZE],
>> +        uint32_t extension_count, const char * const 
>> *enabled_extensions,
>> +        void (*test_func_instance)(VkInstance, VkPhysicalDevice), 
>> void (*test_func)(VkPhysicalDevice))
>
> Could we instead use for_each_device_instance(), and match the UUID in 
> the callback?

I added this in v11, with the one drawback being that we no longer 
ensure that there exists a physical device in the child process that 
matches.
>
>> +            if (test_func_instance)
>> +                test_func_instance(vk_instance, 
>> vk_physical_devices[i]);
>> +            else
>> +                test_func(vk_physical_devices[i]);
>
> This isn't new, but it strikes me as really awkward. Can't we just 
> pass a VkInstance to the callback and leave it unused?

Agreed, but yeah normalizing this will have to come into another patch, 
as I removed test_unique_device in v11.
>>   # Either internal extensions which aren't present on the win32 
>> platform which
>>   # winevulkan may nonetheless use, or extensions we want to generate 
>> headers for
>>   # but not expose to applications (useful for test commits)
>> -UNEXPOSED_EXTENSIONS = {}
>> +UNEXPOSED_EXTENSIONS = {
>> +    "VK_KHR_external_memory_win32",
>> +}
>>     # The Vulkan loader provides entry-points for core functionality 
>> and important
>>   # extensions. Based on vulkan-1.def this amounts to WSI extensions 
>> on 1.0.51.
>>
>
> This doesn't look like it belongs in this patch.


Ended up keeping this in one patch for v11.