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

[Merged by Bors] - Replace old renderer with new renderer #3312

Closed
wants to merge 9 commits into from

Conversation

cart
Copy link
Member

@cart cart commented Dec 12, 2021

This makes the New Bevy Renderer 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: Add vertex buffer layout to Mesh #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 cart added the A-Rendering Drawing game state to the screen label Dec 12, 2021
@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Dec 12, 2021
@cart cart added this to the Bevy 0.6 milestone Dec 12, 2021
@mockersf mockersf removed the S-Needs-Triage This issue needs to be labelled label Dec 12, 2021
@alice-i-cecile
Copy link
Member

What bits I've seen look good, but this is effectively unreviewable due to limitations in Github's tooling. Fortunately, it's already mostly been reviewed! IMO we should merge this ASAP and open issues for any problems with the new examples and for the missing features listed above.

crates/bevy_pbr/Cargo.toml Outdated Show resolved Hide resolved
examples/3d/lighting.rs Outdated Show resolved Hide resolved
@bjorn3
Copy link
Contributor

bjorn3 commented Dec 13, 2021

Could this be split into one commit removing the old renderer and another to rename the new renderer crates? This would improve the results of git blame. Currently git blame will consider most diffs in this commit as changing the old render source rather than renaming the files of the new renderer.

Cargo.toml Outdated Show resolved Hide resolved
@Shatur Shatur mentioned this pull request Dec 13, 2021
@cart
Copy link
Member Author

cart commented Dec 13, 2021

Could this be split into one commit removing the old renderer and another to rename the new renderer crates? This would improve the results of git blame. Currently git blame will consider most diffs in this commit as changing the old render source rather than renaming the files of the new renderer.

Hmm I'm torn on this one. In some cases (ex: bevy_text, bevy_ui, bevy_sprite, bevy_pbr), theres enough overlap that it might be worth treating this as an iterative change rather than a wholesale replacement. That should make it easier to identify + attribute the original authors of a line of code. Even bevy_render (which was basically a full rewrite) has some largely untouched code, such as the mesh module.

@cart
Copy link
Member Author

cart commented Dec 14, 2021

Finally got CI to run the examples properly, but it now takes a very long time. Curious to see what aspect of the new renderer is causing the slowness (maybe the extra complexity from shadows?). We might want to revisit how / when we run them if the extra CI time persists.

@cart
Copy link
Member Author

cart commented Dec 14, 2021

bors r+

bors bot pushed a commit that referenced this pull request Dec 14, 2021
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.
@bors bors bot changed the title Replace old renderer with new renderer [Merged by Bors] - Replace old renderer with new renderer Dec 14, 2021
@bors bors bot closed this Dec 14, 2021
@0x6273 0x6273 mentioned this pull request Dec 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants