After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 793181 - Use pkg-config for Vulkan
Use pkg-config for Vulkan
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Scene Graph
3.93.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2018-02-05 11:06 UTC by Daniel Stone
Modified: 2018-02-05 14:23 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
meson: Use pkg-config for Vulkan dep (2.15 KB, patch)
2018-02-05 11:18 UTC, Daniel Stone
needs-work Details | Review
build: Use pkg-config to find Vulkan (2.71 KB, patch)
2018-02-05 11:43 UTC, Emmanuele Bassi (:ebassi)
none Details | Review
build: Use pkg-config to find Vulkan (2.93 KB, patch)
2018-02-05 14:03 UTC, Emmanuele Bassi (:ebassi)
committed Details | Review

Description Daniel Stone 2018-02-05 11:06:25 UTC
As GL predates pkg-config and had no sensible single provider until GLVND, depending on pkg-config for it can be ropey as not everyone provides.

On the other hand, Vulkan provides pkg-config via the official loader. Use pkg-config for Vulkan where available, with the old-school lib/cflag lookup as a last resort.

I haven't tested this on Windows.
Comment 1 Daniel Stone 2018-02-05 11:18:45 UTC
Created attachment 367901 [details] [review]
meson: Use pkg-config for Vulkan dep

The standard Vulkan loader/SDK ships a pkg-config file with it. Use that
instead of relying on it already being in the system include/library
paths.
Comment 2 Emmanuele Bassi (:ebassi) 2018-02-05 11:40:34 UTC
Review of attachment 367901 [details] [review]:

::: meson.build
@@ +581,3 @@
+    if vulkan_dep.found() and cc.has_function('vkCreateInstance', dependencies: vulkan_dep) and cc.has_header('vulkan/vulkan.h')
+      have_vulkan = true
+    endif

This is missing the bit to add `-lvulkan-1` to the pkg-config file on Windows…

@@ +584,3 @@
+  else
+    vulkan_dep = dependency('vulkan', required: false)
+    have_vulkan = vulkan_dep.found()

And here we need to add `vulkan` to the list of dependencies on the pkg-config file on Linux.

@@ +585,3 @@
+    vulkan_dep = dependency('vulkan', required: false)
+    have_vulkan = vulkan_dep.found()
+  endif

I'd restructure the whole Vulkan check.
Comment 3 Emmanuele Bassi (:ebassi) 2018-02-05 11:43:55 UTC
Created attachment 367903 [details] [review]
build: Use pkg-config to find Vulkan

The standard Vulkan SDK ships with a pkg-config file, like a modern
library should.

We should fall back to finding the library and header only for platforms
where pkg-config is not really a thing.
Comment 4 Daniel Stone 2018-02-05 12:19:52 UTC
Good catch, and thanks for the fixup. I'm happy with that, but won't this break passing '-Dvulkan=true' on Windows? dependency(required: true) means we'll never hit the 'if !vulkan_dep.found()' path.
Comment 5 Emmanuele Bassi (:ebassi) 2018-02-05 12:21:36 UTC
You're right; let me rejig it a little bit.
Comment 6 Emmanuele Bassi (:ebassi) 2018-02-05 14:03:26 UTC
Created attachment 367912 [details] [review]
build: Use pkg-config to find Vulkan

The standard Vulkan SDK ships with a pkg-config file, like a modern
library should.

We should fall back to finding the library and header only for platforms
where pkg-config is not really a thing.

Based on a patch by: Daniel Stone <daniels@collabora.com>
Comment 7 Daniel Stone 2018-02-05 14:08:28 UTC
Review of attachment 367912 [details] [review]:

No idea if the comment about pkg-config is relevant at all, let alone worth the workaround. +1 from me either way - thanks!

::: meson.build
@@ -625,2 +633,4 @@
 endif
 
+if vulkan_pkg_found
+  gdk_packages += 'vulkan'

This now enforces that if someone uses GTK+ via pkg-config, they must also have a vulkan.pc. I don't think that's too big a deal though, right?
Comment 8 Emmanuele Bassi (:ebassi) 2018-02-05 14:18:18 UTC
(In reply to Daniel Stone from comment #7)
> Review of attachment 367912 [details] [review] [review]:
> 
> No idea if the comment about pkg-config is relevant at all, let alone worth
> the workaround. +1 from me either way - thanks!

I prefer to leave it for things like building GTK on Windows with MSYS2 or other Unix-transplants toolchains.
 
> ::: meson.build
> @@ -625,2 +633,4 @@
>  endif
>  
> +if vulkan_pkg_found
> +  gdk_packages += 'vulkan'
> 
> This now enforces that if someone uses GTK+ via pkg-config, they must also
> have a vulkan.pc. I don't think that's too big a deal though, right?

Yeah, I think it's perfectly fair.
Comment 9 Emmanuele Bassi (:ebassi) 2018-02-05 14:23:19 UTC
Attachment 367912 [details] pushed as 3b0e672 - build: Use pkg-config to find Vulkan