Add opt-out background motion vectors#23187
Conversation
greeble-dev
added
C-Bug
alice-i-cecile
added
D-Modest
…motion vectors, with an opt-out component.
aevyrie
changed the title
Atmosphere
mate-h
left a comment
mate-h
left a comment
There was a problem hiding this comment.
Nice fix thanks for getting this done! I don't have any blocking comments but leaving some feedback
| @group(0) @binding(0) var<uniform> view: View; | ||
| @group(0) @binding(1) var<uniform> previous_view: PreviousViewUniforms; | ||
|
|
||
| /// Writes motion vectors for sky pixels (depth == 0 in reversed-Z) based on camera rotation. |
There was a problem hiding this comment.
For consistency in the terminology say "Writes motion vectors for background pixels" and "cleared depth at background pixels"
There was a problem hiding this comment.
Somewhat related to this PR, but not in-scope.
Following up from the discussion on discord , we should really consider renaming SkyBox to something else like:
SceneBackground or BackgroundCubemap to distinguish it from the Atmosphere and remove any implication that this can only be a Sky. Seems to be a common point of confusion. SkyBox represents a visual background layer, not a sky model and not an environment lighting source. The current name conflates concepts.
There was a problem hiding this comment.
Could shorten the file name to just motion_vectors.rs, I think it's a better option for the file name itself. (don't feel too strongly about this) everything else in the file using a background terms should stay the same.
|
|
||
| impl Plugin for BackgroundMotionVectorsPlugin { | ||
| fn build(&self, app: &mut App) { | ||
| embedded_asset!(app, "background_motion_vectors.wgsl"); |
There was a problem hiding this comment.
Same for this feels cleaner to not include the background_ prefix for the file name
| let world_pos = view.world_from_clip * vec4(clip_pos, 0.0, 1.0); | ||
| // Use unjittered_clip_from_world for the current frame to strip TAA jitter from the | ||
| // motion vector. | ||
| let curr_clip_pos = (view.unjittered_clip_from_world * world_pos).xy; |
There was a problem hiding this comment.
This part I don't fully understand, TAA jitters the clip_from_world by default? what does stripping the TAA jitter do, and why wasn't it there before? is this a bugfix for TAA + motion blur?
There was a problem hiding this comment.
Yeah, I was seeing issues when TAA was enabled - this code is what the mesh motion vectors do as well iirc, so the camera jitter doesn't add noise to the motion vectors.
ecoskey
added
S-Ready-For-Final-Review
5 participants
Objective
Note the sun does not make a trail, and mesh edges do not blend with background
Solution
Testing
bevy_citywith motion blur enabledThis is an exaggerated stress test to show the blur across edges is smooth: