From c8b4a00482981cef643bd73860a554d84f28e1de Mon Sep 17 00:00:00 2001
From: Jakob Bornecrantz <jakob@collabora.com>
Date: Wed, 15 Apr 2020 17:37:34 +0100
Subject: [PATCH] comp: Delay swapchain destruction until a safe time, like end
 frame.

---
 doc/changes/compositor/mr.278.md          |  1 +
 src/xrt/compositor/main/comp_compositor.c | 20 +++++++++++++++
 src/xrt/compositor/main/comp_compositor.h | 30 ++++++++++++++++++++++-
 src/xrt/compositor/main/comp_swapchain.c  | 30 +++++++++++++++++------
 4 files changed, 73 insertions(+), 8 deletions(-)
 create mode 100644 doc/changes/compositor/mr.278.md

diff --git a/doc/changes/compositor/mr.278.md b/doc/changes/compositor/mr.278.md
new file mode 100644
index 000000000..8f63c9d70
--- /dev/null
+++ b/doc/changes/compositor/mr.278.md
@@ -0,0 +1 @@
+main: Delay the destruction of swapchains until a time where it is safe, this allows swapchains to be destroyed from other threads.
diff --git a/src/xrt/compositor/main/comp_compositor.c b/src/xrt/compositor/main/comp_compositor.c
index a32823d84..96cc1edba 100644
--- a/src/xrt/compositor/main/comp_compositor.c
+++ b/src/xrt/compositor/main/comp_compositor.c
@@ -73,6 +73,9 @@ compositor_destroy(struct xrt_compositor *xc)
 
 	COMP_DEBUG(c, "DESTROY");
 
+	// Make sure we don't have anything to destroy.
+	comp_compositor_garbage_collect(c);
+
 	if (c->r) {
 		comp_renderer_destroy(c->r);
 		c->r = NULL;
@@ -105,6 +108,8 @@ compositor_destroy(struct xrt_compositor *xc)
 		free(c->compositor_frame_times.debug_var);
 	}
 
+	u_threading_stack_fini(&c->threading.destroy_swapchains);
+
 	free(c);
 }
 
@@ -305,6 +310,9 @@ compositor_end_frame(struct xrt_compositor *xc,
 	//! @todo do a time-weighted average or something.
 	c->expected_app_duration_ns =
 	    c->app_profiling.last_end - c->app_profiling.last_begin;
+
+	// Now is a good point to garbage collect.
+	comp_compositor_garbage_collect(c);
 }
 
 
@@ -806,6 +814,8 @@ xrt_gfx_provider_create_fd(struct xrt_device *xdev, bool flip_y)
 	c->base.base.destroy = compositor_destroy;
 	c->xdev = xdev;
 
+	u_threading_stack_init(&c->threading.destroy_swapchains);
+
 	COMP_DEBUG(c, "Doing init %p", (void *)c);
 
 	// Init the settings to default.
@@ -868,3 +878,13 @@ xrt_gfx_provider_create_fd(struct xrt_device *xdev, bool flip_y)
 
 	return &c->base;
 }
+
+void
+comp_compositor_garbage_collect(struct comp_compositor *c)
+{
+	struct comp_swapchain *sc;
+
+	while ((sc = u_threading_stack_pop(&c->threading.destroy_swapchains))) {
+		comp_swapchain_really_destroy(sc);
+	}
+}
diff --git a/src/xrt/compositor/main/comp_compositor.h b/src/xrt/compositor/main/comp_compositor.h
index 5f7b75fde..45a917954 100644
--- a/src/xrt/compositor/main/comp_compositor.h
+++ b/src/xrt/compositor/main/comp_compositor.h
@@ -10,11 +10,14 @@
 
 #pragma once
 
+#include "xrt/xrt_gfx_vk.h"
+
+#include "util/u_threading.h"
+
 #include "main/comp_settings.h"
 #include "main/comp_window.h"
 #include "main/comp_renderer.h"
 
-#include "xrt/xrt_gfx_vk.h"
 
 #ifdef __cplusplus
 extern "C" {
@@ -143,6 +146,12 @@ struct comp_compositor
 		uint32_t width;
 		uint32_t height;
 	} current;
+
+	struct
+	{
+		//! Thread object for safely destroying swapchain.
+		struct u_threading_stack destroy_swapchains;
+	} threading;
 };
 
 
@@ -174,6 +183,15 @@ comp_compositor(struct xrt_compositor *xc)
 	return (struct comp_compositor *)xc;
 }
 
+/*!
+ * Do garbage collection, destroying any resources that has been scheduled for
+ * destruction from other threads.
+ *
+ * @ingroup comp_main
+ */
+void
+comp_compositor_garbage_collect(struct comp_compositor *c);
+
 /*!
  * A compositor function that is implemented in the swapchain code.
  *
@@ -191,6 +209,16 @@ comp_swapchain_create(struct xrt_compositor *xc,
                       uint32_t array_size,
                       uint32_t mip_count);
 
+/*!
+ * Swapchain destruct is delayed until it is safe to destroy them, this function
+ * does the actual destruction and is called from @ref
+ * comp_compositor_garbage_collect.
+ *
+ * @ingroup comp_main
+ */
+void
+comp_swapchain_really_destroy(struct comp_swapchain *sc);
+
 /*!
  * Free and destroy any initialized fields on the given image, safe to pass in
  * images that has one or all fields set to NULL.
diff --git a/src/xrt/compositor/main/comp_swapchain.c b/src/xrt/compositor/main/comp_swapchain.c
index 23b98fd1f..332350679 100644
--- a/src/xrt/compositor/main/comp_swapchain.c
+++ b/src/xrt/compositor/main/comp_swapchain.c
@@ -19,16 +19,10 @@ static void
 swapchain_destroy(struct xrt_swapchain *xsc)
 {
 	struct comp_swapchain *sc = comp_swapchain(xsc);
-	struct vk_bundle *vk = &sc->c->vk;
 
 	COMP_SPEW(sc->c, "DESTROY");
 
-	for (uint32_t i = 0; i < sc->base.base.num_images; i++) {
-		comp_swapchain_image_cleanup(vk, sc->base.base.array_size,
-		                             &sc->images[i]);
-	}
-
-	free(sc);
+	u_threading_stack_push(&sc->c->threading.destroy_swapchains, sc);
 }
 
 static bool
@@ -205,6 +199,13 @@ err_image:
 	return ret;
 }
 
+
+/*
+ *
+ * Exported functions.
+ *
+ */
+
 struct xrt_swapchain *
 comp_swapchain_create(struct xrt_compositor *xc,
                       enum xrt_swapchain_create_flags create,
@@ -334,3 +335,18 @@ comp_swapchain_image_cleanup(struct vk_bundle *vk,
 		image->memory = VK_NULL_HANDLE;
 	}
 }
+
+void
+comp_swapchain_really_destroy(struct comp_swapchain *sc)
+{
+	struct vk_bundle *vk = &sc->c->vk;
+
+	COMP_SPEW(sc->c, "REALLY DESTROY");
+
+	for (uint32_t i = 0; i < sc->base.base.num_images; i++) {
+		comp_swapchain_image_cleanup(vk, sc->base.base.array_size,
+		                             &sc->images[i]);
+	}
+
+	free(sc);
+}