From ad3817cc1b7785dbc166a05b5fcf7fd073cdcbb9 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Sat, 22 Feb 2025 00:19:43 -0800 Subject: [PATCH] Reallocate materials when they change. (#17979) PR #17898 regressed this, causing much of #17970. This commit fixes the issue by freeing and reallocating materials in the `MaterialBindGroupAllocator` on change. Note that more efficiency is possible, but I opted for the simple approach because (1) we should fix this bug ASAP; (2) I'd like #17965 to land first, because that unlocks the biggest potential optimization, which is not recreating the bind group if it isn't necessary to do so. --- crates/bevy_pbr/src/material.rs | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/crates/bevy_pbr/src/material.rs b/crates/bevy_pbr/src/material.rs index a1c31fe26b737..b32d574400825 100644 --- a/crates/bevy_pbr/src/material.rs +++ b/crates/bevy_pbr/src/material.rs @@ -29,6 +29,7 @@ use bevy_ecs::{ SystemParamItem, }, }; +use bevy_platform_support::collections::hash_map::Entry; use bevy_platform_support::collections::{HashMap, HashSet}; use bevy_platform_support::hash::FixedHasher; use bevy_reflect::std_traits::ReflectDefault; @@ -1306,12 +1307,24 @@ impl RenderAsset for PreparedMaterial { false, ) { Ok(unprepared) => { - let binding = *render_material_bindings - .entry(material_id.into()) - .or_insert_with(|| { + // Allocate or update the material. + let binding = match render_material_bindings.entry(material_id.into()) { + Entry::Occupied(mut occupied_entry) => { + // TODO: Have a fast path that doesn't require + // recreating the bind group if only buffer contents + // change. For now, we just delete and recreate the bind + // group. + bind_group_allocator.free(*occupied_entry.get()); + let new_binding = bind_group_allocator + .allocate_unprepared(unprepared, &pipeline.material_layout); + *occupied_entry.get_mut() = new_binding; + new_binding + } + Entry::Vacant(vacant_entry) => *vacant_entry.insert( bind_group_allocator - .allocate_unprepared(unprepared, &pipeline.material_layout) - }); + .allocate_unprepared(unprepared, &pipeline.material_layout), + ), + }; Ok(PreparedMaterial { binding,