Renderer Rework: Initial Merge Tracking Issue #2535

cart · 2021-07-24T20:40:29Z

The Bevy Renderer Rework is starting to stabilize and it is time to start planning how to upstream it! The intent of this issue is to track the work required to merge the renderer rework as soon as possible. This isn't a "renderer feature wishlist". But feel free to discuss what you think should be on this list!

This is an issue to track work that still needs to be done. If there is a name in parentheses, that work has been claimed by someone.

For some history on this effort, check out:

Bevy Render Rework: Initial Framing and Proof of Concept #2265
Bevy Render Rework Round 2: SubGraphs, Views, and Shadows #2351
experimental pull requests in @cart's repo: https://github.com/cart/bevy/pulls?q=is%3Apr

Here is a list of open prs against the pipelined-rendering branch

Missing Required Features

The new renderer must have (approximate) feature parity with the old renderer.

Missing Nice-To-Have Features

These aren't required for a merge, but would be very nice to have.

Discussions To Have Before Merging

Consider RenderDevice + RenderQueue -> GpuDevice + GpuQueue
Consider using atomic counter instead of UUID for render resource ids
Should we keep the BevyDefault trait?
Agree on the final "shadow enable/disable" api

Steps to Merge

The text was updated successfully, but these errors were encountered:

superdump · 2021-07-24T21:48:10Z

Here's the PR for bevy_gltf2: #2537

superdump · 2021-07-25T06:30:24Z

I’ve got MSAA working but it’s hacky because we need one of:

the ability to configure what node slots are available dynamically
the ability to leave node slots disconnected and make them optional, doing a sensible automatic fallback where it makes sense
hacks like connecting something to a node slot for the sake of filling the slot and passing a run sub graph input that isn’t used

superdump · 2021-07-25T11:20:26Z

Here's a work-in-progress PR for MSAA that I would appreciate some guidance on: #2541

I was trying to support dynamically switching MSAA sample count at run time, which is why it looks more complicated than it perhaps needs to be. I hit the wall mentioned in my last message when switching between 1 sample and multiple samples or vice versa.

However, I wanted to see if I could at least support switching between 2/4/8, but it turned out my Radeon Pro 5500M only supports 1 sample or 4x MSAA samples, so I couldn't test that.

I thought that as I had done most of the work for dynamically adjusting the MSAA sample count, I'd leave it in the PR and see what feedback I got.

superdump · 2021-07-25T11:24:30Z

And a small PR to log the adapter and wgpu backend being used at initialisation time. It's useful to see. #2542

superdump · 2021-07-25T11:27:48Z

I'll do ClearColor as well when there is feedback on MSAA as it's a small change and that touches the same code.

superdump · 2021-07-25T11:46:43Z

Infinite reverse right-handed PR here: #2543

It has a known bug of all geometry being black when looking along -Z that I am still debugging. Help welcome.

superdump · 2021-07-25T15:32:17Z

* Update BufferVec to use Queue (like UniformVec does)

What does this mean? I don't see anything about UniformVec that uses a Queue type.

cart · 2021-07-25T17:37:37Z

Great work @superdump. I'll dig in tomorrow / answer questions. Just a heads up that CI on pipelined-rendering is broken. Almost done fixing it in #2538

superdump · 2021-07-26T09:39:03Z

* the ability to configure what node slots are available dynamically

I added support for removing nodes, node edges, slot edges, and subgraphs. I've only tested use of the APIs for removal of slot edges but now MSAA can be toggled on the fly and I added that into the msaa_pipelined example - press 'm' to try it out.

aevyrie · 2021-07-26T17:08:52Z

Is there anything I can do to help move the primitive shapes RFC forward? I'd be happy to contribute towards implementation PRs once you've reviewed it.

cart · 2021-07-26T18:11:53Z

@aevyrie haha sadly not really (other than continuing to ping me so I don't forget and trying to get more reviews on it). I'll try to get to it in the next day or so as we do want to get the ball rolling on visibility.

aevyrie · 2021-07-26T19:01:34Z

Sounds good 🙂. I'd like to help with visibility/culling if you don't mind including me in those discussions.

cart · 2021-07-26T19:05:47Z

I'd like to help with visibility/culling if you don't mind including me in those discussions.

I'd love to include you in those discussions :)

notverymoe · 2021-07-27T11:14:40Z

Hi there, fresh face, just wanted to volunteer for some of the writing/updating documentation and porting examples - if you're looking for helpers on that

superdump · 2021-07-27T20:04:49Z

Hi there, fresh face, just wanted to volunteer for some of the writing/updating documentation and porting examples - if you're looking for helpers on that

Just dive in! :) Mention here what you’re working on and go for it.

notverymoe · 2021-07-28T13:14:14Z

Alright, I've made a small start just going through the changes. ~~Next I'll begin porting the examples, probably a good way to get familiar.~~

~~Edit: Converted most of the 3d examples, excl. render_to_texture. Only broken examples so far are those that rely on things already noted as unimplemented.~~

Edit 2: Bit too eager apparently 😅 As cart pointed out it's probably best to hold off on that. Doco it is then! Things will change as other tasks complete, but I can at least identify sections and rewrite some sections now.

Edit 3: Still here and keen, just had an up-tick in my day job's workload for a bit there

cart · 2021-07-28T17:14:08Z

Just a heads up that most of the examples wont need porting once we remove the old renderer and replace it with the new one. I don't think we should migrate examples until we've removed the old renderer (which we should wait to do until we have feature parity with the old renderer).

cart · 2021-07-28T23:17:58Z

@superdump

Update BufferVec to use Queue (like UniformVec does)

Oops! I understand why this is confusing: I haven't merged those changes yet. They're available on my custom-shaders branch though: https://github.com/cart/bevy/blob/custom-shaders/pipelined/bevy_render2/src/render_resource/uniform_vec.rs. Sorry for the confusion. I crossed some wires 😄

kirawi · 2021-07-29T18:43:45Z

Will this be in 0.6?

cart · 2021-07-29T19:18:48Z

That is the goal!

Davier · 2021-08-09T20:01:53Z

I'm starting to work on porting bevy_ui

zicklag · 2021-08-09T22:11:50Z

Just ran into the need to set the clear color with the new renderer. Would it be helpful if I added a way to do that? Also, would we want to use the same, ClearColor resource strategy as we did with the old renderer?

Edit: Sorry, just saw the TODO in the list:

Clear Color resource

I'll take that! 😄

zicklag · 2021-08-10T16:12:00Z

I'll also work on "SubApp ZST labels instead of integers", but I could use some guidance on where to put the implementation: #2629.

Weibye · 2021-08-12T10:47:47Z

This should have the label C-Tracking-Issue (?)

cart · 2021-08-13T00:33:45Z

Just ran into the need to set the clear color with the new renderer. Would it be helpful if I added a way to do that? Also, would we want to use the same, ClearColor resource strategy as we did with the old renderer?
I'll take that! smile

Cool cool! Yeah I think we should use the "current" clear color approach. Eventually for "environmental" things I think we might need something that plays nicer with scenes, but for now I think we should go with the simple solution that works.

I'll also work on "SubApp ZST labels instead of integers", but I could use some guidance on where to put the implementation: #2629.

Sounds good!

superdump · 2021-11-13T19:36:19Z

[Merged by Bors] - Per-light toggleable shadow mapping #3126

I'm going to get clustered-forward rendering in shape next. I rebased it once but something broke and gave everything a green tint. I'm suspecting a change in bindings somewhere.

superdump · 2021-11-20T14:29:36Z

I'm looking at my clustered forward rendering branch and trying to get it in shape to make a PR.

superdump · 2021-11-20T18:00:24Z

[Merged by Bors] - Clustered forward rendering #3153

I know @cart wants to keep things moving so I've made the PR without writing the thorough description that I would like to, and without adding in comments with reference materials. I'll back-fill with that information as my top priority with my time.

superdump · 2021-11-22T12:13:11Z

I haven't specifically sought out problem cases, but the updated crevice std140_size_static seems to be correct for the main 2D/3D cases.

[Merged by Bors] - Use crevice std140_size_static everywhere #3168

cart · 2021-11-23T08:19:45Z

I just created the pr to merge the pipelined-rendering work into main. Read the PR for more details. Still a bit more work to do, but we're close!

superdump · 2021-11-23T08:22:00Z

I just created the pr to merge the pipelined-rendering work into main. Read the PR for more details. Still a bit more work to do, but we're close!

Sqweeee!!

lukors · 2021-11-24T10:40:21Z

Here's a link to the PR: #3175

cart · 2021-11-24T21:33:14Z

#3175 has been merged! All renderer development will happen on main from now on!

colepoirier · 2021-12-05T20:47:56Z

@cart heads up the crevice issue has been closed but the task has not been marked as completed

cart · 2021-12-09T03:38:16Z

Just merged clustered forward rendering! We're getting close :)

cart · 2021-12-09T03:40:24Z

@colepoirier yeah I think crevice might be good now. I checked off the subtask because the crevice repo closed it out, but we should still do one last validation pass before considering crevice "done".

aloucks · 2021-12-10T18:17:30Z

The bevy_render2 pipelined renderer seems to be tightly coupled to wgpu. Is this temporary to get things moving along, or is the intent for the new renderer to only integrate with wgpu?

cart · 2021-12-10T21:46:25Z

This was an intentional choice with a lot of discussion behind it. Wgpu is already a "generic gpu abstraction" and makes exactly the tradeoffs we want for a user-facing gpu api. Creating our own would literally be mirroring wgpu for zero added benefit (we did this in the old renderer and it brought nothing but pain and increased complexity). Instead, we've chosen to let wgpu be the "generic bevy gpu api". Feedback on this design has been very positive so far (from Bevy developers building renderer features). Additionally, the wgpu team has proven to be a responsive partner. Read the link above (and the discussions that followed), for examples of how they adjusted project structure and licensing as a direct response to our feedback. The result is a much simpler API that I'm comfortable relying on (and maintaining ... if it ever comes to that).

# Objective Port bevy_ui to pipelined-rendering (see #2535 ) ## Solution I did some changes during the port: - [X] separate color from the texture asset (as suggested [here](https://discord.com/channels/691052431525675048/743663924229963868/874353914525413406)) - [X] ~give the vertex shader a per-instance buffer instead of per-vertex buffer~ (incompatible with batching) Remaining features to implement to reach parity with the old renderer: - [x] textures - [X] TextBundle I'd also like to add these features, but they need some design discussion: - [x] batching - [ ] separate opaque and transparent phases - [ ] multiple windows - [ ] texture atlases - [ ] (maybe) clipping

cart · 2021-12-10T22:51:43Z

The bevy_ui port is merged! New plan: I'm going to focus on fixing the remaining show stopping bugs (such as #3043), then I'll immediately swap in the new renderer and remove the old one. My "actual pipelining" work will take a few days to finish, review, and merge. I'd rather get the new renderer in users' hands asap so we can identity and fix issues as early as possible.

This makes the [New Bevy Renderer](#2535) the default (and only) renderer. The new renderer isn't _quite_ ready for the final release yet, but I want as many people as possible to start testing it so we can identify bugs and address feedback prior to release. The examples are all ported over and operational with a few exceptions: * I removed a good portion of the examples in the `shader` folder. We still have some work to do in order to make these examples possible / ergonomic / worthwhile: #3120 and "high level shader material plugins" are the big ones. This is a temporary measure. * Temporarily removed the multiple_windows example: doing this properly in the new renderer will require the upcoming "render targets" changes. Same goes for the render_to_texture example. * Removed z_sort_debug: entity visibility sort info is no longer available in app logic. we could do this on the "render app" side, but i dont consider it a priority.

cart · 2021-12-14T23:54:27Z

The old render has been replaced by the new renderer #3312. Give it a try (on main) and report any issues you encounter!

kirawi · 2021-12-29T17:31:05Z

What's the current performance improvement looking like on the benchmark?

Tarnadas · 2022-01-06T14:41:49Z

Hey,
I'm trying to make this work with WASM, but I failed because wgpu seems to add ash as a dependency, which cannot be compiled on WASM.
ash is a library for Vulkan, so it should not be included.

❯ cargo tree -i -p ash --edges features --target wasm32-unknown-unknown --no-default-features
ash v0.34.0+1.2.203
├── ash feature "debug"
│   └── wgpu-hal v0.12.1
│       ├── wgpu-hal feature "default"
│       │   └── wgpu-core v0.12.1
│       │       ├── wgpu-core feature "default"
│       │       │   └── wgpu v0.12.0

but if I look into Cargo.toml from wgpu-hal, I see this:

[features]
default = []
metal = ["naga/msl-out", "block", "foreign-types"]
vulkan = ["naga/spv-out", "ash", "gpu-alloc", "gpu-descriptor", "libloading", "inplace_it"]

[dependencies]
ash = { version = "0.34", optional = true }

Someone suggested to bump the Rust edition to 2021, which partially fixed the problem for me.
I've been trying to bump bevy-webgl2 to use latest bevy main branch (I know it doesn't make sense. I just wanted it to compile, but it failed while compiling the dependencies, but the Rust edition fixed it). I then tried to update bevy-rapier to use latest bevy main branch, changed the Rust edition to 2021, but ash is still included as a dependency for no apparent reason.

This bug cannot be reproduced with the examples from the repository btw.

Link to Discord discussion: https://discord.com/channels/691052431525675048/750833140746158140/923451977181044746

cart · 2022-01-08T04:18:16Z

I'm closing this out as we have merged the renderer and a release is incoming. We will open new issues to track upcoming work.

# Objective - alternative to #2895 - as mentioned in #2535 the uuid based ids in the render module should be replaced with atomic-counted ones ## Solution - instead of generating a random UUID for each render resource, this implementation increases an atomic counter - this might be replaced by the ids of wgpu if they expose them directly in the future - I have not benchmarked this solution yet, but this should be slightly faster in theory. - Bevymark does not seem to be affected much by this change, which is to be expected. - Nothing of our API has changed, other than that the IDs have lost their IMO rather insignificant documentation. - Maybe the documentation could be added back into the macro, but this would complicate the code.

# Objective - alternative to bevyengine#2895 - as mentioned in bevyengine#2535 the uuid based ids in the render module should be replaced with atomic-counted ones ## Solution - instead of generating a random UUID for each render resource, this implementation increases an atomic counter - this might be replaced by the ids of wgpu if they expose them directly in the future - I have not benchmarked this solution yet, but this should be slightly faster in theory. - Bevymark does not seem to be affected much by this change, which is to be expected. - Nothing of our API has changed, other than that the IDs have lost their IMO rather insignificant documentation. - Maybe the documentation could be added back into the macro, but this would complicate the code.

cart added C-Enhancement A new feature S-Needs-Triage This issue needs to be labelled A-Meta About the project itself A-Rendering Drawing game state to the screen and removed S-Needs-Triage This issue needs to be labelled labels Jul 24, 2021

cart mentioned this issue Nov 23, 2021

Merge New Renderer #3175

Merged

cart closed this as completed Jan 8, 2022

nothingnesses mentioned this issue Jan 9, 2022

Readback HigherOrderCO/HVM#5

Merged

2 tasks

cart unpinned this issue Feb 6, 2022

This was referenced Dec 16, 2022

Rename Render* to Gpu* #6968

Closed

[Merged by Bors] - Replace UUID based IDs with a atomic-counted ones #6988

Closed

Renderer Rework: Initial Merge Tracking Issue #2535

Renderer Rework: Initial Merge Tracking Issue #2535

Comments

cart commented Jul 24, 2021 • edited Loading

Missing Required Features

Missing Nice-To-Have Features

Discussions To Have Before Merging

Steps to Merge

superdump commented Jul 24, 2021

superdump commented Jul 25, 2021

superdump commented Jul 25, 2021

superdump commented Jul 25, 2021

superdump commented Jul 25, 2021

superdump commented Jul 25, 2021

superdump commented Jul 25, 2021

cart commented Jul 25, 2021

superdump commented Jul 26, 2021

aevyrie commented Jul 26, 2021

cart commented Jul 26, 2021

aevyrie commented Jul 26, 2021

cart commented Jul 26, 2021

notverymoe commented Jul 27, 2021 • edited Loading

superdump commented Jul 27, 2021

notverymoe commented Jul 28, 2021 • edited Loading

cart commented Jul 28, 2021

cart commented Jul 28, 2021

kirawi commented Jul 29, 2021

cart commented Jul 29, 2021

Davier commented Aug 9, 2021

zicklag commented Aug 9, 2021 • edited Loading

zicklag commented Aug 10, 2021

Weibye commented Aug 12, 2021 • edited Loading

cart commented Aug 13, 2021

superdump commented Nov 13, 2021 • edited Loading

superdump commented Nov 20, 2021

superdump commented Nov 20, 2021

superdump commented Nov 22, 2021

cart commented Nov 23, 2021 • edited Loading

superdump commented Nov 23, 2021

lukors commented Nov 24, 2021

cart commented Nov 24, 2021

colepoirier commented Dec 5, 2021

cart commented Dec 9, 2021

cart commented Dec 9, 2021

aloucks commented Dec 10, 2021

cart commented Dec 10, 2021

cart commented Dec 10, 2021

cart commented Dec 14, 2021 • edited Loading

kirawi commented Dec 29, 2021 • edited Loading

Tarnadas commented Jan 6, 2022

cart commented Jan 8, 2022

cart commented Jul 24, 2021 •

edited

Loading

notverymoe commented Jul 27, 2021 •

edited

Loading

notverymoe commented Jul 28, 2021 •

edited

Loading

zicklag commented Aug 9, 2021 •

edited

Loading

Weibye commented Aug 12, 2021 •

edited

Loading

superdump commented Nov 13, 2021 •

edited

Loading

cart commented Nov 23, 2021 •

edited

Loading

cart commented Dec 14, 2021 •

edited

Loading

kirawi commented Dec 29, 2021 •

edited

Loading