Skip to content

Fix motion blur on skinned meshes #18712

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

Merged

Conversation

greeble-dev
Copy link
Contributor

@greeble-dev greeble-dev commented Apr 4, 2025

Objective

Fix motion blur not working on skinned meshes.

Solution

set_mesh_motion_vector_flags can set RenderMeshInstanceFlags::HAS_PREVIOUS_SKIN after specialization has already cached the material. This can lead to MeshPipelineKey::HAS_PREVIOUS_SKIN never getting set, disabling motion blur.

The fix is to make sure set_mesh_motion_vector_flags happens before specialization.

Note that the bug is fixed in a different way by #18074, which includes other fixes but is a much larger change.

Testing

Open the animated_mesh example and add these components to the Camera3d entity:

MotionBlur {
    shutter_angle: 5.0,
    samples: 2,
    #[cfg(all(feature = "webgl2", target_arch = "wasm32", not(feature = "webgpu")))]
    _webgl2_padding: Default::default(),
},
#[cfg(all(feature = "webgl2", target_arch = "wasm32", not(feature = "webgpu")))]
Msaa::Off,

Tested on animated_mesh, many_foxes, custom_skinned_mesh, Win10/Nvidia with Vulkan, WebGL/Chrome, WebGPU/Chrome. Note that testing many_foxes WebGL requires #18715.

…s`. This fixes specialization caching the wrong flag.
@greeble-dev greeble-dev marked this pull request as ready for review April 4, 2025 07:44
@alice-i-cecile alice-i-cecile added this to the 0.16 milestone Apr 4, 2025
@IceSentry IceSentry added C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen labels Apr 4, 2025
@mockersf mockersf added this pull request to the merge queue Apr 4, 2025
Merged via the queue into bevyengine:main with commit 5da64dd Apr 4, 2025
37 checks passed
@github-project-automation github-project-automation bot moved this to Done in Rendering Apr 4, 2025
mockersf pushed a commit that referenced this pull request Apr 4, 2025
## Objective

Fix motion blur not working on skinned meshes.

## Solution

`set_mesh_motion_vector_flags` can set
`RenderMeshInstanceFlags::HAS_PREVIOUS_SKIN` after specialization has
already cached the material. This can lead to
`MeshPipelineKey::HAS_PREVIOUS_SKIN` never getting set, disabling motion
blur.

The fix is to make sure `set_mesh_motion_vector_flags` happens before
specialization.

Note that the bug is fixed in a different way by #18074, which includes
other fixes but is a much larger change.

## Testing

Open the `animated_mesh` example and add these components to the
`Camera3d` entity:

```rust
MotionBlur {
    shutter_angle: 5.0,
    samples: 2,
    #[cfg(all(feature = "webgl2", target_arch = "wasm32", not(feature = "webgpu")))]
    _webgl2_padding: Default::default(),
},
#[cfg(all(feature = "webgl2", target_arch = "wasm32", not(feature = "webgpu")))]
Msaa::Off,
```

Tested on `animated_mesh`, `many_foxes`, `custom_skinned_mesh`,
Win10/Nvidia with Vulkan, WebGL/Chrome, WebGPU/Chrome. Note that testing
`many_foxes` WebGL requires #18715.
@JMS55
Copy link
Contributor

JMS55 commented Apr 5, 2025

As a side note, I really dislike the _webgl2_padding field on MotionBlur. We should remove it, and make a separate RenderMotionBlur struct with the padding that only exists in the render world so that users don't have to deal with the padding.

@greeble-dev
Copy link
Contributor Author

I have a branch that removes webgl2_padding: main...greeble-dev:bevy:motion-blur-component

I've been sitting on it because I was hoping #18074 would land first and I wouldn't have to fix merge conflicts. But that PR looks like it's still got a long road to walk, so I'll make a PR now for webgl2_padding.

@greeble-dev
Copy link
Contributor Author

_webgl2_padding PR is here: #18727

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-Bug An unexpected or incorrect behavior
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants