vulkan-go / vulkan

Vulkan API bindings for Go programming language
MIT License
742 stars 55 forks source link

Please provide Stuffs needed cube to run on wayland linux #64

Closed neurlang closed 1 year ago

neurlang commented 1 year ago

Recently I got running the go-wayland-cube demo on Nvidia on Wayland Ubuntu Linux. This fixed the last big gpu vendor (Intel, Amd worked fine).

I've used egl-wayland-1.1.11, and driver was 515.65.01

The things missing from your awesome vulkan package to make this work are available in the vulkan/ subdir of my repo github.com/neurlang/wayland/tree/master/vulkan

Please consider adding those things to your repo, because it's really vulkan related and should be provided by you. Thanks.

xlab commented 1 year ago

@neurlang mind creating a simple PR pls? :) I can quickly merge if it looks fine

neurlang commented 1 year ago

Please don't merge yet, Just take a look at the fork and tell me what you think about the changes https://github.com/neurlang/vulkan

Big problem is there are now two vkCreateInstance, the old one CreateInstance and a new one I renamed as WaylandCreateInstance.

The behavior of the old one needs to support both cases.

neurlang commented 1 year ago

I'd say the old CreateInstance should not be auto generated anymore, and it should iterate InstanceCreateInfo.PpEnabledExtensionNames to see if there's string "VK_KHR_wayland_surface\000". When that string is found, it could trigger the wayland behavior.

But I'm no expert on the matter.

xlab commented 1 year ago

Hi, I think there is some confusion between vk.CreateInstance and vk.CreateWindowSurface, the latter is platform-specific, and the former is not. For instance, in this case:

Screenshot 2022-09-11 at 11 15 17

I don't see that create instance function has anything specific to Wayland.

So, the platform-dependent workflow is like this: 1) get capabilities, check for surface type 2) use platform-specific surface constructor

Within VK package I am providing some platform-dependent code: https://github.com/vulkan-go/vulkan/blob/master/vulkan_android.go#L31

I think the approach of having the support of Wayland is following: 1) Generate the platform specific surface creator by setting VK_USE_PLATFORM_WAYLAND_KHR 2) Make a standalone file that uses that generated code, but adds some tags and conditions, so the file is being used only if Go compiles with -tags wayland on Linux. 3) Make a PR to this repo.

xlab commented 1 year ago

Also, there is a high-level abstraction over platform init that I made: https://github.com/vulkan-go/asche

Specifically, your App has to implement a single interface, for example, there is an Android app that creates own surface: https://github.com/vulkan-go/demos/blob/7232238edeba325ac4f953a9e77feaf18d4b414e/vulkancube/vulkancube_android/main.go#L24-L25 (using the platform-dependent method)

So Asche app is platform-agnostic, but your specific app that implements the interface can initialize platform-dependent surfaces.

neurlang commented 1 year ago

Yeah I didn't do anything platform specific in vkCreateInstance. In my case the problem was that I didn't call vk.Init() after vk.SetDefaultGetInstanceProcAddr() Now after fix, I removed the my version of vkCreateInstance, used yours and cube still works.

neurlang commented 1 year ago

What remains needed is the following:

Optionally also these two vars, they are difficult to make in go: VkPipeline nilPipeline = (VkPipeline) { 0 }; VkPipelineCache nilPipelineCache = (VkPipelineCache) { VK_NULL_HANDLE };

var NilPipeline = (Pipeline)(unsafe.Pointer(&C.nilPipeline)) var NilPipelineCache = (PipelineCache)(unsafe.Pointer(&C.nilPipelineCache))

neurlang commented 1 year ago

Done.

vkGetPhysicalDeviceFeatures2 also isn't strictly needed, and will be added by other's effort.

Pls have a look: https://github.com/vulkan-go/vulkan/pull/65

neurlang commented 1 year ago

Any feedback for the PR? Any possible problems?

Is the approach of setting the build define VK_USE_PLATFORM_ANDROID_KHR using -tags wayland ok with you?

Users who don't use this tag will be unaffected by this merge.

And vkGetPhysicalDeviceFeatures2 can be done later by upgrading to newer vulkan.

xlab commented 1 year ago

Everything looks fine, I merged! :)