RenderAssetPersistencePolicy → RenderAssetUsage
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:
-
TextureAtlas / FontAtlas: This one's huge. The individual
Imageassets 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. - If one loads an Image and needs to transform it in a system before rendering it, kind of like the decompression example, 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 and BufferUsages, that it may as well be just like that.
// 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:
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:
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(orRenderAssetUsage::default()) -
RenderAssetPersistencePolicy::Unload→RenderAssetUsage::RENDER_WORLD - For types implementing the
RenderAssettrait, changefn persistence_policy(&self) -> RenderAssetPersistencePolicytofn asset_usage(&self) -> RenderAssetUsages. - Change any references to
cpu_persistent_access(RenderAssetPersistencePolicy) toasset_usage(RenderAssetUsage). This applies toImage,Mesh, and a few other types.
I'm not sure how I feel about changing the name to Main/Render worlds. You could have the asset data only on the CPU, and still want to access it in the render world for instance, so it's not entirely accurate. It's not a bad approximation though.
@JMS55 We could add a third bitfield GPU that controls whether or not the asset gets processed in prepare_assets so that there is that ultimate control. What do you think?
That sounds a lot more complicated. I think for that case, people shouldn't be using RenderAsset at all.
I'm fine with the current two options, just I feel like there's maybe a better name for them. They're not too bad as-is though.
I don't think we need to land this in 0.13 given that unloading render assets was disabled by default, but it's a large breaking change so better we don't push it to 0.14 and break things twice.
Agreed. There's also the added performance perk for atlases. I just submitted the change from usage to asset_usage, so I think this is good to go, barring any additional feedback.
Once merge conflicts are resolved ping me and I'll merge this.
@alice-i-cecile All set