Renderer Rework: Initial Merge Tracking Issue #2535
Comments
cart
added
C-Feature
|
Here's the PR for bevy_gltf2: #2537 |
|
I’ve got MSAA working but it’s hacky because we need one of:
|
|
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. |
|
And a small PR to log the adapter and wgpu backend being used at initialisation time. It's useful to see. #2542 |
|
I'll do |
|
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. |
What does this mean? I don't see anything about |
|
Great work @superdump. I'll dig in tomorrow / answer questions. Just a heads up that CI on |
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. |
|
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. |
|
@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. |
|
Sounds good 🙂. 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 :) |
|
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. |
|
Alright, I've made a small start just going through the changes.
Edit 3: Still here and keen, just had an up-tick in my day job's workload for a bit there |
|
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). |
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 😄 |
|
Will this be in 0.6? |
|
That is the goal! |
|
I'm starting to work on porting |
|
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, Edit: Sorry, just saw the TODO in the list:
I'll take that! 😄 |
|
I'll also work on "SubApp ZST labels instead of integers", but I could use some guidance on where to put the implementation: #2629. |
|
This should have the label |
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.
Sounds good! |
|
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. |
|
I'm looking at my clustered forward rendering branch and trying to get it in shape to make a PR. |
|
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. |
|
I haven't specifically sought out problem cases, but the updated crevice |
|
I just created the pr to merge the |
Sqweeee!! |
|
Here's a link to the PR: #3175 |
|
#3175 has been merged! All renderer development will happen on |
|
@cart heads up the crevice issue has been closed but the task has not been marked as completed |
|
Just merged clustered forward rendering! We're getting close :) |
|
@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". |
|
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? |
|
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
|
The |
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.
|
The old render has been replaced by the new renderer #3312. Give it a try (on main) and report any issues you encounter! |
|
What's the current performance improvement looking like on the benchmark? |
|
Hey, but if I look into Cargo.toml from wgpu-hal, I see this: Someone suggested to bump the Rust edition to 2021, which partially fixed the problem for me. This bug cannot be reproduced with the examples from the repository btw. Link to Discord discussion: https://discord.com/channels/691052431525675048/750833140746158140/923451977181044746 |
|
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 #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.
# 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.
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:
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.
Add MSAA support to pipelined-rendering #2541(@superdump @cart)run_sub_graph()input apiMissing Nice-To-Have Features
These aren't required for a merge, but would be very nice to have.
Implement Sprite Batching #2642(@cart @zicklag)depth pre-pass(@superdump)MaterialPlugin<T: Material>for low boilerplate custom shaders (@cart)Discussions To Have Before Merging
Steps to Merge
The text was updated successfully, but these errors were encountered: