-
-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Render Graph Rewrite #13397
Open
ecoskey
wants to merge
149
commits into
bevyengine:main
Choose a base branch
from
ecoskey:resource_render_graph
base: main
Could not load branches
Branch not found: {{ refName }}
Could not load tags
Nothing to show
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Render Graph Rewrite #13397
+4,434
−115
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
This reverts commit f584096.
ecoskey
force-pushed
the
resource_render_graph
branch
from
June 1, 2024 07:05
c12db2c
to
9667308
Compare
proc derive macro for RenderGraphDebug to be added in followup PR
ecoskey
force-pushed
the
resource_render_graph
branch
from
June 1, 2024 07:08
9667308
to
1e532a9
Compare
ecoskey
force-pushed
the
resource_render_graph
branch
from
June 1, 2024 07:40
b9d47c8
to
45f2f56
Compare
ecoskey
force-pushed
the
resource_render_graph
branch
from
June 3, 2024 03:21
db85a11
to
b0abaef
Compare
ecoskey
force-pushed
the
resource_render_graph
branch
from
June 6, 2024 19:51
6778ab4
to
96c2523
Compare
You added a new feature but didn't update the readme. Please run |
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
C-Enhancement
A new feature
C-Needs-Release-Note
Work that should be called out in the blog due to impact
D-Complex
Quite challenging from either a design or technical perspective. Ask for help!
D-Domain-Expert
Requires deep knowledge in a given domain
S-Needs-Review
Needs reviewer attention (from anyone!) to move forward
S-Needs-SME
Decision or review from an SME is required
X-Controversial
There is active debate or serious implications around merging this PR
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.
Objective
Bevy's current render graph is quite rigid; Since it's a pure-ecs system, all the inputs and outputs are fixed. This makes it both difficult to customize for third-party users and difficult to maintain internally. ViewNode and similar abstractions relieve some of the pain of querying the World, but don't succeed in making the graph more dynamic.
Goals:
Non-goals:
wgpu
if they need custom nodes.Solution
The main differences between the old and new graph are as follows:
The graph has no notion of a "view entity" and manages the rendering loop itself (THIS IS NOT FINAL AND IS UP FOR DEBATE, SEE THE SECTION BELOW)RenderHandle: graph resources as IDs
The render graph defers much of its resource creation until after graph configuration is done, so it provides an opaque handle to resources that may not yet exist in the form of
RenderHandle
. These are lifetimed to the current graph execution, so it's impossible to use them outside of their original context. See the migration guide for more detail about resource creation and the utilities the graph provides.In addition, the graph stores metadata alongside each resource, which might be just their plain descriptors, but might also contain extra information such as which entries are writable in a bind group layout.
Render graph nodes
In the current render graph, a node is a simple unit of rendering work with defined inputs and outputs. The same applies here, except we also track which resources a node reads from and which it writes to.
In order to provide the simplest API, graph.add_node takes a plain closure (and a
RenderDependencies
, discussed in the migration guide) with some normal rendering necessities, as well as something calledNodeContext
.NodeContext::get()
allows dereferencing a handle into a normal resource, and then you can do whatever you want!From there, rendering features can mostly be reduced to plain functions that take an
&mut RenderGraphBuilder
as well as handles to whatever inputs they need!Debate: single entry-point/view-less vs. dynamic view queuing
I'm still open to further debate on this, but single-entry-point/single-config was mostly a technical compromise, and I've figured out the issues involved. Though, I'll be leaving it slightly incomplete for now to not introduce any unsafe code to review. The graph is configured per-camera through a component so the user doesn't have to manage the rendering loop themself, but for now it's not possible to access the results of other views, such as UI.
Where the current single-entry-point system has no concept of "view entities" and manages the rendering loop itself, associating each graph with a view entity allows better modularity and separating concerns. It also allows putting an EntityRef in the builder to avoid passing it around everywhere. The main issue is how to order these graphs and pass data between them (in the case of UI especially). The simplest possible API would look likelet ui_tex = graph.depends_on::<RenderHandle<'g, Texture>>(entity)
whereentity
is any entity with a graph attached, for examplegraph.world_query_filtered::<Entity, With<UiView>>().single()
, that would return a texture handle. However, this would likely requireunsafe
code and untyped pointers to manage.In the interest of simplicity for this initial PR, Jasmine convinced me to stay with a single entry-point system, though I did want to show what the alternative would be if we put the extra effort in. If the maintainers/community decide it's worth it to have dynamic views immediately, I don't mind delaying the PR to add those features.NOTE: this would not allow configuring a single graph from multiple places. Merely configuring multiple separate graphs, each operating on their own "view." (need a better word for this. not all views are cameras or shadows)
Debate: Bind Groups
Currently there's two ways we could proceed with handling bind groups. Firstly, bind groups could be handled in the graph entirely behind the scenes, using caching to prevent duplication. This would look something like every node declaring a set of entries, the graph inferring their usage, and handing back a
BindGroup
as a callback parameter. This would be simpler, though might make things more verbose especially in the case of the view bind group.The other way would involve making
BindGroup
s a first-classRenderResource
, which is what is in place in the crate currently. This does involve extra complexity when tracking node dependencies: when a user marks write access to a bind group handle, that has to propagate to every bind group entry that was possibly written to, like storage textures and buffers. This might make declaring a bind group in the graph more confusing, which is worth considering for a system about ease of use. However, I think abstracting around this withBindGroupBuilder
and perhaps a version ofAsBindGroup
for render graph handles would solve most of these issues by inferring the right usage.Current Limitations
no dynamic queuing of views (up for debate to delay for this feature)graph.last_frame(texture)
, this requires more design work),resource metadata storage needs a rework(DONE!)graph resources aren't labelled yet, so debug messages aren't great(DONE!)panic!()
s in a few places where it could gracefully stop rendering.Note: these three below all have the foundations already in place
To-do for this PR
I would greatly appreciate any help with docs and unit tests, it's a lot to cover! :)
Testing
This is intended to be merged as an experimental feature. The basic API should be in its final form, and essentially ready for production. Unit tests are in progress, as is better documentation, though large-scale testing will essentially have to happen as we proceed with the renderer refactor.
Migration Guide
Since actual migration will happen in the form of the big refactor™️ during the next milestone, this will consist of a usage/style guide. This might seem out of place for a PR, but for such a big new system I figure it would help maintainers figure out what they're looking at.
Lifetimes and types added for comprehension. These are generally inferred :)
How do I make a resource?
There are a few ways to create graph resources:
Note: when borrowing/taking an already-made resource from the World, users must also supply metadata that matches that resource. This might make things more difficult to import to the graph, but it lets the graph infer much more about how each resource is used.
What is RenderDependencies, and why doesn't my node work?
RenderDependencies
marks what resources your node reads from and writes to, in order to properly track ordering dependencies between nodes. This must be manually specified, since the only way to infer it would be to intercept all rendering calls (think Unity's SRP render graph) which I felt would be both too complicated and worse to use. If you try to get a resource from the node context which isn't declared in the dependencies, the graph will panic. It can't detect if you write to a resource declared as read-only or vice-versa, so that's up to you.Note: the
deps![]
macro works likevec![]
and infers usage based on using a mutable or immutable reference (see traitIntoRenderDependencies
). This is the preferred way to create aRenderDependencies
. A trait is used here to allow for wrappers around handles to be included in this list as well..add_usages()
Oh no! I have a function that creates a texture/buffer and gives it back to the user, but I don't know what usages to assign! Have no fear, citizen, for the render graph tracks this for you! (sort of). For resources created by descriptor,
.add_usages()
will add the specified usage flags to the descriptor, since the resource hasn't actually been created yet. You can trust users later in the graph will call this based on their needs. Otherwise, if the resource is imported and has an associated descriptor, the graph will panic if the needed usage isn't present..is_fresh()
Use
graph.is_fresh(resource_handle)
to check if a resource has been written to yet in the current frame or not. This is most useful when determining if a render pass should clear the color attachment or not..meta()
Use
graph.meta()
to get the metata of a handle or the layout of a bind group handle respectively. This is meant to reduce parameter bloat when effects need to produce textures of the same size as their input, for example. Or, for the bind group case, when creating a pipeline given only a handle to a bind group.In-depth example: a full-screen render pass
See
crate::std::fullscreen
for the actual code