mirror of
https://github.com/bevyengine/bevy
synced 2024-11-23 21:23:05 +00:00
9 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Joseph
|
5876352206
|
Optimize common usages of AssetReader (#14082)
# Objective
The `AssetReader` trait allows customizing the behavior of fetching
bytes for an `AssetPath`, and expects implementors to return `dyn
AsyncRead + AsyncSeek`. This gives implementors of `AssetLoader` great
flexibility to tightly integrate their asset loading behavior with the
asynchronous task system.
However, almost all implementors of `AssetLoader` don't use the async
functionality at all, and just call `AsyncReadExt::read_to_end(&mut
Vec<u8>)`. This is incredibly inefficient, as this method repeatedly
calls `poll_read` on the trait object, filling the vector 32 bytes at a
time. At my work we have assets that are hundreds of megabytes which
makes this a meaningful overhead.
## Solution
Turn the `Reader` type alias into an actual trait, with a provided
method `read_to_end`. This provided method should be more efficient than
the existing extension method, as the compiler will know the underlying
type of `Reader` when generating this function, which removes the
repeated dynamic dispatches and allows the compiler to make further
optimizations after inlining. Individual implementors are able to
override the provided implementation -- for simple asset readers that
just copy bytes from one buffer to another, this allows removing a large
amount of overhead from the provided implementation.
Now that `Reader` is an actual trait, I also improved the ergonomics for
implementing `AssetReader`. Currently, implementors are expected to box
their reader and return it as a trait object, which adds unnecessary
boilerplate to implementations. This PR changes that trait method to
return a pseudo trait alias, which allows implementors to return `impl
Reader` instead of `Box<dyn Reader>`. Now, the boilerplate for boxing
occurs in `ErasedAssetReader`.
## Testing
I made identical changes to my company's fork of bevy. Our app, which
makes heavy use of `read_to_end` for asset loading, still worked
properly after this. I am not aware if we have a more systematic way of
testing asset loading for correctness.
---
## Migration Guide
The trait method `bevy_asset::io::AssetReader::read` (and `read_meta`)
now return an opaque type instead of a boxed trait object. Implementors
of these methods should change the type signatures appropriately
```rust
impl AssetReader for MyReader {
// Before
async fn read<'a>(&'a self, path: &'a Path) -> Result<Box<Reader<'a>>, AssetReaderError> {
let reader = // construct a reader
Box::new(reader) as Box<Reader<'a>>
}
// After
async fn read<'a>(&'a self, path: &'a Path) -> Result<impl Reader + 'a, AssetReaderError> {
// create a reader
}
}
```
`bevy::asset::io::Reader` is now a trait, rather than a type alias for a
trait object. Implementors of `AssetLoader::load` will need to adjust
the method signature accordingly
```rust
impl AssetLoader for MyLoader {
async fn load<'a>(
&'a self,
// Before:
reader: &'a mut bevy::asset::io::Reader,
// After:
reader: &'a mut dyn bevy::asset::io::Reader,
_: &'a Self::Settings,
load_context: &'a mut LoadContext<'_>,
) -> Result<Self::Asset, Self::Error> {
}
```
Additionally, implementors of `AssetReader` that return a type
implementing `futures_io::AsyncRead` and `AsyncSeek` might need to
explicitly implement `bevy::asset::io::Reader` for that type.
```rust
impl bevy::asset::io::Reader for MyAsyncReadAndSeek {}
```
|
||
Ricky Taylor
|
efcb6d6c11
|
Make LoadContext use the builder pattern for loading dependent assets (#13465)
# Objective - Fixes #13445. ## Solution - Removes all `load_` methods from `LoadContext`. - Introduces `fn loader()` which returns a builder. ## Testing - I've tested with `cargo test --package=bevy_asset` and run the two relevant examples (`asset_processing` & `asset_decompression`). --- ## Changelog - Replaced all `load_` methods on `LoadContext` with the new `loader()` pattern. ## Migration Guide - Several LoadContext method calls will need to be updated: - `load_context.load_with_settings(path, settings)` => `load_context.loader().with_settings(settings).load(path)` - `load_context.load_untyped(path)` => `load_context.loader().untyped().load(path)` - `load_context.load_direct(path)` => `load_context.loader().direct().load(path)` - `load_context.load_direct_untyped(path)` => `load_context.loader().direct().untyped().load(path)` - `load_context.load_direct_with_settings(path, settings)` => `load_context.loader().with_settings(settings).direct().load(path)` - `load_context.load_direct_with_reader(reader, path)` => `load_context.loader().direct().with_reader(reader).load(path)` - `load_context.load_direct_with_reader_and_settings(reader, path, settings)` => `load_context.loader().with_settings(settings).direct().with_reader(reader).load(path)` - `load_context.load_direct_untyped_with_reader(reader, path)` => `load_context.loader().direct().with_reader(reader).untyped().load(path)` --- CC @alice-i-cecile / @bushrat011899 Examples: ```rust load_context.loader() .with_asset_type::<A>() .with_asset_type_id(TypeId::of::<A>()) .with_settings(|mut settings| { settings.key = value; }) // Then, for a Handle<A>: .load::<A>() // Or, for a Handle<LoadedUntypedAsset>: .untyped() .load() // Or, to load an `A` directly: .direct() .load::<A>() .await // Or, to load an `ErasedLoadedAsset` directly: .direct() .untyped() .load() .await ``` |
||
|
Ricky Taylor
|
26df1c1179
|
Add more load_direct implementations (#13415)
# Objective - Introduce variants of `LoadContext::load_direct` which allow picking asset type & configuring settings. - Fixes #12963. ## Solution - Implements `ErasedLoadedAsset::downcast` and adds some accessors to `LoadedAsset<A>`. - Changes `load_direct`/`load_direct_with_reader` to be typed, and introduces `load_direct_untyped`/`load_direct_untyped_with_reader`. - Introduces `load_direct_with_settings` and `load_direct_with_reader_and_settings`. ## Testing - I've run cargo test and played with the examples which use `load_direct`. - I also extended the `asset_processing` example to use the new typed version of `load_direct` and use `load_direct_with_settings`. --- ## Changelog - Introduced new `load_direct` methods in `LoadContext` to allow specifying type & settings ## Migration Guide - `LoadContext::load_direct` has been renamed to `LoadContext::load_direct_untyped`. You may find the new `load_direct` is more appropriate for your use case (and the migration may only be moving one type parameter). - `LoadContext::load_direct_with_reader` has been renamed to `LoadContext::load_direct_untyped_with_reader`. --- This might not be an obvious win as a solution because it introduces quite a few new `load_direct` alternatives - but it does follow the existing pattern pretty well. I'm very open to alternatives. 😅 |
||
Ame
|
0256dacba4
|
Fix some doc warnings (#12961)
# Objective - Fix some doc warnings - Add doc-scrape-examples to all examples Moved from #12692 I run `cargo +nightly doc --workspace --all-features --no-deps -Zunstable-options -Zrustdoc-scrape-examples` <details> ``` warning: public documentation for `GzAssetLoaderError` links to private item `GzAssetLoader` --> examples/asset/asset_decompression.rs:24:47 | 24 | /// Possible errors that can be produced by [`GzAssetLoader`] | ^^^^^^^^^^^^^ this item is private | = note: this link will resolve properly if you pass `--document-private-items` = note: `#[warn(rustdoc::private_intra_doc_links)]` on by default warning: `bevy` (example "asset_decompression") generated 1 warning warning: unresolved link to `shape::Quad` --> examples/2d/mesh2d.rs:3:15 | 3 | //! [`Quad`]: shape::Quad | ^^^^^^^^^^^ no item named `shape` in scope | = note: `#[warn(rustdoc::broken_intra_doc_links)]` on by default warning: `bevy` (example "mesh2d") generated 1 warning warning: unresolved link to `WorldQuery` --> examples/ecs/custom_query_param.rs:1:49 | 1 | //! This example illustrates the usage of the [`WorldQuery`] derive macro, which allows | ^^^^^^^^^^ no item named `WorldQuery` in scope | = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` = note: `#[warn(rustdoc::broken_intra_doc_links)]` on by default warning: `bevy` (example "custom_query_param") generated 1 warning warning: unresolved link to `shape::Quad` --> examples/2d/mesh2d_vertex_color_texture.rs:4:15 | 4 | //! [`Quad`]: shape::Quad | ^^^^^^^^^^^ no item named `shape` in scope | = note: `#[warn(rustdoc::broken_intra_doc_links)]` on by default warning: `bevy` (example "mesh2d_vertex_color_texture") generated 1 warning warning: public documentation for `TextPlugin` links to private item `CoolText` --> examples/asset/processing/asset_processing.rs:48:9 | 48 | /// * [`CoolText`]: a custom RON text format that supports dependencies and embedded dependencies | ^^^^^^^^ this item is private | = note: this link will resolve properly if you pass `--document-private-items` = note: `#[warn(rustdoc::private_intra_doc_links)]` on by default warning: public documentation for `TextPlugin` links to private item `Text` --> examples/asset/processing/asset_processing.rs:49:9 | 49 | /// * [`Text`]: a "normal" plain text file | ^^^^ this item is private | = note: this link will resolve properly if you pass `--document-private-items` warning: public documentation for `TextPlugin` links to private item `CoolText` --> examples/asset/processing/asset_processing.rs:51:57 | 51 | /// It also defines an asset processor that will load [`CoolText`], resolve embedded dependenc... | ^^^^^^^^ this item is private | = note: this link will resolve properly if you pass `--document-private-items` warning: `bevy` (example "asset_processing") generated 3 warnings warning: public documentation for `CustomAssetLoaderError` links to private item `CustomAssetLoader` --> examples/asset/custom_asset.rs:20:47 | 20 | /// Possible errors that can be produced by [`CustomAssetLoader`] | ^^^^^^^^^^^^^^^^^ this item is private | = note: this link will resolve properly if you pass `--document-private-items` = note: `#[warn(rustdoc::private_intra_doc_links)]` on by default warning: public documentation for `BlobAssetLoaderError` links to private item `CustomAssetLoader` --> examples/asset/custom_asset.rs:61:47 | 61 | /// Possible errors that can be produced by [`CustomAssetLoader`] | ^^^^^^^^^^^^^^^^^ this item is private | = note: this link will resolve properly if you pass `--document-private-items` ``` ``` warning: `bevy` (example "mesh2d") generated 1 warning warning: public documentation for `log_layers_ecs` links to private item `update_subscriber` --> examples/app/log_layers_ecs.rs:6:18 | 6 | //! Inside the [`update_subscriber`] function we will create a [`mpsc::Sender`] and a [`mpsc::R... | ^^^^^^^^^^^^^^^^^ this item is private | = note: this link will resolve properly if you pass `--document-private-items` = note: `#[warn(rustdoc::private_intra_doc_links)]` on by default warning: unresolved link to `AdvancedLayer` --> examples/app/log_layers_ecs.rs:7:72 | 7 | ... will go into the [`AdvancedLayer`] and the [`Receiver`](mpsc::Receiver) will | ^^^^^^^^^^^^^ no item named `AdvancedLayer` in scope | = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` = note: `#[warn(rustdoc::broken_intra_doc_links)]` on by default warning: unresolved link to `LogEvents` --> examples/app/log_layers_ecs.rs:8:42 | 8 | //! go into a non-send resource called [`LogEvents`] (It has to be non-send because [`Receiver`... | ^^^^^^^^^ no item named `LogEvents` in scope | = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: public documentation for `log_layers_ecs` links to private item `transfer_log_events` --> examples/app/log_layers_ecs.rs:9:30 | 9 | //! From there we will use [`transfer_log_events`] to transfer log events from [`LogEvents`] to... | ^^^^^^^^^^^^^^^^^^^ this item is private | = note: this link will resolve properly if you pass `--document-private-items` warning: unresolved link to `LogEvents` --> examples/app/log_layers_ecs.rs:9:82 | 9 | ...nsfer log events from [`LogEvents`] to an ECS event called [`LogEvent`]. | ^^^^^^^^^ no item named `LogEvents` in scope | = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: public documentation for `log_layers_ecs` links to private item `LogEvent` --> examples/app/log_layers_ecs.rs:9:119 | 9 | ...nts`] to an ECS event called [`LogEvent`]. | ^^^^^^^^ this item is private | = note: this link will resolve properly if you pass `--document-private-items` warning: public documentation for `log_layers_ecs` links to private item `LogEvent` --> examples/app/log_layers_ecs.rs:11:49 | 11 | //! Finally, after all that we can access the [`LogEvent`] event from our systems and use it. | ^^^^^^^^ this item is private | = note: this link will resolve properly if you pass `--document-private-items` ``` <details/> |
||
Arthur Brussee
|
ac49dce4ca
|
Use async-fn in traits rather than BoxedFuture (#12550)
# Objective
Simplify implementing some asset traits without Box::pin(async move{})
shenanigans.
Fixes (in part) https://github.com/bevyengine/bevy/issues/11308
## Solution
Use async-fn in traits when possible in all traits. Traits with return
position impl trait are not object safe however, and as AssetReader and
AssetWriter are both used with dynamic dispatch, you need a Boxed
version of these futures anyway.
In the future, Rust is [adding
](https://blog.rust-lang.org/2023/12/21/async-fn-rpit-in-traits.html)proc
macros to generate these traits automatically, and at some point in the
future dyn traits should 'just work'. Until then.... this seemed liked
the right approach given more ErasedXXX already exist, but, no clue if
there's plans here! Especially since these are public now, it's a bit of
an unfortunate API, and means this is a breaking change.
In theory this saves some performance when these traits are used with
static dispatch, but, seems like most code paths go through dynamic
dispatch, which boxes anyway.
I also suspect a bunch of the lifetime annotations on these function
could be simplified now as the BoxedFuture was often the only thing
returned which needed a lifetime annotation, but I'm not touching that
for now as traits + lifetimes can be so tricky.
This is a revival of
[pull/11362](https://github.com/bevyengine/bevy/pull/11362) after a
spectacular merge f*ckup, with updates to the latest Bevy. Just to recap
some discussion:
- Overall this seems like a win for code quality, especially when
implementing these traits, but a loss for having to deal with ErasedXXX
variants.
- `ConditionalSend` was the preferred name for the trait that might be
Send, to deal with wasm platforms.
- When reviewing be sure to disable whitespace difference, as that's 95%
of the PR.
## Changelog
- AssetReader, AssetWriter, AssetLoader, AssetSaver and Process now use
async-fn in traits rather than boxed futures.
## Migration Guide
- Custom implementations of AssetReader, AssetWriter, AssetLoader,
AssetSaver and Process should switch to async fn rather than returning a
bevy_utils::BoxedFuture.
- Simultaniously, to use dynamic dispatch on these traits you should
instead use dyn ErasedXXX.
|
||
James Liu
|
512b7463a3
|
Disentangle bevy_utils/bevy_core's reexported dependencies (#12313)
# Objective Make bevy_utils less of a compilation bottleneck. Tackle #11478. ## Solution * Move all of the directly reexported dependencies and move them to where they're actually used. * Remove the UUID utilities that have gone unused since `TypePath` took over for `TypeUuid`. * There was also a extraneous bytemuck dependency on `bevy_core` that has not been used for a long time (since `encase` became the primary way to prepare GPU buffers). * Remove the `all_tuples` macro reexport from bevy_ecs since it's accessible from `bevy_utils`. --- ## Changelog Removed: Many of the reexports from bevy_utils (petgraph, uuid, nonmax, smallvec, and thiserror). Removed: bevy_core's reexports of bytemuck. ## Migration Guide bevy_utils' reexports of petgraph, uuid, nonmax, smallvec, and thiserror have been removed. bevy_core' reexports of bytemuck's types has been removed. Add them as dependencies in your own crate instead. |
||
|
Ame
|
9d67edc3a6
|
fix some typos (#12038)
# Objective Split - containing only the fixed typos - https://github.com/bevyengine/bevy/pull/12036#pullrequestreview-1894738751 # Migration Guide In `crates/bevy_mikktspace/src/generated.rs` ```rs // before pub struct SGroup { pub iVertexRepresentitive: i32, .. } // after pub struct SGroup { pub iVertexRepresentative: i32, .. } ``` In `crates/bevy_core_pipeline/src/core_2d/mod.rs` ```rs // before Node2D::ConstrastAdaptiveSharpening // after Node2D::ContrastAdaptiveSharpening ``` --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Co-authored-by: James Liu <contact@jamessliu.com> Co-authored-by: François <mockersf@gmail.com> |
||
Tristan Guichaoua
|
694c06f3d0
|
Inverse missing_docs logic (#11676)
# Objective Currently the `missing_docs` lint is allowed-by-default and enabled at crate level when their documentations is complete (see #3492). This PR proposes to inverse this logic by making `missing_docs` warn-by-default and mark crates with imcomplete docs allowed. ## Solution Makes `missing_docs` warn at workspace level and allowed at crate level when the docs is imcomplete. |
||
Zachary Harrold
|
46b8e904f4
|
Added Method to Allow Pipelined Asset Loading (#10565)
# Objective - Fixes #10518 ## Solution I've added a method to `LoadContext`, `load_direct_with_reader`, which mirrors the behaviour of `load_direct` with a single key difference: it is provided with the `Reader` by the caller, rather than getting it from the contained `AssetServer`. This allows for an `AssetLoader` to process its `Reader` stream, and then directly hand the results off to the `LoadContext` to handle further loading. The outer `AssetLoader` can control how the `Reader` is interpreted by providing a relevant `AssetPath`. For example, a Gzip decompression loader could process the asset `images/my_image.png.gz` by decompressing the bytes, then handing the decompressed result to the `LoadContext` with the new path `images/my_image.png.gz/my_image.png`. This intuitively reflects the nature of contained assets, whilst avoiding unintended behaviour, since the generated path cannot be a real file path (a file and folder of the same name cannot coexist in most file-systems). ```rust #[derive(Asset, TypePath)] pub struct GzAsset { pub uncompressed: ErasedLoadedAsset, } #[derive(Default)] pub struct GzAssetLoader; impl AssetLoader for GzAssetLoader { type Asset = GzAsset; type Settings = (); type Error = GzAssetLoaderError; fn load<'a>( &'a self, reader: &'a mut Reader, _settings: &'a (), load_context: &'a mut LoadContext, ) -> BoxedFuture<'a, Result<Self::Asset, Self::Error>> { Box::pin(async move { let compressed_path = load_context.path(); let file_name = compressed_path .file_name() .ok_or(GzAssetLoaderError::IndeterminateFilePath)? .to_string_lossy(); let uncompressed_file_name = file_name .strip_suffix(".gz") .ok_or(GzAssetLoaderError::IndeterminateFilePath)?; let contained_path = compressed_path.join(uncompressed_file_name); let mut bytes_compressed = Vec::new(); reader.read_to_end(&mut bytes_compressed).await?; let mut decoder = GzDecoder::new(bytes_compressed.as_slice()); let mut bytes_uncompressed = Vec::new(); decoder.read_to_end(&mut bytes_uncompressed)?; // Now that we have decompressed the asset, let's pass it back to the // context to continue loading let mut reader = VecReader::new(bytes_uncompressed); let uncompressed = load_context .load_direct_with_reader(&mut reader, contained_path) .await?; Ok(GzAsset { uncompressed }) }) } fn extensions(&self) -> &[&str] { &["gz"] } } ``` Because this example is so prudent, I've included an `asset_decompression` example which implements this exact behaviour: ```rust fn main() { App::new() .add_plugins(DefaultPlugins) .init_asset::<GzAsset>() .init_asset_loader::<GzAssetLoader>() .add_systems(Startup, setup) .add_systems(Update, decompress::<Image>) .run(); } fn setup(mut commands: Commands, asset_server: Res<AssetServer>) { commands.spawn(Camera2dBundle::default()); commands.spawn(( Compressed::<Image> { compressed: asset_server.load("data/compressed_image.png.gz"), ..default() }, Sprite::default(), TransformBundle::default(), VisibilityBundle::default(), )); } fn decompress<A: Asset>( mut commands: Commands, asset_server: Res<AssetServer>, mut compressed_assets: ResMut<Assets<GzAsset>>, query: Query<(Entity, &Compressed<A>)>, ) { for (entity, Compressed { compressed, .. }) in query.iter() { let Some(GzAsset { uncompressed }) = compressed_assets.remove(compressed) else { continue; }; let uncompressed = uncompressed.take::<A>().unwrap(); commands .entity(entity) .remove::<Compressed<A>>() .insert(asset_server.add(uncompressed)); } } ``` A key limitation to this design is how to type the internally loaded asset, since the example `GzAssetLoader` is unaware of the internal asset type `A`. As such, in this example I store the contained asset as an `ErasedLoadedAsset`, and leave it up to the consumer of the `GzAsset` to handle typing the final result, which is the purpose of the `decompress` system. This limitation can be worked around by providing type information to the `GzAssetLoader`, such as `GzAssetLoader<Image, ImageAssetLoader>`, but this would require registering the asset loader for every possible decompression target. Aside from this limitation, nested asset containerisation works as an end user would expect; if the user registers a `TarAssetLoader`, and a `GzAssetLoader`, then they can load assets with compound containerisation, such as `images.tar.gz`. --- ## Changelog - Added `LoadContext::load_direct_with_reader` - Added `asset_decompression` example ## Notes - While I believe my implementation of a Gzip asset loader is reasonable, I haven't included it as a public feature of `bevy_asset` to keep the scope of this PR as focussed as possible. - I have included `flate2` as a `dev-dependency` for the example; it is not included in the main dependency graph. |