Bloom tweaks #1
Merged
Bloom tweaks #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
# Objective Currently, Bevy only supports rendering to the current "surface texture format". This means that "render to texture" scenarios must use the exact format the primary window's surface uses, or Bevy will crash. This is even harder than it used to be now that we detect preferred surface formats at runtime instead of using hard coded BevyDefault values. ## Solution 1. Look up and store each window surface's texture format alongside other extracted window information 2. Specialize the upscaling pass on the current `RenderTarget`'s texture format, now that we can cheaply correlate render targets to their current texture format 3. Remove the old `SurfaceTextureFormat` and `AvailableTextureFormats`: these are now redundant with the information stored on each extracted window, and probably should not have been globals in the first place (as in theory each surface could have a different format). This means you can now use any texture format you want when rendering to a texture! For example, changing the `render_to_texture` example to use `R16Float` now doesn't crash / properly only stores the red component: 
# Objective
Currently scenes define components using a list:
```rust
[
(
entity: 0,
components: [
{
"bevy_transform::components::transform::Transform": (
translation: (
x: 0.0,
y: 0.0,
z: 0.0
),
rotation: (0.0, 0.0, 0.0, 1.0),
scale: (
x: 1.0,
y: 1.0,
z: 1.0
),
),
},
{
"my_crate::Foo": (
text: "Hello World",
),
},
{
"my_crate::Bar": (
baz: 123,
),
},
],
),
]
```
However, this representation has some drawbacks (as pointed out by @Metadorius in [this](bevyengine#4561 (comment)) comment):
1. Increased nesting and more characters (minor effect on overall size)
2. More importantly, by definition, entities cannot have more than one instance of any given component. Therefore, such data is best stored as a map— where all values are meant to have unique keys.
## Solution
Change `components` to store a map of components rather than a list:
```rust
[
(
entity: 0,
components: {
"bevy_transform::components::transform::Transform": (
translation: (
x: 0.0,
y: 0.0,
z: 0.0
),
rotation: (0.0, 0.0, 0.0, 1.0),
scale: (
x: 1.0,
y: 1.0,
z: 1.0
),
),
"my_crate::Foo": (
text: "Hello World",
),
"my_crate::Bar": (
baz: 123
),
},
),
]
```
#### Code Representation
This change only affects the scene format itself. `DynamicEntity` still stores its components as a list. The reason for this is that storing such data as a map is not really needed since:
1. The "key" of each value is easily found by just calling `Reflect::type_name` on it
2. We should be generating such structs using the `World` itself which upholds the one-component-per-entity rule
One could in theory create manually create a `DynamicEntity` with duplicate components, but this isn't something I think we should focus on in this PR. `DynamicEntity` can be broken in other ways (i.e. storing a non-component in the components list), and resolving its issues can be done in a separate PR.
---
## Changelog
* The scene format now uses a map to represent the collection of components rather than a list
## Migration Guide
The scene format now uses a map to represent the collection of components. Scene files will need to update from the old list format.
<details>
<summary>Example Code</summary>
```rust
// OLD
[
(
entity: 0,
components: [
{
"bevy_transform::components::transform::Transform": (
translation: (
x: 0.0,
y: 0.0,
z: 0.0
),
rotation: (0.0, 0.0, 0.0, 1.0),
scale: (
x: 1.0,
y: 1.0,
z: 1.0
),
),
},
{
"my_crate::Foo": (
text: "Hello World",
),
},
{
"my_crate::Bar": (
baz: 123,
),
},
],
),
]
// NEW
[
(
entity: 0,
components: {
"bevy_transform::components::transform::Transform": (
translation: (
x: 0.0,
y: 0.0,
z: 0.0
),
rotation: (0.0, 0.0, 0.0, 1.0),
scale: (
x: 1.0,
y: 1.0,
z: 1.0
),
),
"my_crate::Foo": (
text: "Hello World",
),
"my_crate::Bar": (
baz: 123
),
},
),
]
```
</details>
# Objective - Fixes bevyengine#5876 . ## Solution - added pub use statements to re-export the following traits in bevy_audio: rodio::source::Source, rodio::Sample, rodio::cpal::Sample. - rodio::cpal::Sample was re-exported as CpalSample to avoid naming conflict with rodio::Sample.
# Objective Bevy's internal plugins have lots of execution-order ambiguities, which makes the ambiguity detection tool very noisy for our users. ## Solution Silence every last ambiguity that can currently be resolved. Each time an ambiguity is silenced, it is accompanied by a comment describing why it is correct. This description should be based on the public API of the respective systems. Thus, I have added documentation to some systems describing how they use some resources. # Future work Some ambiguities remain, due to issues out of scope for this PR. * The ambiguity checker does not respect `Without<>` filters, leading to false positives. * Ambiguities between `bevy_ui` and `bevy_animation` cannot be resolved, since neither crate knows that the other exists. We will need a general solution to this problem.
These tiny changes answer question I had when using the Timer class.
…ne#6382) # Objective Currently, `DynamicSceneBuilder` keeps track of entities via a `HashMap`. This has an unintended side-effect in that, when building the full `DynamicScene`, we aren't guaranteed any particular order. In other words, inserting Entity A then Entity B can result in either `[A, B]` or `[B, A]`. This can be rather annoying when running tests on scenes generated via the builder as it will work sometimes but not other times. There's also the potential that this might unnecessarily clutter up VCS diffs for scene files (assuming they had an intentional order). ## Solution Store `DynamicSceneBuilder`'s entities in a `Vec` rather than a `HashMap`. --- ## Changelog * Stablized entity order in `DynamicSceneBuilder` (0.9.0-dev)
# Objective Clean up code surrounding fetch by pulling out the common parts into the iteration code. ## Solution Merge `Fetch::table_fetch` and `Fetch::archetype_fetch` into a single API: `Fetch::fetch(&mut self, entity: &Entity, table_row: &usize)`. This provides everything any fetch requires to internally decide which storage to read from and get the underlying data. All of these functions are marked as `#[inline(always)]` and the arguments are passed as references to attempt to optimize out the argument that isn't being used. External to `Fetch`, Query iteration has been changed to keep track of the table row and entity outside of fetch, which moves a lot of the expensive bookkeeping `Fetch` structs had previously done internally into the outer loop. ~~TODO: Benchmark, docs~~ Done. --- ## Changelog Changed: `Fetch::table_fetch` and `Fetch::archetype_fetch` have been merged into a single `Fetch::fetch` function. ## Migration Guide TODO Co-authored-by: Brian Merchant <[email protected]> Co-authored-by: Saverio Miroddi <[email protected]>
fixes bevyengine#5944 Uses the second solution: > 2. keep track of the old viewport in the computed_state, and if camera.viewport != camera.computed_state.old_viewport, then update the projection. This is more reliable, but needs to store two UVec2s more in the camera (probably not a big deal).
# Objective  ^ enable this Concretely, I need to - list all handle ids for an asset type - fetch the asset as `dyn Reflect`, given a `HandleUntyped` - when encountering a `Handle<T>`, find out what asset type that handle refers to (`T`'s type id) and turn the handle into a `HandleUntyped` ## Solution - add `ReflectAsset` type containing function pointers for working with assets ```rust pub struct ReflectAsset { type_uuid: Uuid, assets_resource_type_id: TypeId, // TypeId of the `Assets<T>` resource get: fn(&World, HandleUntyped) -> Option<&dyn Reflect>, get_mut: fn(&mut World, HandleUntyped) -> Option<&mut dyn Reflect>, get_unchecked_mut: unsafe fn(&World, HandleUntyped) -> Option<&mut dyn Reflect>, add: fn(&mut World, &dyn Reflect) -> HandleUntyped, set: fn(&mut World, HandleUntyped, &dyn Reflect) -> HandleUntyped, len: fn(&World) -> usize, ids: for<'w> fn(&'w World) -> Box<dyn Iterator<Item = HandleId> + 'w>, remove: fn(&mut World, HandleUntyped) -> Option<Box<dyn Reflect>>, } ``` - add `ReflectHandle` type relating the handle back to the asset type and providing a way to create a `HandleUntyped` ```rust pub struct ReflectHandle { type_uuid: Uuid, asset_type_id: TypeId, downcast_handle_untyped: fn(&dyn Any) -> Option<HandleUntyped>, } ``` - add the corresponding `FromType` impls - add a function `app.register_asset_reflect` which is supposed to be called after `.add_asset` and registers `ReflectAsset` and `ReflectHandle` in the type registry --- ## Changelog - add `ReflectAsset` and `ReflectHandle` types, which allow code to use reflection to manipulate arbitrary assets without knowing their types at compile time
Simply fix an outdated tag in `CHANGELOG.md`.
# Objective
- fix new clippy lints before they get stable and break CI
## Solution
- run `clippy --fix` to auto-fix machine-applicable lints
- silence `clippy::should_implement_trait` for `fn HandleId::default<T: Asset>`
## Changes
- always prefer `format!("{inline}")` over `format!("{}", not_inline)`
- prefer `Box::default` (or `Box::<T>::default` if necessary) over `Box::new(T::default())`
Alternative to bevyengine#4799 Tested and this works as expected
# Objective
Currently for entities we serialize only `id`. But this is not very expected behavior. For example, in networking, when the server sends its state, it contains entities and components. On the client, I create new objects and map them (using `EntityMap`) to those received from the server (to know which one matches which). And if `generation` field is missing, this mapping can be broken. Example:
1. Server sends an entity `Entity{ id: 2, generation: 1}` with components.
2. Client puts the received entity in a map and create a new entity that maps to this received entity. The new entity have different `id` and `generation`. Let's call it `Entity{ id: 12, generation: 4}`.
3. Client sends a command for `Entity{ id: 12, generation: 4}`. To do so, it maps local entity to the one from server. But `generation` field is 0 because it was omitted for serialization on the server. So it maps to `Entity{ id: 2, generation: 0}`.
4. Server receives `Entity{ id: 2, generation: 0}` which is invalid.
In my game I worked around it by [writing custom serialization](https://github.com/dollisgame/dollis/blob/master/src/core/network/entity_serde.rs) and using `serde(with = "...")`. But it feels like a bad default to me.
Using `Entity` over a custom `NetworkId` also have the following advantages:
1. Re-use `MapEntities` trait to map `Entity`s in replicated components.
2. Instead of server `Entity <-> NetworkId ` and `Entity <-> NetworkId`, we map entities only on client.
3. No need to handling uniqueness. It's a rare case, but makes things simpler. For example, I don't need to query for a resource to create an unique ID.
Closes bevyengine#6143.
## Solution
Use default serde impls. If anyone want to avoid wasting memory on `generation`, they can create a new type that holds `u32`. This is what Bevy do for [DynamicEntity](https://docs.rs/bevy/latest/bevy/scene/struct.DynamicEntity.html) to serialize scenes. And I don't see any use case to serialize an entity id expect this one.
---
## Changelog
### Changed
- Entity now serializes / deserializes `generation` field.
## Migration Guide
- Entity now fully serialized. If you want to serialze only `id`, as it was before, you can create a new type that wraps `u32`.
# Objective Following discussion on bevyengine#3536 and bevyengine#3522, `Handle::as_weak()` takes a type `U`, reinterpreting the handle as of another asset type while keeping the same ID. This is mainly used today in font atlas code. This PR does two things: - Rename the method to `cast_weak()` to make its intent more clear - Actually change the type uuid in the handle if it's not an asset path variant. ## Migration Guide - Rename `Handle::as_weak` uses to `Handle::cast_weak` The method now properly sets the associated type uuid if the handle is a direct reference (e.g. not a reference to an `AssetPath`), so adjust you code accordingly if you relied on the previous behavior.
# Objective - Allows bevy users to dispatch `multi_draw_indirect`, `multi_draw_indexed_indirect`, `multi_draw_indirect_count`, `multi_draw_indexed_indirect_count` draw calls. - Fixes bevyengine#6216 ## Solution - Added the corresponding wrapper methods to `TrackedRenderPass` --- ## Changelog > Added `multi_draw_*` draw calls to `TrackedRenderPass` Co-authored-by: Zhixing Zhang <[email protected]>
…#6401) # Objective Fix the soundness issue outlined in bevyengine#5866. In short the problem is that `query.to_readonly().get_component_mut::<T>()` can provide unsound mutable access to the component. This PR is an alternative to just removing the offending api. Given that `to_readonly` is a useful tool, I think this approach is a preferable short term solution. Long term I think theres a better solution out there, but we can find that on its own time. ## Solution Add what amounts to a "dirty flag" that marks Queries that have been converted to their read-only variant via `to_readonly` as dirty. When this flag is set to true, `get_component_mut` will fail with an error, preventing the unsound access.
# Objective Bevy still has many instances of using single-tuples `(T,)` to create a bundle. Due to bevyengine#2975, this is no longer necessary. ## Solution Search for regex `\(.+\s*,\)`. This should have found every instance.
# Objective - `Visibility` don't have `FromReflect` derive. ## Solution - Add it. --- ## Changelog ### Added - `FromReflect` for `Visibility`.
# Objective Noticed bevy_window doesn't ever use web-sys. That logic resides to bevy_winit and winit. ## Solution Remove web-sys dependency
# Objective - Time have `Reflect`, but doesn't have `FromReflect`. ## Solution - Add it for `Timer`, `Stopwatch` and `TimerMode`. --- ## Changelog ### Added * `FromReflect` derive for `Timer`, `Stopwatch` and `TimerMode`.
- Freeing unused memory held by visible entities - Fixed comment style # Objective With Rust 1.56 it's possible to shrink vectors to a specified capacity. Visibility system had a comment before asking for that feature to free unused memory by a vector if its capacity is two times larger than the length. ## Solution Shrinking the vector of visible entities to the nearest power of 2 elements next to `len()`, if capacity exceeds it more than two times.
…r descendants and ancestors (bevyengine#6185) # Objective Add methods to `Query<&Children>` and `Query<&Parent>` to iterate over descendants and ancestors, respectively. ## Changelog * Added extension trait for `Query` in `bevy_hierarchy`, `HierarchyQueryExt` * Added method `iter_descendants` to `Query<&Children>` via `HierarchyQueryExt` for iterating over the descendants of an entity. * Added method `iter_ancestors` to `Query<&Parent>` via `HierarchyQueryExt` for iterating over the ancestors of an entity. Co-authored-by: devil-ira <[email protected]>
# Objective
Currently toggling an `AudioSink` (for example from a game menu) requires writing
```rs
if sink.is_paused() {
sink.play();
} else {
sink.pause();
}
```
It would be nicer if we could reduce this down to a single line
```rs
sink.toggle();
```
## Solution
Add an `AudioSink::toggle` method which does exactly that.
---
## Changelog
- Added `AudioSink::toggle` which can be used to toggle state of a sink.
# Objective Fixes bevyengine#6378 `bevy_transform` is missing a feature corresponding to the `serialize` feature on the `bevy` crate. ## Solution Adds a `serialize` feature to `bevy_transform`. Derives `serde::Serialize` and `Deserialize` when feature is enabled.
…e#6381) # Objective - Bevy main crashs on Safari mobile - On Safari mobile, calling winit_window.set_cursor_grab(true) fails as the API is not implemented (as there is no cursor on Safari mobile, the api doesn't make sense there). I don't know about other mobile browsers ## Solution - Do not call the api to release cursor grab on window creation, as the cursor is not grabbed anyway at this point - This is bevyengine#3617 which was lost in bevyengine#6218
# Objective - Make it impossible to add a plugin twice - This is going to be more a risk for plugins with configurations, to avoid things like `App::new().add_plugins(DefaultPlugins).add_plugin(ImagePlugin::default_nearest())` ## Solution - Panic when a plugin is added twice - It's still possible to mark a plugin as not unique by overriding `is_unique` - ~~Simpler version of~~ bevyengine#3988 (not simpler anymore because of how `PluginGroupBuilder` implements `PluginGroup`)
# Objective
Entities are unique, however, this is not reflected in the scene format. Currently, entities are stored in a list where a user could inadvertently create a duplicate of the same entity.
## Solution
Switch from the list representation to a map representation for entities.
---
## Changelog
* The `entities` field in the scene format is now a map of entity ID to entity data
## Migration Guide
The scene format now stores its collection of entities in a map rather than a list:
```rust
// OLD
(
entities: [
(
entity: 12,
components: {
"bevy_transform::components::transform::Transform": (
translation: (
x: 0.0,
y: 0.0,
z: 0.0
),
rotation: (0.0, 0.0, 0.0, 1.0),
scale: (
x: 1.0,
y: 1.0,
z: 1.0
),
),
},
),
],
)
// NEW
(
entities: {
12: (
components: {
"bevy_transform::components::transform::Transform": (
translation: (
x: 0.0,
y: 0.0,
z: 0.0
),
rotation: (0.0, 0.0, 0.0, 1.0),
scale: (
x: 1.0,
y: 1.0,
z: 1.0
),
),
},
),
},
)
```
# Objective - Fixes bevyengine#6311 - Make it clearer what should be done in the example (close the Bevy app window) ## Solution - Remove the second windowed Bevy App [since winit does not support this](https://github.com/rust-windowing/winit/blob/v0.27.4/src/event_loop.rs#L82-L83) - Add title to the Bevy window asking the user to close it This is more of a quick fix to have a working example. It would be nicer if we had a small real usecase for this functionality. Another alternativ that I tried out: If we want to showcase a second Bevy app as it was before, we could still do this as long as one of them does not have a window. But I don't see how this is helpful in the context of the example, so I stuck with only one Bevy app and a simple print afterwards.
# Objective - `ReflectDefault` can be used to create default values for reflected types - `std` primitives that are `Default`-constructable should register `ReflectDefault` ## Solution - register `ReflectDefault`
…ngine#6432) # Objective - actions from actions-rs are outdated and use deprecated function - They haven't been updated for the last two years (https://github.com/actions-rs/toolchain) ## Solution - use the newer and up-to-date https://github.com/dtolnay/rust-toolchain
# Objective Post processing effects cannot read and write to the same texture. Currently they must own their own intermediate texture and redundantly copy from that back to the main texture. This is very inefficient. Additionally, working with ViewTarget is more complicated than it needs to be, especially when working with HDR textures. ## Solution `ViewTarget` now stores two copies of the "main texture". It uses an atomic value to track which is currently the "main texture" (this interior mutability is necessary to accommodate read-only RenderGraph execution). `ViewTarget` now has a `post_process_write` method, which will return a source and destination texture. Each call to this method will flip between the two copies of the "main texture". ```rust let post_process = render_target.post_process_write(); let source_texture = post_process.source; let destination_texture = post_process.destination; ``` The caller _must_ read from the source texture and write to the destination texture, as it is assumed that the destination texture will become the new "main texture". For simplicity / understandability `ViewTarget` is now a flat type. "hdr-ness" is a property of the `TextureFormat`. The internals are fully private in the interest of providing simple / consistent apis. Developers can now easily access the main texture by calling `view_target.main_texture()`. HDR ViewTargets no longer have an "ldr texture" with `TextureFormat::bevy_default`. They _only_ have their two "hdr" textures. This simplifies the mental model. All we have is the "currently active hdr texture" and the "other hdr texture", which we flip between for post processing effects. The tonemapping node has been rephrased to use this "post processing pattern". The blit pass has been removed, and it now only runs a pass when HDR is enabled. Notably, both the input and output texture are assumed to be HDR. This means that tonemapping behaves just like any other "post processing effect". It could theoretically be moved anywhere in the "effect chain" and continue to work. In general, I think these changes will make the lives of people making post processing effects much easier. And they better position us to start building higher level / more structured "post processing effect stacks". --- ## Changelog - `ViewTarget` now stores two copies of the "main texture". Calling `ViewTarget::post_process_write` will flip between copies of the main texture.
# Objective `bevy_core` is missing a feature corresponding to the `serialize` feature on the `bevy` crate. Similar to bevyengine#6378 and bevyengine#6379 to serialize `Name` easily. ## Solution Add this feature and hand-written serialization for `Name` (to avoid storing `hash` field). --- ## Changelog ### Added * `Serialize` and `Deserialize` derives for `Name` under `serialize` feature.
…engine#6309) # Objective Add documentation `#[world_query(ignore)]`. Fixes bevyengine#6283. --- I've only described it's behavior so far (which appears to be the same as with `system_param`). Is there another use-case for this besides with `PhantomData`? I could only find a single usage of this construct on GitHub, which is [here](https://github.com/tqwewe/bevy-editor-2/blob/ffcb816927a1bbdcf1cb0136ce47864e5040f9fb/bevy/examples/ecs/custom_query_param.rs#L102). I was also wondering if it would make sense to add a usage example to the `custom_query_example`? 🤔 That's why it's currently still in there. Co-authored-by: Lucas Jenß <[email protected]>
# Objective * Add benchmarks for `Query::get_many`. * Speed up `Query::get_many`. ## Solution Previously, `get_many` and `get_many_mut` used the method `array::map`, which tends to optimize very poorly. This PR replaces uses of that method with loops. ## Benchmarks | Benchmark name | Execution time | Change from this PR | |--------------------------------------|----------------|---------------------| | query_get_many_2/50000_calls_table | 1.3732 ms | -24.967% | | query_get_many_2/50000_calls_sparse | 1.3826 ms | -24.572% | | query_get_many_5/50000_calls_table | 2.6833 ms | -30.681% | | query_get_many_5/50000_calls_sparse | 2.9936 ms | -30.672% | | query_get_many_10/50000_calls_table | 5.7771 ms | -36.950% | | query_get_many_10/50000_calls_sparse | 7.4345 ms | -36.987% |
…ngine#6437) # Objective Currently, `bevy_dynamic_plugin` simply panics on error. This makes it impossible to handle failures in applications that use this feature. For example, I'd like to build an optional expansion for my game, that may not be distributed to all users. I want to use `bevy_dynamic_plugin` for loading it. I want my game to try to load it on startup, but continue without it if it cannot be loaded. ## Solution - Make the `dynamically_load_plugin` function return a `Result`, so it can gracefully return loading errors. - Create an error enum type, to provide useful information about the kind of error. This adds `thiserror` to the dependencies of `bevy_dynamic_plugin`, but that dependency is already used in other parts of bevy (such as `bevy_asset`), so not a big deal. I chose not to change the behavior of the builder method in the App extension trait. I kept it as panicking. There is no clean way (that I'm aware of) to make a builder-style API that has fallible methods. So it is either a panic or a warning. I feel the panic is more appropriate. --- ## Changelog ### Changed - `bevy_dynamic_plugin::dynamically_load_plugin` now returns `Result` instead of panicking, to allow for error handling
For `derive(WorldQuery)`, there are three structs generated, `Item`, `Fetch` and `State`. These inherit the visibility of the derived structure, thus `#![warn(missing_docs)]` would warn about missing documentation for these structures. - [ ] I'd like some advice on what to write here, as I personally don't really understand `Fetch` nor `State`.
# Objective - Add post processing passes for FXAA (Fast Approximate Anti-Aliasing) - Add example comparing MSAA and FXAA ## Solution When the FXAA plugin is added, passes for FXAA are inserted between the main pass and the tonemapping pass. Supports using either HDR or LDR output from the main pass. --- ## Changelog - Add a new FXAANode that runs after the main pass when the FXAA plugin is added. Co-authored-by: Carter Anderson <[email protected]>
DGriffin91
added a commit
that referenced
this pull request
Feb 16, 2023
Tonemapping example refactor
JMS55
pushed a commit
that referenced
this pull request
Jan 14, 2024
Compensate for exposure after sampling background color, adjust transmission example
JMS55
pushed a commit
that referenced
this pull request
Jun 25, 2024
…3905) # Objective - first part of bevyengine#13900 ## Solution - split `check_light_mesh_visibility `into `check_dir_light_mesh_visibility `and `check_point_light_mesh_visibility` for better review
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.