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

Mesh: added static and dynamic mesh for less runtime memory usage #1782

Closed
wants to merge 9 commits into from

Conversation

julhe
Copy link
Contributor

@julhe julhe commented Mar 29, 2021

This addresses the unloading issue in #756.

Problem

Any mesh that is created will keep it's data (vertex attributes and indices) in the memory. This data is however never needed, unless the user wishes to make changes at runtime.

Changes

  • All mesh data will be unloaded after beeing uploaded to the GPU. If the user want's runtime changes hes has to use mesh::new_dynamic().
  • Internally, a new enum called MeshDataState indicates wther the mesh is static or dynamic.
  • mesh now keep direct track of changes to mesh::attributes and mesh::indices.
  • Remove mesh::indices_mut and mesh::attribute_mut, because any changes to these can't be tracked. If the user wishes to make chages to existing data, he has to clone them first from the immutable getter.

@julhe julhe changed the title Mesh: Static and Dynamic for Mesh: added static and dynamic-mesh for less runtime memory usage Mar 29, 2021
@julhe julhe changed the title Mesh: added static and dynamic-mesh for less runtime memory usage Mesh: added static and dynamic mesh for less runtime memory usage Mar 29, 2021
@julhe julhe mentioned this pull request Mar 29, 2021
7 tasks
@julhe julhe marked this pull request as draft March 29, 2021 01:53
@inodentry
Copy link
Contributor

When this gets merged, we should probably also implement something similar for Texture data. There is no need to keep texture data (and any mipmaps) in system RAM after copying to the GPU, if the texture will not be modified.

@julhe
Copy link
Contributor Author

julhe commented Mar 29, 2021

Forgot to add:

The current API will just print an error if, for example,set_attribute is called but the mesh is GPU only. This is similary to how Unity deals with the problem, but might not be super rust idiomatic?

I tried other API aproaches, like having a extra struct MeshData that you can borrow from mesh, but there is no way to check when that borrow returns to update vertex_layout or indices_count etc. . Could also let that struct MeshDataborrowable over a scope wiht fn(&mut meshData : MeshData) though, but maybe afterall this non-idomatic problem isn't big enough to justify this amount of extra code... 🤷 What do you people think?

@alice-i-cecile alice-i-cecile added C-Performance A change motivated by improving speed, memory usage or compile times A-Rendering Drawing game state to the screen labels Mar 29, 2021
Julian Heinken added 3 commits March 30, 2021 21:49
Separated Mesh internally into MeshData and MeshMetaInfo
Added enum MeshDataState
Adapted examples and shape crate to new Mesh API
@julhe
Copy link
Contributor Author

julhe commented Mar 30, 2021

Round 2!

  • Separated Mesh internally into MeshData and MeshMetaInfo
  • Creating any mesh requieres the MeshDatato be defined up-front. Example
let mut mesh_data = MeshData::default();
mesh_data.set_attribute(MeshData::ATTRIBUTE_POSITION, vec![[1.0, 0.0, 0.0], [0.0, 1.0, 0.0], [1.0, 1.0, 0.0]]);
mesh_data.set_indices(Some(Indices::U32(vec![0,1,2])));
Mesh::new_static(PrimitiveTopology::TriangleList, mesh_data)
  • Altering mesh data happens now via Mesh::get_mesh_data_mut() which returns a Option<MeshData>, whether the mesh is static or dynamic. Example for altering a dynamic mesh: myMesh.get_mesh_data_mut().unwrap().set_attribute("Vertex_Position", new_vertex_data).
  • The vertice count, etc. is now acquired via myMesh.meta().get_vertex_count().
  • Adapted examples and bevy_shape crate to new Mesh API. Shapes are now always getting created as dynamically.

Open Questions

  • Indices and attributes can't be borrowed mutable anymore, because:
    • The meta data is only updated uppon set_indices or set_attribute. (Changeable)
    • Changes made can't be validated and could even crash bevy.
  • To work arround this, the user would need to get a temporarly copy of the mesh first, which needs additional resources.
  • Is there a use case where this is a problem? Could only think of enviroments with low-RAM, but is there something else?

@julhe julhe marked this pull request as ready for review March 31, 2021 15:37
@julhe
Copy link
Contributor Author

julhe commented Apr 30, 2021

Any futher feedback on this?

Copy link
Member

@cart cart left a comment

Choose a reason for hiding this comment

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

In the context of the current renderer I'm pretty much sold on this impl. However moving to "pipelined rendering" creates new "asset dataflow" problems, such as the need to share the "same" asset across parallel contexts. rafx gets around this by just making these shared assets immutable Arcs. We could do something similar, but I'm not convinced thats a good design for things like dynamic meshes with large numbers of vertices or constantly changing materials, due to the need for expensive "copy on write" behavior.

The "handoff" style works well for pipelined access to "immutable" meshes and could be done without Arcs, which is pretty cool. But it still creates the "copy on write" requirement for Meshes with the OnGpuAndLocal setting, unless we decide to fully copy mesh data during the "extract" step (the point where game updates are paused to copy data over to the renderer). A full copy of a large mesh is a relatively expensive operation to do in the "extract" step (which is supposed to take as little time as possible because it blocks the ability to start work on the next frame in parallel).

Long story short, I currently have no clue what the best approach to dataflow is in the event that we adopt a "pipelined renderer" (which its looking like we will. check out recent #renderer conversations on discord if you havent already). And "Mesh data flow" will almost certainly be informed by "pipelined rendering".

I'm curious to hear your thoughts on that.

Indices and attributes can't be borrowed mutable anymore, because:
The meta data is only updated uppon set_indices or set_attribute. (Changeable)
Changes made can't be validated and could even crash bevy.
To work arround this, the user would need to get a temporarly copy of the mesh first, which needs additional resources.

This also relates to pipelined rendering, which depending on the asset-sharing approach we choose might force us to adopt a copy-on-write model anyway.

However I think the "editing a very large Mesh without massive copies" problem is real. If someone has a 100 MB terrain mesh that they want to dynamically update, we probably shouldn't be copying it every time it changes (although they also shouldn't be doing a full upload to the gpu each time, but thats a separate problem to solve).

self.vertex_layout.clone()
}

pub fn get_vertices_count(&self) -> usize {
Copy link
Member

Choose a reason for hiding this comment

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

lets make these names "accessor style". Ex: fn get_vertices_count(&self) -> usize -> fn vertex_count(&self) -> usize

self.primitive_topology
}

pub fn get_mesh_data(&self) -> Option<&MeshData> {
Copy link
Member

Choose a reason for hiding this comment

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

I think I like fn data(&self) more than fn get_mesh_data(&self)

mut mesh_events: EventReader<AssetEvent<Mesh>>,
mut queries: QuerySet<(
Query<&mut RenderPipelines, With<Handle<Mesh>>>,
Query<(Entity, &Handle<Mesh>, &mut RenderPipelines), Changed<Handle<Mesh>>>,
)>,
) {
let mut changed_meshes = HashSet::default();
let mut meshes_to_upload_to_gpu = HashSet::default();
let mut meshes_to_locally_unload = HashSet::default();
Copy link
Member

Choose a reason for hiding this comment

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

Nit: i think it makes sense to move this right before line 528 (i like to declare variables as close to their first usage as possible)

]),
);
cube_with_vertex_colors
.get_mesh_data_mut()
Copy link
Member

Choose a reason for hiding this comment

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

We could circumvent this "convert to mesh first then extract" pattern if we impl From<Cube> for MeshData and then From<MeshData> for Mesh. That would give us a better dataflow for cases like this and shouldn't affect ergonomics. We'd need to move PrimitiveTopology into MeshData, which I'm personally ok with. Meshes are constructed with a particular PrimitiveTopology (or at least, a particular type of topology algorithm) in mind, and its not like we support re-using MeshData across meshes anyway.

@julhe
Copy link
Contributor Author

julhe commented May 9, 2021

I gave this some thoughts:
We're always dealing with two representations of the Mesh on CPU side:

  1. CPU form: bunch of arrays for each attribute like MyNormals: Vec<Vec3>.
  2. GPU form: big array for all attributes in interleaved form like in MyVertexData: Vec<u8> + a vertex layout descriptor.

In my current implementation we're (optionally) carrying 1. inside Mesh and convert it into 2. once the mesh-system is called.

But I now see this as a problem (see your 100MB mesh example), because the user has it's own, likely more efficient, representation of f.e. a terrain system already. So if the user commits his changes to Mesh we would have up three different representations simultaneously in memory (user, CPU, GPU). 😬

I would therefore suggest to remove any possibility to store a CPU-Mesh in Mesh entirely and only allow GPU meshes in byte form.

struct MeshBlob{
    pub vertex_bytes: &[u8],
    pub index_bytes: &[u8],
    pub meta: MeshMeta,
}

Dealing with raw GPU data has their own pitfalls, so I wouldn't want most user to deal with them directly, and instead offer some API for common cases.

let my_mesh_blob = MeshBlob::from_attributes(my_positions, Some(my_normals),/*...*/).unwrap();
mesh.set_gpu_data(my_mesh_blob);

In the case of the dynamic terrain, the user would just write his own MeshBlob creator. The GLTF loader would use MeshBlob::from_attributes().
This approach would solve a ton of problems:

  • Memory consumption is minimized
  • Less data would be moved around (see User Mesh -> CPU Mesh -> GPU Mesh)
  • Vertex data can easily be validated (either by MeshBlob::from_attributes or Mesh::set_gpu_data)
  • Missing attributes can easily be filled with defaults (fixes many panics by missing attributes (sorry for that!))
  • Unloading of mesh data is done in a reasonable way (once the blob is uploaded to the GPU)
  • Attribute packaging and compression can be realized (See Mesh API overhaul (#599 Followup) #756)
  • MeshBlobs can be serialized for faster loading times in builds
  • Vertex-Layouts are more predictable
    • Less Render-pipeline permutations
    • Makes it easier to write a custom shader
  • Cleaner API of Mesh

This would mean that I remove all of the API that deals with the CPU-Mesh and also change the scope of the PR quite a bit.

Downside (as far as I can see):

  • Loaded meshes can't be manipulated, since the user will never get the chance to access it. (just offer additional asset API?)
  • MeshBlob::from_attributes needs some internal logic to choose from multiple VertexLayouts. Otherwise we would get overkill meshes that define only f.e. positions and normals, but store four channels of UVs 😃 .
  • Practically, only one system can write to the mesh. Third Party systems that change a mesh would be difficult. (Altrough they could be implemented as static function).

@cart cart added the S-Pre-Relicense This PR was made before Bevy added the Apache license. Cannot be merged or used for other work label Jul 23, 2021
@garyttierney
Copy link

Hi @julhe, @cart!

I'm looking at reducing memory usage for my prototype and the quickest win was picking this up, just looking for some feedback on the approach based on the comments above. I've added support for meshes with vertex data that is already interleaved and unloading that data after an upload to the GPU like was done here. Now I'm optimizing for this case:

However I think the "editing a very large Mesh without massive copies" problem is real. If someone has a 100 MB terrain mesh that they want to dynamically update, we probably shouldn't be copying it every time it changes (although they also shouldn't be doing a full upload to the gpu each time, but thats a separate problem to solve).

At the moment I've went with 3 types of VertexStorage strategies and extended RenderAsset to allow an attempt to update existing PreparedAssets:

  • GpuAndLocal (buffer created with MAP_WRITE, limited to small meshes (my 1660 Ti only has 256 MiB of host coherent memory))
  • GpuCow (store changed data in a staging buffer and copy to the vertex buffer during extraction, changes to the underlying buffer are stored as buffer address ranges with and CPU buffers (Vec<u8>) of the modified data)
  • Cpu (full CPU->GPU re-uploads)

Under the hood in Mesh there's now an abstraction to determine where an attribute should be written to in its storage. This allows set_attributes() et al. to work with loose or interleaved data regardless of storage to preserve API usability, but I'm still improving the ergonomics of that (to e.g. avoid replacing the entire buffer when changing a handful of vertices and dealing with having no attribute names in interleaved data).

Every mesh is created with loose attributes by default and is converted to interleaved data on the first GPU upload, but the loose -> interleaved -> GPU copy is avoided by going from loose -> interleaved GPU storage and draining each loose attribute buffer as we go. From is implemented for LooseVertexData on InterleavedVertexData and vice versa, so converting back to a flat format to e.g. add more vertices is still possible, but as mentioned will fail with a GpuCow mesh.

To summarize:

  • CPU vertex data is unloaded after upload, optionally converted to a reference to a CoW staging buffer or host-coherent buffer
  • Vertex data can now be stored as interleaved or loose data
    • This is somewhat transparent to the user, who is still able to modify attributes.
  • Interleaved vertex data has several storage mechanisms
    • Host-coherent buffer (GpuAndLocal)
    • Staging buffer that tracks changed buffer regions (GpuCow)
    • Same full CPU->GPU copy that happens today (Cpu)

Some concerns:

  • you can't serialize a mesh to disk after the CPU data has been unloaded
  • care needs to be taken to modify GpuAndLocal meshes in the correct place, maybe it's better to expose dynamic mesh data through some other render system specific handle?
  • storing CPU buffers for GpuCow could be avoided by assuming the mesh can directly map the storage buffer, but introduces the above problem
  • GpuCow meshes can only be written to, not read
  • efficiency of GpuCow uploads is ultimately down to access patterns on the underlying vertex data

@DJMcNab DJMcNab removed the S-Pre-Relicense This PR was made before Bevy added the Apache license. Cannot be merged or used for other work label Aug 7, 2021
@alice-i-cecile alice-i-cecile added the X-Controversial There is active debate or serious implications around merging this PR label Apr 22, 2022
@JMS55
Copy link
Contributor

JMS55 commented Jan 27, 2023

Small request: rather than "dynamic" and "static", I would rather "mutable" and "immutable".

@inodentry
Copy link
Contributor

That rename might make sense. In the context of rendering, typically the terms "dynamic" and "static" are used for whether objects can move around the scene (whether their transform will change) or not. Hearing "static mesh optimization" makes my brain think about things like batching geometry and baking lightmaps, assuming that those meshes are at a fixed Transform forever.

@JMS55 JMS55 added this to the 0.11 milestone Mar 16, 2023
@JMS55
Copy link
Contributor

JMS55 commented Mar 16, 2023

Hi @julhe! Are you interested in continuing this PR? If not, I'll happily adopt it.

@JMS55 JMS55 removed this from the 0.11 milestone Mar 20, 2023
@julhe
Copy link
Contributor Author

julhe commented Mar 31, 2023

Hey @JMS55 sorry for the delay!

I'm fine with you taking over, but I would like to also recevie credit for the contribution. 🙂

@JMS55
Copy link
Contributor

JMS55 commented May 17, 2023

@julhe sorry, I just saw your response! Of course you will receive credit if I end up merging anything.

I tried some things and came to the conclusion that it would be better to wait for the asset rework to get finished #8624 rather then attempt this now, however.

@Phyyl
Copy link

Phyyl commented Sep 17, 2023

Hey @JMS55, just a simple ping here, checking if there's any updates on this. I'm interested in this feature and I'm offering my help if needed. Let me know! 🚀

@JMS55
Copy link
Contributor

JMS55 commented Sep 17, 2023

Assets v2 has been merged, which means it's now feasible to do something like this. However the way we handle meshes in general is likely to change soon as part of the ongoing rendering rewrites.

I have no plans to work on this myself at the moment, but if you're interested in working on it feel free to join #rendering-dev in the bevy discord and talk to us on what needs to be done :)

@Patryk27
Copy link

I just wanted to leave my two cents:

This data is however never needed, unless the user wishes to make changes at runtime.

I'm writing a Bevy-based path tracer and I need to have a constant access to those raw data in order to correctly build the BVH (one could argue BLAS / TLAS approach is better, but in my case I'm "flattening" meshes and instances into a list of triangles and build BVH out of that).

I'm not sure on the data flow, but if Bevy's to unload a mesh, it should at least give one frame for another systems (in particular inside RenderApp) to extract the mesh for storage on their own side (maybe the current implementation already does that, I haven't analyzed the code thoroughly) 🙂

@inodentry
Copy link
Contributor

I believe there should be an easy API toggle to choose whether data should be kept in system ram, or freed. There are valid use cases for both. If you need CPU-side code that deals with the data of meshes/textures/etc, you want them to be kept (in an ideal world, though, they could be put into unified memory on hardware that supports it (consoles, integrated gpus, etc). For most games, you want them to be freed.

The default for assets loaded from disk should be to free the data on the CPU side, but it should be possible to reconfigure that (both globally (like we can do with the default texture filtering, etc) and per-asset). Assets that are added from code should default to keeping the data.

@JMS55
Copy link
Contributor

JMS55 commented Nov 23, 2023

Closing in favor of #10520

@JMS55 JMS55 closed this Nov 23, 2023
github-merge-queue bot pushed a commit that referenced this pull request Jan 3, 2024
# Objective
- No point in keeping Meshes/Images in RAM once they're going to be sent
to the GPU, and kept in VRAM. This saves a _significant_ amount of
memory (several GBs) on scenes like bistro.
- References
  - #1782
  - #8624 

## Solution
- Augment RenderAsset with the capability to unload the underlying asset
after extracting to the render world.
- Mesh/Image now have a cpu_persistent_access field. If this field is
RenderAssetPersistencePolicy::Unload, the asset will be unloaded from
Assets<T>.
- A new AssetEvent is sent upon dropping the last strong handle for the
asset, which signals to the RenderAsset to remove the GPU version of the
asset.

---

## Changelog
- Added `AssetEvent::NoLongerUsed` and
`AssetEvent::is_no_longer_used()`. This event is sent when the last
strong handle of an asset is dropped.
- Rewrote the API for `RenderAsset` to allow for unloading the asset
data from the CPU.
- Added `RenderAssetPersistencePolicy`.
- Added `Mesh::cpu_persistent_access` for memory savings when the asset
is not needed except for on the GPU.
- Added `Image::cpu_persistent_access` for memory savings when the asset
is not needed except for on the GPU.
- Added `ImageLoaderSettings::cpu_persistent_access`.
- Added `ExrTextureLoaderSettings`.
- Added `HdrTextureLoaderSettings`.

## Migration Guide
- Asset loaders (GLTF, etc) now load meshes and textures without
`cpu_persistent_access`. These assets will be removed from
`Assets<Mesh>` and `Assets<Image>` once `RenderAssets<Mesh>` and
`RenderAssets<Image>` contain the GPU versions of these assets, in order
to reduce memory usage. If you require access to the asset data from the
CPU in future frames after the GLTF asset has been loaded, modify all
dependent `Mesh` and `Image` assets and set `cpu_persistent_access` to
`RenderAssetPersistencePolicy::Keep`.
- `Mesh` now requires a new `cpu_persistent_access` field. Set it to
`RenderAssetPersistencePolicy::Keep` to mimic the previous behavior.
- `Image` now requires a new `cpu_persistent_access` field. Set it to
`RenderAssetPersistencePolicy::Keep` to mimic the previous behavior.
- `MorphTargetImage::new()` now requires a new `cpu_persistent_access`
parameter. Set it to `RenderAssetPersistencePolicy::Keep` to mimic the
previous behavior.
- `DynamicTextureAtlasBuilder::add_texture()` now requires that the
`TextureAtlas` you pass has an `Image` with `cpu_persistent_access:
RenderAssetPersistencePolicy::Keep`. Ensure you construct the image
properly for the texture atlas.
- The `RenderAsset` trait has significantly changed, and requires
adapting your existing implementations.
  - The trait now requires `Clone`.
- The `ExtractedAsset` associated type has been removed (the type itself
is now extracted).
  - The signature of `prepare_asset()` is slightly different
- A new `persistence_policy()` method is now required (return
RenderAssetPersistencePolicy::Unload to match the previous behavior).
- Match on the new `NoLongerUsed` variant for exhaustive matches of
`AssetEvent`.
github-merge-queue bot pushed a commit that referenced this pull request Jan 30, 2024
# Objective

Right now, all assets in the main world get extracted and prepared in
the render world (if the asset's using the RenderAssetPlugin). This is
unfortunate for two cases:

1. **TextureAtlas** / **FontAtlas**: This one's huge. The individual
`Image` assets that make up the atlas are cloned and prepared
individually when there's no reason for them to be. The atlas textures
are built on the CPU in the main world. *There can be hundreds of images
that get prepared for rendering only not to be used.*
2. If one loads an Image and needs to transform it in a system before
rendering it, kind of like the [decompression
example](https://github.com/bevyengine/bevy/blob/main/examples/asset/asset_decompression.rs#L120),
there's a price paid for extracting & preparing the asset that's not
intended to be rendered yet.

------

* References #10520
* References #1782

## Solution

This changes the `RenderAssetPersistencePolicy` enum to bitflags. I felt
that the objective with the parameter is so similar in nature to wgpu's
[`TextureUsages`](https://docs.rs/wgpu/latest/wgpu/struct.TextureUsages.html)
and
[`BufferUsages`](https://docs.rs/wgpu/latest/wgpu/struct.BufferUsages.html),
that it may as well be just like that.

```rust
// This asset only needs to be in the main world. Don't extract and prepare it.
RenderAssetUsages::MAIN_WORLD

// Keep this asset in the main world and  
RenderAssetUsages::MAIN_WORLD | RenderAssetUsages::RENDER_WORLD

// This asset is only needed in the render world. Remove it from the asset server once extracted.
RenderAssetUsages::RENDER_WORLD
```

### Alternate Solution

I considered introducing a third field to `RenderAssetPersistencePolicy`
enum:
```rust
enum RenderAssetPersistencePolicy {
    /// Keep the asset in the main world after extracting to the render world.
    Keep,
    /// Remove the asset from the main world after extracting to the render world.
    Unload,
    /// This doesn't need to be in the render world at all.
    NoExtract, // <-----
}
```
Functional, but this seemed like shoehorning. Another option is renaming
the enum to something like:
```rust
enum RenderAssetExtractionPolicy {
    /// Extract the asset and keep it in the main world.
    Extract,
    /// Remove the asset from the main world after extracting to the render world.
    ExtractAndUnload,
    /// This doesn't need to be in the render world at all.
    NoExtract,
}
```
I think this last one could be a good option if the bitflags are too
clunky.

## Migration Guide

* `RenderAssetPersistencePolicy::Keep` → `RenderAssetUsage::MAIN_WORLD |
RenderAssetUsage::RENDER_WORLD` (or `RenderAssetUsage::default()`)
* `RenderAssetPersistencePolicy::Unload` →
`RenderAssetUsage::RENDER_WORLD`
* For types implementing the `RenderAsset` trait, change `fn
persistence_policy(&self) -> RenderAssetPersistencePolicy` to `fn
asset_usage(&self) -> RenderAssetUsages`.
* Change any references to `cpu_persistent_access`
(`RenderAssetPersistencePolicy`) to `asset_usage` (`RenderAssetUsage`).
This applies to `Image`, `Mesh`, and a few other types.
tjamaan pushed a commit to tjamaan/bevy that referenced this pull request Feb 6, 2024
# Objective

Right now, all assets in the main world get extracted and prepared in
the render world (if the asset's using the RenderAssetPlugin). This is
unfortunate for two cases:

1. **TextureAtlas** / **FontAtlas**: This one's huge. The individual
`Image` assets that make up the atlas are cloned and prepared
individually when there's no reason for them to be. The atlas textures
are built on the CPU in the main world. *There can be hundreds of images
that get prepared for rendering only not to be used.*
2. If one loads an Image and needs to transform it in a system before
rendering it, kind of like the [decompression
example](https://github.com/bevyengine/bevy/blob/main/examples/asset/asset_decompression.rs#L120),
there's a price paid for extracting & preparing the asset that's not
intended to be rendered yet.

------

* References bevyengine#10520
* References bevyengine#1782

## Solution

This changes the `RenderAssetPersistencePolicy` enum to bitflags. I felt
that the objective with the parameter is so similar in nature to wgpu's
[`TextureUsages`](https://docs.rs/wgpu/latest/wgpu/struct.TextureUsages.html)
and
[`BufferUsages`](https://docs.rs/wgpu/latest/wgpu/struct.BufferUsages.html),
that it may as well be just like that.

```rust
// This asset only needs to be in the main world. Don't extract and prepare it.
RenderAssetUsages::MAIN_WORLD

// Keep this asset in the main world and  
RenderAssetUsages::MAIN_WORLD | RenderAssetUsages::RENDER_WORLD

// This asset is only needed in the render world. Remove it from the asset server once extracted.
RenderAssetUsages::RENDER_WORLD
```

### Alternate Solution

I considered introducing a third field to `RenderAssetPersistencePolicy`
enum:
```rust
enum RenderAssetPersistencePolicy {
    /// Keep the asset in the main world after extracting to the render world.
    Keep,
    /// Remove the asset from the main world after extracting to the render world.
    Unload,
    /// This doesn't need to be in the render world at all.
    NoExtract, // <-----
}
```
Functional, but this seemed like shoehorning. Another option is renaming
the enum to something like:
```rust
enum RenderAssetExtractionPolicy {
    /// Extract the asset and keep it in the main world.
    Extract,
    /// Remove the asset from the main world after extracting to the render world.
    ExtractAndUnload,
    /// This doesn't need to be in the render world at all.
    NoExtract,
}
```
I think this last one could be a good option if the bitflags are too
clunky.

## Migration Guide

* `RenderAssetPersistencePolicy::Keep` → `RenderAssetUsage::MAIN_WORLD |
RenderAssetUsage::RENDER_WORLD` (or `RenderAssetUsage::default()`)
* `RenderAssetPersistencePolicy::Unload` →
`RenderAssetUsage::RENDER_WORLD`
* For types implementing the `RenderAsset` trait, change `fn
persistence_policy(&self) -> RenderAssetPersistencePolicy` to `fn
asset_usage(&self) -> RenderAssetUsages`.
* Change any references to `cpu_persistent_access`
(`RenderAssetPersistencePolicy`) to `asset_usage` (`RenderAssetUsage`).
This applies to `Image`, `Mesh`, and a few other types.
Maximetinu pushed a commit to Sophya/bevy that referenced this pull request Feb 12, 2024
- No point in keeping Meshes/Images in RAM once they're going to be sent
to the GPU, and kept in VRAM. This saves a _significant_ amount of
memory (several GBs) on scenes like bistro.
- References
  - bevyengine#1782
  - bevyengine#8624

- Augment RenderAsset with the capability to unload the underlying asset
after extracting to the render world.
- Mesh/Image now have a cpu_persistent_access field. If this field is
RenderAssetPersistencePolicy::Unload, the asset will be unloaded from
Assets<T>.
- A new AssetEvent is sent upon dropping the last strong handle for the
asset, which signals to the RenderAsset to remove the GPU version of the
asset.

---

- Added `AssetEvent::NoLongerUsed` and
`AssetEvent::is_no_longer_used()`. This event is sent when the last
strong handle of an asset is dropped.
- Rewrote the API for `RenderAsset` to allow for unloading the asset
data from the CPU.
- Added `RenderAssetPersistencePolicy`.
- Added `Mesh::cpu_persistent_access` for memory savings when the asset
is not needed except for on the GPU.
- Added `Image::cpu_persistent_access` for memory savings when the asset
is not needed except for on the GPU.
- Added `ImageLoaderSettings::cpu_persistent_access`.
- Added `ExrTextureLoaderSettings`.
- Added `HdrTextureLoaderSettings`.

- Asset loaders (GLTF, etc) now load meshes and textures without
`cpu_persistent_access`. These assets will be removed from
`Assets<Mesh>` and `Assets<Image>` once `RenderAssets<Mesh>` and
`RenderAssets<Image>` contain the GPU versions of these assets, in order
to reduce memory usage. If you require access to the asset data from the
CPU in future frames after the GLTF asset has been loaded, modify all
dependent `Mesh` and `Image` assets and set `cpu_persistent_access` to
`RenderAssetPersistencePolicy::Keep`.
- `Mesh` now requires a new `cpu_persistent_access` field. Set it to
`RenderAssetPersistencePolicy::Keep` to mimic the previous behavior.
- `Image` now requires a new `cpu_persistent_access` field. Set it to
`RenderAssetPersistencePolicy::Keep` to mimic the previous behavior.
- `MorphTargetImage::new()` now requires a new `cpu_persistent_access`
parameter. Set it to `RenderAssetPersistencePolicy::Keep` to mimic the
previous behavior.
- `DynamicTextureAtlasBuilder::add_texture()` now requires that the
`TextureAtlas` you pass has an `Image` with `cpu_persistent_access:
RenderAssetPersistencePolicy::Keep`. Ensure you construct the image
properly for the texture atlas.
- The `RenderAsset` trait has significantly changed, and requires
adapting your existing implementations.
  - The trait now requires `Clone`.
- The `ExtractedAsset` associated type has been removed (the type itself
is now extracted).
  - The signature of `prepare_asset()` is slightly different
- A new `persistence_policy()` method is now required (return
RenderAssetPersistencePolicy::Unload to match the previous behavior).
- Match on the new `NoLongerUsed` variant for exhaustive matches of
`AssetEvent`.
Maximetinu pushed a commit to Sophya/bevy that referenced this pull request Feb 12, 2024
Right now, all assets in the main world get extracted and prepared in
the render world (if the asset's using the RenderAssetPlugin). This is
unfortunate for two cases:

1. **TextureAtlas** / **FontAtlas**: This one's huge. The individual
`Image` assets that make up the atlas are cloned and prepared
individually when there's no reason for them to be. The atlas textures
are built on the CPU in the main world. *There can be hundreds of images
that get prepared for rendering only not to be used.*
2. If one loads an Image and needs to transform it in a system before
rendering it, kind of like the [decompression
example](https://github.com/bevyengine/bevy/blob/main/examples/asset/asset_decompression.rs#L120),
there's a price paid for extracting & preparing the asset that's not
intended to be rendered yet.

------

* References bevyengine#10520
* References bevyengine#1782

This changes the `RenderAssetPersistencePolicy` enum to bitflags. I felt
that the objective with the parameter is so similar in nature to wgpu's
[`TextureUsages`](https://docs.rs/wgpu/latest/wgpu/struct.TextureUsages.html)
and
[`BufferUsages`](https://docs.rs/wgpu/latest/wgpu/struct.BufferUsages.html),
that it may as well be just like that.

```rust
// This asset only needs to be in the main world. Don't extract and prepare it.
RenderAssetUsages::MAIN_WORLD

// Keep this asset in the main world and
RenderAssetUsages::MAIN_WORLD | RenderAssetUsages::RENDER_WORLD

// This asset is only needed in the render world. Remove it from the asset server once extracted.
RenderAssetUsages::RENDER_WORLD
```

I considered introducing a third field to `RenderAssetPersistencePolicy`
enum:
```rust
enum RenderAssetPersistencePolicy {
    /// Keep the asset in the main world after extracting to the render world.
    Keep,
    /// Remove the asset from the main world after extracting to the render world.
    Unload,
    /// This doesn't need to be in the render world at all.
    NoExtract, // <-----
}
```
Functional, but this seemed like shoehorning. Another option is renaming
the enum to something like:
```rust
enum RenderAssetExtractionPolicy {
    /// Extract the asset and keep it in the main world.
    Extract,
    /// Remove the asset from the main world after extracting to the render world.
    ExtractAndUnload,
    /// This doesn't need to be in the render world at all.
    NoExtract,
}
```
I think this last one could be a good option if the bitflags are too
clunky.

* `RenderAssetPersistencePolicy::Keep` → `RenderAssetUsage::MAIN_WORLD |
RenderAssetUsage::RENDER_WORLD` (or `RenderAssetUsage::default()`)
* `RenderAssetPersistencePolicy::Unload` →
`RenderAssetUsage::RENDER_WORLD`
* For types implementing the `RenderAsset` trait, change `fn
persistence_policy(&self) -> RenderAssetPersistencePolicy` to `fn
asset_usage(&self) -> RenderAssetUsages`.
* Change any references to `cpu_persistent_access`
(`RenderAssetPersistencePolicy`) to `asset_usage` (`RenderAssetUsage`).
This applies to `Image`, `Mesh`, and a few other types.
Maximetinu pushed a commit to Sophya/bevy that referenced this pull request Feb 12, 2024
- No point in keeping Meshes/Images in RAM once they're going to be sent
to the GPU, and kept in VRAM. This saves a _significant_ amount of
memory (several GBs) on scenes like bistro.
- References
  - bevyengine#1782
  - bevyengine#8624

- Augment RenderAsset with the capability to unload the underlying asset
after extracting to the render world.
- Mesh/Image now have a cpu_persistent_access field. If this field is
RenderAssetPersistencePolicy::Unload, the asset will be unloaded from
Assets<T>.
- A new AssetEvent is sent upon dropping the last strong handle for the
asset, which signals to the RenderAsset to remove the GPU version of the
asset.

---

- Added `AssetEvent::NoLongerUsed` and
`AssetEvent::is_no_longer_used()`. This event is sent when the last
strong handle of an asset is dropped.
- Rewrote the API for `RenderAsset` to allow for unloading the asset
data from the CPU.
- Added `RenderAssetPersistencePolicy`.
- Added `Mesh::cpu_persistent_access` for memory savings when the asset
is not needed except for on the GPU.
- Added `Image::cpu_persistent_access` for memory savings when the asset
is not needed except for on the GPU.
- Added `ImageLoaderSettings::cpu_persistent_access`.
- Added `ExrTextureLoaderSettings`.
- Added `HdrTextureLoaderSettings`.

- Asset loaders (GLTF, etc) now load meshes and textures without
`cpu_persistent_access`. These assets will be removed from
`Assets<Mesh>` and `Assets<Image>` once `RenderAssets<Mesh>` and
`RenderAssets<Image>` contain the GPU versions of these assets, in order
to reduce memory usage. If you require access to the asset data from the
CPU in future frames after the GLTF asset has been loaded, modify all
dependent `Mesh` and `Image` assets and set `cpu_persistent_access` to
`RenderAssetPersistencePolicy::Keep`.
- `Mesh` now requires a new `cpu_persistent_access` field. Set it to
`RenderAssetPersistencePolicy::Keep` to mimic the previous behavior.
- `Image` now requires a new `cpu_persistent_access` field. Set it to
`RenderAssetPersistencePolicy::Keep` to mimic the previous behavior.
- `MorphTargetImage::new()` now requires a new `cpu_persistent_access`
parameter. Set it to `RenderAssetPersistencePolicy::Keep` to mimic the
previous behavior.
- `DynamicTextureAtlasBuilder::add_texture()` now requires that the
`TextureAtlas` you pass has an `Image` with `cpu_persistent_access:
RenderAssetPersistencePolicy::Keep`. Ensure you construct the image
properly for the texture atlas.
- The `RenderAsset` trait has significantly changed, and requires
adapting your existing implementations.
  - The trait now requires `Clone`.
- The `ExtractedAsset` associated type has been removed (the type itself
is now extracted).
  - The signature of `prepare_asset()` is slightly different
- A new `persistence_policy()` method is now required (return
RenderAssetPersistencePolicy::Unload to match the previous behavior).
- Match on the new `NoLongerUsed` variant for exhaustive matches of
`AssetEvent`.
Maximetinu pushed a commit to Sophya/bevy that referenced this pull request Feb 12, 2024
Right now, all assets in the main world get extracted and prepared in
the render world (if the asset's using the RenderAssetPlugin). This is
unfortunate for two cases:

1. **TextureAtlas** / **FontAtlas**: This one's huge. The individual
`Image` assets that make up the atlas are cloned and prepared
individually when there's no reason for them to be. The atlas textures
are built on the CPU in the main world. *There can be hundreds of images
that get prepared for rendering only not to be used.*
2. If one loads an Image and needs to transform it in a system before
rendering it, kind of like the [decompression
example](https://github.com/bevyengine/bevy/blob/main/examples/asset/asset_decompression.rs#L120),
there's a price paid for extracting & preparing the asset that's not
intended to be rendered yet.

------

* References bevyengine#10520
* References bevyengine#1782

This changes the `RenderAssetPersistencePolicy` enum to bitflags. I felt
that the objective with the parameter is so similar in nature to wgpu's
[`TextureUsages`](https://docs.rs/wgpu/latest/wgpu/struct.TextureUsages.html)
and
[`BufferUsages`](https://docs.rs/wgpu/latest/wgpu/struct.BufferUsages.html),
that it may as well be just like that.

```rust
// This asset only needs to be in the main world. Don't extract and prepare it.
RenderAssetUsages::MAIN_WORLD

// Keep this asset in the main world and
RenderAssetUsages::MAIN_WORLD | RenderAssetUsages::RENDER_WORLD

// This asset is only needed in the render world. Remove it from the asset server once extracted.
RenderAssetUsages::RENDER_WORLD
```

I considered introducing a third field to `RenderAssetPersistencePolicy`
enum:
```rust
enum RenderAssetPersistencePolicy {
    /// Keep the asset in the main world after extracting to the render world.
    Keep,
    /// Remove the asset from the main world after extracting to the render world.
    Unload,
    /// This doesn't need to be in the render world at all.
    NoExtract, // <-----
}
```
Functional, but this seemed like shoehorning. Another option is renaming
the enum to something like:
```rust
enum RenderAssetExtractionPolicy {
    /// Extract the asset and keep it in the main world.
    Extract,
    /// Remove the asset from the main world after extracting to the render world.
    ExtractAndUnload,
    /// This doesn't need to be in the render world at all.
    NoExtract,
}
```
I think this last one could be a good option if the bitflags are too
clunky.

* `RenderAssetPersistencePolicy::Keep` → `RenderAssetUsage::MAIN_WORLD |
RenderAssetUsage::RENDER_WORLD` (or `RenderAssetUsage::default()`)
* `RenderAssetPersistencePolicy::Unload` →
`RenderAssetUsage::RENDER_WORLD`
* For types implementing the `RenderAsset` trait, change `fn
persistence_policy(&self) -> RenderAssetPersistencePolicy` to `fn
asset_usage(&self) -> RenderAssetUsages`.
* Change any references to `cpu_persistent_access`
(`RenderAssetPersistencePolicy`) to `asset_usage` (`RenderAssetUsage`).
This applies to `Image`, `Mesh`, and a few other types.
Maximetinu pushed a commit to Sophya/bevy that referenced this pull request Feb 12, 2024
- No point in keeping Meshes/Images in RAM once they're going to be sent
to the GPU, and kept in VRAM. This saves a _significant_ amount of
memory (several GBs) on scenes like bistro.
- References
  - bevyengine#1782
  - bevyengine#8624

- Augment RenderAsset with the capability to unload the underlying asset
after extracting to the render world.
- Mesh/Image now have a cpu_persistent_access field. If this field is
RenderAssetPersistencePolicy::Unload, the asset will be unloaded from
Assets<T>.
- A new AssetEvent is sent upon dropping the last strong handle for the
asset, which signals to the RenderAsset to remove the GPU version of the
asset.

---

- Added `AssetEvent::NoLongerUsed` and
`AssetEvent::is_no_longer_used()`. This event is sent when the last
strong handle of an asset is dropped.
- Rewrote the API for `RenderAsset` to allow for unloading the asset
data from the CPU.
- Added `RenderAssetPersistencePolicy`.
- Added `Mesh::cpu_persistent_access` for memory savings when the asset
is not needed except for on the GPU.
- Added `Image::cpu_persistent_access` for memory savings when the asset
is not needed except for on the GPU.
- Added `ImageLoaderSettings::cpu_persistent_access`.
- Added `ExrTextureLoaderSettings`.
- Added `HdrTextureLoaderSettings`.

- Asset loaders (GLTF, etc) now load meshes and textures without
`cpu_persistent_access`. These assets will be removed from
`Assets<Mesh>` and `Assets<Image>` once `RenderAssets<Mesh>` and
`RenderAssets<Image>` contain the GPU versions of these assets, in order
to reduce memory usage. If you require access to the asset data from the
CPU in future frames after the GLTF asset has been loaded, modify all
dependent `Mesh` and `Image` assets and set `cpu_persistent_access` to
`RenderAssetPersistencePolicy::Keep`.
- `Mesh` now requires a new `cpu_persistent_access` field. Set it to
`RenderAssetPersistencePolicy::Keep` to mimic the previous behavior.
- `Image` now requires a new `cpu_persistent_access` field. Set it to
`RenderAssetPersistencePolicy::Keep` to mimic the previous behavior.
- `MorphTargetImage::new()` now requires a new `cpu_persistent_access`
parameter. Set it to `RenderAssetPersistencePolicy::Keep` to mimic the
previous behavior.
- `DynamicTextureAtlasBuilder::add_texture()` now requires that the
`TextureAtlas` you pass has an `Image` with `cpu_persistent_access:
RenderAssetPersistencePolicy::Keep`. Ensure you construct the image
properly for the texture atlas.
- The `RenderAsset` trait has significantly changed, and requires
adapting your existing implementations.
  - The trait now requires `Clone`.
- The `ExtractedAsset` associated type has been removed (the type itself
is now extracted).
  - The signature of `prepare_asset()` is slightly different
- A new `persistence_policy()` method is now required (return
RenderAssetPersistencePolicy::Unload to match the previous behavior).
- Match on the new `NoLongerUsed` variant for exhaustive matches of
`AssetEvent`.
Maximetinu pushed a commit to Sophya/bevy that referenced this pull request Feb 12, 2024
Right now, all assets in the main world get extracted and prepared in
the render world (if the asset's using the RenderAssetPlugin). This is
unfortunate for two cases:

1. **TextureAtlas** / **FontAtlas**: This one's huge. The individual
`Image` assets that make up the atlas are cloned and prepared
individually when there's no reason for them to be. The atlas textures
are built on the CPU in the main world. *There can be hundreds of images
that get prepared for rendering only not to be used.*
2. If one loads an Image and needs to transform it in a system before
rendering it, kind of like the [decompression
example](https://github.com/bevyengine/bevy/blob/main/examples/asset/asset_decompression.rs#L120),
there's a price paid for extracting & preparing the asset that's not
intended to be rendered yet.

------

* References bevyengine#10520
* References bevyengine#1782

This changes the `RenderAssetPersistencePolicy` enum to bitflags. I felt
that the objective with the parameter is so similar in nature to wgpu's
[`TextureUsages`](https://docs.rs/wgpu/latest/wgpu/struct.TextureUsages.html)
and
[`BufferUsages`](https://docs.rs/wgpu/latest/wgpu/struct.BufferUsages.html),
that it may as well be just like that.

```rust
// This asset only needs to be in the main world. Don't extract and prepare it.
RenderAssetUsages::MAIN_WORLD

// Keep this asset in the main world and
RenderAssetUsages::MAIN_WORLD | RenderAssetUsages::RENDER_WORLD

// This asset is only needed in the render world. Remove it from the asset server once extracted.
RenderAssetUsages::RENDER_WORLD
```

I considered introducing a third field to `RenderAssetPersistencePolicy`
enum:
```rust
enum RenderAssetPersistencePolicy {
    /// Keep the asset in the main world after extracting to the render world.
    Keep,
    /// Remove the asset from the main world after extracting to the render world.
    Unload,
    /// This doesn't need to be in the render world at all.
    NoExtract, // <-----
}
```
Functional, but this seemed like shoehorning. Another option is renaming
the enum to something like:
```rust
enum RenderAssetExtractionPolicy {
    /// Extract the asset and keep it in the main world.
    Extract,
    /// Remove the asset from the main world after extracting to the render world.
    ExtractAndUnload,
    /// This doesn't need to be in the render world at all.
    NoExtract,
}
```
I think this last one could be a good option if the bitflags are too
clunky.

* `RenderAssetPersistencePolicy::Keep` → `RenderAssetUsage::MAIN_WORLD |
RenderAssetUsage::RENDER_WORLD` (or `RenderAssetUsage::default()`)
* `RenderAssetPersistencePolicy::Unload` →
`RenderAssetUsage::RENDER_WORLD`
* For types implementing the `RenderAsset` trait, change `fn
persistence_policy(&self) -> RenderAssetPersistencePolicy` to `fn
asset_usage(&self) -> RenderAssetUsages`.
* Change any references to `cpu_persistent_access`
(`RenderAssetPersistencePolicy`) to `asset_usage` (`RenderAssetUsage`).
This applies to `Image`, `Mesh`, and a few other types.
Maximetinu pushed a commit to Sophya/bevy that referenced this pull request Feb 12, 2024
- No point in keeping Meshes/Images in RAM once they're going to be sent
to the GPU, and kept in VRAM. This saves a _significant_ amount of
memory (several GBs) on scenes like bistro.
- References
  - bevyengine#1782
  - bevyengine#8624

- Augment RenderAsset with the capability to unload the underlying asset
after extracting to the render world.
- Mesh/Image now have a cpu_persistent_access field. If this field is
RenderAssetPersistencePolicy::Unload, the asset will be unloaded from
Assets<T>.
- A new AssetEvent is sent upon dropping the last strong handle for the
asset, which signals to the RenderAsset to remove the GPU version of the
asset.

---

- Added `AssetEvent::NoLongerUsed` and
`AssetEvent::is_no_longer_used()`. This event is sent when the last
strong handle of an asset is dropped.
- Rewrote the API for `RenderAsset` to allow for unloading the asset
data from the CPU.
- Added `RenderAssetPersistencePolicy`.
- Added `Mesh::cpu_persistent_access` for memory savings when the asset
is not needed except for on the GPU.
- Added `Image::cpu_persistent_access` for memory savings when the asset
is not needed except for on the GPU.
- Added `ImageLoaderSettings::cpu_persistent_access`.
- Added `ExrTextureLoaderSettings`.
- Added `HdrTextureLoaderSettings`.

- Asset loaders (GLTF, etc) now load meshes and textures without
`cpu_persistent_access`. These assets will be removed from
`Assets<Mesh>` and `Assets<Image>` once `RenderAssets<Mesh>` and
`RenderAssets<Image>` contain the GPU versions of these assets, in order
to reduce memory usage. If you require access to the asset data from the
CPU in future frames after the GLTF asset has been loaded, modify all
dependent `Mesh` and `Image` assets and set `cpu_persistent_access` to
`RenderAssetPersistencePolicy::Keep`.
- `Mesh` now requires a new `cpu_persistent_access` field. Set it to
`RenderAssetPersistencePolicy::Keep` to mimic the previous behavior.
- `Image` now requires a new `cpu_persistent_access` field. Set it to
`RenderAssetPersistencePolicy::Keep` to mimic the previous behavior.
- `MorphTargetImage::new()` now requires a new `cpu_persistent_access`
parameter. Set it to `RenderAssetPersistencePolicy::Keep` to mimic the
previous behavior.
- `DynamicTextureAtlasBuilder::add_texture()` now requires that the
`TextureAtlas` you pass has an `Image` with `cpu_persistent_access:
RenderAssetPersistencePolicy::Keep`. Ensure you construct the image
properly for the texture atlas.
- The `RenderAsset` trait has significantly changed, and requires
adapting your existing implementations.
  - The trait now requires `Clone`.
- The `ExtractedAsset` associated type has been removed (the type itself
is now extracted).
  - The signature of `prepare_asset()` is slightly different
- A new `persistence_policy()` method is now required (return
RenderAssetPersistencePolicy::Unload to match the previous behavior).
- Match on the new `NoLongerUsed` variant for exhaustive matches of
`AssetEvent`.
Maximetinu pushed a commit to Sophya/bevy that referenced this pull request Feb 12, 2024
Right now, all assets in the main world get extracted and prepared in
the render world (if the asset's using the RenderAssetPlugin). This is
unfortunate for two cases:

1. **TextureAtlas** / **FontAtlas**: This one's huge. The individual
`Image` assets that make up the atlas are cloned and prepared
individually when there's no reason for them to be. The atlas textures
are built on the CPU in the main world. *There can be hundreds of images
that get prepared for rendering only not to be used.*
2. If one loads an Image and needs to transform it in a system before
rendering it, kind of like the [decompression
example](https://github.com/bevyengine/bevy/blob/main/examples/asset/asset_decompression.rs#L120),
there's a price paid for extracting & preparing the asset that's not
intended to be rendered yet.

------

* References bevyengine#10520
* References bevyengine#1782

This changes the `RenderAssetPersistencePolicy` enum to bitflags. I felt
that the objective with the parameter is so similar in nature to wgpu's
[`TextureUsages`](https://docs.rs/wgpu/latest/wgpu/struct.TextureUsages.html)
and
[`BufferUsages`](https://docs.rs/wgpu/latest/wgpu/struct.BufferUsages.html),
that it may as well be just like that.

```rust
// This asset only needs to be in the main world. Don't extract and prepare it.
RenderAssetUsages::MAIN_WORLD

// Keep this asset in the main world and
RenderAssetUsages::MAIN_WORLD | RenderAssetUsages::RENDER_WORLD

// This asset is only needed in the render world. Remove it from the asset server once extracted.
RenderAssetUsages::RENDER_WORLD
```

I considered introducing a third field to `RenderAssetPersistencePolicy`
enum:
```rust
enum RenderAssetPersistencePolicy {
    /// Keep the asset in the main world after extracting to the render world.
    Keep,
    /// Remove the asset from the main world after extracting to the render world.
    Unload,
    /// This doesn't need to be in the render world at all.
    NoExtract, // <-----
}
```
Functional, but this seemed like shoehorning. Another option is renaming
the enum to something like:
```rust
enum RenderAssetExtractionPolicy {
    /// Extract the asset and keep it in the main world.
    Extract,
    /// Remove the asset from the main world after extracting to the render world.
    ExtractAndUnload,
    /// This doesn't need to be in the render world at all.
    NoExtract,
}
```
I think this last one could be a good option if the bitflags are too
clunky.

* `RenderAssetPersistencePolicy::Keep` → `RenderAssetUsage::MAIN_WORLD |
RenderAssetUsage::RENDER_WORLD` (or `RenderAssetUsage::default()`)
* `RenderAssetPersistencePolicy::Unload` →
`RenderAssetUsage::RENDER_WORLD`
* For types implementing the `RenderAsset` trait, change `fn
persistence_policy(&self) -> RenderAssetPersistencePolicy` to `fn
asset_usage(&self) -> RenderAssetUsages`.
* Change any references to `cpu_persistent_access`
(`RenderAssetPersistencePolicy`) to `asset_usage` (`RenderAssetUsage`).
This applies to `Image`, `Mesh`, and a few other types.
dekirisu pushed a commit to dekirisu/bevy_gltf_trait that referenced this pull request Jul 7, 2024
# Objective
- No point in keeping Meshes/Images in RAM once they're going to be sent
to the GPU, and kept in VRAM. This saves a _significant_ amount of
memory (several GBs) on scenes like bistro.
- References
  - bevyengine/bevy#1782
  - bevyengine/bevy#8624 

## Solution
- Augment RenderAsset with the capability to unload the underlying asset
after extracting to the render world.
- Mesh/Image now have a cpu_persistent_access field. If this field is
RenderAssetPersistencePolicy::Unload, the asset will be unloaded from
Assets<T>.
- A new AssetEvent is sent upon dropping the last strong handle for the
asset, which signals to the RenderAsset to remove the GPU version of the
asset.

---

## Changelog
- Added `AssetEvent::NoLongerUsed` and
`AssetEvent::is_no_longer_used()`. This event is sent when the last
strong handle of an asset is dropped.
- Rewrote the API for `RenderAsset` to allow for unloading the asset
data from the CPU.
- Added `RenderAssetPersistencePolicy`.
- Added `Mesh::cpu_persistent_access` for memory savings when the asset
is not needed except for on the GPU.
- Added `Image::cpu_persistent_access` for memory savings when the asset
is not needed except for on the GPU.
- Added `ImageLoaderSettings::cpu_persistent_access`.
- Added `ExrTextureLoaderSettings`.
- Added `HdrTextureLoaderSettings`.

## Migration Guide
- Asset loaders (GLTF, etc) now load meshes and textures without
`cpu_persistent_access`. These assets will be removed from
`Assets<Mesh>` and `Assets<Image>` once `RenderAssets<Mesh>` and
`RenderAssets<Image>` contain the GPU versions of these assets, in order
to reduce memory usage. If you require access to the asset data from the
CPU in future frames after the GLTF asset has been loaded, modify all
dependent `Mesh` and `Image` assets and set `cpu_persistent_access` to
`RenderAssetPersistencePolicy::Keep`.
- `Mesh` now requires a new `cpu_persistent_access` field. Set it to
`RenderAssetPersistencePolicy::Keep` to mimic the previous behavior.
- `Image` now requires a new `cpu_persistent_access` field. Set it to
`RenderAssetPersistencePolicy::Keep` to mimic the previous behavior.
- `MorphTargetImage::new()` now requires a new `cpu_persistent_access`
parameter. Set it to `RenderAssetPersistencePolicy::Keep` to mimic the
previous behavior.
- `DynamicTextureAtlasBuilder::add_texture()` now requires that the
`TextureAtlas` you pass has an `Image` with `cpu_persistent_access:
RenderAssetPersistencePolicy::Keep`. Ensure you construct the image
properly for the texture atlas.
- The `RenderAsset` trait has significantly changed, and requires
adapting your existing implementations.
  - The trait now requires `Clone`.
- The `ExtractedAsset` associated type has been removed (the type itself
is now extracted).
  - The signature of `prepare_asset()` is slightly different
- A new `persistence_policy()` method is now required (return
RenderAssetPersistencePolicy::Unload to match the previous behavior).
- Match on the new `NoLongerUsed` variant for exhaustive matches of
`AssetEvent`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Performance A change motivated by improving speed, memory usage or compile times X-Controversial There is active debate or serious implications around merging this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants