Skip to content

Commit

Permalink
Rendering: Better Asset deduplication
Browse files Browse the repository at this point in the history
  • Loading branch information
MeFisto94 committed Feb 16, 2025
1 parent 9b36f1c commit b03d4da
Show file tree
Hide file tree
Showing 6 changed files with 15 additions and 19 deletions.
6 changes: 3 additions & 3 deletions src/entity/systems/display_id_resolver_system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,10 @@ impl DisplayIdResolverSystem {

let base_path = name.split_at(name.rfind('\\').expect("No \\ in name")).0;

let result = self.m2_resolver.resolve(name.clone());
let result = self.m2_resolver.resolve(name.as_str());

for reference in &result.tex_reference {
let resolve = self.tex_resolver.resolve(reference.reference_str.clone());
let resolve = self.tex_resolver.resolve(reference.reference_str.as_str());
reference.reference.store(Some(resolve));
}

Expand Down Expand Up @@ -183,7 +183,7 @@ impl DisplayIdResolverSystem {
Some((
reference.texture_type,
reference.texture_flags,
self.tex_resolver.resolve(tex_name.clone()),
self.tex_resolver.resolve(tex_name.as_str()),
))
})
.collect_vec();
Expand Down
8 changes: 4 additions & 4 deletions src/game/map_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,15 +252,15 @@ impl MapManager {

let result = self
.wmo_resolver
.resolve(wmo.reference.reference_str.clone());
.resolve(wmo.reference.reference_str.as_str());

// TODO: should we resolve WMO Groups right here or rather after all WMOs are resolved? Technically groups could even be lazily resolved?
for sub_group in &result.subgroups {
let resolver = self.wmo_group_resolver.clone();
let sub_group_cloned = sub_group.clone();
set.spawn_blocking_on(
move || {
let group_result = resolver.resolve(sub_group_cloned.reference_str.to_string());
let group_result = resolver.resolve(sub_group_cloned.reference_str.as_str());
sub_group_cloned.reference.store(Some(group_result));
},
self.runtime.handle(),
Expand Down Expand Up @@ -354,7 +354,7 @@ impl MapManager {
let result = handle_clone
.spawn_blocking(move || {
// TODO: PERF: get rid of duplicated references here, make DoodadReference#reference an Arc and cache them at least adt wide.
m2_resolver.resolve(reference_str)
m2_resolver.resolve(reference_str.as_str())
})
.await
.unwrap();
Expand Down Expand Up @@ -393,7 +393,7 @@ impl MapManager {
let resolver = tex_resolver.clone();
set.spawn_blocking_on(
move || {
let result_tex = resolver.resolve(tex_reference.reference_str.clone());
let result_tex = resolver.resolve(tex_reference.reference_str.as_str());
tex_reference.reference.store(Some(result_tex));
},
handle,
Expand Down
2 changes: 1 addition & 1 deletion src/rendering/application.rs
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,7 @@ impl RenderingApplication {
.filter_map(|tex| {
if tex.texture_type == M2TextureType::None {
Some((
tex_resolver.resolve(tex.filename.clone()),
tex_resolver.resolve(tex.filename.as_str()),
tex.filename.as_str(),
))
} else {
Expand Down
14 changes: 5 additions & 9 deletions src/rendering/asset_graph/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,9 @@ impl<G: GraphNodeGenerator<T>, T> Resolver<G, T> {
}
}

// TODO: maybe take name by reference and only own it when inserting.
// also canonicalize paths: uppercase and forward slashes as in MPQ?
// -> Those two requirements do conflict, though.
pub fn resolve(&self, name: String) -> Arc<T> {
// optimistic path
// can be removed without impacting correctness
pub fn resolve(&self, key: &str) -> Arc<T> {
let name = key.to_ascii_uppercase(); // TODO: Canonicalize (i.e. forward slashes?)
// optimistic path, can be removed without impacting correctness
if let Some(existing) = self.ref_cache.get(&name).and_then(|x| x.upgrade()) {
return existing;
}
Expand All @@ -38,13 +35,12 @@ impl<G: GraphNodeGenerator<T>, T> Resolver<G, T> {
// benefit: we can call `generate` outside of the critical section
// drawback: if we're wrong, `generate` gets called more than once
// let new = self.generator.generate(&name);
// Then, we can also move in name instead of cloning it

// clone can be removed, when generating outside the critical section
match self.ref_cache.entry(name.clone()) {
Entry::Occupied(mut o) => {
if let Some(existing) = o.get().upgrade() {
// the optimistic path failed earlier,
// but someone slipped an entry in since then
// the optimistic path failed earlier, but someone slipped an entry in since then
existing
} else {
// there was already an entry, but it died
Expand Down
2 changes: 1 addition & 1 deletion src/rendering/loader/m2_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ impl M2Loader {
// In theory, we could investigate the number of LoD Levels, but we will just use "0"
let mut skin_file = std::io::Cursor::new(
loader
.load_raw_owned(&name.replace(".m2", "00.skin"))
.load_raw_owned(&name.replace(".M2", "00.skin"))
.unwrap(),
);

Expand Down
2 changes: 1 addition & 1 deletion src/rendering/loader/wmo_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ impl WMOLoader {
let name = wmo.modn.doodadNameList[idx].as_str();

// fix name: currently it ends with .mdx, but we need .m2
let name = name.replace(".MDX", ".m2").replace(".MDL", ".m2");
let name = name.replace(".MDX", ".M2").replace(".MDL", ".M2");
if name.to_lowercase().contains("emitter") {
continue;
}
Expand Down

0 comments on commit b03d4da

Please sign in to comment.