Refactor SceneResources (#67)

* Refactor SceneResources

- Make `Material` a value type.
- Enable choosing between Lit and Unlit materials on Add.
- Separate persistent `InspectData`.

* Move ResourceArray to its own header

 - Add docs.

Author: karnkaul

lib/engine/CMakeLists.txt

@@ -43,6 +43,7 @@ target_sources(${PROJECT_NAME} PRIVATE
   include/${target_prefix}/engine/editor/browse_file.hpp
   include/${target_prefix}/engine/editor/common.hpp
   include/${target_prefix}/engine/editor/drag_drop_id.hpp
+  include/${target_prefix}/engine/editor/inspect_data.hpp
   include/${target_prefix}/engine/editor/inspector.hpp
   include/${target_prefix}/engine/editor/log.hpp
   include/${target_prefix}/engine/editor/reflector.hpp

lib/engine/include/facade/engine/editor/inspect_data.hpp

@@ -0,0 +1,9 @@
+#pragma once
+#include <string>
+
+namespace facade::editor {
+struct InspectData {
+	std::string input_buffer{};
+	int material_type{};
+};
+} // namespace facade::editor

lib/engine/include/facade/engine/editor/inspector.hpp

@@ -1,5 +1,6 @@
 #pragma once
 #include <facade/engine/editor/common.hpp>
+#include <facade/engine/editor/inspect_data.hpp>
 #include <facade/scene/scene.hpp>
 
 namespace facade::editor {
@@ -20,7 +21,7 @@ class Inspector {
 
 	NotClosed<Window> m_target;
 	Scene& m_scene;
-	SceneResourcesMut m_resources;
+	SceneResources& m_resources;
 };
 
 ///
@@ -67,15 +68,17 @@ class ResourceInspector : public Inspector {
 ///
 class SceneInspector : public Inspector {
   public:
+	using Data = InspectData;
+
 	SceneInspector(NotClosed<Window> target, Scene& out_scene) : Inspector(target, out_scene) {}
 
 	///
 	/// \brief View/edit all resources.
-	/// \param out_name_buf Persistent buffer for popups
+	/// \param out_data Persistent data for popups
 	///
 	/// Uses ResourceInspector.
 	///
-	void resources(std::string& out_name_buf) const;
+	void resources(Data& out_data) const;
 	///
 	/// \brief Inspect the Scene's camera.
 	///

lib/engine/src/editor/inspector.cpp

@@ -26,7 +26,7 @@ void edit_material(NotClosed<Window> target, SceneResources const& resources, Li
 	make_id_slot(lit.emissive, "Emissive", name.c_str(), {true});
 }
 
-void edit_material(SceneResourcesMut resources, UnlitMaterial& unlit) {
+void edit_material(SceneResources const& resources, UnlitMaterial& unlit) {
 	auto name = FixedString<128>{};
 	if (unlit.texture) { name = FixedString<128>{"{} ({})", resources.textures[*unlit.texture].name(), *unlit.texture}; }
 	make_id_slot(unlit.texture, "Texture", name.c_str(), {true});
@@ -57,12 +57,13 @@ void ResourceInspector::view(StaticMesh const& mesh, Id<StaticMesh> id) const {
 }
 
 void ResourceInspector::edit(Material& out_material, Id<Material> id) const {
-	auto const name = FixedString<128>{"{} ({})", out_material.name, id};
+	auto const name = FixedString<128>{"{} ({})", out_material.name(), id};
 	auto tn = TreeNode{name.c_str()};
 	drag_payload(id, name.c_str());
 	if (tn) {
-		if (auto* lit = dynamic_cast<LitMaterial*>(&out_material)) { return edit_material(m_target, m_resources, *lit); }
-		if (auto* unlit = dynamic_cast<UnlitMaterial*>(&out_material)) { return edit_material(m_resources, *unlit); }
+		auto& mat = out_material.base();
+		if (auto* lit = dynamic_cast<LitMaterial*>(&mat)) { return edit_material(m_target, m_resources, *lit); }
+		if (auto* unlit = dynamic_cast<UnlitMaterial*>(&mat)) { return edit_material(m_resources, *unlit); }
 	}
 }
 
@@ -78,7 +79,7 @@ void ResourceInspector::edit(Mesh& out_mesh, Id<Mesh> id) const {
 				make_id_slot(primitive.static_mesh, "Static Mesh", name.c_str());
 
 				name = {};
-				if (primitive.material) { name = FixedString<128>{"{} ({})", m_resources.materials[*primitive.material]->name, *primitive.material}; }
+				if (primitive.material) { name = FixedString<128>{"{} ({})", m_resources.materials[*primitive.material].name(), *primitive.material}; }
 				make_id_slot(primitive.material, "Material", name.c_str(), {true});
 
 				if (small_button_red("x###remove_primitive")) { to_erase = index; }
@@ -95,46 +96,54 @@ void ResourceInspector::edit(Mesh& out_mesh, Id<Mesh> id) const {
 	}
 }
 
-void SceneInspector::resources(std::string& out_name_buf) const {
+void SceneInspector::resources(Data& out_data) const {
 	auto const ri = ResourceInspector{m_target, m_scene};
 	static constexpr auto flags_v = ImGuiTreeNodeFlags_Framed;
 	bool add_material{}, add_mesh{};
 	if (auto tn = TreeNode("Textures", flags_v)) {
-		for (auto [texture, id] : enumerate(m_resources.textures)) { ri.view(texture, id); }
+		for (auto [texture, id] : enumerate(m_resources.textures.view())) { ri.view(texture, id); }
 	}
 	if (auto tn = TreeNode("Static Meshes", flags_v)) {
-		for (auto [static_mesh, id] : enumerate(m_resources.static_meshes)) { ri.view(static_mesh, id); }
+		for (auto [static_mesh, id] : enumerate(m_resources.static_meshes.view())) { ri.view(static_mesh, id); }
 	}
 	if (auto tn = TreeNode("Materials", flags_v)) {
-		for (auto [material, id] : enumerate(m_resources.materials)) { ri.edit(*material, id); }
+		for (auto [material, id] : enumerate(m_resources.materials.view())) { ri.edit(material, id); }
 		if (ImGui::Button("Add...")) { add_material = true; }
 	}
 	if (auto tn = TreeNode{"Meshes", flags_v}) {
-		for (auto [mesh, id] : enumerate(m_resources.meshes)) { ri.edit(mesh, id); }
+		for (auto [mesh, id] : enumerate(m_resources.meshes.view())) { ri.edit(mesh, id); }
 		if (ImGui::Button("Add...")) { add_mesh = true; }
 	}
 
 	if (add_material) { Popup::open("Add Material"); }
+	if (out_data.input_buffer.empty()) { out_data.input_buffer.resize(128, '\0'); }
+
+	if (auto popup = Popup{"Add Mesh"}) {
+		ImGui::InputText("Name", out_data.input_buffer.data(), out_data.input_buffer.size());
+		if (ImGui::Button("Add") && *out_data.input_buffer.c_str()) {
+			m_scene.add(Mesh{.name = out_data.input_buffer.c_str()});
+			Popup::close_current();
+		}
+	}
+
 	if (add_mesh) { Popup::open("Add Mesh"); }
-	if (out_name_buf.empty()) { out_name_buf.resize(128, '\0'); }
-	auto open_popup = [&out_name_buf](char const* id, auto func) {
-		if (auto popup = Popup{id}) {
-			ImGui::InputText("Name", out_name_buf.data(), out_name_buf.size());
-			if (ImGui::Button(id)) {
-				auto str = out_name_buf.c_str();
-				if (!*str) { return; }
-				func(str);
-				Popup::close_current();
+	if (auto popup = Popup{"Add Material"}) {
+		ImGui::InputText("Name", out_data.input_buffer.data(), out_data.input_buffer.size());
+		static constexpr char const* mat_types_v[] = {"Lit", "Unlit"};
+		static constexpr auto mat_types_size_v{static_cast<int>(std::size(mat_types_v))};
+		char const* mat_type = out_data.material_type >= 0 && out_data.material_type < mat_types_size_v ? mat_types_v[out_data.material_type] : "Invalid";
+		ImGui::SliderInt("Type", &out_data.material_type, 0, mat_types_size_v - 1, mat_type);
+		if (ImGui::Button("Add") && *out_data.input_buffer.c_str()) {
+			auto mat = std::unique_ptr<MaterialBase>{};
+			switch (out_data.material_type) {
+			case 0: mat = std::make_unique<LitMaterial>(); break;
+			case 1: mat = std::make_unique<UnlitMaterial>(); break;
 			}
+			mat->name = out_data.input_buffer.c_str();
+			m_scene.add(Material{std::move(mat)});
+			Popup::close_current();
 		}
-	};
-	auto push_material = [this](std::string name) {
-		auto mat = std::make_unique<LitMaterial>();
-		mat->name = std::move(name);
-		m_scene.add(std::move(mat));
-	};
-	open_popup("Add Material", [&push_material](char const* name) { push_material(name); });
-	open_popup("Add Mesh", [this](char const* name) { m_scene.add(Mesh{.name = name}); });
+	}
 }
 
 void SceneInspector::camera() const {
@@ -218,7 +227,7 @@ void SceneInspector::mesh(Node& out_node) const {
 	auto* mesh_id = out_node.find<Id<Mesh>>();
 	if (auto tn = TreeNode{"Mesh", ImGuiTreeNodeFlags_DefaultOpen | ImGuiTreeNodeFlags_Framed}) {
 		auto name = FixedString<128>{};
-		if (mesh_id) { name = FixedString<128>{"{} ({})", m_scene.find(*mesh_id)->name, *mesh_id}; }
+		if (mesh_id) { name = FixedString<128>{"{} ({})", m_resources.meshes.find(*mesh_id)->name, *mesh_id}; }
 		make_id_slot<Mesh>(out_node, "Mesh", name.c_str());
 	}
 }

lib/engine/src/scene_renderer.cpp

@@ -90,17 +90,17 @@ std::span<glm::mat4x4 const> SceneRenderer::make_instances(Node const& node, glm
 }
 
 void SceneRenderer::render(Renderer& renderer, vk::CommandBuffer cb, Node const& node, glm::mat4 const& parent) {
-	auto const resources = m_scene->resources();
+	auto const& resources = m_scene->resources();
 	if (auto const* mesh_id = node.find<Id<Mesh>>()) {
-		static auto const s_default_material = LitMaterial{};
+		static auto const s_default_material = Material{std::make_unique<LitMaterial>()};
 		auto const& mesh = resources.meshes[*mesh_id];
 		for (auto const& primitive : mesh.primitives) {
-			auto const& material = primitive.material ? *resources.materials[primitive.material->value()] : static_cast<Material const&>(s_default_material);
+			auto const& material = primitive.material ? resources.materials[primitive.material->value()] : static_cast<Material const&>(s_default_material);
 			auto pipeline = renderer.bind_pipeline(cb, m_scene->pipeline_state, material.shader_id());
 			pipeline.set_line_width(m_scene->pipeline_state.line_width);
 
 			update_view(pipeline);
-			auto const store = TextureStore{resources.textures, m_white, m_black};
+			auto const store = TextureStore{resources.textures.view(), m_white, m_black};
 			material.write_sets(pipeline, store);
 
 			auto const& static_mesh = resources.static_meshes[primitive.static_mesh];

lib/scene/CMakeLists.txt

@@ -43,6 +43,7 @@ target_sources(${PROJECT_NAME} PRIVATE
   include/${target_prefix}/scene/mesh.hpp
   include/${target_prefix}/scene/node_data.hpp
   include/${target_prefix}/scene/node.hpp
+  include/${target_prefix}/scene/resource_array.hpp
   include/${target_prefix}/scene/scene_resources.hpp
   include/${target_prefix}/scene/scene.hpp
 

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

@@ -39,27 +39,18 @@ struct TextureStore {
 };
 
 ///
-/// \brief Base Material: stores render parameters for a mesh.
+/// \brief Base class for concrete materials.
 ///
-class Material {
+class MaterialBase {
   public:
 	///
-	/// \brief The alpha blend mode.
+	/// \brief Alpha blend mode.
 	///
 	enum class AlphaMode : std::uint32_t { eOpaque = 0, eBlend, eMask };
 
-	///
-	/// \brief Convert sRGB encoded colour to linear.
-	///
-	static glm::vec4 to_linear(glm::vec4 const& srgb);
-	///
-	/// \brief Convert linear encoded colour to sRGB.
-	///
-	static glm::vec4 to_srgb(glm::vec4 const& linear);
-
 	inline static std::string const default_shader_id{"default"};
 
-	virtual ~Material() = default;
+	virtual ~MaterialBase() = default;
 
 	///
 	/// \brief Obtain the ID for the shader used by this material.
@@ -73,13 +64,46 @@ class Material {
 	///
 	virtual void write_sets(Pipeline& pipeline, TextureStore const& store) const = 0;
 
+	///
+	/// \brief Name of this instance.
+	///
 	std::string name{"(Unnamed)"};
 };
 
+///
+/// \brief Value-semantic strategy wrapper for concrete materials.
+///
+class Material {
+  public:
+	using AlphaMode = MaterialBase::AlphaMode;
+
+	Material(std::unique_ptr<MaterialBase>&& base) : m_base(std::move(base)) {}
+
+	std::string const& shader_id() const {
+		assert(m_base);
+		return m_base->shader_id();
+	}
+
+	void write_sets(Pipeline& pipeline, TextureStore const& store) const {
+		assert(m_base);
+		m_base->write_sets(pipeline, store);
+	}
+
+	std::string_view name() const { return m_base->name; }
+
+	MaterialBase& base() const {
+		assert(m_base);
+		return *m_base;
+	}
+
+  private:
+	std::unique_ptr<MaterialBase> m_base;
+};
+
 ///
 /// \brief Unlit Material.
 ///
-class UnlitMaterial : public Material {
+class UnlitMaterial : public MaterialBase {
   public:
 	inline static std::string const shader_id_v{"unlit"};
 
@@ -99,7 +123,7 @@ class UnlitMaterial : public Material {
 ///
 /// \brief PBR lit material.
 ///
-class LitMaterial : public Material {
+class LitMaterial : public MaterialBase {
   public:
 	///
 	/// \brief Base colour factor.

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

@@ -0,0 +1,82 @@
+#pragma once
+#include <facade/scene/id.hpp>
+#include <facade/util/ptr.hpp>
+#include <span>
+#include <utility>
+#include <vector>
+
+namespace facade {
+///
+/// \brief Wrapper over an array of resources, each identified by the index as its Id.
+///
+/// Only Scene has access to the underlying vector (via friend). Other types can operate
+/// on const / non-const spans, but cannot modify the array itself.
+///
+template <typename T>
+class ResourceArray {
+  public:
+	///
+	/// \brief Obtain an immutable view into the underlying array.
+	/// \returns Immutable span
+	///
+	std::span<T const> view() const { return m_array; }
+	///
+	/// \brief Obtain a mutable view into the underlying array.
+	/// \returns Mutable span
+	///
+	std::span<T> view() { return m_array; }
+
+	///
+	/// \brief Obtain an immutable pointer to the element at index id.
+	/// \param id Id (index) of element
+	/// \returns nullptr if id is out of bounds
+	///
+	Ptr<T const> find(Id<T> id) const {
+		if (id >= m_array.size()) { return {}; }
+		return &m_array[id];
+	}
+
+	///
+	/// \brief Obtain a mutable pointer to the element at index id.
+	/// \param id Id (index) of element
+	/// \returns nullptr if id is out of bounds
+	///
+	Ptr<T> find(Id<T> index) { return const_cast<Ptr<T>>(std::as_const(*this).find(index)); }
+
+	///
+	/// \brief Obtain an immutable reference to the element at index id.
+	/// \param id Id (index) of element
+	/// \returns const reference to element
+	///
+	/// id must be valid / in range
+	///
+	T const& operator[](Id<T> id) const {
+		auto ret = find(id);
+		assert(ret);
+		return *ret;
+	}
+
+	///
+	/// \brief Obtain an immutable reference to the element at index id.
+	/// \param id Id (index) of element
+	/// \returns const reference to element
+	///
+	/// id must be valid / in range
+	///
+	T& operator[](Id<T> index) { return const_cast<T&>(std::as_const(*this).operator[](index)); }
+
+	///
+	/// \brief Obtain the size of the underlying array.
+	///
+	constexpr std::size_t size() const { return m_array.size(); }
+	///
+	/// \brief Check if the underlying array is empty.
+	///
+	constexpr bool empty() const { return m_array.empty(); }
+
+  private:
+	std::vector<T> m_array{};
+
+	friend class Scene;
+};
+} // namespace facade