Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Further bloom improvements #4

Merged

Conversation

StarLederer
Copy link

I am assuming I don't have to follow Bevys PR guidelines here.

This PR Updates the fork to the version I demonstrate here.

I might take some time to think about how to tackle aspect ratio issues. If you want to make changes to the code in the mean time i think it is will make everyone's job easier if you merge this first (if you agree with the changes of course)

james7132 and others added 30 commits November 15, 2022 20:31
# Objective
Fix bevyengine#6555.

## Solution
Bump `gilrs` version to 0.10.
…evyengine#4927)

# Objective
Archetype is a deceptively large type in memory. It stores metadata about which components are in which storage in multiple locations, which is only used when creating new Archetypes while moving entities.

## Solution
Remove the redundant `Box<[ComponentId]>`s and iterate over the sparse set of component metadata instead. Reduces Archetype's size by 4 usizes (32 bytes on 64-bit systems), as well as the additional allocations for holding these slices.

It'd seem like there's a downside that the origin archetype has it's component metadata iterated over twice when creating a new archetype, but this change also removes the extra `Vec<ArchetypeComponentId>` allocations when creating a new archetype which may amortize out to a net gain here. This change likely negatively impacts creating new archetypes with a large number of components, but that's a cost mitigated by the fact that these archetypal relationships are cached in Edges and is incurred only once for each edge created.

## Additional Context
There are several other in-flight PRs that shrink Archetype:

 - bevyengine#4800 merges the entities and rows Vecs together (shaves off 24 bytes per archetype) 
 - bevyengine#4809 removes unique_components and moves it to it's own dedicated storage (shaves off 72 bytes per archetype)

---

## Changelog
Changed: `Archetype::table_components` and `Archetype::sparse_set_components` return iterators instead of slices. `Archetype::new` requires iterators instead of parallel slices/vecs.

## Migration Guide
Do I still need to do this? I really hope people were not relying on the public facing APIs changed here.
# Objective
Make core types in ECS smaller. The column sparse set in Tables is never updated after creation.

## Solution
Create `ImmutableSparseSet` which removes the capacity fields in the backing vec's and the APIs for inserting or removing elements. Drops the size of the sparse set by 3 usizes (24 bytes on 64-bit systems)

## Followup
~~After bevyengine#4809, Archetype's component SparseSet should be replaced with it.~~ This has been done.

---

## Changelog
Removed: `Table::component_capacity`

## Migration Guide
`Table::component_capacity()` has been removed as Tables do not support adding/removing columns after construction.

Co-authored-by: Carter Anderson <mcanders1@gmail.com>
# Objective

* Enable `Res` and `Query` parameter mutual exclusion
* Required for bevyengine#5080

The `FilteredAccessSet::get_conflicts` methods didn't work properly with
`Res` and `ResMut` parameters. Because those added their access by using
the `combined_access_mut` method and directly modifying the global
access state of the FilteredAccessSet. This caused an inconsistency,
because get_conflicts assumes that ALL added access have a corresponding
`FilteredAccess` added to the `filtered_accesses` field.

In practice, that means that SystemParam that adds their access through
the `Access` returned by `combined_access_mut` and the ones that add
their access using the `add` method lived in two different universes. As
a result, they could never be mutually exclusive.

## Solution

This commit fixes it by removing the `combined_access_mut` method. This
ensures that the `combined_access` field of FilteredAccessSet is always
updated consistently with the addition of a filter. When checking for
filtered access, it is now possible to account for `Res` and `ResMut`
invalid access. This is currently not needed, but might be in the
future.

We add the `add_unfiltered_{read,write}` methods to replace previous
usages of `combined_access_mut`.

We also add improved Debug implementations on FixedBitSet so that their
meaning is much clearer in debug output.


---

## Changelog

* Fix `Res` and `Query` parameter never being mutually exclusive.

## Migration Guide

Note: this mostly changes ECS internals, but since the API is public, it is technically breaking:
* Removed `FilteredAccessSet::combined_access_mut`
  * Replace _immutable_ usage of those by `combined_access`
  * For _mutable_ usages, use the new `add_unfiltered_{read,write}` methods instead of `combined_access_mut` followed by `add_{read,write}`
…#4853)

# Objective
BlobVec currently relies on a scratch piece of memory allocated at initialization to make a temporary copy of a component when using `swap_remove_and_{forget/drop}`. This is potentially suboptimal as it writes to a, well-known, but random part of memory instead of using the stack.

## Solution
As the `FIXME` in the file states, replace `swap_scratch` with a call to `swap_nonoverlapping::<u8>`. The swapped last entry is returned as a `OwnedPtr`.

In theory, this should be faster as the temporary swap is allocated on the stack, `swap_nonoverlapping` allows for easier vectorization for bigger types, and the same memory is used between the swap and the returned `OwnedPtr`.
# Objective

- The `contributors` example panics when attempting to generate an empty range if the window height is smaller than the sprites
- Don't do that

## Solution

- Clamp the bounce height to be 0 minimum, and generate an inclusive range when passing it to `rng.gen_range`
When I was upgrading to 0.9 noticed there were some changes to the timer, mainly the `TimerMode`.  When switching from the old `is_repeating()`  and `set_repeating()` to the new `mode()` and `set_mode()` noticed the docs still had the old description.
# Objective

- Fix a panic in wasm when using globals in a shader

## Solution

- Similar to bevyengine#6460
# Objective

Fixes bevyengine#6661 

## Solution

Make `SceneSpawner::spawn_dynamic` return `InstanceId` like other functions there.

---

## Changelog

Make `SceneSpawner::spawn_dynamic` return `InstanceId` instead of `()`.
# Objective

Delete `ImageMode`. It doesn't do anything except mislead people into thinking it controls the aspect ratio of images somehow.

Fixes bevyengine#3933 and bevyengine#6637

## Solution

Delete `ImageMode`

## Changelog

Removes the `ImageMode` enum.
Removes the `image_mode` field from `ImageBundle`
Removes the `With<ImageMode>` query filter from `image_node_system`
Renames `image_node_system` to` update_image_calculated_size_system`
# Objective

structs containing wgpu types take a long time to compile. this is particularly bad for generics containing the wgpu structs (like the depth pipeline builder with `#[derive(SystemParam)]` i've been working on).

we can avoid that by boxing and type-erasing in the bevy `render_resource` wrappers.

type system magic is not a strength of mine so i guess there will be a cleaner way to achieve this, happy to take feedback or for it to be taken as a proof of concept if someone else wants to do a better job.

## Solution

- add macros to box and type-erase in debug mode
- leave current impl for release mode

timings:


<html xmlns:v="urn:schemas-microsoft-com:vml"
xmlns:o="urn:schemas-microsoft-com:office:office"
xmlns:x="urn:schemas-microsoft-com:office:excel"
xmlns="http://www.w3.org/TR/REC-html40">

<head>

<meta name=ProgId content=Excel.Sheet>
<meta name=Generator content="Microsoft Excel 15">
<link id=Main-File rel=Main-File
href="file:///C:/Users/robfm/AppData/Local/Temp/msohtmlclip1/01/clip.htm">
<link rel=File-List
href="file:///C:/Users/robfm/AppData/Local/Temp/msohtmlclip1/01/clip_filelist.xml">
<!--table
	{mso-displayed-decimal-separator:"\.";
	mso-displayed-thousand-separator:"\,";}
@page
	{margin:.75in .7in .75in .7in;
	mso-header-margin:.3in;
	mso-footer-margin:.3in;}
tr
	{mso-height-source:auto;}
col
	{mso-width-source:auto;}
br
	{mso-data-placement:same-cell;}
td
	{padding-top:1px;
	padding-right:1px;
	padding-left:1px;
	mso-ignore:padding;
	color:black;
	font-size:11.0pt;
	font-weight:400;
	font-style:normal;
	text-decoration:none;
	font-family:Calibri, sans-serif;
	mso-font-charset:0;
	mso-number-format:General;
	text-align:general;
	vertical-align:bottom;
	border:none;
	mso-background-source:auto;
	mso-pattern:auto;
	mso-protection:locked visible;
	white-space:nowrap;
	mso-rotate:0;}
.xl65
	{mso-number-format:0%;}
.xl66
	{vertical-align:middle;
	white-space:normal;}
.xl67
	{vertical-align:middle;}
-->
</head>

<body link="#0563C1" vlink="#954F72">



current |   |   |  
-- | -- | -- | --
  | Total time: | 64.9s |  
  | bevy_pbr v0.9.0-dev | 19.2s |  
  | bevy_render v0.9.0-dev | 17.0s |  
  | bevy_sprite v0.9.0-dev | 15.1s |  
  | DepthPipelineBuilder | 18.7s |  
  |   |   |  
with type-erasing |   |   | diff
  | Total time: | 49.0s | -24%
  | bevy_render v0.9.0-dev | 12.0s | -38%
  | bevy_pbr v0.9.0-dev | 8.7s | -49%
  | bevy_sprite v0.9.0-dev | 6.1s | -60%
  | DepthPipelineBuilder | 1.2s | -94%



</body>

</html>

the depth pipeline builder is a binary with body: 
```rust
use std::{marker::PhantomData, hash::Hash};
use bevy::{prelude::*, ecs::system::SystemParam, pbr::{RenderMaterials, MaterialPipeline, ShadowPipeline}, render::{renderer::RenderDevice, render_resource::{SpecializedMeshPipelines, PipelineCache}, render_asset::RenderAssets}};

fn main() {
    println!("Hello, world p!\n");
}

#[derive(SystemParam)]
pub struct DepthPipelineBuilder<'w, 's, M: Material> 
where M::Data: Eq + Hash + Clone,
{
    render_device: Res<'w, RenderDevice>,
    material_pipeline: Res<'w, MaterialPipeline<M>>,
    material_pipelines: ResMut<'w, SpecializedMeshPipelines<MaterialPipeline<M>>>,
    shadow_pipeline: Res<'w, ShadowPipeline>,
    pipeline_cache: ResMut<'w, PipelineCache>,
    render_meshes: Res<'w, RenderAssets<Mesh>>,
    render_materials: Res<'w, RenderMaterials<M>>,
    msaa: Res<'w, Msaa>,
    #[system_param(ignore)]
    _p: PhantomData<&'s M>,
}
```
# Objective

Fix android touch events being flipped.  Only removed test for android, don't have ios device to test with.  Tested with emulator and physical device.

## Solution

Remove check, no longer needed with coordinate change in 0.9
# Objective

Make core pipeline graphic nodes, including `BloomNode`, `FxaaNode`, `TonemappingNode` and `UpscalingNode` public.
This will allow users to construct their own render graphs with these build-in nodes.

## Solution

Make them public.
Also put node names into bevy's core namespace (`core_2d::graph::node`, `core_3d::graph::node`) which makes them consistent.
…er (bevyengine#5214)

# Objective

Fix bevyengine#5149

## Solution

Instead of returning the **total count** of elements in the `QueryIter` in
`size_hint`, we return the **count of remaining elements**. This
Fixes bevyengine#5149 even when bevyengine#5148 gets merged.

- bevyengine#5149
- bevyengine#5148

---

## Changelog

- Fix partially consumed `QueryIter` and `QueryCombinationIter` having invalid `size_hint`


Co-authored-by: Nicola Papale <nicopap@users.noreply.github.com>
# Objective
Fix bevyengine#6453. 

## Solution
Use the solution mentioned in the issue by catching the unwind and dropping the error. Wrap the `executor.try_tick` calls with `std::catch::unwind`.

Ideally this would be moved outside of the hot loop, but the mut ref to the `spawned` future is not `UnwindSafe`.

This PR only addresses the bug, we can address the perf issues (should there be any) later.
# Objective
Fixes bevyengine#4884. `ComponentTicks` stores both added and changed ticks contiguously in the same 8 bytes. This is convenient when passing around both together, but causes half the bytes fetched from memory for the purposes of change detection to effectively go unused. This is inefficient when most queries (no filter, mutating *something*) only write out to the changed ticks.

## Solution
Split the storage for change detection ticks into two separate `Vec`s inside `Column`. Fetch only what is needed during iteration.

This also potentially also removes one blocker from autovectorization of dense queries.

EDIT: This is confirmed to enable autovectorization of dense queries in `for_each` and `par_for_each`  where possible.  Unfortunately `iter` has other blockers that prevent it.

### TODO

 - [x] Microbenchmark
 - [x] Check if this allows query iteration to autovectorize simple loops.
 - [x] Clean up all of the spurious tuples now littered throughout the API

### Open Questions

 - ~~Is `Mut::is_added` absolutely necessary? Can we not just use `Added` or `ChangeTrackers`?~~ It's optimized out if unused.
 - ~~Does the fetch of the added ticks get optimized out if not used?~~ Yes it is.

---

## Changelog
Added: `Tick`, a wrapper around a single change detection tick.
Added: `Column::get_added_ticks`
Added: `Column::get_column_ticks`
Added: `SparseSet::get_added_ticks`
Added: `SparseSet::get_column_ticks`
Changed: `Column` now stores added and changed ticks separately internally.
Changed: Most APIs returning `&UnsafeCell<ComponentTicks>` now returns `TickCells` instead, which contains two separate `&UnsafeCell<Tick>` for either component ticks.
Changed: `Query::for_each(_mut)`, `Query::par_for_each(_mut)` will now leverage autovectorization to speed up query iteration where possible.

## Migration Guide
TODO
# Objective

- Bevy should be usable to create 'overlay' type apps, where the input is not captured by Bevy, but passed down/into a target app, or to allow passive displays/widgets etc.
 
## Solution

- the `winit::window::Window` already has a `set_cursor_hittest()` which basically does this for mouse input events, so I've exposed it (trying to copy the style laid out in the existing wrappings, and added a simple demo.

---

## Changelog

- Added `hittest` to `WindowAttributes`
- Added the `hittest`'s setters/getters
- Modified the `WindowBuilder`
- Modifed the `WindowDescriptor`'s `Default` impl.
- Added an example `cargo run --example fallthrough`
# Objective

This add a ctor to `Box` to aid the creation of non-centred boxes. The PR adopts @rezural's work on PR bevyengine#3322, taking into account the feedback on that PR from @james7132.

## Solution

`Box::from_corners()` creates a `Box` from two opposing corners and automatically determines the min and max extents to ensure that the `Box` is well-formed.

Co-authored-by: rezural <rezural@protonmail.com>
…ngine#6676)

# Objective

Latest Release, "bevy 0.9" move the FrameCount updater into RenderPlugin, it leads to user who only run app with Core/Minimal Plugin cannot get the right number of FrameCount, it always return 0.

As for use cases like a server app, we don't want to add render dependencies to the app.

More detail in bevyengine#6656 

## Solution

- Move the `update_frame_count` into CorePlugin
Add a method to rotate a transform to point towards a direction.

Also updated the docs to link to `forward` and `up` instead of mentioning local negative `Z` and local `Y`.

Unfortunately, links to methods don't work in rust-analyzer :(

Co-authored-by: Devil Ira <justthecooldude@gmail.com>
# Objective

Allow more use cases where the user may benefit from both `ExtractComponentPlugin` _and_ `UniformComponentPlugin`.

## Solution

Add an associated type to `ExtractComponent` in order to allow specifying the output component (or bundle).

Make `extract_component` return an `Option<_>` such that components can be extracted only when needed.

What problem does this solve?

`ExtractComponentPlugin` allows extracting components, but currently the output type is the same as the input.
This means that use cases such as having a settings struct which turns into a uniform is awkward.

For example we might have:

```rust
struct MyStruct {
    enabled: bool,
    val: f32
}

struct MyStructUniform {
    val: f32
}
```

With the new approach, we can extract `MyStruct` only when it is enabled, and turn it into its related uniform.

This chains well with `UniformComponentPlugin`.

The user may then:

```rust
app.add_plugin(ExtractComponentPlugin::<MyStruct>::default());
app.add_plugin(UniformComponentPlugin::<MyStructUniform>::default());
```

This then saves the user a fair amount of boilerplate.


## Changelog

### Changed

- `ExtractComponent` can specify output type, and outputting is optional.



Co-authored-by: Torstein Grindvik <52322338+torsteingrindvik@users.noreply.github.com>
Without this fix, piped systems containing exclusive systems fail to run, giving a runtime panic.
With this PR, running piped systems that contain exclusive systems now works.

## Explanation of the bug

This is because, unless overridden, the default implementation of `run` from the `System` trait simply calls `run_unsafe`. That is not valid for exclusive systems. They must always be called via `run`, as `run_unsafe` takes `&World` instead of `&mut World`.

Trivial reproduction example:
```rust
fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_system(exclusive.pipe(another))
        .run();
}

fn exclusive(_world: &mut World) {}
fn another() {}
```
If you run this, you will get a panic 'Cannot run exclusive systems with a shared World reference' and the backtrace shows how bevy (correctly) tries to call the `run` method (because the system is exclusive), but it is the implementation from the `System` trait (because `PipeSystem` does not have its own), which calls `run_unsafe` (incorrect):
 - 3: <bevy_ecs::system::system_piping::PipeSystem<SystemA,SystemB> as bevy_ecs::system::system::System>::run_unsafe
 - 4: bevy_ecs::system::system::System::run
# Objective

Fixes bevyengine#6498.

## Solution

Adding a parent node with properties AlignItems::Center and JustifyContent::Center to centered child nodes and removing their auto-margin properties.
# Objective
Fixes bevyengine#4697. Hierarchical propagation of properties, currently only Transform -> GlobalTransform, can be a very expensive operation. Transform propagation is a strict dependency for anything positioned in world-space. In large worlds, this can take quite a bit of time, so limiting it to a single thread can result in poor CPU utilization as it bottlenecks the rest of the frame's systems.

## Solution

 - Move transforms without a parent or a child (free-floating (Global)Transform) entities into a separate parallel system.
 - Chunk the hierarchy based on the root entities and process it in parallel with `Query::par_for_each_mut`. 
 - Utilize the hierarchy's specific properties introduced in bevyengine#4717 to allow for safe use of `Query::get_unchecked` on multiple threads. Assuming each child is unique in the hierarchy, it is impossible to have an aliased `&mut GlobalTransform` so long as we verify that the parent for a child is the same one propagated from.

---

## Changelog
Removed: `transform_propagate_system` is no longer `pub`.
# Objective

- Fix dead links on this docs page: https://docs.rs/bevy/0.9.0/bevy/struct.DefaultPlugins.html

## Solution

- Point links to the imported versions instead of the versions in external crates.
# Objective

`add_node_edge` and `add_slot_edge` are fallible methods, but are always used with `.unwrap()`.
`input_node` is often unwrapped as well.
This points to having an infallible behaviour as default, with an alternative fallible variant if needed.

Improves readability and ergonomics.

## Solution

- Change `add_node_edge` and `add_slot_edge` to panic on error.
- Change `input_node` to panic on `None`.
- Add `try_add_node_edge` and `try_add_slot_edge` in case fallible methods are needed.
- Add `get_input_node` to still be able to get an `Option`.
---

## Changelog

### Added

- `try_add_node_edge`
- `try_add_slot_edge`
- `get_input_node`

### Changed

- `add_node_edge` is now infallible (panics on error)
- `add_slot_edge` is now infallible (panics on error)
- `input_node` now panics on `None`

## Migration Guide

Remove `.unwrap()` from `add_node_edge` and `add_slot_edge`.
For cases where the error was handled, use `try_add_node_edge` and `try_add_slot_edge` instead.

Remove `.unwrap()` from `input_node`.
For cases where the option was handled, use `get_input_node` instead.


Co-authored-by: Torstein Grindvik <52322338+torsteingrindvik@users.noreply.github.com>
# Objective

- shaders defs can now have a `bool` or `int` value
- `#if SHADER_DEF <operator> 3`
  - ok if `SHADER_DEF` is defined, has the correct type and pass the comparison
  - `==`, `!=`, `>=`, `>`, `<`, `<=` supported
- `#SHADER_DEF` or `#{SHADER_DEF}`
  - will be replaced by the value in the shader code
---

## Migration Guide

- replace `shader_defs.push(String::from("NAME"));` by `shader_defs.push("NAME".into());`
- if you used shader def `NO_STORAGE_BUFFERS_SUPPORT`, check how `AVAILABLE_STORAGE_BUFFER_BINDINGS` is now used in Bevy default shaders
silvestrpredko and others added 5 commits December 28, 2022 17:09
# Objective

I am new to Bevy. And during my development, I noticed that the `iOS` example doesn't work.
Example panics with next message: ```panicked at 'Resource requested by bevy_ui::widget::text::text_system does not exist: bevy_asset::assets::Assets```.

I have asked for help in a `discord` iOS chat and there I receive a recommendation that it is possible that some bevy features missing.

## Solution

So, I used ```bevy``` with default features.
# Objective

- Fixes bevyengine#7047 

## Solution

- Pass the correct view entity
# Objective

`xlibsWrapper` is being deprecated: NixOS/nixpkgs#194054, this pr removes the deprecated xlibsWrapper and makes a couple more improvements

## Solution

- rename NixOS to Nix since this is not specific to NixOS
- remove usage of `xlibsWrapper`
- add instructions for nix flakes with `nix develop`
- add example of a packaged bevy program in nixpkgs
- minor cosmetic/grammatical changes
Old comment is Z-up ,  Fix comment for bevy Y-up

# Objective

- Update Box vertices comment for bevy Y-up

## Solution

- Update comment for Y-up

---

## Changelog

None

## Migration Guide

None
@JMS55
Copy link
Owner

JMS55 commented Jan 2, 2023

Happy near year. I wanted to let you know that I will be somewhat away for the next ~2 weeks. I'll have internet access, but somewhat limited.

This PR is almost ready. As I'll be away, you should take over the bloom PR (assuming you have time). I've invited you as a "collaborator" to this repository (please don't modify other branches besides the bloom one). Feel free to merge this once you feel it's ready, and then update the PR description on the bevy repo so that people can start reviewing it before the next release :)

@StarLederer
Copy link
Author

Happy new year! Thanks for assisting me in this PR so far. I've accepted the invitation and will treat this fork responsibly

StarLederer and others added 18 commits January 2, 2023 17:25
…e#5911)

# Objective
Solve bevyengine#5464 

## Solution
Adds a `SystemInformationDiagnosticsPlugin` to add diagnostics.

Adds `Cargo.toml` flags to fix building on different platforms. 

---

## Changelog

Adds `sysinfo` crate to `bevy-diagnostics`. 

Changes in import order are due to clippy.

Co-authored-by: l1npengtul <35755164+l1npengtul@users.noreply.github.com>
Co-authored-by: IceSentry <c.giguere42@gmail.com>
# Objective

There isn't really a way to test that code using bevy_reflect compiles or doesn't compile for certain scenarios. This would be especially useful for macro-centric PRs like bevyengine#6511 and bevyengine#6042.

## Solution

Using `bevy_ecs_compile_fail_tests` as reference, added the `bevy_reflect_compile_fail_tests` crate.

Currently, this crate contains a very simple test case. This is so that we can get the basic foundation of this crate agreed upon and merged so that more tests can be added by other PRs.

### Open Questions

- [x] Should this be added to CI? (Answer: Yes)

---

## Changelog

- Added the `bevy_reflect_compile_fail_tests` crate for testing compilation errors
# Objective
`Query::get` and other random access methods require looking up `EntityLocation` for every provided entity, then always looking up the `Archetype` to get the table ID and table row. This requires 4 total random fetches from memory: the `Entities` lookup, the `Archetype` lookup, the table row lookup, and the final fetch from table/sparse sets. If `EntityLocation` contains the table ID and table row, only the `Entities` lookup and the final storage fetch are required.

## Solution
Add `TableId` and table row to `EntityLocation`. Ensure it's updated whenever entities are moved around. To ensure `EntityMeta` does not grow bigger, both `TableId` and `ArchetypeId` have been shrunk to u32, and the archetype index and table row are stored as u32s instead of as usizes. This should shrink `EntityMeta` by 4 bytes, from 24 to 20 bytes, as there is no padding anymore due to the change in alignment.

This idea was partially concocted by @BoxyUwU. 

## Performance
This should restore the `Query::get` "gains" lost to bevyengine#6625 that were introduced in bevyengine#4800 without being unsound, and also incorporates some of the memory usage reductions seen in bevyengine#3678.

This also removes the same lookups during add/remove/spawn commands, so there may be a bit of a speedup in commands and `Entity{Ref,Mut}`.

---

## Changelog
Added: `EntityLocation::table_id`
Added: `EntityLocation::table_row`.
Changed: `World`s can now only hold a maximum of 2<sup>32</sup>- 1 archetypes.
Changed: `World`s can now only hold a maximum of 2<sup>32</sup> - 1 tables.

## Migration Guide

A `World` can only hold a maximum of 2<sup>32</sup> - 1 archetypes and tables now. If your use case requires more than this, please file an issue explaining your use case.
…ine#7043)

# Objective

- The recently merged PR bevyengine#7013 does not allow multiple `RenderPhase`s to share the same `RenderPass`.
- Due to the introduced overhead we want to minimize the number of `RenderPass`es recorded during each frame.

## Solution

- Take a constructed `TrackedRenderPass` instead of a `RenderPassDiscriptor` as a parameter to the `RenderPhase::render` method.

---

## Changelog

To enable multiple `RenderPhases` to share the same `TrackedRenderPass`,
the `RenderPhase::render` signature has changed.

```rust
pub fn render<'w>(
  &self,
  render_pass: &mut TrackedRenderPass<'w>,
  world: &'w World,
  view: Entity)
```


Co-authored-by: Kurt Kühnert <51823519+kurtkuehnert@users.noreply.github.com>
…ngine#7069)

# Objective

- The bevyengine#7064 PR had poor performance on an M1 Max in MacOS due to significant overuse of registers resulting in 'register spilling' where data that would normally be stored in registers on the GPU is instead stored in VRAM. The latency to read from/write to VRAM instead of registers incurs a significant performance penalty.
- Use of registers is a limiting factor in shader performance. Assignment of a struct from memory to a local variable can incur copies. Passing a variable that has struct type as an argument to a function can also incur copies. As such, these two cases can incur increased register usage and decreased performance.

## Solution

- Remove/avoid a number of assignments of light struct type data to local variables.
- Remove/avoid a number of passing light struct type variables/data as value arguments to shader functions.
@StarLederer
Copy link
Author

Merged upstream.

Tried implementing bevyengine#6802 first but it used newer code that was very different in some parts. I'm surprised merging was not too bad considering the change with uniform extraction. bevyengine#6802 should be really easy to implement now

@StarLederer
Copy link
Author

All done! 🎉🎉🎉

I all planned changes have been implemented, change requests addressed and bevyengine#6802 implemented. Transparent windows and custom view ports both work. I will now proceed to merge this and update the PR on upstream.

@StarLederer StarLederer merged commit 38d7c45 into JMS55:bloom-transparent-fix Jan 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.