-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Add MSAA support to pipelined-rendering #2541
Add MSAA support to pipelined-rendering #2541
Conversation
I don't really think this is good enough to merge as it is, but I would appreciate some guidance on how to proceed with it so I don't waste time doing unnecessary work. |
I added support for removing nodes, node edges, slot edges, and subgraphs. And now dynamically changing MSAA samples works. Run the |
For the most part I think this is pretty uncontroversial and we'll be able to build on this for whatever the final solution is. I think the biggest missing pieces / open questions are:
I'll probably let this simmer for a bit, but unless we come up with alternatives before 0.6, i think we should roll with this impl. |
(also we've got to resolve the new merge conflicts that came from breaking out "core_pipeline" into its own crate) |
1e15b88
to
cff1017
Compare
I don't know why check-bans is failing on glam, ndk, and ndk-glue but I haven't touched those. |
I believe these have been fixed on main. Happy to ignore them for now (bors lets us skip them). |
9982ce9
to
2260b8a
Compare
Updated on top of pipelined-rendering again. |
2260b8a
to
ed5f3fc
Compare
Updated on top of pipelined-rendering again. |
8b1eb0d
to
383d7a0
Compare
Note: this is now on top of @cart 's |
So I've thought a lot about MSAA and the complexity around graph edge management (which we've both complained about in the bast). Decided to experiment with moving color attachments and resolve targets into View entities (as a new ViewTarget component). I think it is much cleaner. Curious what your thoughts are: I'll PR it if you're on board. |
Do it! I looked over the commit and it looks good except that you hard coded the msaa samples to 4 when creating the pbr pipeline key, and the example got lost. I find the example really useful for checking it doesn’t panic when changing the msaa sample count on the fly, or raising the window. I had similar thoughts but didn’t want to conflate changes. ExtractedWindow felt like the wrong place for the sampled colour attachment stuff and i thought it would make more sense to be part of the render target. I like how at some point the code just tells you that it doesn’t make sense. :) |
Closing in favor of #3042 |
Adds support for MSAA to the new renderer. This is done using the new [pipeline specialization](#3031) support to specialize on sample count. This is an alternative implementation to #2541 that cuts out the need for complicated render graph edge management by moving the relevant target information into View entities. This reuses @superdump's clever MSAA bitflag range code from #2541. Note that wgpu currently only supports 1 or 4 samples due to those being the values supported by WebGPU. However they do plan on exposing ways to [enable/query for natively supported sample counts](gfx-rs/wgpu#1832). When this happens we should integrate
Objective
Add MSAA support to the
pipelined-rendering
branch.Solution