[Mesa-dev] [PATCH 05/17] vulkan/wsi: Generate wayland protocol headers separately from EGL

[Emil Velikov]

On 8 March 2017 at 17:31, Jason Ekstrand <jason at jlekstrand.net> wrote:
> On Wed, Mar 8, 2017 at 9:15 AM, Emil Velikov <emil.l.velikov at gmail.com>
> wrote:
>>
>> From: Jason Ekstrand <jason.ekstrand at intel.com>
>>
>> Previously, we were depending on EGL for generating the headers and
>> providing the protocol symbols. However, since neither Vulkan driver
>> actually wants to link against EGL, this is kind of pointless. It also
>> creates a weird build dependency.
>>
>> v2 [Jason]
>>  - Add missing wsi/ prefix, MKDIR_GEN
>>
>> v3 [Emil Velikov]
>>  - include BUILT_SOURCES/generation rules outside of conditional
>>
>> Reviewed-by: Emil Velikov <emil.velikov at collabora.com>
>> ---
>>  src/amd/vulkan/Makefile.am          |  3 ---
>>  src/intel/vulkan/Makefile.am        |  7 -------
>>  src/vulkan/Makefile.am              | 20 ++++++++++++++++----
>>  src/vulkan/Makefile.sources         |  4 ++++
>>  src/vulkan/wsi/.gitignore           |  2 ++
>>  src/vulkan/wsi/wsi_common_wayland.c |  2 +-
>>  6 files changed, 23 insertions(+), 15 deletions(-)
>>  create mode 100644 src/vulkan/wsi/.gitignore
>>
>> diff --git a/src/amd/vulkan/Makefile.am b/src/amd/vulkan/Makefile.am
>> index 3379a3d6a8..fbd9f5a030 100644
>> --- a/src/amd/vulkan/Makefile.am
>> +++ b/src/amd/vulkan/Makefile.am
>> @@ -77,15 +77,12 @@ endif
>>
>>  if HAVE_PLATFORM_WAYLAND
>>  AM_CPPFLAGS += \
>> -       -I$(top_builddir)/src/egl/wayland/wayland-drm \
>> -       -I$(top_srcdir)/src/egl/wayland/wayland-drm \
>>         $(WAYLAND_CFLAGS) \
>>         -DVK_USE_PLATFORM_WAYLAND_KHR
>>
>>  VULKAN_SOURCES += $(VULKAN_WSI_WAYLAND_FILES)
>>
>>  VULKAN_LIB_DEPS += \
>> -       $(top_builddir)/src/egl/wayland/wayland-drm/libwayland-drm.la \
>>         $(WAYLAND_LIBS)
>>  endif
>>
>> diff --git a/src/intel/vulkan/Makefile.am b/src/intel/vulkan/Makefile.am
>> index 3cd01e4d02..6c902b52e1 100644
>> --- a/src/intel/vulkan/Makefile.am
>> +++ b/src/intel/vulkan/Makefile.am
>> @@ -63,12 +63,6 @@ AM_CPPFLAGS = \
>>         -I$(top_builddir)/src/intel \
>>         -I$(top_srcdir)/src/intel
>>
>> -if HAVE_PLATFORM_WAYLAND
>> -AM_CPPFLAGS += \
>> -       -I$(top_builddir)/src/egl/wayland/wayland-drm \
>> -       -I$(top_srcdir)/src/egl/wayland/wayland-drm
>> -endif
>> -
>>  AM_CPPFLAGS += \
>>         $(LIBDRM_CFLAGS) \
>>         $(INTEL_CFLAGS) \
>> @@ -119,7 +113,6 @@ AM_CPPFLAGS += \
>>  VULKAN_SOURCES += $(VULKAN_WSI_WAYLAND_FILES)
>>
>>  VULKAN_LIB_DEPS += \
>> -       $(top_builddir)/src/egl/wayland/wayland-drm/libwayland-drm.la \
>>         $(WAYLAND_LIBS)
>>  endif
>>
>> diff --git a/src/vulkan/Makefile.am b/src/vulkan/Makefile.am
>> index f7aca8e937..f8d6fae40b 100644
>> --- a/src/vulkan/Makefile.am
>> +++ b/src/vulkan/Makefile.am
>> @@ -48,17 +48,29 @@ VULKAN_WSI_SOURCES += $(VULKAN_WSI_X11_FILES)
>>  VULKAN_LIB_DEPS += $(XCB_DRI3_LIBS) -lX11-xcb
>>  endif
>>
>> +BUILT_SOURCES += $(VULKAN_WSI_WAYLAND_GENERATED_FILES)
>> +CLEANFILES = $(BUILT_SOURCES)
>> +
>> +WL_DRM_XML = $(top_srcdir)/src/egl/wayland/wayland-drm/wayland-drm.xml
>> +
>> +wsi/wayland-drm-protocol.c : $(WL_DRM_XML)
>> +       $(MKDIR_GEN)
>> +       $(AM_V_GEN)$(WAYLAND_SCANNER) code < $< > $@
>> +
>> +wsi/wayland-drm-client-protocol.h : $(WL_DRM_XML)
>> +       $(MKDIR_GEN)
>> +       $(AM_V_GEN)$(WAYLAND_SCANNER) client-header < $< > $@
>
>
> Is this why you replace WAYLAND_SCANNER with ":" when it doesn't exist?  My
> understanding of the two options (in or outside the if) are as follows:
>
> If we put it inside the if then a reconfigure can cause it to not "make
> clean" properly.
>
> If we put it outside the if, then we have to be able to run the above two
> rules without wayland-scanner
>
> Is that a good sum-up?  If so, then replacing it with ":" seems reasonable
> assuming that actually works. :-)
>
Yes that's roughly it. Might need to double-check how reconfigure/make
will behave as we toggle the wayland switch w/o make clean... then
again, not sure if we should bother supporting that ?

-Emil