From d3f964bb2fafd3bca06519154752e80127cdb619 Mon Sep 17 00:00:00 2001
From: chazmm <balloverwatchh@gmail.com>
Date: Wed, 9 Sep 2026 02:52:56 +0100
Subject: [PATCH] Avoid identical character lighting asset updates based on
 trace evidence

---
 crates/skate-game/src/retail_character.rs | 49 ++++++++++++++++--
 docs/university-performance-assessment.md | 61 +++++++++++++++++++++++
 2 files changed, 107 insertions(+), 3 deletions(-)

diff --git a/crates/skate-game/src/retail_character.rs b/crates/skate-game/src/retail_character.rs
index 98cd43c4..7f057bab 100644
--- a/crates/skate-game/src/retail_character.rs
+++ b/crates/skate-game/src/retail_character.rs
@@ -279,6 +279,7 @@ fn update(
     mut materials: ResMut<Assets<CharacterMaterial>>,
     mut shadow: ResMut<crate::retail_render::ShadowState>,
     time: Res<Time>,
+    mut changed: Local<Vec<AssetId<CharacterMaterial>>>,
 ) {
     let (Some(mut lighting), Ok(root)) = (lighting, root.single()) else {
         return;
@@ -292,19 +293,61 @@ fn update(
         let weight = 1. - (-time.delta_secs().clamp(0., 0.05) / 0.35).exp();
         std::array::from_fn(|i| old[i].lerp(sh[i], weight))
     });
-    for (_, material) in materials.iter_mut() {
-        material.params.sh = displayed;
-    }
+    publish_sh(&mut materials, displayed, &mut changed);
     lighting.display_sh = Some(displayed);
     // Adapter floor: the local probe's direction-independent ambient term.
     // The native per-frame c8 shadow-colour controller remains unrecovered.
     shadow.approach(sh[0].truncate(), time.delta_secs());
 }
 
+fn publish_sh(materials: &mut Assets<CharacterMaterial>, displayed: [Vec4; 9], changed: &mut Vec<AssetId<CharacterMaterial>>) {
+    // iter_mut emits Modified even for identical assignments, rebuilding GPU
+    // material bindings. Read first and mutate only bitwise-different SH payloads.
+    // Check every material so newly bound pieces still receive the current SH.
+    changed.clear();
+    changed.extend(materials.iter().filter_map(|(id, material)| {
+        (!material.params.sh.iter().zip(&displayed).all(|(a,b)| a.to_array().map(f32::to_bits)==b.to_array().map(f32::to_bits))).then_some(id)
+    }));
+    for &id in changed.iter() {
+        materials.get_mut(id).expect("material enumerated in this call").params.sh = displayed;
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
 
+    #[test]
+    fn unchanged_sh_emits_no_asset_modification_but_new_values_and_pieces_do() {
+        let mut app=App::new();
+        app.add_plugins((MinimalPlugins,AssetPlugin::default())).init_asset::<CharacterMaterial>();
+        fn material() -> CharacterMaterial {
+            CharacterMaterial { params:CharacterParams {light:Vec4::ZERO,tint:Vec4::ONE,options:Vec4::ZERO,rows:[Vec4::ZERO;9],sh:[Vec4::ZERO;9]},diffuse:None,normal:None,mask:None,alpha:AlphaMode::Opaque }
+        }
+        let first=app.world_mut().resource_mut::<Assets<CharacterMaterial>>().add(material());
+        app.update();
+        app.world_mut().resource_mut::<Messages<AssetEvent<CharacterMaterial>>>().clear();
+        let mut scratch=Vec::new();
+        let mut displayed=[Vec4::ZERO;9];
+        publish_sh(&mut app.world_mut().resource_mut::<Assets<CharacterMaterial>>(),displayed,&mut scratch);
+        assert!(scratch.is_empty());
+        app.update();
+        assert_eq!(app.world_mut().resource_mut::<Messages<AssetEvent<CharacterMaterial>>>().drain().count(),0);
+        // Signed zero and NaN payloads must retain their bits, without epsilon tests.
+        displayed[0]=Vec4::new(-0.0,f32::from_bits(0x7fc01234),1.0,0.0);
+        publish_sh(&mut app.world_mut().resource_mut::<Assets<CharacterMaterial>>(),displayed,&mut scratch);
+        assert_eq!(scratch,vec![first.id()]);
+        app.update();
+        assert!(app.world_mut().resource_mut::<Messages<AssetEvent<CharacterMaterial>>>().drain().any(|e|matches!(e,AssetEvent::Modified{id} if id==first.id())));
+        publish_sh(&mut app.world_mut().resource_mut::<Assets<CharacterMaterial>>(),displayed,&mut scratch);
+        assert!(scratch.is_empty());
+        let second=app.world_mut().resource_mut::<Assets<CharacterMaterial>>().add(material());
+        publish_sh(&mut app.world_mut().resource_mut::<Assets<CharacterMaterial>>(),displayed,&mut scratch);
+        assert_eq!(scratch,vec![second.id()]);
+        let assets=app.world().resource::<Assets<CharacterMaterial>>();
+        for handle in [&first,&second] { assert_eq!(assets.get(handle).unwrap().params.sh[0].to_array().map(f32::to_bits),displayed[0].to_array().map(f32::to_bits)); }
+    }
+
     #[test]
     fn world_receiver_source_excludes_world_casters_and_emits_no_light() {
         let mut world = World::new();
diff --git a/docs/university-performance-assessment.md b/docs/university-performance-assessment.md
index a08d6180..5088cc0c 100644
--- a/docs/university-performance-assessment.md
+++ b/docs/university-performance-assessment.md
@@ -255,3 +255,64 @@ analyser reproduced the capture's event/sample counts and limitation flags. The
 new private static-CRT release EXE/PDB built successfully; embedded revision and PE
 imports were verified. The original recording and executable were retained. No
 gameplay or GPU capture was launched by the agent.
+
+## Second user capture: named CPU systems and measured GPU durations
+
+Build `2a74581`, GPU diagnostics enabled, SHA-256
+`03d97c8b8cd3980123ad6ea7c6e7c36d9d41ff2c5877077851ce93086b1747de`.
+The useful recording covers 24.62 seconds and 6,997 frame intervals. It ended at
+the byte bound, with zero queue drops and 12,213,517 intentionally filtered short
+spans. This is sufficient for the observations below despite not reaching 30 seconds.
+Another small file from this session contains only metadata and no active capture.
+
+Frame interval: mean **3.52 ms**, median **3.11 ms**, p95 **6.33 ms**, p99 **7.11 ms**,
+maximum **9.71 ms**. The mean corresponds to about 284 main-app updates per second,
+not verified display FPS. Settings and actual target size match the first capture.
+The samples cover a different interval and instrumentation configuration, so their
+lower timings are **not an A/B performance improvement**.
+
+| Scope | Mean | p95 | Measurement |
+|---|---:|---:|---|
+| Render schedule | 3.27 ms | 6.07 ms | Inclusive CPU wall time |
+| Main app | 2.25 ms | 2.98 ms | Inclusive CPU wall time; overlaps render work |
+| Command-buffer generation tasks | 1.27 ms | 3.59 ms | CPU work/waits |
+| Directional-light mesh visibility | 0.448 ms | 0.974 ms | CPU system |
+| Controller polling | 0.342 ms | 0.443 ms | CPU/OS calls |
+| Character-material preparation | 0.124 ms | 0.160 ms | CPU system |
+| Physics advance | 0.644 ms | 0.880 ms | Per fixed tick: 1,477 ticks, not per display frame |
+| Main opaque pass | 0.191 ms | 0.259 ms | GPU timestamp duration |
+| Retail exposure meter | 0.00781 ms | 0.00794 ms | GPU timestamp duration |
+| Retail exposed tone | 0.0157 ms | 0.0159 ms | GPU timestamp duration |
+
+CPU rendering/visibility is a stronger optimisation lead than main-pass shading
+for this scene. GPU figures cover instrumented passes, not the entire GPU frame;
+some anonymized shadow/other pass labels cannot be attributed individually from
+this export. CPU scope durations include scheduler and profiler overhead. The
+recorder's `end_frame` system itself averaged 0.115 ms; this is only one component
+of capture overhead, not its total. No GPU saturation claim follows from these data.
+
+Across 117 resource snapshots there were consistently 4,936 prepared meshes,
+2,195 prepared images, and zero waiting pipelines. There is still no indication
+of resource churn or shader compilation driving this interval. These counts
+differ from the first capture, so the two recordings are not identical workloads.
+
+The visibility source checks two distinct shadow sources: world/skater shadows
+for character lighting, and layer-28 player-only shadows received by the baked
+world. Removing casters or reducing cascades would change the image. It remains a
+substantial lead, but no visibility shortcut is applied without an exact equivalent.
+Controller polling calls XInput state and capabilities at the existing cadence;
+throttling absent slots or caching subtype data could alter reconnect/input semantics,
+so it is not changed under the timing constraint.
+
+One directly supported change is implemented in `retail_character.rs::update`.
+It previously used `Assets::iter_mut` to assign SH lighting to every character
+material every frame. Bevy emits `AssetEvent::Modified` for those mutable accesses,
+including identical assignments, causing needless preparation/binding work after
+lighting has settled. The new path reads each material first and only obtains a
+mutable reference if any SH float's **bits** differ. It still computes probe choice,
+smoothing and shadow state every frame and updates every changing/new material in
+the same frame. It uses no tolerance, cadence reduction or shader change. Signed
+zeros and NaN payloads are compared explicitly. Reusable scratch storage avoids a
+new per-frame allocation. This is expected to help settled-lighting frames; it may
+save nothing while every material's lighting changes. No FPS gain is claimed before
+a matching post-change capture and user visual test.