Retain asset without data for RENDER_WORLD-only assets

[robtfm]

Objective

when RenderAssets with RenderAssetUsages::RENDER_WORLD and without RenderAssetUsages::MAIN_WORLD are extracted, the asset is removed from the assets collection. this causes some issues:

• systems which rely on the asset, like picking with meshes, fail with "asset not found" errors which are unintuitive.
• loading the asset by path a second time results in the asset being reloaded from storage, re-extracted and re-transferred to gpu, replacing the existing asset
• knowledge about the asset state is lost, we cannot tell if an asset is already loaded with AssetServer::get_handle
• metadata (image size, e.g.) is no longer available for the asset

Solution

extraction:

• add take_gpu_data to the RenderAsset trait. use it to pull the data out of the asset for transfer, and leave the empty asset in the collection. default implementation just clones the asset.
• if the data has already been taken, panic. this follows from modifying an asset after extraction, which is always a code error, so i think panic here makes sense

Mesh/RenderMesh:

• make Mesh::attributes and Mesh::indices options
• take them on extraction
• expect operations which access or modify the vertex data or indices if it has been extracted. accessing the vertex data after extraction is always a code error. fixes Somehow prevent confusion caused by Assets being removed due to not having RenderAssetUsages::MAIN_WORLD #19737 by resulting in the error Mesh has been extracted to RenderWorld. To access vertex attributes, the mesh must have RenderAssetUsages::MAIN_WORLD
• compute the mesh Aabb when gpu data is taken and store the result. this allows extracted meshes to still use frustum culling (otherwise using multiple copies of an extracted mesh now panics as compute_aabb relied on vertex positions). there's a bit of a tradeoff here: users may not need the Aabb and we needlessly compute it. but i think users almost always do want them, and computing once (for extracted meshes) is cheaper than the alternative, keeping position data and computing a fresh Aabb every time the mesh is used on a new entity.

Image/GpuImage:

images are a little more complex because the data can be deliberately None for render-targets / GPU-written textures where we only want an uninitialized gpu-side texture.

• take Image::data on extraction
• record on the resulting GpuImage whether any data was found initially
• on subsequent modifications with no data, panic if there was data previously

corner case / issue: when used with RenderAssetBytesPerFrameLimiter there may be no previous gpu asset if it is still queued pending upload due to the bandwidth limit. this can result in a modified image with initial data skipping the had_data check, resulting in a blank texture. i think this is sufficiently rare that it's not a real problem, users would still hit the panic if the asset is transferred in time and the problem/solution should be clear when they do hit it.

ShaderStorageBuffer/GpuShaderStorageBuffer

follows the same pattern as Image/GpuImage:

• take ShaderStorageBuffer::data on extraction
• record on the resulting GpuShaderStorageBuffer whether any data was found initially
• on modifications with no data, panic if there was data previously

we don't have the queue issue here because GpuShaderStorageBuffer doesn't implement byte_len so we can't end up queueing them.

other RenderAssets

i didn't modify the other RenderAsset types (GpuAutoExposureCompensationCurve, GpuLineGizmo, RenderWireframeMaterial, PreparedMaterial, PreparedMaterial2d, PreparedUiMaterial) on the assumption that cloning these is cheap enough anyway.

Testing

only really tested within my work project. i can add some explicit tests if required.

[hukasu]

I wonder if it would be better having an enum like this just to have some more information

enum RenderAssetData {
    Available(Vec<u8>),
    Unavailable,
    SentToRenderWorld
}

[robtfm]

maybe ... i tried to avoid changing the api of the main-world types (particularly Image::data which is pub) but maybe it's worthwhile

[jasmine-nominal]

Suggested change

	/// mersh vertex data, etc) will be removed from the cpu-side asset once it's been extracted and prepared
	/// mesh vertex data, etc) will be removed from the cpu-side asset once it's been extracted and prepared

[alice-i-cecile]

Could we swap these methods to return Result and avoid the expects here?

[robtfm]

We could … it is a usage error though, rather than “something that can happen and you should deal with”, like result usually implies.

I don’t feel strongly, will change if you prefer it.