Skip to content

Commit 0517b96

Browse files
authored
Fix motion vector computation after #17688. (#17717)
PR #17688 broke motion vector computation, and therefore motion blur, because it enabled retention of `MeshInputUniform`s, and `MeshInputUniform`s contain the indices of the previous frame's transform and the previous frame's skinned mesh joint matrices. On frame N, if a `MeshInputUniform` is retained on GPU from the previous frame, the `previous_input_index` and `previous_skin_index` would refer to the indices for frame N - 2, not the index for frame N - 1. This patch fixes the problems. It solves these issues in two different ways, one for transforms and one for skins: 1. To fix transforms, this patch supplies the *frame index* to the shader as part of the view uniforms, and specifies which frame index each mesh's previous transform refers to. So, in the situation described above, the frame index would be N, the previous frame index would be N - 1, and the `previous_input_frame_number` would be N - 2. The shader can now detect this situation and infer that the mesh has been retained, and can therefore conclude that the mesh's transform hasn't changed. 2. To fix skins, this patch replaces the explicit `previous_skin_index` with an invariant that the index of the joints for the current frame and the index of the joints for the previous frame are the same. This means that the `MeshInputUniform` never has to be updated even if the skin is animated. The downside is that we have to copy joint matrices from the previous frame's buffer to the current frame's buffer in `extract_skins`. The rationale behind (2) is that we currently have no mechanism to detect when joints that affect a skin have been updated, short of comparing all the transforms and setting a flag for `extract_meshes_for_gpu_building` to consume, which would regress performance as we want `extract_skins` and `extract_meshes_for_gpu_building` to be able to run in parallel. To test this change, use `cargo run --example motion_blur`.
1 parent 5e569af commit 0517b96

File tree

9 files changed

+52
-23
lines changed

9 files changed

+52
-23
lines changed

crates/bevy_pbr/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ bevy_asset = { path = "../bevy_asset", version = "0.16.0-dev" }
3636
bevy_color = { path = "../bevy_color", version = "0.16.0-dev" }
3737
bevy_core_pipeline = { path = "../bevy_core_pipeline", version = "0.16.0-dev" }
3838
bevy_derive = { path = "../bevy_derive", version = "0.16.0-dev" }
39+
bevy_diagnostic = { path = "../bevy_diagnostic", version = "0.16.0-dev" }
3940
bevy_ecs = { path = "../bevy_ecs", version = "0.16.0-dev" }
4041
bevy_image = { path = "../bevy_image", version = "0.16.0-dev" }
4142
bevy_math = { path = "../bevy_math", version = "0.16.0-dev" }

crates/bevy_pbr/src/render/gpu_preprocess.rs

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -704,12 +704,10 @@ impl Node for EarlyGpuPreprocessNode {
704704
continue;
705705
};
706706

707-
// If we're drawing indirectly, make sure the mesh preprocessing
708-
// shader has access to the view info it needs to do culling.
709-
let mut dynamic_offsets: SmallVec<[u32; 1]> = smallvec![];
710-
if !no_indirect_drawing {
711-
dynamic_offsets.push(view_uniform_offset.offset);
712-
}
707+
// Make sure the mesh preprocessing shader has access to the
708+
// view info it needs to do culling and motion vector
709+
// computation.
710+
let dynamic_offsets = [view_uniform_offset.offset];
713711

714712
// Are we drawing directly or indirectly?
715713
match *phase_bind_groups {
@@ -1416,6 +1414,11 @@ fn preprocess_direct_bind_group_layout_entries() -> DynamicBindGroupLayoutEntrie
14161414
DynamicBindGroupLayoutEntries::new_with_indices(
14171415
ShaderStages::COMPUTE,
14181416
(
1417+
// `view`
1418+
(
1419+
0,
1420+
uniform_buffer::<ViewUniform>(/* has_dynamic_offset= */ true),
1421+
),
14191422
// `current_input`
14201423
(3, storage_buffer_read_only::<MeshInputUniform>(false)),
14211424
// `previous_input`
@@ -1471,11 +1474,6 @@ fn gpu_culling_bind_group_layout_entries() -> DynamicBindGroupLayoutEntries {
14711474
9,
14721475
storage_buffer_read_only::<MeshCullingData>(/* has_dynamic_offset= */ false),
14731476
),
1474-
// `view`
1475-
(
1476-
0,
1477-
uniform_buffer::<ViewUniform>(/* has_dynamic_offset= */ true),
1478-
),
14791477
))
14801478
}
14811479

@@ -1930,6 +1928,7 @@ impl<'a> PreprocessBindGroupBuilder<'a> {
19301928
"preprocess_direct_bind_group",
19311929
&self.pipelines.direct_preprocess.bind_group_layout,
19321930
&BindGroupEntries::with_indices((
1931+
(0, self.view_uniforms.uniforms.binding()?),
19331932
(3, self.current_input_buffer.as_entire_binding()),
19341933
(4, self.previous_input_buffer.as_entire_binding()),
19351934
(

crates/bevy_pbr/src/render/mesh.rs

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use bevy_core_pipeline::{
88
prepass::MotionVectorPrepass,
99
};
1010
use bevy_derive::{Deref, DerefMut};
11+
use bevy_diagnostic::FrameCount;
1112
use bevy_ecs::{
1213
prelude::*,
1314
query::ROQueryItem,
@@ -551,12 +552,18 @@ pub struct MeshInputUniform {
551552
/// Low 16 bits: index of the material inside the bind group data.
552553
/// High 16 bits: index of the lightmap in the binding array.
553554
pub material_and_lightmap_bind_group_slot: u32,
555+
/// The number of the frame on which this [`MeshInputUniform`] was built.
556+
///
557+
/// This is used to validate the previous transform and skin. If this
558+
/// [`MeshInputUniform`] wasn't updated on this frame, then we know that
559+
/// neither this mesh's transform nor that of its joints have been updated
560+
/// on this frame, and therefore the transforms of both this mesh and its
561+
/// joints must be identical to those for the previous frame.
562+
pub timestamp: u32,
554563
/// User supplied tag to identify this mesh instance.
555564
pub tag: u32,
556565
/// Padding.
557-
pub pad_a: u32,
558-
/// Padding.
559-
pub pad_b: u32,
566+
pub pad: u32,
560567
}
561568

562569
/// Information about each mesh instance needed to cull it on GPU.
@@ -1117,6 +1124,7 @@ impl RenderMeshInstanceGpuBuilder {
11171124
render_material_bindings: &RenderMaterialBindings,
11181125
render_lightmaps: &RenderLightmaps,
11191126
skin_uniforms: &SkinUniforms,
1127+
timestamp: FrameCount,
11201128
) -> u32 {
11211129
let (first_vertex_index, vertex_count) =
11221130
match mesh_allocator.mesh_vertex_slice(&self.shared.mesh_asset_id) {
@@ -1164,6 +1172,7 @@ impl RenderMeshInstanceGpuBuilder {
11641172
lightmap_uv_rect: self.lightmap_uv_rect,
11651173
flags: self.mesh_flags.bits(),
11661174
previous_input_index: u32::MAX,
1175+
timestamp: timestamp.0,
11671176
first_vertex_index,
11681177
first_index_index,
11691178
index_count: if mesh_is_indexed {
@@ -1176,8 +1185,7 @@ impl RenderMeshInstanceGpuBuilder {
11761185
self.shared.material_bindings_index.slot,
11771186
) | ((lightmap_slot as u32) << 16),
11781187
tag: self.shared.tag,
1179-
pad_a: 0,
1180-
pad_b: 0,
1188+
pad: 0,
11811189
};
11821190

11831191
// Did the last frame contain this entity as well?
@@ -1607,6 +1615,7 @@ pub fn collect_meshes_for_gpu_building(
16071615
render_material_bindings: Res<RenderMaterialBindings>,
16081616
render_lightmaps: Res<RenderLightmaps>,
16091617
skin_uniforms: Res<SkinUniforms>,
1618+
frame_count: Res<FrameCount>,
16101619
) {
16111620
let RenderMeshInstances::GpuBuilding(ref mut render_mesh_instances) =
16121621
render_mesh_instances.into_inner()
@@ -1646,6 +1655,7 @@ pub fn collect_meshes_for_gpu_building(
16461655
&render_material_bindings,
16471656
&render_lightmaps,
16481657
&skin_uniforms,
1658+
*frame_count,
16491659
);
16501660
}
16511661

@@ -1673,6 +1683,7 @@ pub fn collect_meshes_for_gpu_building(
16731683
&render_material_bindings,
16741684
&render_lightmaps,
16751685
&skin_uniforms,
1686+
*frame_count,
16761687
);
16771688
mesh_culling_builder
16781689
.update(&mut mesh_culling_data_buffer, instance_data_index as usize);

crates/bevy_pbr/src/render/mesh_preprocess.wgsl

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -209,14 +209,22 @@ fn main(@builtin(global_invocation_id) global_invocation_id: vec3<u32>) {
209209
}
210210
#endif
211211

212-
// Look up the previous model matrix.
212+
// See whether the `MeshInputUniform` was updated on this frame. If it
213+
// wasn't, then we know the transforms of this mesh must be identical to
214+
// those on the previous frame, and therefore we don't need to access the
215+
// `previous_input_index` (in fact, we can't; that index are only valid for
216+
// one frame and will be invalid).
217+
let timestamp = current_input[input_index].timestamp;
218+
let mesh_changed_this_frame = timestamp == view.frame_count;
219+
220+
// Look up the previous model matrix, if it could have been.
213221
let previous_input_index = current_input[input_index].previous_input_index;
214222
var previous_world_from_local_affine_transpose: mat3x4<f32>;
215-
if (previous_input_index == 0xffffffff) {
216-
previous_world_from_local_affine_transpose = world_from_local_affine_transpose;
217-
} else {
223+
if (mesh_changed_this_frame && previous_input_index != 0xffffffffu) {
218224
previous_world_from_local_affine_transpose =
219225
previous_input[previous_input_index].world_from_local;
226+
} else {
227+
previous_world_from_local_affine_transpose = world_from_local_affine_transpose;
220228
}
221229
let previous_world_from_local =
222230
maths::affine3_to_square(previous_world_from_local_affine_transpose);

crates/bevy_pbr/src/render/skinning.wgsl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ fn skin_model(
3434
+ weights.z * joint_matrices.data[indexes.z]
3535
+ weights.w * joint_matrices.data[indexes.w];
3636
#else // SKINS_USE_UNIFORM_BUFFERS
37-
let skin_index = mesh[instance_index].current_skin_index;
37+
var skin_index = mesh[instance_index].current_skin_index;
3838
return weights.x * joint_matrices[skin_index + indexes.x]
3939
+ weights.y * joint_matrices[skin_index + indexes.y]
4040
+ weights.z * joint_matrices[skin_index + indexes.z]

crates/bevy_render/src/experimental/occlusion_culling/mesh_preprocess_types.wgsl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,10 @@ struct MeshInput {
1818
// Low 16 bits: index of the material inside the bind group data.
1919
// High 16 bits: index of the lightmap in the binding array.
2020
material_and_lightmap_bind_group_slot: u32,
21+
timestamp: u32,
2122
// User supplied index to identify the mesh instance
2223
tag: u32,
23-
pad_a: u32,
24-
pad_b: u32,
24+
pad: u32,
2525
}
2626

2727
// The `wgpu` indirect parameters structure. This is a union of two structures.

crates/bevy_render/src/render_resource/buffer_vec.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,11 @@ impl<T: NoUninit> RawBufferVec<T> {
103103
self.values.append(&mut other.values);
104104
}
105105

106+
/// Returns the value at the given index.
107+
pub fn get(&self, index: u32) -> Option<&T> {
108+
self.values.get(index as usize)
109+
}
110+
106111
/// Sets the value at the given index.
107112
///
108113
/// The index must be less than [`RawBufferVec::len`].

crates/bevy_render/src/view/mod.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ pub mod visibility;
22
pub mod window;
33

44
use bevy_asset::{load_internal_asset, weak_handle, Handle};
5+
use bevy_diagnostic::FrameCount;
56
pub use visibility::*;
67
pub use window::*;
78

@@ -568,6 +569,7 @@ pub struct ViewUniform {
568569
pub frustum: [Vec4; 6],
569570
pub color_grading: ColorGradingUniform,
570571
pub mip_bias: f32,
572+
pub frame_count: u32,
571573
}
572574

573575
#[derive(Resource)]
@@ -889,6 +891,7 @@ pub fn prepare_view_uniforms(
889891
Option<&TemporalJitter>,
890892
Option<&MipBias>,
891893
)>,
894+
frame_count: Res<FrameCount>,
892895
) {
893896
let view_iter = views.iter();
894897
let view_count = view_iter.len();
@@ -942,6 +945,7 @@ pub fn prepare_view_uniforms(
942945
frustum,
943946
color_grading: extracted_view.color_grading.clone().into(),
944947
mip_bias: mip_bias.unwrap_or(&MipBias(0.0)).0,
948+
frame_count: frame_count.0,
945949
}),
946950
};
947951

crates/bevy_render/src/view/view.wgsl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,4 +60,5 @@ struct View {
6060
frustum: array<vec4<f32>, 6>,
6161
color_grading: ColorGrading,
6262
mip_bias: f32,
63+
frame_count: u32,
6364
};

0 commit comments

Comments
 (0)