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 <[email protected]>
gioui · Sep 23, 2021 · 94f7fa3 · 94f7fa3
1 parent 53fe2e5
commit 94f7fa3
Show file tree

Hide file tree

Showing 5 changed files with 34 additions and 33 deletions.
diff --git 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,32 +135,21 @@ func (c *vkContext) destroyImageViews() {
 	c.views = nil
 }
 
-func (c *vkContext) destroySurface() {
+func (c *vkContext) destroySwapchain() {
 	vk.DeviceWaitIdle(c.dev)
 
 	c.destroyImageViews()
 	if c.swchain != 0 {
 		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
@@ -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
@@ -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
@@ -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
@@ -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