Add and use ThreadPool (#63)

* Add Scene::Loader

- Remove now unused headers from scene.hpp.
- Move all loading logic into `Scene::Loader`.
- WIP: temp code with futures commented out.

* Add ThreadPool

- Use for loading scenes and decompressing images.
- Rename `Scene::Loader` to `Scene::GltfLoader`.

* Upload Vulkan resources using ThreadPool

* Add docs

* Increment load status inside async task

- More accurate status reporting.

* Robust thread count

- `ThreadPool`: Skip creating any threads if requested count or hardware concurrency is 0.
- `Engine`: special-case thread count being 0 or 1.

* Acquire image under mutex lock

- Not seeing any issues without it, but best to assume WSI can interfere with device queues.

* Expose force-threads as Cli Opt

* Display load thread count in frame stats

Author: karnkaul

lib/engine/include/facade/engine/engine.hpp

@@ -1,6 +1,7 @@
 #pragma once
 #include <facade/build_version.hpp>
 #include <facade/glfw/glfw.hpp>
+#include <facade/scene/load_status.hpp>
 #include <facade/scene/scene.hpp>
 #include <facade/util/time.hpp>
 #include <facade/util/unique_task.hpp>
@@ -20,6 +21,7 @@ struct EngineCreateInfo {
 	std::uint8_t desired_msaa{2};
 	bool auto_show{false};
 	Validation validation{Validation::eDefault};
+	std::optional<std::uint32_t> force_thread_count{};
 };
 
 ///
@@ -81,15 +83,19 @@ class Engine {
 	void request_stop();
 
 	///
-	/// \brief Load a GLTF scene asynchronously
+	/// \brief Load a GLTF scene asynchronously.
 	///
 	/// Subsequent requests will be rejected if one is in flight
 	///
 	bool load_async(std::string gltf_json_path, UniqueTask<void()> on_loaded = {});
 	///
-	/// \brief Obtain status of in-flight async load request (if active)
+	/// \brief Obtain status of in-flight async load request (if active).
 	///
 	LoadStatus load_status() const;
+	///
+	/// \brief Obtain the number of loading threads.
+	///
+	std::size_t load_thread_count() const;
 
 	glm::ivec2 window_position() const;
 	glm::uvec2 window_extent() const;

lib/engine/include/facade/engine/scene_renderer.hpp

@@ -1,6 +1,7 @@
 #pragma once
 #include <facade/render/renderer.hpp>
 #include <facade/scene/scene.hpp>
+#include <facade/vk/buffer.hpp>
 
 namespace facade {
 class SceneRenderer {

lib/engine/src/engine.cpp

@@ -7,16 +7,17 @@
 #include <facade/engine/scene_renderer.hpp>
 #include <facade/glfw/glfw_wsi.hpp>
 #include <facade/render/renderer.hpp>
+#include <facade/scene/gltf_loader.hpp>
 #include <facade/util/data_provider.hpp>
 #include <facade/util/env.hpp>
 #include <facade/util/error.hpp>
 #include <facade/util/logger.hpp>
+#include <facade/util/thread_pool.hpp>
 #include <facade/vk/cmd.hpp>
 #include <facade/vk/vk.hpp>
 #include <glm/gtc/color_space.hpp>
 #include <glm/mat4x4.hpp>
 #include <filesystem>
-#include <future>
 
 namespace facade {
 namespace fs = std::filesystem;
@@ -159,10 +160,10 @@ struct RenderWindow {
 		  renderer(gfx, this->window, gui.get(), Renderer::CreateInfo{command_buffers_v, msaa}), gui(std::move(gui)) {}
 };
 
-bool load_gltf(Scene& out_scene, char const* path, AtomicLoadStatus* out_status) {
+bool load_gltf(Scene& out_scene, char const* path, AtomicLoadStatus& out_status, ThreadPool* thread_pool) {
 	auto const provider = FileDataProvider::mount_parent_dir(path);
 	auto json = dj::Json::from_file(path);
-	return out_scene.load_gltf(json, provider, out_status);
+	return Scene::GltfLoader{out_scene, out_status}(json, provider, thread_pool);
 }
 
 template <typename T>
@@ -195,15 +196,17 @@ struct Engine::Impl {
 
 	std::uint8_t msaa;
 
+	ThreadPool thread_pool{};
 	std::mutex mutex{};
 
 	struct {
 		LoadRequest request{};
 		UniqueTask<void()> callback{};
 	} load{};
 
-	Impl(UniqueWin window, std::uint8_t msaa, bool validation)
-		: window(std::move(window), std::make_unique<DearImGui>(), msaa, validation), renderer(this->window.gfx), scene(this->window.gfx), msaa(msaa) {
+	Impl(UniqueWin window, std::uint8_t msaa, bool validation, std::optional<std::uint32_t> thread_count)
+		: window(std::move(window), std::make_unique<DearImGui>(), msaa, validation), renderer(this->window.gfx), scene(this->window.gfx), msaa(msaa),
+		  thread_pool(thread_count) {
 		s_instance = this;
 		load.request.status.reset();
 	}
@@ -225,7 +228,8 @@ bool Engine::is_instance_active() { return s_instance != nullptr; }
 
 Engine::Engine(CreateInfo const& info) noexcept(false) {
 	if (s_instance) { throw Error{"Engine: active instance exists and has not been destroyed"}; }
-	m_impl = std::make_unique<Impl>(make_window(info.extent, info.title), info.desired_msaa, determine_validation(info.validation));
+	if (info.force_thread_count) { logger::info("[Engine] Forcing load thread count: [{}]", *info.force_thread_count); }
+	m_impl = std::make_unique<Impl>(make_window(info.extent, info.title), info.desired_msaa, determine_validation(info.validation), info.force_thread_count);
 	if (info.auto_show) { show(true); }
 }
 
@@ -269,6 +273,18 @@ bool Engine::load_async(std::string gltf_json_path, UniqueTask<void()> on_loaded
 		logger::error("[Engine] Invalid GLTF JSON path: [{}]", gltf_json_path);
 		return false;
 	}
+
+	// ensure thread pool queue has at least one worker thread, else load on this thread
+	if (m_impl->thread_pool.thread_count() == 0) {
+		auto const start = time::since_start();
+		if (!load_gltf(m_impl->scene, gltf_json_path.c_str(), m_impl->load.request.status, nullptr)) {
+			logger::error("[Engine] Failed to load GLTF: [{}]", gltf_json_path);
+			return false;
+		}
+		logger::info("...GLTF [{}] loaded in [{:.2f}s]", env::to_filename(gltf_json_path), time::since_start() - start);
+		return true;
+	}
+
 	// shared state will need to be accessed, lock the mutex
 	auto lock = std::scoped_lock{m_impl->mutex};
 	if (m_impl->load.request.future.valid()) {
@@ -284,14 +300,16 @@ bool Engine::load_async(std::string gltf_json_path, UniqueTask<void()> on_loaded
 	m_impl->load.request.path = std::move(gltf_json_path);
 	m_impl->load.request.status.reset();
 	m_impl->load.request.start_time = time::since_start();
-	auto func = [path = m_impl->load.request.path, gfx = m_impl->window.gfx, status = &m_impl->load.request.status] {
+	// if thread pool queue has only one worker thread, can't dispatch tasks from within a task and then wait for them (deadlock)
+	auto* tp = m_impl->thread_pool.thread_count() > 1 ? &m_impl->thread_pool : nullptr;
+	auto func = [path = m_impl->load.request.path, gfx = m_impl->window.gfx, status = &m_impl->load.request.status, tp] {
 		auto scene = Scene{gfx};
-		if (!load_gltf(scene, path.c_str(), status)) { logger::error("[Engine] Failed to load GLTF: [{}]", path); }
+		if (!load_gltf(scene, path.c_str(), *status, tp)) { logger::error("[Engine] Failed to load GLTF: [{}]", path); }
 		// return the scene even on failure, it will be empty but valid
 		return scene;
 	};
 	// store future
-	m_impl->load.request.future = std::async(std::launch::async, func);
+	m_impl->load.request.future = m_impl->thread_pool.enqueue(func);
 	return true;
 }
 
@@ -301,6 +319,8 @@ LoadStatus Engine::load_status() const {
 	return {.stage = status.stage.load(), .total = status.total, .done = status.done};
 }
 
+std::size_t Engine::load_thread_count() const { return m_impl->thread_pool.thread_count(); }
+
 Scene& Engine::scene() const { return m_impl->scene; }
 GLFWwindow* Engine::window() const { return m_impl->window.window.get(); }
 Glfw::State const& Engine::state() const { return m_impl->window.window.get().state(); }

lib/render/src/renderer.cpp

@@ -179,7 +179,10 @@ bool Renderer::next_frame(std::span<vk::CommandBuffer> out) {
 
 	// acquire swapchain image
 	auto acquired = ImageView{};
-	if (m_impl->swapchain.acquire(m_impl->window.framebuffer_extent(), acquired, *frame.sync.draw) != vk::Result::eSuccess) { return false; }
+	{
+		auto lock = std::scoped_lock{m_impl->gfx.shared->mutex};
+		if (m_impl->swapchain.acquire(lock, m_impl->window.framebuffer_extent(), acquired, *frame.sync.draw) != vk::Result::eSuccess) { return false; }
+	}
 	m_impl->gfx.reset(*frame.sync.drawn);
 	m_impl->gfx.shared->defer_queue.next();
 

lib/scene/CMakeLists.txt

@@ -35,6 +35,7 @@ endif()
 target_sources(${PROJECT_NAME} PRIVATE
   include/${target_prefix}/scene/camera.hpp
   include/${target_prefix}/scene/fly_cam.hpp
+  include/${target_prefix}/scene/gltf_loader.hpp
   include/${target_prefix}/scene/id.hpp
   include/${target_prefix}/scene/lights.hpp
   include/${target_prefix}/scene/load_status.hpp
@@ -48,6 +49,7 @@ target_sources(${PROJECT_NAME} PRIVATE
   src/detail/gltf.hpp
 
   src/camera.cpp
+  src/gltf_loader.cpp
   src/material.cpp
   src/scene.cpp
   src/transform.cpp

lib/scene/include/facade/scene/gltf_loader.hpp

@@ -0,0 +1,53 @@
+#include <djson/json.hpp>
+#include <facade/scene/load_status.hpp>
+#include <facade/scene/scene.hpp>
+#include <atomic>
+
+namespace facade {
+class ThreadPool;
+
+///
+/// \brief Concurrent status for multi-threaded loading
+///
+struct AtomicLoadStatus {
+	std::atomic<LoadStage> stage{};
+	std::atomic<std::size_t> total{};
+	std::atomic<std::size_t> done{};
+
+	void reset() {
+		stage = LoadStage::eNone;
+		total = {};
+		done = {};
+	}
+};
+
+class Scene::GltfLoader {
+  public:
+	///
+	/// \brief Construct a GltfLoader.
+	/// \param out_scene The scene to load into
+	/// \param out_status AtomicLoadStatus to be updated as the scene is loaded
+	///
+	GltfLoader(Scene& out_scene, AtomicLoadStatus& out_status) : m_scene(out_scene), m_status(out_status) { m_status.reset(); }
+
+	///
+	/// \brief Load data from a GLTF file.
+	/// \param json The root JSON node for the GLTF asset
+	/// \param provider Data provider with the JSON parent directory mounted
+	/// \param thread_pool Optional pointer to thread pool to use for async loading of images and buffers
+	/// \returns true If successfully loaded
+	///
+	/// If the GLTF data fails to load, the scene data will remain unchanged.
+	/// This function purposely throws on fatal errors.
+	///
+	bool operator()(dj::Json const& json, DataProvider const& provider, ThreadPool* thread_pool = {}) noexcept(false);
+	///
+	/// \brief Obtain the load status.
+	///
+	AtomicLoadStatus const& load_status() const { return m_status; }
+
+  private:
+	Scene& m_scene;
+	AtomicLoadStatus& m_status;
+};
+} // namespace facade

lib/scene/include/facade/scene/load_status.hpp

@@ -9,19 +9,15 @@ namespace facade {
 enum class LoadStage : std::uint8_t {
 	eNone,
 	eParsingJson,
-	eLoadingImages,
-	eUploadingTextures,
-	eUploadingMeshes,
+	eUploadingResources,
 	eBuildingScenes,
 	eCOUNT_,
 };
 
 ///
 /// \brief String map for LoadStage.
 ///
-constexpr auto load_stage_str = EnumArray<LoadStage, std::string_view>{
-	"None", "Parsing JSON", "Loading Images", "Uploading Textures", "Uploading Meshes", "Building Scenes",
-};
+constexpr auto load_stage_str = EnumArray<LoadStage, std::string_view>{"None", "Parsing JSON", "Uploading Resources", "Building Scenes"};
 static_assert(std::size(load_stage_str.t) == static_cast<std::size_t>(LoadStage::eCOUNT_));
 
 struct LoadStatus {

lib/scene/include/facade/scene/scene.hpp

@@ -2,47 +2,23 @@
 #include <facade/scene/camera.hpp>
 #include <facade/scene/id.hpp>
 #include <facade/scene/lights.hpp>
-#include <facade/scene/load_status.hpp>
 #include <facade/scene/material.hpp>
 #include <facade/scene/mesh.hpp>
 #include <facade/scene/node.hpp>
 #include <facade/scene/node_data.hpp>
-#include <facade/util/enum_array.hpp>
 #include <facade/util/image.hpp>
 #include <facade/util/ptr.hpp>
 #include <facade/util/transform.hpp>
-#include <facade/vk/buffer.hpp>
 #include <facade/vk/pipeline.hpp>
 #include <facade/vk/static_mesh.hpp>
 #include <facade/vk/texture.hpp>
-#include <atomic>
 #include <memory>
-#include <optional>
 #include <span>
 #include <vector>
 
-namespace dj {
-class Json;
-}
-
 namespace facade {
 struct DataProvider;
 
-///
-/// \brief Concurrent status for multi-threaded loading
-///
-struct AtomicLoadStatus {
-	std::atomic<LoadStage> stage{};
-	std::atomic<std::size_t> total{};
-	std::atomic<std::size_t> done{};
-
-	void reset() {
-		stage = LoadStage::eNone;
-		total = {};
-		done = {};
-	}
-};
-
 ///
 /// \brief Immutable view of the resources stored in the scene.
 ///
@@ -64,6 +40,8 @@ struct SceneResources {
 class Scene {
   public:
 	using Resources = SceneResources;
+	// defined in loader.hpp
+	class GltfLoader;
 
 	///
 	/// \brief Represents a single GTLF scene.
@@ -85,18 +63,6 @@ class Scene {
 	///
 	explicit Scene(Gfx const& gfx);
 
-	///
-	/// \brief Load data from a GLTF file.
-	/// \param root The root JSON node for the GLTF asset
-	/// \param provider Data provider with the JSON parent directory mounted
-	/// \param out_status Optional pointer to AtomicLoadStatus to be updated by the Scene
-	/// \returns true If successfully loaded
-	///
-	/// If the GLTF data fails to load, the scene data will remain unchanged.
-	/// This function purposely throws on fatal errors.
-	///
-	bool load_gltf(dj::Json const& root, DataProvider const& provider, AtomicLoadStatus* out_status = {}) noexcept(false);
-
 	///
 	/// \brief Add a Camera.
 	/// \param camera Camera instance to add