Add ExtractableAsset trait for accessing extractable data

[beicause]

Objective

Follow-up to #21732. This attempts to provide a unified interface for extractable data. Additionally, this attempts to simplify the Mesh interface.

Solution

Adds ExtractableAsset trait and implements it for Mesh,Image,ShaderStorageBuffer. Adds is_extracted_to_render_world field to Image,ShaderStorageBuffer so that take_gpu_data doesn't need previous_gpu_asset.

Moves all try_xxx methods of Mesh to MeshExtractableData, and try_xxx_option is removed, the corresponding methods always return Option if them can. The methods on Mesh may panic. To handle panics, use extractable_data_ref / extractable_data_mut and get the MeshExtractableData interface for access.

The benefit of accessing data through the ExtractableAsset trait is you can more clearly known if the data is extractable, and you only need to check once for continuous access.

Methods accept a Mesh parameter can be changed to accept MeshExtractableData to avoid unwrap. I also added topology field to MeshExtractableData (though it won’t be extracted) and implemented Into<Mesh> for it. This makes it possible to construct a Mesh from MeshExtractableData, similar to InfallibleMesh but I’m unsure if this is a viable replacement for InfallibleMesh.

Testing

CI

[robtfm]

i've not been through in depth, but i think this approach is probably cleaner than the InfallibleMesh approach.

user code may now panic if it accesses mesh data directly, but there is a way to handle it if that user code (or engine code like mesh picking) chooses to. we don't force users to handle errors that they know can't occur on existing meshes and don't require use of a separate "builder" struct (InfallibleMesh) for avoiding impossible-error handling in primitive construction.

as written we would lose the ability to rewrite render-world-only images (previously setting image.data = Some(new_data) would successfully trigger re-upload), but assuming we get that working there's no missing functionality, less api changes, and only one significant addition (get_extractable_data_*).

[beicause]

as written we would lose the ability to rewrite render-world-only images (previously setting image.data = Some(new_data) would successfully trigger re-upload)

This can work in this PR after changing the take_gpu_data logic to use the is_extracted_to_render_world field on Image, but users must manually set this field to false after modifying to make re-uploading succeed.

[robtfm]

right, i guess it's ok to require every modification of render-world-only assets to also unset that flag.

i guess the key question is, do we intend to make render-world-only the default in future?
if we do, then maybe infallible builder plus result-handling in all other cases is better.
if not, then i think default panic, optional result-handling (i.e. this) is better.

[JMS55]

Request: For any change to extraction, please think about Solari BLAS building. It shouldn't ever be a problem, Solari operates post extract, but let's be careful.

[robtfm]

Request: For any change to extraction, please think about Solari BLAS building. It shouldn't ever be a problem, Solari operates post extract, but let's be careful.

is it possible to add an example or tests? it's the only reliable way to ensure that prs don't break stuff.

[greeble-dev]

I've got some directional concerns about this PR:

1. Panicing. I think this should be a last resort, especially when it's easy to write code that works fine in some cases, but panics when the data is extracted. The counter-argument would be that people can use MeshExtractableData instead, which doesn't panic. But that leads to:

2. MeshExtractableData is caught between two roles - a mutable builder and a (mostly) immutable result. It can play them both now because the internals are simple. But that will become harder if validation and optimization steps are added - I think the most likely outcome is that it gets split into separate types.

So I think the InfallibleMesh PR is a better first step, even if I'm not sold on some of the details. It gets the type split done now, so there's a type that only plays the role of builder. That means validation and optimization can be added to Mesh (or another type) while minimising breaking changes for people who just want to build meshes.

I'm assuming that if InfallibleMesh happens then this PR can still be relevant - it would be limited to refactoring the extraction and wouldn't affect the rest of the Mesh interface.

One big caveat: I don't really know what people's plans are for mesh validation and optimization. So maybe I'm making assumptions that are invalid.

[beicause]

In my opinion:

1. If we don’t want to keep the panic APIs, we can remove them and only keep the extractable data interface. But that requires more changes and I don't feel strongly about it now, as handling the panics here should not be difficult for users. Alright, I found that mesh_picking panics with render world only meshes. This is a real footgun. Perhaps we should remove these methods now to ensure all panics are properly handled.

2. Making Mesh immutable seems unnecessary to me and isn't guaranteed at the moment. Assets are currently designed to be mutable, unless we ensure immutability at the Asset level, there isn’t enough motivation to just make Mesh immutable.

[beicause]

I have removed all panic methods. These methods could easily become a footgun for systems that iterate over Meshs, causing unexpected panics such as bevy mesh_picking (The issue exists on the main branch, a fix will be required if this PR isn't included in 0.18. Edit: I opened an issue)

[JMS55]

is it possible to add an example or tests? it's the only reliable way to ensure that prs don't break stuff.

There's a solari example you can run.