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
83ffc90c6c
commit
000b9e52c9
@ -358,7 +358,10 @@ pub fn allocate_and_free_meshes(
|
|||||||
render_device: Res<RenderDevice>,
|
render_device: Res<RenderDevice>,
|
||||||
render_queue: Res<RenderQueue>,
|
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.allocate_meshes(
|
||||||
&mesh_allocator_settings,
|
&mesh_allocator_settings,
|
||||||
&extracted_meshes,
|
&extracted_meshes,
|
||||||
@ -366,9 +369,6 @@ pub fn allocate_and_free_meshes(
|
|||||||
&render_device,
|
&render_device,
|
||||||
&render_queue,
|
&render_queue,
|
||||||
);
|
);
|
||||||
|
|
||||||
// Process removed meshes.
|
|
||||||
mesh_allocator.free_meshes(&extracted_meshes);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
impl MeshAllocator {
|
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>) {
|
fn free_meshes(&mut self, extracted_meshes: &ExtractedAssets<RenderMesh>) {
|
||||||
let mut empty_slabs = <HashSet<_>>::default();
|
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) {
|
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);
|
self.free_allocation_in_slab(mesh_id, slab_id, &mut empty_slabs);
|
||||||
}
|
}
|
||||||
|
@ -151,14 +151,19 @@ impl<A: RenderAsset> RenderAssetDependency for A {
|
|||||||
#[derive(Resource)]
|
#[derive(Resource)]
|
||||||
pub struct ExtractedAssets<A: RenderAsset> {
|
pub struct ExtractedAssets<A: RenderAsset> {
|
||||||
/// The assets extracted this frame.
|
/// The assets extracted this frame.
|
||||||
|
///
|
||||||
|
/// These are assets that were either added or modified this frame.
|
||||||
pub extracted: Vec<(AssetId<A::SourceAsset>, A::SourceAsset)>,
|
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`].
|
/// These assets will not be present in [`ExtractedAssets::extracted`].
|
||||||
pub removed: HashSet<AssetId<A::SourceAsset>>,
|
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>>,
|
pub added: HashSet<AssetId<A::SourceAsset>>,
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -167,6 +172,7 @@ impl<A: RenderAsset> Default for ExtractedAssets<A> {
|
|||||||
Self {
|
Self {
|
||||||
extracted: Default::default(),
|
extracted: Default::default(),
|
||||||
removed: Default::default(),
|
removed: Default::default(),
|
||||||
|
modified: Default::default(),
|
||||||
added: Default::default(),
|
added: Default::default(),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@ -235,8 +241,9 @@ pub(crate) fn extract_render_asset<A: RenderAsset>(
|
|||||||
|world, mut cached_state: Mut<CachedExtractRenderAssetSystemState<A>>| {
|
|world, mut cached_state: Mut<CachedExtractRenderAssetSystemState<A>>| {
|
||||||
let (mut events, mut assets) = cached_state.state.get_mut(world);
|
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 removed = <HashSet<_>>::default();
|
||||||
|
let mut modified = <HashSet<_>>::default();
|
||||||
|
|
||||||
for event in events.read() {
|
for event in events.read() {
|
||||||
#[expect(
|
#[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."
|
reason = "LoadedWithDependencies is marked as a TODO, so it's likely this will no longer lint soon."
|
||||||
)]
|
)]
|
||||||
match event {
|
match event {
|
||||||
AssetEvent::Added { id } | AssetEvent::Modified { id } => {
|
AssetEvent::Added { id } => {
|
||||||
changed_assets.insert(*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 } => {
|
AssetEvent::Unused { id } => {
|
||||||
changed_assets.remove(id);
|
needs_extracting.remove(id);
|
||||||
|
modified.remove(id);
|
||||||
removed.insert(*id);
|
removed.insert(*id);
|
||||||
}
|
}
|
||||||
AssetEvent::LoadedWithDependencies { .. } => {
|
AssetEvent::LoadedWithDependencies { .. } => {
|
||||||
@ -260,7 +275,7 @@ pub(crate) fn extract_render_asset<A: RenderAsset>(
|
|||||||
|
|
||||||
let mut extracted_assets = Vec::new();
|
let mut extracted_assets = Vec::new();
|
||||||
let mut added = <HashSet<_>>::default();
|
let mut added = <HashSet<_>>::default();
|
||||||
for id in changed_assets.drain() {
|
for id in needs_extracting.drain() {
|
||||||
if let Some(asset) = assets.get(id) {
|
if let Some(asset) = assets.get(id) {
|
||||||
let asset_usage = A::asset_usage(asset);
|
let asset_usage = A::asset_usage(asset);
|
||||||
if asset_usage.contains(RenderAssetUsages::RENDER_WORLD) {
|
if asset_usage.contains(RenderAssetUsages::RENDER_WORLD) {
|
||||||
@ -280,6 +295,7 @@ pub(crate) fn extract_render_asset<A: RenderAsset>(
|
|||||||
commands.insert_resource(ExtractedAssets::<A> {
|
commands.insert_resource(ExtractedAssets::<A> {
|
||||||
extracted: extracted_assets,
|
extracted: extracted_assets,
|
||||||
removed,
|
removed,
|
||||||
|
modified,
|
||||||
added,
|
added,
|
||||||
});
|
});
|
||||||
cached_state.state.apply(world);
|
cached_state.state.apply(world);
|
||||||
|
Loading…
Reference in New Issue
Block a user