From 99bd0d8e78a2f7739344f2a55e0ae7babd91514b Mon Sep 17 00:00:00 2001 From: HampusM Date: Wed, 16 Sep 2026 17:43:44 +0200 Subject: refactor(engine): simplify how model materials are handled --- engine/src/material/asset.rs | 16 +---- engine/src/model.rs | 108 ++++--------------------------- engine/src/model/asset.rs | 45 ++++++++----- engine/src/rendering/main_render_pass.rs | 2 +- 4 files changed, 45 insertions(+), 126 deletions(-) diff --git a/engine/src/material/asset.rs b/engine/src/material/asset.rs index 31d4ce5..f87b768 100644 --- a/engine/src/material/asset.rs +++ b/engine/src/material/asset.rs @@ -37,10 +37,6 @@ fn import_wavefront_mtl_asset( .map_err(|err| Error::ReadFailed(err, path.to_path_buf()))?, )?; - let mut mat_asset_map = Map { - assets: HashMap::with_capacity(named_materials.len()), - }; - for material in named_materials { let mut material_builder = Material::builder() .ambient(material.ambient) @@ -66,19 +62,9 @@ fn import_wavefront_mtl_asset( ); } - let material_name = material.name; - let material = material_builder.build(); - - let material_asset = - asset_submitter.submit_store_named(material_name.clone(), material); - - mat_asset_map - .assets - .insert(material_name.into(), material_asset); + asset_submitter.submit_store_named(material.name, material_builder.build()); } - asset_submitter.submit_store(mat_asset_map); - Ok(()) } diff --git a/engine/src/model.rs b/engine/src/model.rs index 60a5615..1269afd 100644 --- a/engine/src/model.rs +++ b/engine/src/model.rs @@ -1,9 +1,5 @@ -use std::borrow::Cow; -use std::collections::HashMap; - use crate::asset::{Assets, Handle as AssetHandle}; use crate::ecs::Component; -use crate::material::asset::Map as MaterialAssetMap; use crate::material::Material; use crate::mesh::Mesh; @@ -29,8 +25,7 @@ impl Model pub struct Spec { pub mesh_asset: Option>, - pub materials: Materials, - pub material_names: Vec>, + pub materials: Vec, } impl Spec @@ -45,31 +40,16 @@ impl Spec assets: &'assets Assets, ) -> MaterialSearchResult<'assets> { - let Some(material_name) = self.material_names.first() else { + let Some(material_desc) = self.materials.first() else { return MaterialSearchResult::NoMaterials; }; - let material_asset = match &self.materials { - Materials::Maps(material_asset_map_assets) => material_asset_map_assets - .iter() - .find_map(|mat_asset_map_asset| { - let mat_asset_map = assets.get(mat_asset_map_asset)?; - - mat_asset_map.assets.get(material_name) - }), - Materials::Direct(material_assets) => material_assets.get(material_name), - }; - - let Some(material_asset) = material_asset else { - return MaterialSearchResult::NotFound; - }; - - if assets.get(material_asset).is_none() { + if assets.get(&material_desc.asset).is_none() { tracing::trace!("Missing material asset"); return MaterialSearchResult::NotFound; } - MaterialSearchResult::Found(material_asset) + MaterialSearchResult::Found(&material_desc.asset) } } @@ -77,8 +57,7 @@ impl Spec pub struct SpecBuilder { mesh_asset: Option>, - materials: Materials, - material_names: Vec>, + materials: Vec, } impl SpecBuilder @@ -90,29 +69,12 @@ impl SpecBuilder self } - pub fn materials(mut self, materials: Materials) -> Self - { - self.materials = materials; - - self - } - - pub fn material_name(mut self, material_name: impl Into>) -> Self - { - self.material_names.push(material_name.into()); - - self - } - - pub fn material_names( + pub fn materials( mut self, - material_names: impl IntoIterator, + materials: impl IntoIterator, ) -> Self - where - MaterialName: Into>, { - self.material_names - .extend(material_names.into_iter().map(|mat_name| mat_name.into())); + self.materials = materials.into_iter().collect(); self } @@ -120,67 +82,25 @@ impl SpecBuilder #[tracing::instrument(skip_all)] pub fn build(self) -> Spec { - if !self.materials.is_empty() && self.material_names.is_empty() { - tracing::warn!("Model spec will have materials but no material names"); - } - - if self.materials.is_empty() && !self.material_names.is_empty() { - tracing::warn!("Model spec will have material names but no materials"); - } - Spec { mesh_asset: self.mesh_asset, materials: self.materials, - material_names: self.material_names, } } } #[derive(Debug, Clone)] -pub enum Materials -{ - Direct(HashMap, AssetHandle>), - Maps(Vec>), -} - -impl Materials +#[non_exhaustive] +pub struct MaterialDescription { - pub fn direct( - material_assets: impl IntoIterator)>, - ) -> Self - where - MaterialName: Into>, - { - Self::Direct( - material_assets - .into_iter() - .map(|(material_name, mat_asset)| (material_name.into(), mat_asset)) - .collect(), - ) - } - - pub fn is_empty(&self) -> bool - { - match self { - Self::Direct(material_assets) => material_assets.is_empty(), - Self::Maps(material_asset_map_assets) => material_asset_map_assets.is_empty(), - } - } - - pub fn len(&self) -> usize - { - match self { - Self::Direct(material_assets) => material_assets.len(), - Self::Maps(material_asset_map_assets) => material_asset_map_assets.len(), - } - } + pub asset: AssetHandle, } -impl Default for Materials +impl MaterialDescription { - fn default() -> Self + pub fn new(asset: AssetHandle) -> Self { - Self::Maps(Vec::new()) + Self { asset } } } diff --git a/engine/src/model/asset.rs b/engine/src/model/asset.rs index 5487203..52a6733 100644 --- a/engine/src/model/asset.rs +++ b/engine/src/model/asset.rs @@ -1,9 +1,11 @@ use std::fs::read_to_string; use std::path::{Path, PathBuf}; -use crate::asset::{Assets, Submitter as AssetSubmitter}; -use crate::material::asset::Map as MaterialAssetMap; -use crate::model::{Materials, Spec}; +use ecs::util::Either; + +use crate::asset::{Assets, Label as AssetLabel, Submitter as AssetSubmitter}; +use crate::material::Material; +use crate::model::{MaterialDescription, Spec}; #[derive(Debug, Clone)] #[non_exhaustive] @@ -69,24 +71,32 @@ fn import_wavefront_obj_asset( let mesh_asset = asset_submitter.submit_store_named("mesh", mesh); - let mut material_asset_map_assets = Vec::with_capacity(obj.mtl_libs.len()); - - for mtl_lib_path in &obj.mtl_libs { - let mtl_lib_asset = asset_submitter - .submit_load_other::(parent_path.join(mtl_lib_path)); - - material_asset_map_assets.push(mtl_lib_asset); + if obj.mtl_libs.len() > 1 { + return Err(Error::MoreThanOneMaterialLibrary); } asset_submitter.submit_store( Spec::builder() .mesh(mesh_asset) - .materials(Materials::Maps(material_asset_map_assets)) - .material_names( - obj.unique_used_material_names - .into_iter() - .map(|material_name| material_name.into_string()), - ) + .materials(if obj.mtl_libs.is_empty() { + Either::A([].into_iter()) + } else { + Either::B( + obj.unique_used_material_names + .iter() + .zip(std::iter::repeat(obj.mtl_libs.iter()).flatten()) + .map(|(material_name, mtl_lib)| { + MaterialDescription::new( + asset_submitter.submit_load_other::( + AssetLabel { + path: parent_path.join(mtl_lib).into(), + name: Some(material_name.as_ref().into()), + }, + ), + ) + }), + ) + }) .build(), ); @@ -102,6 +112,9 @@ enum Error #[error("Failed to read file {}", .1.display())] ReadFailed(#[source] std::io::Error, PathBuf), + #[error("More than one material library is specified. This is not supported")] + MoreThanOneMaterialLibrary, + #[error(transparent)] Other(#[from] crate::file_format::wavefront::obj::Error), } diff --git a/engine/src/rendering/main_render_pass.rs b/engine/src/rendering/main_render_pass.rs index 7364024..ce6e040 100644 --- a/engine/src/rendering/main_render_pass.rs +++ b/engine/src/rendering/main_render_pass.rs @@ -487,7 +487,7 @@ fn add_renderable_creation_commands( return; } - debug_assert!(model_spec.material_names.len() <= 1); + debug_assert!(model_spec.materials.len() <= 1); let model_material = match model_spec.find_first_material(&assets) { MaterialSearchResult::Found(model_material_asset) => { -- cgit v1.2.3-18-g5258