Assets: Use ArcSwap to simplify asset graph API (and make it more loc…

…k free)

Cargo.toml

@@ -82,6 +82,9 @@ wgpu-profiler = { version = "0.17.0", features = ["tracy"] }
 # Error handling
 anyhow = "1.0.95"
 
+# For the asset graph implementation (RwLock<Option<Arc<T>>>)
+arc-swap = "1.7.1"
+
 # asset parsing
 mpq = { path = "mpq-rust" } # mpq = "0.8"
 image-blp = "1"

README.md

@@ -33,8 +33,11 @@ Things that still need to be implemented (loosely sorted by priority):
 - Third Person Camera Controller (and sending `MOVE_FACING` packets / reworking the movement tracker)
 - Instanced Rendering of M2s (UnitMaterial is currently created in-place, even if the same texture has already been
   used.) It remains to be seen if that is enough for rend3 to auto instance, though.
+    - TODO: Does Rend3 even have instancing?
 - Debug Shader Reloading
 - Lights Improvement (especially the SkyLight: Implement a game time that ticks, support ambient color)
+- Move WMOs to "UnitsMaterial" (and probably rename/refactor it too.) This requires Unit to support more things (
+  transparency, albedo colors)
 - Configuration
 - Cross Platform support (i.e. investigate MBP 2011 failure)
 - massive Map Manager rework
@@ -60,5 +63,4 @@ Things that still need to be implemented (loosely sorted by priority):
   a lot of API surface and especially the related event handling and layout management
   needs to be handled from scratch.
 - Advanced game "logic" (e.g. chat, friend list, guilds, trading, auction house)
-- Advanced rendering techniques: AO, TAA, VXGI?
-- https://docs.rs/arc-swap/latest/arc_swap/
+- Advanced rendering techniques: AO, TAA, VXGI?

src/entity/systems/display_id_resolver_system.rs

@@ -8,8 +8,8 @@ use crate::rendering::asset_graph::nodes::adt_node::M2Node;
 use crate::rendering::asset_graph::resolver::Resolver;
 use crate::rendering::rend3_backend::IRTexture;
 use hecs::Without;
-use itertools::{Itertools, enumerate};
-use log::{info, warn};
+use itertools::Itertools;
+use log::warn;
 use sargerust_files::m2::types::M2TextureType;
 use std::collections::HashSet;
 use std::sync::{Arc, RwLock};
@@ -99,7 +99,7 @@ impl DisplayIdResolverSystem {
 
             for reference in &result.tex_reference {
                 let resolve = self.tex_resolver.resolve(reference.reference_str.clone());
-                *reference.reference.write().expect("Write Lock") = Some(resolve);
+                reference.reference.store(Some(resolve));
             }
 
             let base_skin_cdie_opt =

src/game/map_light_settings_provider.rs

@@ -1,10 +1,9 @@
-use crate::io::common::loader::RawAssetLoader;
 use crate::io::mpq::load_dbc;
 use crate::io::mpq::loader::MPQLoader;
 use crate::rendering::common::coordinate_systems;
 use glam::{Vec3, Vec4};
 use itertools::Itertools;
-use log::{trace, warn};
+use log::warn;
 use std::sync::Arc;
 use wow_dbc::wrath_tables::light::{Light, LightRow};
 use wow_dbc::wrath_tables::light_float_band::{LightFloatBand, LightFloatBandRow};

src/game/map_manager.rs

@@ -1,6 +1,5 @@
 use std::collections::HashMap;
 use std::io::Cursor;
-use std::ops::DerefMut;
 use std::sync::{Arc, RwLock};
 use std::time::Instant;
 
@@ -243,14 +242,7 @@ impl MapManager {
 
         // TODO: Resolving should be a matter of the rendering app, not this code here? But then their code relies on things being preloaded?
         for wmo in &wmos {
-            if wmo
-                .reference
-                .reference
-                .read()
-                .expect("Reference ReadLock")
-                .as_ref()
-                .is_some()
-            {
+            if wmo.reference.reference.load().as_ref().is_some() {
                 // Apparently our reference was cloned and thus is pre-loaded.
                 continue;
             }
@@ -266,13 +258,7 @@ impl MapManager {
                 set.spawn_blocking_on(
                     move || {
                         let group_result = resolver.resolve(sub_group_cloned.reference_str.to_string());
-
-                        let mut write_lock_group = sub_group_cloned
-                            .reference
-                            .write()
-                            .expect("Write lock on sub group reference");
-
-                        *write_lock_group.deref_mut() = Some(group_result);
+                        sub_group_cloned.reference.store(Some(group_result));
                     },
                     self.runtime.handle(),
                 );
@@ -310,13 +296,7 @@ impl MapManager {
                 );
             }
 
-            let mut write_lock = wmo
-                .reference
-                .reference
-                .write()
-                .expect("Write lock on wmo reference");
-
-            *write_lock.deref_mut() = Some(result);
+            wmo.reference.reference.store(Some(result));
         }
 
         for dad in &direct_doodad_refs {
@@ -387,12 +367,7 @@ impl MapManager {
                 // TODO: With an async friendly RwLock, this could become regular async code, without spawn_blocking.
                 handle_clone
                     .spawn_blocking(move || {
-                        let mut write_lock = dad
-                            .reference
-                            .reference
-                            .write()
-                            .expect("Write lock on doodad reference");
-                        *write_lock.deref_mut() = Some(result);
+                        dad.reference.reference.store(Some(result));
                     })
                     .await
                     .unwrap();
@@ -416,13 +391,7 @@ impl MapManager {
             set.spawn_blocking_on(
                 move || {
                     let result_tex = resolver.resolve(tex_reference.reference_str.clone());
-
-                    let mut ref_wlock = tex_reference
-                        .reference
-                        .write()
-                        .expect("texture reference write lock");
-
-                    *ref_wlock.deref_mut() = Some(result_tex);
+                    tex_reference.reference.store(Some(result_tex));
                 },
                 handle,
             );

src/physics/collider_factory.rs

@@ -69,8 +69,7 @@ impl ColliderFactory {
     ) {
         // TODO: We should also use the WeakKeyDashMap for WMOs themselves.
         for wmo_ref in &adt.wmos {
-            let resolved_wmo = wmo_ref.reference.reference.read().expect("poisoned lock");
-            if let Some(wmo) = resolved_wmo.deref() {
+            if let Some(wmo) = wmo_ref.reference.reference.load().deref() {
                 let weak_wmo = Arc::downgrade(wmo);
 
                 if !wmo_colliders
@@ -102,8 +101,7 @@ impl ColliderFactory {
     ) {
         let (scale, rotation, translation) = wmo_ref.transform.to_scale_rotation_translation();
         for group_reference in groups {
-            let resolved_group = group_reference.reference.read().expect("poisoned lock");
-            if let Some(group) = resolved_group.deref() {
+            if let Some(group) = group_reference.reference.load().deref() {
                 let weak_grp = Arc::downgrade(group);
 
                 {
@@ -201,9 +199,7 @@ impl ColliderFactory {
     ) {
         // TODO: A lot of those doodads probably shouldn't be collidable (lilypad, jugs, wall shield)
         for doodad in doodads {
-            let resolved_doodad = doodad.reference.reference.read().expect("poisoned lock");
-
-            let Some(dad) = resolved_doodad.deref() else {
+            let Some(dad) = doodad.reference.reference.load_full() else {
                 continue;
             };
 

src/rendering/application.rs

@@ -1,3 +1,4 @@
+use arc_swap::ArcSwapOption;
 use std::collections::HashMap;
 use std::f32::consts::PI;
 use std::hash::BuildHasher;
@@ -22,7 +23,7 @@ use crate::rendering::rend3_backend::{
 };
 use glam::{Mat4, UVec2, Vec3A, Vec4};
 use itertools::Itertools;
-use log::{debug, trace, warn};
+use log::{trace, warn};
 use rend3::graph::RenderGraph;
 use rend3::types::{
     Camera, CameraProjection, Handedness, MaterialHandle, PresentMode, SampleCount, Texture, Texture2DHandle,
@@ -38,11 +39,8 @@ use rend3_routine::common::CameraSpecifier;
 use rend3_routine::forward::ForwardRoutineArgs;
 use rend3_routine::{clear, forward};
 use sargerust_files::m2::types::{M2TextureFlags, M2TextureType};
-use winit::error::EventLoopError;
 use winit::event::{ElementState, KeyEvent, WindowEvent};
-use winit::event_loop::EventLoop;
 use winit::keyboard::{KeyCode, PhysicalKey};
-use winit::window::{Window, WindowBuilder};
 
 // #[derive(Debug)] // TODO: Ensure Grabber implements Display
 pub struct RenderingApplication {
@@ -214,12 +212,12 @@ impl RenderingApplication {
     fn load_wmos(&self, renderer: &Arc<Renderer>, graph: &Arc<ADTNode>) {
         for wmo_ref in &graph.wmos {
             let wmo = {
-                let wmo_rlock = wmo_ref.reference.reference.read().expect("WMO Read Lock");
-                if wmo_rlock.is_none() {
-                    continue; // WMO is not loaded yet.
+                let wmo_arc = wmo_ref.reference.reference.load();
+                if wmo_arc.is_none() {
+                    continue; // WMO is not resolved yet.
                 }
 
-                wmo_rlock
+                wmo_arc
                     .as_ref()
                     .expect("WMO has to be loaded (see lines above)")
                     .clone()
@@ -268,14 +266,14 @@ impl RenderingApplication {
                 }
 
                 let subgroup = {
-                    let subgroup_rlock = subgroup_ref.reference.read().expect("Subgroup Read Lock");
+                    let subgroup_arc = subgroup_ref.reference.load();
 
-                    if subgroup_rlock.is_none() {
+                    if subgroup_arc.is_none() {
                         // not loaded yet
                         continue;
                     }
 
-                    subgroup_rlock
+                    subgroup_arc
                         .as_ref()
                         .expect("Subgroup has to be loaded (see lines above)")
                         .clone()
@@ -436,13 +434,16 @@ impl RenderingApplication {
 
             // TODO: technically we have a race condition here, while we load the stuff on the GPU, it may have changed loader side. In general we have no concept of updating yet.
             let m2 = {
-                // TODO: Async aware RwLock
-                let m2_rlock = doodad.reference.reference.read().expect("M2 Read Lock");
-                if m2_rlock.is_none() {
+                // TODO: This pattern is so common, but due to the continue I think we can't deduplicate this into a method
+                let doodad_arc = doodad.reference.reference.load();
+                if doodad_arc.is_none() {
                     continue;
                 }
 
-                m2_rlock.as_ref().expect("previous is_none check.").clone()
+                doodad_arc
+                    .as_ref()
+                    .expect("previous is_none check.")
+                    .clone()
             };
 
             //  TODO: This ignores dynamic textures, but I think they can't be supported here anyway.
@@ -502,12 +503,9 @@ impl RenderingApplication {
     }
 
     pub fn are_all_textures_loaded(tex_reference: &Vec<Arc<IRTextureReference>>) -> bool {
-        !tex_reference.iter().any(|tex| {
-            tex.reference
-                .read()
-                .expect("tex reference read lock")
-                .is_none()
-        })
+        !tex_reference
+            .iter()
+            .any(|tex| tex.reference.load().is_none())
     }
 
     pub fn load_material(
@@ -572,7 +570,7 @@ impl RenderingApplication {
                         .map(|(_, _, tex)| tex.clone())
                 }
             })
-            .map(|tex| gpu_loaders::gpu_load_texture(renderer, &RwLock::new(Some(tex))))
+            .map(|tex| gpu_loaders::gpu_load_texture(renderer, &ArcSwapOption::new(Some(tex))))
             .collect_vec();
 
         // TODO: There's still the edge-case where loading may have failed and it's thus declared as None. But that requires

src/rendering/asset_graph/nodes/adt_node.rs

@@ -2,6 +2,7 @@ use crate::rendering::common::special_types::TerrainTextureLayerRend3;
 use crate::rendering::common::types::{Material, Mesh};
 use crate::rendering::importer::m2_importer::M2Material;
 use crate::rendering::rend3_backend::{IRM2Material, IRMaterial, IRMesh, IRTextureReference};
+use arc_swap::ArcSwapOption;
 use glam::{Affine3A, Mat4, Vec3A};
 use rend3::types::{MaterialHandle, MeshHandle, ObjectHandle};
 use sargerust_files::m2::types::M2Texture;
@@ -125,14 +126,14 @@ pub struct WMOGroupNode {
 #[derive(Debug)]
 pub struct NodeReference<T> {
     pub reference_str: String,
-    pub reference: RwLock<Option<Arc<T>>>,
+    pub reference: ArcSwapOption<T>,
 }
 
 impl<T> NodeReference<T> {
     pub fn new(reference_str: String) -> Self {
         Self {
             reference_str,
-            reference: RwLock::new(None),
+            reference: ArcSwapOption::empty(),
         }
     }
 }
@@ -155,7 +156,7 @@ impl<T> Eq for NodeReference<T> {}
 #[derive(Debug)]
 pub struct IRObjectReference<T> {
     pub reference_str: String,
-    pub reference: RwLock<Option<Arc<RwLock<T>>>>, // the inner RwLock is to mutate the IRObject, most notably it's handle. Could be put into handle, but then perspectively data needs to be mutable as well.
+    pub reference: ArcSwapOption<RwLock<T>>, // the RwLock is to mutate the IRObject, most notably it's handle. Could be put into handle, but then perspectively data needs to be mutable as well.
 }
 
 #[derive(Debug)]
@@ -187,7 +188,7 @@ impl From<Material> for IRObject<Material, MaterialHandle> {
 impl From<M2Texture> for IRTextureReference {
     fn from(value: M2Texture) -> Self {
         Self {
-            reference: RwLock::new(None),
+            reference: ArcSwapOption::empty(),
             reference_str: value.filename,
         }
     }
@@ -196,7 +197,7 @@ impl From<M2Texture> for IRTextureReference {
 impl From<String> for IRTextureReference {
     fn from(value: String) -> Self {
         Self {
-            reference: RwLock::new(None),
+            reference: ArcSwapOption::empty(),
             reference_str: value,
         }
     }

src/rendering/importer/m2_importer.rs

@@ -1,6 +1,5 @@
-use crate::rendering::common::types::{AlbedoType, Material, Mesh, TransparencyType, VertexBuffers};
-use glam::{Vec2, Vec3, Vec4};
-use image_blp::BlpImage;
+use crate::rendering::common::types::{Mesh, VertexBuffers};
+use glam::{Vec2, Vec3};
 use itertools::Itertools;
 use sargerust_files::m2::types::{M2Asset, M2Batch, M2SkinProfile, M2SkinSection, M2Texture};
 

src/rendering/loader/wmo_loader.rs

@@ -4,6 +4,7 @@ use crate::rendering::asset_graph::nodes::adt_node::{DoodadReference, NodeRefere
 use crate::rendering::common::highlevel_types::PlaceableDoodad;
 use crate::rendering::common::types::{AlbedoType, Material, TransparencyType};
 use crate::rendering::rend3_backend::IRTextureReference;
+use arc_swap::ArcSwapOption;
 use glam::{Affine3A, Quat, Vec3, Vec4};
 use log::debug;
 use sargerust_files::wmo::reader::WMOReader;
@@ -61,7 +62,7 @@ impl WMOLoader {
                 //  is rather minor, just some locking and resolving.
                 tex_references.push(Arc::new(IRTextureReference {
                     reference_str: texname_1,
-                    reference: RwLock::new(None),
+                    reference: ArcSwapOption::empty(),
                 }))
             }
         }