[Mesa-dev] [PATCH] vulkan: simplify VK_USE_PLATFORM_*_KHR handling

[Emil Velikov]

On 9 March 2018 at 16:53, Eric Engestrom <eric.engestrom at imgtec.com> wrote:
> On Friday, 2018-03-09 15:24:18 +0000, Emil Velikov wrote:
>> From: Emil Velikov <emil.velikov at collabora.com>
>>
>> Instead of having multiple guards littered through the code, simply
>> introduce static inline no-op functions when the respective macros are
>> not set.
>>
>> Inspired by the same convention from the kernel.
>
> Oh right, I didn't get what you meant at first, but this makes sense. As
> the number of supported platform increases, this will be much cleaner.
>
>>
>> Cc: Eric Engestrom <eric.engestrom at imgtec.com>
>> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
>> ---
>> Here is an example of my earlier suggestion.
>> NOTE: not even compile tested!
>> ---
>>  src/vulkan/wsi/wsi_common.c         |  8 --------
>>  src/vulkan/wsi/wsi_common_private.h | 27 +++++++++++++++++++++++++++
>>  2 files changed, 27 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/vulkan/wsi/wsi_common.c b/src/vulkan/wsi/wsi_common.c
>> index fe262b4968d..c91075325b9 100644
>> --- a/src/vulkan/wsi/wsi_common.c
>> +++ b/src/vulkan/wsi/wsi_common.c
>> @@ -79,17 +79,13 @@ wsi_device_init(struct wsi_device *wsi,
>>     WSI_GET_CB(WaitForFences);
>>  #undef WSI_GET_CB
>>
>> -#ifdef VK_USE_PLATFORM_XCB_KHR
>>     result = wsi_x11_init_wsi(wsi, alloc);
>>     if (result != VK_SUCCESS)
>>        goto fail;
>> -#endif
>>
>> -#ifdef VK_USE_PLATFORM_WAYLAND_KHR
>>     result = wsi_wl_init_wsi(wsi, alloc, pdevice);
>>     if (result != VK_SUCCESS)
>>        goto fail;
>> -#endif
>>
>>     return VK_SUCCESS;
>>
>> @@ -102,12 +98,8 @@ void
>>  wsi_device_finish(struct wsi_device *wsi,
>>                    const VkAllocationCallbacks *alloc)
>>  {
>> -#ifdef VK_USE_PLATFORM_WAYLAND_KHR
>>     wsi_wl_finish_wsi(wsi, alloc);
>> -#endif
>> -#ifdef VK_USE_PLATFORM_XCB_KHR
>>     wsi_x11_finish_wsi(wsi, alloc);
>> -#endif
>>  }
>>
>>  VkResult
>> diff --git a/src/vulkan/wsi/wsi_common_private.h b/src/vulkan/wsi/wsi_common_private.h
>> index b608119b969..501bbf0498e 100644
>> --- a/src/vulkan/wsi/wsi_common_private.h
>> +++ b/src/vulkan/wsi/wsi_common_private.h
>> @@ -129,16 +129,43 @@ struct wsi_interface {
>>                                  struct wsi_swapchain **swapchain);
>>  };
>>
>> +#ifdef VK_USE_PLATFORM_XCB_KHR
>>  VkResult wsi_x11_init_wsi(struct wsi_device *wsi_device,
>>                            const VkAllocationCallbacks *alloc);
>>  void wsi_x11_finish_wsi(struct wsi_device *wsi_device,
>>                          const VkAllocationCallbacks *alloc);
>> +#else
>> +static inline
>> +VkResult wsi_x11_init_wsi(struct wsi_device *wsi_device,
>> +                          const VkAllocationCallbacks *alloc)
>> +{
>> +   return VK_SUCCESS;
>> +}
>> +
>> +static inline
>> +void wsi_x11_finish_wsi(struct wsi_device *wsi_device,
>> +                        const VkAllocationCallbacks *alloc) { }
>
> Nit: can we have all the `{` and `}` on their own lines, just like the
> `init` stub above?
>
Ack will do.

> Provided you compile-test :P
> Reviewed-by: Eric Engestrom <eric.engestrom at imgtec.com>
>
Thanks.

Can we get an ack/nack from the Vulkan devs? I don't want to be called
out for sneaking something behind their back.
Despite how trivial ;-)

-Emil