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

[Eric Engestrom]

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.

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 :)

> +                     const VkAllocationCallbacks *alloc,
> +                     int display_fd)
> +{
> +   return VK_SUCCESS;
> +}
> +
> +static inline void
> +wsi_display_finish_wsi(struct wsi_device *wsi_device,
> +                       const VkAllocationCallbacks *alloc)
> +{
> +}
> +#endif
>  
>  #define WSI_DEFINE_NONDISP_HANDLE_CASTS(__wsi_type, __VkType)              \
>                                                                             \
> -- 
> 2.18.0
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev