Skip to content

Fix many_foxes + motion blur = crash on WebGL #18715

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

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