Skip to content

Conversation

greeble-dev
Copy link
Contributor

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

Objective

Fix #18714.

Solution

Make sure SkinUniforms::prev_buffer is resized at the same time as current_buffer.

There will be a one frame visual glitch when the buffers are resized, since prev_buffer is incorrectly initialised with the current joint transforms.

Note that #18074 includes the same fix. I'm assuming this smaller PR will land first.

Testing

See repro instructions in #18714. Tested on animated_mesh, many_foxes, custom_skinned_mesh, Win10/Nvidia with Vulkan, WebGL/Chrome, WebGPU/Chrome.

@JMS55
Copy link
Contributor

JMS55 commented Apr 4, 2025

Dropping by rq, but iirc morphed meshes use the same code. I think we need the same change there.

@greeble-dev
Copy link
Contributor Author

greeble-dev commented Apr 4, 2025

I think morphs might have been similar to skins in 0.15, but skins changed quite a bit in 0.16 while morphs stayed the same. As far as I can tell this particular issue was introduced in 0.16 and is isolated to skins.

github-merge-queue bot 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.
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.
@alice-i-cecile alice-i-cecile added this to the 0.16 milestone Apr 5, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Apr 6, 2025
Merged via the queue into bevyengine:main with commit 2f80d08 Apr 6, 2025
37 checks passed
@github-project-automation github-project-automation bot moved this to Done in Rendering Apr 6, 2025
mockersf pushed a commit that referenced this pull request Apr 8, 2025
## Objective

Fix  #18714.

## Solution

Make sure `SkinUniforms::prev_buffer` is resized at the same time as
`current_buffer`.

There will be a one frame visual glitch when the buffers are resized,
since `prev_buffer` is incorrectly initialised with the current joint
transforms.

Note that #18074 includes the same fix. I'm assuming this smaller PR
will land first.

## Testing

See repro instructions in #18714. Tested on `animated_mesh`,
`many_foxes`, `custom_skinned_mesh`, Win10/Nvidia with Vulkan,
WebGL/Chrome, WebGPU/Chrome.
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.

many_foxes + motion blur = crash on WebGL
4 participants