Have the mesh allocator handle modified meshes (#18531)
# Objective Fixes https://github.com/bevyengine/bevy/issues/16586. ## Solution - Free meshes before allocating new ones (so hopefully the existing allocation is used, but it's not guaranteed since it might end up getting used by a smaller mesh first). - Keep track of modified render assets, and have the mesh allocator free their allocations. - Cleaned up some render asset code to make it more understandable, since it took me several minutes to reverse engineer/remember how it was supposed to work. Long term we'll probably want to explicitly reusing allocations for modified meshes that haven't grown in size, or do delta uploads using a compute shader or something, but this is an easy fix for the near term. ## Testing Ran the example provided in the issue. No crash after a few minutes, and memory usage remains steady.
This commit is contained in:
parent
96d5f1e5de
commit
5e8dec8a90
@ -358,7 +358,10 @@ pub fn allocate_and_free_meshes(
|
||||
render_device: Res<RenderDevice>,
|
||||
render_queue: Res<RenderQueue>,
|
||||
) {
|
||||
// Process newly-added meshes.
|
||||
// Process removed or modified meshes.
|
||||
mesh_allocator.free_meshes(&extracted_meshes);
|
||||
|
||||
// Process newly-added or modified meshes.
|
||||
mesh_allocator.allocate_meshes(
|
||||
&mesh_allocator_settings,
|
||||
&extracted_meshes,
|
||||
@ -366,9 +369,6 @@ pub fn allocate_and_free_meshes(
|
||||
&render_device,
|
||||
&render_queue,
|
||||
);
|
||||
|
||||
// Process removed meshes.
|
||||
mesh_allocator.free_meshes(&extracted_meshes);
|
||||
}
|
||||
|
||||
impl MeshAllocator {
|
||||
@ -607,9 +607,17 @@ impl MeshAllocator {
|
||||
}
|
||||
}
|
||||
|
||||
/// Frees allocations for meshes that were removed or modified this frame.
|
||||
fn free_meshes(&mut self, extracted_meshes: &ExtractedAssets<RenderMesh>) {
|
||||
let mut empty_slabs = <HashSet<_>>::default();
|
||||
for mesh_id in &extracted_meshes.removed {
|
||||
|
||||
// TODO: Consider explicitly reusing allocations for changed meshes of the same size
|
||||
let meshes_to_free = extracted_meshes
|
||||
.removed
|
||||
.iter()
|
||||
.chain(extracted_meshes.modified.iter());
|
||||
|
||||
for mesh_id in meshes_to_free {
|
||||
if let Some(slab_id) = self.mesh_id_to_vertex_slab.remove(mesh_id) {
|
||||
self.free_allocation_in_slab(mesh_id, slab_id, &mut empty_slabs);
|
||||
}
|
||||
|
@ -151,14 +151,19 @@ impl<A: RenderAsset> RenderAssetDependency for A {
|
||||
#[derive(Resource)]
|
||||
pub struct ExtractedAssets<A: RenderAsset> {
|
||||
/// The assets extracted this frame.
|
||||
///
|
||||
/// These are assets that were either added or modified this frame.
|
||||
pub extracted: Vec<(AssetId<A::SourceAsset>, A::SourceAsset)>,
|
||||
|
||||
/// IDs of the assets removed this frame.
|
||||
/// IDs of the assets that were removed this frame.
|
||||
///
|
||||
/// These assets will not be present in [`ExtractedAssets::extracted`].
|
||||
pub removed: HashSet<AssetId<A::SourceAsset>>,
|
||||
|
||||
/// IDs of the assets added this frame.
|
||||
/// IDs of the assets that were modified this frame.
|
||||
pub modified: HashSet<AssetId<A::SourceAsset>>,
|
||||
|
||||
/// IDs of the assets that were added this frame.
|
||||
pub added: HashSet<AssetId<A::SourceAsset>>,
|
||||
}
|
||||
|
||||
@ -167,6 +172,7 @@ impl<A: RenderAsset> Default for ExtractedAssets<A> {
|
||||
Self {
|
||||
extracted: Default::default(),
|
||||
removed: Default::default(),
|
||||
modified: Default::default(),
|
||||
added: Default::default(),
|
||||
}
|
||||
}
|
||||
@ -235,8 +241,9 @@ pub(crate) fn extract_render_asset<A: RenderAsset>(
|
||||
|world, mut cached_state: Mut<CachedExtractRenderAssetSystemState<A>>| {
|
||||
let (mut events, mut assets) = cached_state.state.get_mut(world);
|
||||
|
||||
let mut changed_assets = <HashSet<_>>::default();
|
||||
let mut needs_extracting = <HashSet<_>>::default();
|
||||
let mut removed = <HashSet<_>>::default();
|
||||
let mut modified = <HashSet<_>>::default();
|
||||
|
||||
for event in events.read() {
|
||||
#[expect(
|
||||
@ -244,12 +251,20 @@ pub(crate) fn extract_render_asset<A: RenderAsset>(
|
||||
reason = "LoadedWithDependencies is marked as a TODO, so it's likely this will no longer lint soon."
|
||||
)]
|
||||
match event {
|
||||
AssetEvent::Added { id } | AssetEvent::Modified { id } => {
|
||||
changed_assets.insert(*id);
|
||||
AssetEvent::Added { id } => {
|
||||
needs_extracting.insert(*id);
|
||||
}
|
||||
AssetEvent::Modified { id } => {
|
||||
needs_extracting.insert(*id);
|
||||
modified.insert(*id);
|
||||
}
|
||||
AssetEvent::Removed { .. } => {
|
||||
// We don't care that the asset was removed from Assets<T> in the main world.
|
||||
// An asset is only removed from RenderAssets<T> when its last handle is dropped (AssetEvent::Unused).
|
||||
}
|
||||
AssetEvent::Removed { .. } => {}
|
||||
AssetEvent::Unused { id } => {
|
||||
changed_assets.remove(id);
|
||||
needs_extracting.remove(id);
|
||||
modified.remove(id);
|
||||
removed.insert(*id);
|
||||
}
|
||||
AssetEvent::LoadedWithDependencies { .. } => {
|
||||
@ -260,7 +275,7 @@ pub(crate) fn extract_render_asset<A: RenderAsset>(
|
||||
|
||||
let mut extracted_assets = Vec::new();
|
||||
let mut added = <HashSet<_>>::default();
|
||||
for id in changed_assets.drain() {
|
||||
for id in needs_extracting.drain() {
|
||||
if let Some(asset) = assets.get(id) {
|
||||
let asset_usage = A::asset_usage(asset);
|
||||
if asset_usage.contains(RenderAssetUsages::RENDER_WORLD) {
|
||||
@ -280,6 +295,7 @@ pub(crate) fn extract_render_asset<A: RenderAsset>(
|
||||
commands.insert_resource(ExtractedAssets::<A> {
|
||||
extracted: extracted_assets,
|
||||
removed,
|
||||
modified,
|
||||
added,
|
||||
});
|
||||
cached_state.state.apply(world);
|
||||
|
Loading…
Reference in New Issue
Block a user