Skip to content

Solari: Denoiser motion vector metal compat - #25787

Merged
alice-i-cecile merged 5 commits into
bevyengine:mainfrom
mate-h:m/solari-metal-motionvec
Sep 17, 2026
Merged

alice-i-cecile merged 5 commits into
bevyengine:mainfrom
mate-h:m/solari-metal-motionvec

Conversation

@mate-h

@mate-h mate-h commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

Objective

It changed Solari’s motion vectors from a sampled texture_2d<f32> to texture_storage_2d<rg16float, read_write>.

Presently on main we get a crash on Metal:

2026-09-14T18:43:13.274411Z ERROR bevy_render::error_handler: Caught rendering error: Validation Error

Caused by:
  In Device::create_bind_group_layout, label = 'solari_lighting_bind_group_layout'
    Binding index 5: ReadWrite access to storage textures with format Rg16Float is not supported

rg16float can't be bound as a read-write storage texture on Metal. Vulkan and DX12 usually can.

Solution

ReSTIR only samples motion vectors to reproject last frame's reservoir. The only writer is DLSS Ray Reconstruction PSR, and only for perfect mirrors — everything else already goes to specular_motion_vectors.

At pipeline init we check whether the adapter supports STORAGE_READ_WRITE on rg16float. If it does, we keep the storage binding and write PSR motion in place. If not, we sample a texture_2d<f32> and skip the write. DLSS RR doesn't run on those GPUs anyway.

Testing

  • Ran the Solari example on Metal (Apple M5), no crash
  • Could not test the DLSS in-place path (no DLSS_SDK / NVIDIA GPU on this machine)
  • CI check for Metal + Solari is still missing (no available hw to test on?)

@mate-h
mate-h requested a review from JMS55 September 14, 2026 18:50
@mate-h mate-h added A-Rendering Drawing game state to the screen O-Metal Specific to the Metal render API S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Sep 14, 2026
@github-project-automation github-project-automation Bot moved this to Needs SME Triage in Rendering Sep 14, 2026
@stuartparmenter stuartparmenter added this to the 0.20 milestone Sep 15, 2026
@stuartparmenter

Copy link
Copy Markdown
Contributor

Would like to land this in 0.20 so solari on metal isn't broken

@mate-h mate-h changed the title Solari: Split denoiser motion vector binding for metal compat Solari: Denoiser motion vector metal compat Sep 16, 2026

@JMS55 JMS55 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm but some nits

@mate-h mate-h added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Sep 17, 2026
@alice-i-cecile
alice-i-cecile added this pull request to the merge queue Sep 17, 2026
Merged via the queue into bevyengine:main with commit f4918bb Sep 17, 2026
40 checks passed
@github-project-automation github-project-automation Bot moved this from Needs SME Triage to Done in Rendering Sep 17, 2026
gregcsokas pushed a commit to gregcsokas/bevy that referenced this pull request Sep 21, 2026
# Objective

- bevyengine#25123 introduced Solari on Metal
- bevyengine#25423 broke compatibility with Metal

It changed Solari’s motion vectors from a sampled `texture_2d<f32>` to
`texture_storage_2d<rg16float, read_write>`.

Presently on main we get a crash on Metal:
```
2026-09-14T18:43:13.274411Z ERROR bevy_render::error_handler: Caught rendering error: Validation Error

Caused by:
  In Device::create_bind_group_layout, label = 'solari_lighting_bind_group_layout'
    Binding index 5: ReadWrite access to storage textures with format Rg16Float is not supported
```

`rg16float` can't be bound as a read-write storage texture on Metal.
Vulkan and DX12 usually can.

## Solution

ReSTIR only samples motion vectors to reproject last frame's reservoir.
The only writer is DLSS Ray Reconstruction PSR, and only for perfect
mirrors — everything else already goes to `specular_motion_vectors`.

At pipeline init we check whether the adapter supports
`STORAGE_READ_WRITE` on `rg16float`. If it does, we keep the storage
binding and write PSR motion in place. If not, we sample a
`texture_2d<f32>` and skip the write. DLSS RR doesn't run on those GPUs
anyway.

## Testing

- Ran the Solari example on Metal (Apple M5), no crash
- Could not test the DLSS in-place path (no `DLSS_SDK` / NVIDIA GPU on
this machine)
- CI check for Metal + Solari is still missing (no available hw to test
on?)
mockersf pushed a commit that referenced this pull request Sep 25, 2026
# Objective

- #25123 introduced Solari on Metal
- #25423 broke compatibility with Metal

It changed Solari’s motion vectors from a sampled `texture_2d<f32>` to
`texture_storage_2d<rg16float, read_write>`.

Presently on main we get a crash on Metal:
```
2026-09-14T18:43:13.274411Z ERROR bevy_render::error_handler: Caught rendering error: Validation Error

Caused by:
  In Device::create_bind_group_layout, label = 'solari_lighting_bind_group_layout'
    Binding index 5: ReadWrite access to storage textures with format Rg16Float is not supported
```

`rg16float` can't be bound as a read-write storage texture on Metal.
Vulkan and DX12 usually can.

## Solution

ReSTIR only samples motion vectors to reproject last frame's reservoir.
The only writer is DLSS Ray Reconstruction PSR, and only for perfect
mirrors — everything else already goes to `specular_motion_vectors`.

At pipeline init we check whether the adapter supports
`STORAGE_READ_WRITE` on `rg16float`. If it does, we keep the storage
binding and write PSR motion in place. If not, we sample a
`texture_2d<f32>` and skip the write. DLSS RR doesn't run on those GPUs
anyway.

## Testing

- Ran the Solari example on Metal (Apple M5), no crash
- Could not test the DLSS in-place path (no `DLSS_SDK` / NVIDIA GPU on
this machine)
- CI check for Metal + Solari is still missing (no available hw to test
on?)
pull Bot pushed a commit to AsnZZZ/bevy that referenced this pull request Sep 28, 2026
# Objective

- Filtering always runs for the atmosphere
- `bevy_solari` introduces a new use case where only mip 0 is sampled
- We need a way to disable the filtering to save on the overhead

## Solution

- Add a `bool` field called `filter` on the
`AtmosphereEnvironmentMapLight` struct.
- Add a system to `bevy_solari` that automatically sets this to `filter`
field to `false`
- Generally updated documentation throughout 

## Testing

- Applied the change set from the PR bevyengine#25704 (now also on main)
- Created a new branch at
https://github.com/mate-h/bevy/tree/m/solari-atmo-test
- I had to fix compatibility with metal backend for `bevy_solari`.
posted PR for this bevyengine#25787.
- Ran the `bevy_solari` example with the
`AtmosphereEnvironmentMapLight::default()` and verified with log
statements that the filtering gets disabled and does not run the GPU
pipeline.

---

## Showcase

<img width="1280" height="750" alt="Screenshot 2026-09-14 at 12 45
25 AM"
src="https://github.com/user-attachments/assets/23e35861-9a28-4552-a6ce-723bb5f320d9"
/>

---------

Co-authored-by: JMS55 <47158642+JMS55@users.noreply.github.com>
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 O-Metal Specific to the Metal render API S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants