Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unload render assets from RAM #10520

Merged
merged 48 commits into from
Jan 3, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
e3b4936
Unload render assets from RAM
JMS55 Nov 12, 2023
9e6a6e2
Rename, docs
JMS55 Nov 12, 2023
0e0cf83
Doc clarification
JMS55 Nov 12, 2023
d7796cf
Refactor
JMS55 Nov 12, 2023
bb1b9f1
Merge commit 'ea01c3e387d80994662aa54c2b8f70dd678903c3' into unload_r…
JMS55 Nov 22, 2023
22588ad
Send asset removal event on last strong handle drop, even if the asse…
JMS55 Nov 22, 2023
d28a695
Revert previous dumb commit
JMS55 Nov 22, 2023
3111a57
Add TODO
JMS55 Nov 22, 2023
63055d8
Remove Assets::remove
JMS55 Nov 22, 2023
29b1c71
Clarify doc
JMS55 Nov 22, 2023
5ce91e1
Add NoLongerUsed event
JMS55 Nov 22, 2023
6fe128d
Fix docs
JMS55 Nov 22, 2023
b91ddd7
Fix docs
JMS55 Nov 23, 2023
2d1a5c5
Read asset info for skip
JMS55 Nov 23, 2023
624fe80
Fixes
JMS55 Nov 23, 2023
9d7d16f
Fix tests
JMS55 Nov 23, 2023
1b1eb32
Misc
JMS55 Nov 27, 2023
18ffd40
Fix examples
JMS55 Nov 28, 2023
239b6a3
Ignore clippy
JMS55 Nov 28, 2023
e3de7ce
Merge commit 'a902ea6f85079777e61ea8627d86277c0d9abaf1' into unload_r…
JMS55 Nov 28, 2023
d00e845
Missed a clippy lint
JMS55 Nov 28, 2023
9829eec
Missed more clippy lints again
JMS55 Nov 28, 2023
9b47b6b
Remove unused import
JMS55 Nov 28, 2023
9478355
Fixes
JMS55 Nov 28, 2023
9c0f3bc
Add missed image loader
JMS55 Nov 28, 2023
aa538e2
Fix doc links
JMS55 Nov 28, 2023
19cd2bd
Fix examples
JMS55 Nov 28, 2023
23cf06f
Fix more examples
JMS55 Nov 28, 2023
69061f8
Merge remote-tracking branch 'bevy/gh-readonly-queue/main/pr-10785-0f…
JMS55 Nov 29, 2023
ae035b5
Add image loader persistence setting
JMS55 Dec 3, 2023
03c33de
Fixes
JMS55 Dec 3, 2023
d083e30
Appease clippy
JMS55 Dec 3, 2023
400f324
Merge commit '4d42713e7713bd8abaa3fe3164574b5b6895f40e' into unload_r…
JMS55 Dec 3, 2023
8b863af
Merge commit '11065974d487ca4b9680ae683a9d356f2f1b6c36' into unload_r…
JMS55 Dec 14, 2023
2c9c529
Merge commit '8067e46049f222d37ac394745805bad98979980f' into unload_r…
JMS55 Dec 28, 2023
e1f78c9
Use enum instead of bool
JMS55 Dec 28, 2023
8c92266
Misc changes
JMS55 Dec 28, 2023
6acc91d
Format
JMS55 Dec 28, 2023
6bcc067
Fix example imports
JMS55 Dec 28, 2023
9bb0f46
Fix tests and docs
JMS55 Dec 29, 2023
fb45b81
Rename
JMS55 Dec 30, 2023
ccb8afe
Tweak doc comment
JMS55 Dec 30, 2023
9088603
Merge commit '786abbf3f5e5be4b89c6b53d2d03162079f8e1f4' into unload_r…
JMS55 Dec 30, 2023
ea5af0d
Add missing import
JMS55 Dec 30, 2023
fec04d1
Fix doc
JMS55 Dec 30, 2023
8538833
Rename to Unused
JMS55 Jan 3, 2024
16e320e
Finish rename
JMS55 Jan 3, 2024
6efbb67
Fix another name I missed
JMS55 Jan 3, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions crates/bevy_asset/src/assets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,15 @@ impl<A: Asset> Assets<A> {
}
}

/// Removes the [`Asset`] with the given `id`, if its exists. This always emits [`AssetEvent::Removed`], regardless
/// of whether or not the asset exists.
/// Note that this supports anything that implements `Into<AssetId<A>>`, which includes [`Handle`] and [`AssetId`].
fn remove_always_emit_event(&mut self, id: impl Into<AssetId<A>>) {
let id = id.into();
self.remove_untracked(id);
self.queued_events.push(AssetEvent::Removed { id });
}

/// Returns `true` if there are no assets in this collection.
pub fn is_empty(&self) -> bool {
self.dense_storage.is_empty() && self.hash_map.is_empty()
Expand Down Expand Up @@ -466,18 +475,21 @@ impl<A: Asset> Assets<A> {
let mut not_ready = Vec::new();
while let Ok(drop_event) = assets.handle_provider.drop_receiver.try_recv() {
let id = drop_event.id;
if !assets.contains(id.typed()) {

if !drop_event.last_strong_handle {
not_ready.push(drop_event);
continue;
}

if drop_event.asset_server_managed {
if infos.process_handle_drop(id.untyped(TypeId::of::<A>())) {
assets.remove(id.typed());
assets.remove_always_emit_event(id.typed());
}
} else {
assets.remove(id.typed());
assets.remove_always_emit_event(id.typed());
}
}

// TODO: this is _extremely_ inefficient find a better fix
// This will also loop failed assets indefinitely. Is that ok?
for event in not_ready {
Expand Down
26 changes: 15 additions & 11 deletions crates/bevy_asset/src/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ pub struct AssetHandleProvider {

pub(crate) struct DropEvent {
pub(crate) id: InternalAssetId,
pub(crate) last_strong_handle: bool,
pub(crate) asset_server_managed: bool,
}

Expand Down Expand Up @@ -92,15 +93,6 @@ pub struct StrongHandle {
pub(crate) drop_sender: Sender<DropEvent>,
}

impl Drop for StrongHandle {
fn drop(&mut self) {
let _ = self.drop_sender.send(DropEvent {
id: self.id.internal(),
asset_server_managed: self.asset_server_managed,
});
}
}

impl std::fmt::Debug for StrongHandle {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("StrongHandle")
Expand All @@ -124,7 +116,7 @@ impl std::fmt::Debug for StrongHandle {
#[reflect(Component)]
pub enum Handle<A: Asset> {
/// A "strong" reference to a live (or loading) [`Asset`]. If a [`Handle`] is [`Handle::Strong`], the [`Asset`] will be kept
/// alive until the [`Handle`] is dropped. Strong handles also provide access to additional asset metadata.
/// alive until the [`Handle`] is dropped. Strong handles also provide access to additional asset metadata.
Strong(Arc<StrongHandle>),
/// A "weak" reference to an [`Asset`]. If a [`Handle`] is [`Handle::Weak`], it does not necessarily reference a live [`Asset`],
/// nor will it keep assets alive.
Expand Down Expand Up @@ -189,13 +181,25 @@ impl<A: Asset> Handle<A> {

/// Converts this [`Handle`] to an "untyped" / "generic-less" [`UntypedHandle`], which stores the [`Asset`] type information
/// _inside_ [`UntypedHandle`]. This will return [`UntypedHandle::Strong`] for [`Handle::Strong`] and [`UntypedHandle::Weak`] for
/// [`Handle::Weak`].
/// [`Handle::Weak`].
#[inline]
pub fn untyped(self) -> UntypedHandle {
self.into()
}
}

impl<T: Asset> Drop for Handle<T> {
fn drop(&mut self) {
if let Self::Strong(strong_handle) = self {
let _ = strong_handle.drop_sender.send(DropEvent {
id: strong_handle.id.internal(),
last_strong_handle: Arc::strong_count(strong_handle) == 1,
asset_server_managed: strong_handle.asset_server_managed,
});
}
}
}

impl<A: Asset> Default for Handle<A> {
fn default() -> Self {
Handle::Weak(AssetId::default())
Expand Down
1 change: 1 addition & 0 deletions crates/bevy_core_pipeline/src/tonemapping/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -412,5 +412,6 @@ pub fn lut_placeholder() -> Image {
},
sampler: ImageSampler::Default,
texture_view_descriptor: None,
cpu_persistent_access: false,
}
}
19 changes: 8 additions & 11 deletions crates/bevy_gizmos/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -372,28 +372,25 @@ struct GpuLineGizmo {
}

impl RenderAsset for LineGizmo {
type ExtractedAsset = LineGizmo;

type PreparedAsset = GpuLineGizmo;

type Param = SRes<RenderDevice>;

fn extract_asset(&self) -> Self::ExtractedAsset {
self.clone()
fn unload_after_extract(&self) -> bool {
false
}

fn prepare_asset(
line_gizmo: Self::ExtractedAsset,
self,
render_device: &mut SystemParamItem<Self::Param>,
) -> Result<Self::PreparedAsset, PrepareAssetError<Self::ExtractedAsset>> {
let position_buffer_data = cast_slice(&line_gizmo.positions);
) -> Result<Self::PreparedAsset, PrepareAssetError<Self>> {
let position_buffer_data = cast_slice(&self.positions);
let position_buffer = render_device.create_buffer_with_data(&BufferInitDescriptor {
usage: BufferUsages::VERTEX,
label: Some("LineGizmo Position Buffer"),
contents: position_buffer_data,
});

let color_buffer_data = cast_slice(&line_gizmo.colors);
let color_buffer_data = cast_slice(&self.colors);
let color_buffer = render_device.create_buffer_with_data(&BufferInitDescriptor {
usage: BufferUsages::VERTEX,
label: Some("LineGizmo Color Buffer"),
Expand All @@ -403,8 +400,8 @@ impl RenderAsset for LineGizmo {
Ok(GpuLineGizmo {
position_buffer,
color_buffer,
vertex_count: line_gizmo.positions.len() as u32,
strip: line_gizmo.strip,
vertex_count: self.positions.len() as u32,
strip: self.strip,
})
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_gltf/src/loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ async fn load_gltf<'a, 'b, 'c>(
let primitive_label = primitive_label(&gltf_mesh, &primitive);
let primitive_topology = get_primitive_topology(primitive.mode())?;

let mut mesh = Mesh::new(primitive_topology);
let mut mesh = Mesh::new(primitive_topology, false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we make this a flag in the GltfLoaderSettings (or make settings.load_meshes into an enum maybe)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather not, I think. Users should instead find the specific meshes/textures after loading, and set those ones specifically to Keep.


// Read vertex attributes
for (semantic, accessor) in primitive.attributes() {
Expand Down
46 changes: 27 additions & 19 deletions crates/bevy_render/src/mesh/mesh/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ pub const VERTEX_ATTRIBUTE_BUFFER_ID: u64 = 10;
/// # use bevy_render::render_resource::PrimitiveTopology;
/// fn create_simple_parallelogram() -> Mesh {
/// // Create a new mesh using a triangle list topology, where each set of 3 vertices composes a triangle.
/// Mesh::new(PrimitiveTopology::TriangleList)
/// Mesh::new(PrimitiveTopology::TriangleList, false)
/// // Add 4 vertices, each with its own position attribute (coordinate in
/// // 3D space), for each of the corners of the parallelogram.
/// .with_inserted_attribute(
Expand Down Expand Up @@ -108,8 +108,6 @@ pub const VERTEX_ATTRIBUTE_BUFFER_ID: u64 = 10;
/// - Vertex winding order: by default, `StandardMaterial.cull_mode` is [`Some(Face::Back)`](crate::render_resource::Face),
/// which means that Bevy would *only* render the "front" of each triangle, which
/// is the side of the triangle from where the vertices appear in a *counter-clockwise* order.
///
// TODO: allow values to be unloaded after been submitting to the GPU to conserve memory
#[derive(Asset, Debug, Clone, Reflect)]
pub struct Mesh {
#[reflect(ignore)]
Expand All @@ -123,6 +121,17 @@ pub struct Mesh {
indices: Option<Indices>,
morph_targets: Option<Handle<Image>>,
morph_target_names: Option<Vec<String>>,
/// If false, this asset will be unloaded from `Assets<Mesh>` via `remove_untracked()`
/// once it has been uploaded to the GPU.
///
/// This saves on RAM usage by not keeping a redundant copy of the mesh in memory once
/// it's in the GPU's VRAM.
///
/// The asset unloading will only take place once the asset has been added to `Assets<Mesh>`
/// and a frame has passed. You do not need to set this flag to true in order to build the
/// mesh initially. This flag only controls whether or not the asset is accessible via `Assets<Mesh>`
/// in future frames, once it's been added to `Assets<Mesh>`.
pub cpu_persistent_access: bool,
}

impl Mesh {
Expand Down Expand Up @@ -180,13 +189,14 @@ impl Mesh {
/// Construct a new mesh. You need to provide a [`PrimitiveTopology`] so that the
/// renderer knows how to treat the vertex data. Most of the time this will be
/// [`PrimitiveTopology::TriangleList`].
pub fn new(primitive_topology: PrimitiveTopology) -> Self {
pub fn new(primitive_topology: PrimitiveTopology, cpu_persistent_access: bool) -> Self {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if new assumed cpu_persistent_access = false, and a second constructor, new_with_persistence (name chosen arbitrarily) was used for cases where you want to choose that value? It would reduce the amount of noise in this commit without hurting usability.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only thing I'm worried about with that is that it's non-obvious that the asset is going to disappear. I want users to be aware and consciously consider whether they need to keep the asset or not.

I'm open to either option though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a good point. As long as it's for a good reason (which I think that concern is), the breaking change is worth having.

Mesh {
primitive_topology,
attributes: Default::default(),
indices: None,
morph_targets: None,
morph_target_names: None,
cpu_persistent_access,
}
}

Expand Down Expand Up @@ -1052,50 +1062,48 @@ pub enum GpuBufferInfo {
}

impl RenderAsset for Mesh {
type ExtractedAsset = Mesh;
type PreparedAsset = GpuMesh;
type Param = (SRes<RenderDevice>, SRes<RenderAssets<Image>>);

/// Clones the mesh.
fn extract_asset(&self) -> Self::ExtractedAsset {
self.clone()
fn unload_after_extract(&self) -> bool {
!self.cpu_persistent_access
}

/// Converts the extracted mesh a into [`GpuMesh`].
fn prepare_asset(
mesh: Self::ExtractedAsset,
self,
(render_device, images): &mut SystemParamItem<Self::Param>,
) -> Result<Self::PreparedAsset, PrepareAssetError<Self::ExtractedAsset>> {
let vertex_buffer_data = mesh.get_vertex_buffer_data();
) -> Result<Self::PreparedAsset, PrepareAssetError<Self>> {
let vertex_buffer_data = self.get_vertex_buffer_data();
let vertex_buffer = render_device.create_buffer_with_data(&BufferInitDescriptor {
usage: BufferUsages::VERTEX,
label: Some("Mesh Vertex Buffer"),
contents: &vertex_buffer_data,
});

let buffer_info = if let Some(data) = mesh.get_index_buffer_bytes() {
let buffer_info = if let Some(data) = self.get_index_buffer_bytes() {
GpuBufferInfo::Indexed {
buffer: render_device.create_buffer_with_data(&BufferInitDescriptor {
usage: BufferUsages::INDEX,
contents: data,
label: Some("Mesh Index Buffer"),
}),
count: mesh.indices().unwrap().len() as u32,
index_format: mesh.indices().unwrap().into(),
count: self.indices().unwrap().len() as u32,
index_format: self.indices().unwrap().into(),
}
} else {
GpuBufferInfo::NonIndexed
};

let mesh_vertex_buffer_layout = mesh.get_mesh_vertex_buffer_layout();
let mesh_vertex_buffer_layout = self.get_mesh_vertex_buffer_layout();

Ok(GpuMesh {
vertex_buffer,
vertex_count: mesh.count_vertices() as u32,
vertex_count: self.count_vertices() as u32,
buffer_info,
primitive_topology: mesh.primitive_topology(),
primitive_topology: self.primitive_topology(),
layout: mesh_vertex_buffer_layout,
morph_targets: mesh
morph_targets: self
.morph_targets
.and_then(|mt| images.get(&mt).map(|i| i.texture_view.clone())),
})
Expand Down Expand Up @@ -1237,7 +1245,7 @@ mod tests {
#[test]
#[should_panic]
fn panic_invalid_format() {
let _mesh = Mesh::new(PrimitiveTopology::TriangleList)
let _mesh = Mesh::new(PrimitiveTopology::TriangleList, false)
.with_inserted_attribute(Mesh::ATTRIBUTE_UV_0, vec![[0.0, 0.0, 0.0]]);
}
}
2 changes: 1 addition & 1 deletion crates/bevy_render/src/mesh/shape/capsule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ impl From<Capsule> for Mesh {
assert_eq!(vs.len(), vert_len);
assert_eq!(tris.len(), fs_len);

Mesh::new(PrimitiveTopology::TriangleList)
Mesh::new(PrimitiveTopology::TriangleList, false)
.with_inserted_attribute(Mesh::ATTRIBUTE_POSITION, vs)
.with_inserted_attribute(Mesh::ATTRIBUTE_NORMAL, vns)
.with_inserted_attribute(Mesh::ATTRIBUTE_UV_0, vts)
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_render/src/mesh/shape/cylinder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ impl From<Cylinder> for Mesh {
build_cap(true);
build_cap(false);

Mesh::new(PrimitiveTopology::TriangleList)
Mesh::new(PrimitiveTopology::TriangleList, false)
.with_indices(Some(Indices::U32(indices)))
.with_inserted_attribute(Mesh::ATTRIBUTE_POSITION, positions)
.with_inserted_attribute(Mesh::ATTRIBUTE_NORMAL, normals)
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_render/src/mesh/shape/icosphere.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ impl TryFrom<Icosphere> for Mesh {

let indices = Indices::U32(indices);

Ok(Mesh::new(PrimitiveTopology::TriangleList)
Ok(Mesh::new(PrimitiveTopology::TriangleList, false)
.with_indices(Some(indices))
.with_inserted_attribute(Mesh::ATTRIBUTE_POSITION, points)
.with_inserted_attribute(Mesh::ATTRIBUTE_NORMAL, normals)
Expand Down
6 changes: 3 additions & 3 deletions crates/bevy_render/src/mesh/shape/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ impl From<Box> for Mesh {
20, 21, 22, 22, 23, 20, // bottom
]);

Mesh::new(PrimitiveTopology::TriangleList)
Mesh::new(PrimitiveTopology::TriangleList, false)
.with_inserted_attribute(Mesh::ATTRIBUTE_POSITION, positions)
.with_inserted_attribute(Mesh::ATTRIBUTE_NORMAL, normals)
.with_inserted_attribute(Mesh::ATTRIBUTE_UV_0, uvs)
Expand Down Expand Up @@ -172,7 +172,7 @@ impl From<Quad> for Mesh {
let normals: Vec<_> = vertices.iter().map(|(_, n, _)| *n).collect();
let uvs: Vec<_> = vertices.iter().map(|(_, _, uv)| *uv).collect();

Mesh::new(PrimitiveTopology::TriangleList)
Mesh::new(PrimitiveTopology::TriangleList, false)
.with_indices(Some(indices))
.with_inserted_attribute(Mesh::ATTRIBUTE_POSITION, positions)
.with_inserted_attribute(Mesh::ATTRIBUTE_NORMAL, normals)
Expand Down Expand Up @@ -253,7 +253,7 @@ impl From<Plane> for Mesh {
}
}

Mesh::new(PrimitiveTopology::TriangleList)
Mesh::new(PrimitiveTopology::TriangleList, false)
.with_indices(Some(Indices::U32(indices)))
.with_inserted_attribute(Mesh::ATTRIBUTE_POSITION, positions)
.with_inserted_attribute(Mesh::ATTRIBUTE_NORMAL, normals)
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_render/src/mesh/shape/regular_polygon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ impl From<RegularPolygon> for Mesh {
indices.extend_from_slice(&[0, i + 1, i]);
}

Mesh::new(PrimitiveTopology::TriangleList)
Mesh::new(PrimitiveTopology::TriangleList, false)
.with_inserted_attribute(Mesh::ATTRIBUTE_POSITION, positions)
.with_inserted_attribute(Mesh::ATTRIBUTE_NORMAL, normals)
.with_inserted_attribute(Mesh::ATTRIBUTE_UV_0, uvs)
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_render/src/mesh/shape/torus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ impl From<Torus> for Mesh {
}
}

Mesh::new(PrimitiveTopology::TriangleList)
Mesh::new(PrimitiveTopology::TriangleList, false)
.with_indices(Some(Indices::U32(indices)))
.with_inserted_attribute(Mesh::ATTRIBUTE_POSITION, positions)
.with_inserted_attribute(Mesh::ATTRIBUTE_NORMAL, normals)
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_render/src/mesh/shape/uvsphere.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ impl From<UVSphere> for Mesh {
}
}

Mesh::new(PrimitiveTopology::TriangleList)
Mesh::new(PrimitiveTopology::TriangleList, false)
.with_indices(Some(Indices::U32(indices)))
.with_inserted_attribute(Mesh::ATTRIBUTE_POSITION, vertices)
.with_inserted_attribute(Mesh::ATTRIBUTE_NORMAL, normals)
Expand Down
Loading