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

[Emil Velikov]

On 9 August 2018 at 15:46, Eric Engestrom <eric.engestrom at intel.com> wrote:
> On Tuesday, 2018-08-07 18:17:09 +0100, 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.
>>
>> v2: Also handle PLATFORM_DISPLAY
>>
>> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
>> Reviewed-by: Eric Engestrom <eric.engestrom at imgtec.com> (v1)
>> ---
>>  src/vulkan/wsi/wsi_common.c         | 12 --------
>>  src/vulkan/wsi/wsi_common_private.h | 47 +++++++++++++++++++++++++++++
>>  2 files changed, 47 insertions(+), 12 deletions(-)
>>
>> diff --git a/src/vulkan/wsi/wsi_common.c b/src/vulkan/wsi/wsi_common.c
>> index f2d90a6bba2..d2ba7871a1d 100644
>> --- a/src/vulkan/wsi/wsi_common.c
>> +++ b/src/vulkan/wsi/wsi_common.c
>> @@ -80,23 +80,17 @@ 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
>>
>> -#ifdef VK_USE_PLATFORM_DISPLAY_KHR
>>     result = wsi_display_init_wsi(wsi, alloc, display_fd);
>>     if (result != VK_SUCCESS)
>>        goto fail;
>> -#endif
>>
>>     return VK_SUCCESS;
>>
>> @@ -109,15 +103,9 @@ void
>>  wsi_device_finish(struct wsi_device *wsi,
>>                    const VkAllocationCallbacks *alloc)
>>  {
>> -#ifdef VK_USE_PLATFORM_DISPLAY_KHR
>>     wsi_display_finish_wsi(wsi, alloc);
>> -#endif
>> -#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 9f2aacd6560..7dc1554e38d 100644
>> --- a/src/vulkan/wsi/wsi_common_private.h
>> +++ b/src/vulkan/wsi/wsi_common_private.h
>> @@ -128,17 +128,49 @@ 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)
>> +{
>> +}
>> +#endif
>> +
>> +#ifdef VK_USE_PLATFORM_WAYLAND_KHR
>>  VkResult wsi_wl_init_wsi(struct wsi_device *wsi_device,
>>                           const VkAllocationCallbacks *alloc,
>>                           VkPhysicalDevice physical_device);
>>  void wsi_wl_finish_wsi(struct wsi_device *wsi_device,
>>                         const VkAllocationCallbacks *alloc);
>> +#else
>> +static inline
>> +VkResult wsi_wl_init_wsi(struct wsi_device *wsi_device,
>> +                         const VkAllocationCallbacks *alloc,
>> +                         VkPhysicalDevice physical_device)
>> +{
>> +   return VK_SUCCESS;
>> +}
>>
>> +static inline
>> +void wsi_wl_finish_wsi(struct wsi_device *wsi_device,
>> +                       const VkAllocationCallbacks *alloc)
>> +{
>> +}
>> +#endif
>>
>> +#ifdef VK_USE_PLATFORM_DISPLAY_KHR
>>  VkResult
>>  wsi_display_init_wsi(struct wsi_device *wsi_device,
>>                       const VkAllocationCallbacks *alloc,
>> @@ -147,6 +179,21 @@ wsi_display_init_wsi(struct wsi_device *wsi_device,
>>  void
>>  wsi_display_finish_wsi(struct wsi_device *wsi_device,
>>                         const VkAllocationCallbacks *alloc);
>> +#else
>> +static inline VkResult
>> +wsi_display_init_wsi(struct wsi_device *wsi_device,
>
> To be consistent, `VkResult` (and `void` in the finish one below) should
> be on the same line as the function name.
>
Just following existing wsi_display_init_wsi declaration.

> That said, this is actually much more code and more complicated than the
> current state, and let's be honest: nobody will add another wsi_device_init()
> or anything else that would add a second caller for any of these, so
> I actually retract my r-b: I think it's better as it currently is :)
>
Ack. It is fairly bike-sheddy solution, so I'm glad it's settled
sooner than later ;-)

Thanks
Emil