RenderAssetPersistencePolicy → RenderAssetUsages

[brianreavis]

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, there's a price paid for extracting & preparing the asset that's not intended to be rendered yet.

---

• References Unload render assets from RAM #10520
• References Mesh: added static and dynamic mesh for less runtime memory usage #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 → RenderAssetUsages::MAIN_WORLD | RenderAssetUsages::RENDER_WORLD (or RenderAssetUsage::default())
• RenderAssetPersistencePolicy::Unload → RenderAssetUsages::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.

[doonv]

Perhaps add an option for both worlds?

[brianreavis]

I'm inclined to leave this with only the distinct options. All other current uses of bitflags don't provide constants for combined options. Both worlds can be achieved with RenderAssetUsages::default() or RenderAssetUsages::all() (in addition to classic or-ing)

[JMS55]

Overall I think this is a definite improvement over an enum, nice work.

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.

[brianreavis]

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?

[JMS55]

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.

[JMS55]

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.

[brianreavis]

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.

[NthTensor]

Consider adding documentation for the behavior of RenderAssetUsages::default().

RENDER_WORLD/MAIN_WORLD seems more beginner friendly than CPU/GPU, EXTRACT/UNLOAD or anything else I can think of, but it is a little weird to me that setting just MAIN_WORLD seems to have the same effect as not setting any flags.

[atlv24]

having been the one to propose PeristencePolicy naming originally in the unload assets from RAM PR, i think this is a far better naming that captures the semantics of the api properly instead of the mechanics of the api.

[alice-i-cecile]

Once merge conflicts are resolved ping me and I'll merge this.

[brianreavis]

@alice-i-cecile All set