Improve TAA #8974
Improve TAA #8974
Conversation
JMS55
added
C-Enhancement
|
@DGriffin91 I can only get the smearing when running your example with the noise texture specifically. Using any other texture/material (I tested a few) works fine. I think the reason it smears on your example is due to neighborhood color clamping. Reprojection succeeds, but it's not 100% perfectly accurate, and with such a high frequency, multi-colored texture, the color clamping (which is based on neighboring pixels) performs very poorly. I could be wrong though. I'm guessing that the reason it works in #8847 is that in my previous TAA code (what's currently in main, and what that PR is based off of), I reset the accumulation/blend factor when any motion (non-zero motion vector) is detected. Which in hindsight, wasn't a great idea. However, that did mean you didn't get any ghosting even when neighborhood clamping failed. This PR tries to keep the history even under motion. A possible solution would be to reset/reduce accumulation when color clips by a lot or is near clipping, which iirc is what Brian Karis/Unreal did. That can be very finicky and introduce more artifacts if we're not careful. Doesn't seem like the kind of thing we should try right before 0.11. |
|
Removing this from 0.11 as theres too much controversy here. |
|
Example |
|
Example |
|
Example |
|
Example |
1 similar comment
|
Example |
|
Example |
|
Example |
|
|
You just need to extend the |
|
Thanks, that fixed it! |
|
I've mentioned this in the discord but want to comment here so it doesn't get lost. I think the state of the TAA in the PR currently has much more smearing/blurring under motion than the current implementation in bevy main. And still does not address the edge smearing issue that #8847 fixes. Main as of 5733d24 #8974 (this PR) |
|
I don't think this can be merged until the NaN issues are solved.
Potentially related is that the history texture has negative RGB values for some things for some reason. |
|
Nor because of the ghosting/smearing that @DGriffin91 showed above. |
alice-i-cecile
added
the
X-Controversial
|
@DGriffin91 what does that last example at 60Hz look like on OK, so, we all want to make progress on TAA. None of what I am writing is intended to offend anyone, and all efforts and contributions are sincerely appreciated. It looks to me like this PR causes significant regressions in visual quality in its current form. As this PR currently combines multiple changes into one PR, it makes it difficult to understand what part of the changes introduces what improvements and what problems. I think we need better in-repo test cases for common problems, such as a moving / rotating camera, perhaps if heuristics for when disocclusions are detected are based on depth differences then we need to test disocclusions at different depths, etc. Then this PR could be split into multiple smaller steps to be able to verify that each in isolation is as good or better. That will also make reviewing much easier which increases the likelihood of faster merges. When situations with PRs become unclear, it is difficult to prioritise reviewing them over other PRs that have a clear state. I looked through #8847 and aside from needing to check the FXAA code move line-by-line, it looks like it adds camera movement and a noise texture to stress test TAA functionality. That change to example code should probably be a separate PR so we can first see how it looks on
|
|
@superdump I've created a PR with just the example from #8847: #10183 I've also updated it with a few things:
Bevy main: This pr: I've abandon #8847 in favor of using a 3rd party plugin: https://github.com/DGriffin91/bevy_mod_taa |
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Extracted the easy stuff from #8974 . # Problem 1. Commands from `update_previous_view_projections` would crash when matching entities were despawned. 2. `TaaPipelineId` and `draw_3d_graph` module were not public. 3. When the motion vectors pointed to pixels that are now off screen, a smearing artifact could occur. # Solution 1. Use `try_insert` command instead. 2. Make them public, renaming to `TemporalAntiAliasPipelineId`. 3. Check for this case, and ignore history for pixels that are off-screen.
Extracted the easy stuff from bevyengine#8974 . # Problem 1. Commands from `update_previous_view_projections` would crash when matching entities were despawned. 2. `TaaPipelineId` and `draw_3d_graph` module were not public. 3. When the motion vectors pointed to pixels that are now off screen, a smearing artifact could occur. # Solution 1. Use `try_insert` command instead. 2. Make them public, renaming to `TemporalAntiAliasPipelineId`. 3. Check for this case, and ignore history for pixels that are off-screen.
Extracted the easy stuff from bevyengine#8974 . # Problem 1. Commands from `update_previous_view_projections` would crash when matching entities were despawned. 2. `TaaPipelineId` and `draw_3d_graph` module were not public. 3. When the motion vectors pointed to pixels that are now off screen, a smearing artifact could occur. # Solution 1. Use `try_insert` command instead. 2. Make them public, renaming to `TemporalAntiAliasPipelineId`. 3. Check for this case, and ignore history for pixels that are off-screen.
Objective
Followup
Changelog
TemporalAntiAliasNodepublic