From 94f7fa32186c76e9aa9fc5345beaa4068c3d0e30 Mon Sep 17 00:00:00 2001 From: Elias Naur Date: Thu, 23 Sep 2021 11:26:41 +0200 Subject: [PATCH] app: [Vulkan] keep VkSurfaceKHR ownership to platforms Before this change, it was unclear who owned the platform specific VkSurfaceKHR object, leading to a double-free in the error path for devices with no Vulkan support. This change moves the ownership to the platform specific code. Add vk.EnumeratePhysicalDevices while here (refactor was part of debugging of the double-free). Signed-off-by: Elias Naur --- app/vulkan.go | 26 ++++---------------------- app/vulkan_android.go | 11 ++++++++--- app/vulkan_wayland.go | 5 ++++- app/vulkan_x11.go | 5 ++++- internal/vk/vulkan.go | 20 ++++++++++++++------ 5 files changed, 34 insertions(+), 33 deletions(-) diff --git a/app/vulkan.go b/app/vulkan.go index 91d345f59..037af9c2b 100644 --- a/app/vulkan.go +++ b/app/vulkan.go @@ -17,7 +17,6 @@ import ( type vkContext struct { physDev vk.PhysicalDevice inst vk.Instance - surf vk.Surface dev vk.Device queueFam int queue vk.Queue @@ -35,36 +34,30 @@ type vkContext struct { func newVulkanContext(inst vk.Instance, surf vk.Surface) (*vkContext, error) { physDev, qFam, err := vk.ChoosePhysicalDevice(inst, surf) if err != nil { - vk.DestroySurface(inst, surf) return nil, err } dev, err := vk.CreateDeviceAndQueue(physDev, qFam, "VK_KHR_swapchain") if err != nil { - vk.DestroySurface(inst, surf) return nil, err } if err != nil { - vk.DestroySurface(inst, surf) vk.DestroyDevice(dev) return nil, err } acquireSem, err := vk.CreateSemaphore(dev) if err != nil { - vk.DestroySurface(inst, surf) vk.DestroyDevice(dev) return nil, err } presentSem, err := vk.CreateSemaphore(dev) if err != nil { vk.DestroySemaphore(dev, acquireSem) - vk.DestroySurface(inst, surf) vk.DestroyDevice(dev) return nil, err } c := &vkContext{ physDev: physDev, inst: inst, - surf: surf, dev: dev, queueFam: qFam, queue: vk.GetDeviceQueue(dev, qFam, 0), @@ -120,7 +113,7 @@ func mapErr(err error) error { func (c *vkContext) release() { vk.DeviceWaitIdle(c.dev) - c.destroySurface() + c.destroySwapchain() vk.DestroySemaphore(c.dev, c.acquireSem) vk.DestroySemaphore(c.dev, c.presentSem) vk.DestroyDevice(c.dev) @@ -142,7 +135,7 @@ func (c *vkContext) destroyImageViews() { c.views = nil } -func (c *vkContext) destroySurface() { +func (c *vkContext) destroySwapchain() { vk.DeviceWaitIdle(c.dev) c.destroyImageViews() @@ -150,24 +143,13 @@ func (c *vkContext) destroySurface() { vk.DestroySwapchain(c.dev, c.swchain) c.swchain = 0 } - if c.surf != 0 { - vk.DestroySurface(c.inst, c.surf) - c.surf = 0 - } -} - -func (c *vkContext) setSurface(surf vk.Surface) { - if c.surf != 0 { - panic("another surface is active") - } - c.surf = surf } -func (c *vkContext) refresh(width, height int) error { +func (c *vkContext) refresh(surf vk.Surface, width, height int) error { vk.DeviceWaitIdle(c.dev) c.destroyImageViews() - swchain, imgs, format, err := vk.CreateSwapchain(c.physDev, c.dev, c.surf, width, height, c.swchain) + swchain, imgs, format, err := vk.CreateSwapchain(c.physDev, c.dev, surf, width, height, c.swchain) if c.swchain != 0 { vk.DestroySwapchain(c.dev, c.swchain) c.swchain = 0 diff --git a/app/vulkan_android.go b/app/vulkan_android.go index a3f54fe6e..48df013f1 100644 --- a/app/vulkan_android.go +++ b/app/vulkan_android.go @@ -15,6 +15,7 @@ import ( type wlVkContext struct { win *window inst vk.Instance + surf vk.Surface ctx *vkContext } @@ -39,6 +40,7 @@ func init() { c := &wlVkContext{ win: w, inst: inst, + surf: surf, ctx: ctx, } return c, nil @@ -55,6 +57,7 @@ func (c *wlVkContext) API() gpu.API { func (c *wlVkContext) Release() { c.ctx.release() + vk.DestroySurface(c.inst, c.surf) vk.DestroyInstance(c.inst) *c = wlVkContext{} } @@ -71,11 +74,13 @@ func (c *wlVkContext) Unlock() {} func (c *wlVkContext) Refresh() error { win, w, h := c.win.nativeWindow() - c.ctx.destroySurface() + if c.surf != 0 { + c.ctx.destroySwapchain() + vk.DestroySurface(c.inst, c.surf) + } surf, err := vk.CreateAndroidSurface(c.inst, unsafe.Pointer(win)) if err != nil { return err } - c.ctx.setSurface(surf) - return c.ctx.refresh(w, h) + return c.ctx.refresh(surf, w, h) } diff --git a/app/vulkan_wayland.go b/app/vulkan_wayland.go index c142e99c5..7bf066280 100644 --- a/app/vulkan_wayland.go +++ b/app/vulkan_wayland.go @@ -17,6 +17,7 @@ import ( type wlVkContext struct { win *window inst vk.Instance + surf vk.Surface ctx *vkContext } @@ -42,6 +43,7 @@ func init() { c := &wlVkContext{ win: w, inst: inst, + surf: surf, ctx: ctx, } return c, nil @@ -58,6 +60,7 @@ func (c *wlVkContext) API() gpu.API { func (c *wlVkContext) Release() { c.ctx.release() + vk.DestroySurface(c.inst, c.surf) vk.DestroyInstance(c.inst) *c = wlVkContext{} } @@ -74,5 +77,5 @@ func (c *wlVkContext) Unlock() {} func (c *wlVkContext) Refresh() error { _, w, h := c.win.surface() - return c.ctx.refresh(w, h) + return c.ctx.refresh(c.surf, w, h) } diff --git a/app/vulkan_x11.go b/app/vulkan_x11.go index c1a5d75c3..cf9777401 100644 --- a/app/vulkan_x11.go +++ b/app/vulkan_x11.go @@ -17,6 +17,7 @@ import ( type x11VkContext struct { win *x11Window inst vk.Instance + surf vk.Surface ctx *vkContext } @@ -42,6 +43,7 @@ func init() { c := &x11VkContext{ win: w, inst: inst, + surf: surf, ctx: ctx, } return c, nil @@ -58,6 +60,7 @@ func (c *x11VkContext) API() gpu.API { func (c *x11VkContext) Release() { c.ctx.release() + vk.DestroySurface(c.inst, c.surf) vk.DestroyInstance(c.inst) *c = x11VkContext{} } @@ -74,5 +77,5 @@ func (c *x11VkContext) Unlock() {} func (c *x11VkContext) Refresh() error { _, w, h := c.win.window() - return c.ctx.refresh(w, h) + return c.ctx.refresh(c.surf, w, h) } diff --git a/internal/vk/vulkan.go b/internal/vk/vulkan.go index a6efbcb01..6ef59317a 100644 --- a/internal/vk/vulkan.go +++ b/internal/vk/vulkan.go @@ -816,12 +816,12 @@ func CreateInstance(exts ...string) (Instance, error) { return nil, err } inf := C.VkInstanceCreateInfo{ - sType: C.VK_STRUCTURE_TYPE_INSTANCE_CREATE_INFO, - enabledExtensionCount: C.uint32_t(len(exts)), + sType: C.VK_STRUCTURE_TYPE_INSTANCE_CREATE_INFO, } if len(exts) > 0 { cexts := mallocCStringArr(exts) defer freeCStringArr(cexts) + inf.enabledExtensionCount = C.uint32_t(len(exts)) inf.ppEnabledExtensionNames = &cexts[0] } var inst Instance @@ -861,17 +861,25 @@ func GetPhysicalDeviceQueueFamilyProperties(pd PhysicalDevice) []QueueFamilyProp return queues } -func ChoosePhysicalDevice(inst Instance, surf Surface) (PhysicalDevice, int, error) { +func EnumeratePhysicalDevices(inst Instance) ([]PhysicalDevice, error) { var count C.uint32_t if err := vkErr(C.vkEnumeratePhysicalDevices(funcs.vkEnumeratePhysicalDevices, inst, &count, nil)); err != nil { - return nil, 0, fmt.Errorf("vulkan: vkEnumeratePhysicalDevices: %w", err) + return nil, fmt.Errorf("vulkan: vkEnumeratePhysicalDevices: %w", err) } if count == 0 { - return nil, 0, errors.New("vulkan: no devices available") + return nil, nil } devs := make([]C.VkPhysicalDevice, count) if err := vkErr(C.vkEnumeratePhysicalDevices(funcs.vkEnumeratePhysicalDevices, inst, &count, &devs[0])); err != nil { - return nil, 0, fmt.Errorf("vulkan: vkEnumeratePhysicalDevices: %w", err) + return nil, fmt.Errorf("vulkan: vkEnumeratePhysicalDevices: %w", err) + } + return devs, nil +} + +func ChoosePhysicalDevice(inst Instance, surf Surface) (PhysicalDevice, int, error) { + devs, err := EnumeratePhysicalDevices(inst) + if err != nil { + return nil, 0, err } for _, pd := range devs { const caps = C.VK_QUEUE_GRAPHICS_BIT | C.VK_QUEUE_COMPUTE_BIT