Skip to content

Added SSAO support on WebGPU, with R32Float fallback.#20960

Merged
alice-i-cecile merged 8 commits intobevyengine:mainfrom
diyu-motif:ssao_r32
Oct 29, 2025
Merged

Added SSAO support on WebGPU, with R32Float fallback.#20960
alice-i-cecile merged 8 commits intobevyengine:mainfrom
diyu-motif:ssao_r32

Conversation

@diyu-motif
Copy link
Contributor

Objective

This PR is to add SSAO support on WebGPU.

Solution

Add r16float detect and fallback to r32float if not supported. FYI the initial ssao PR here.

Testing

  • Did you test these changes? If so, how?
    Yes, here is the command to test the ssao example
    RUSTFLAGS="--cfg getrandom_backend=\"wasm_js\"" cargo run --example ssao --target wasm32-unknown-unknown --features webgpu
    and then http://localhost:1334, make sure it supports WebGPU

  • Are there any parts that need more testing?
    N/A

  • How can other people (reviewers) test your changes? Is there anything specific they need to know?

    1. For the R32Float fallback, only the 1st commit is needed. However, I ran into some strange flickering issue on Chrome (and Chrome canary too), so I added a "detect_r16float_support" flag and a temp fix in the 2nd and 3rd commit, which are intended to be reverted when merging.
    2. with the 1st commit, Safari works fine, but Chrome has the flickering, and I fixed it by partially reverting a shader change from Move view transformations to bevy_render #20313, which is very strange. It is probably caused by some shader optimization issue in Chrome, and I have no clue why, as the change is just moving the calculation to a different function.
      Here is the flickering on Mac Chrome(140.0.7339.133 (Official Build) (arm64))
chrome_ssao
  • If relevant, what platforms did you test these changes on, and are there any important ones you can't test?
    Tested on native(mac) and WebGPU. To test on native, set "detect_r16float_support" to false to force R32Float.
    I couldn't make the WebGPU ssao example running on Windows 11 Chrome or Firefox, there is some error in 'taa_pipeline' .

// -----------------

/// Convert a world space position to clip space
/// DEPRECATED: use bevy_render::view::position_world_to_clip instead
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was this part changed?

@JMS55
Copy link
Contributor

JMS55 commented Sep 11, 2025

Oh whoops just read the rest of the PR description.

@JMS55 JMS55 requested a review from atlv24 September 11, 2025 02:17
@JMS55
Copy link
Contributor

JMS55 commented Sep 11, 2025

Also I'm not 100% sure that we'd need this workaround once #8888 (comment) is resolved. But not sure, haven't checked what GPUs don't support texture-formats-tier1.

@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible A-Rendering Drawing game state to the screen O-WebGPU Specific to the WebGPU render API labels Sep 11, 2025
@alice-i-cecile alice-i-cecile added the S-Needs-Review Needs reviewer attention (from anyone!) to move forward label Sep 11, 2025
@diyu-motif
Copy link
Contributor Author

Also I'm not 100% sure that we'd need this workaround once #8888 (comment) is resolved. But not sure, haven't checked what GPUs don't support texture-formats-tier1.

no updates in #8888 for a while, what's the plan?

@diyu-motif
Copy link
Contributor Author

ok, I made the change as suggested, the only issue is the commit 4b09204, which I will revert, but just want to leave it now for atlv24 to take a look.

github-merge-queue bot pushed a commit that referenced this pull request Sep 26, 2025
# Objective

- Fix chrome rendering bug:
<img width="1755" height="985" alt="image"
src="https://github.com/user-attachments/assets/6293a960-b8fa-460e-9d71-e7f2a9fb2d20"
/>

## Solution

- Revert part of #20313
- I don't know why this fixes it, but #20960 also ran into it

## Testing

-
mockersf pushed a commit that referenced this pull request Sep 26, 2025
# Objective

- Fix chrome rendering bug:
<img width="1755" height="985" alt="image"
src="https://github.com/user-attachments/assets/6293a960-b8fa-460e-9d71-e7f2a9fb2d20"
/>

## Solution

- Revert part of #20313
- I don't know why this fixes it, but #20960 also ran into it

## Testing

-
@diyu-motif
Copy link
Contributor Author

OK, I reverted the fix for Chrome, as I think it's been addressed by #21202
The issue is probably a Chrome optimization bug, you can make the flickering go away with this:
view_bindings::view.clip_from_world * vec4(world_pos, 1.00001);

@diyu-motif diyu-motif requested a review from JMS55 September 26, 2025 21:07
@diyu-motif
Copy link
Contributor Author

Hello guys, please let me know what I should do for this MR? Thanks. cc @alice-i-cecile

Comment on lines +11 to +15
@group(0) @binding(1) var preprocessed_depth_mip0: texture_storage_2d<r#{SSAO_DEPTH_FORMAT}float, write>;
@group(0) @binding(2) var preprocessed_depth_mip1: texture_storage_2d<r#{SSAO_DEPTH_FORMAT}float, write>;
@group(0) @binding(3) var preprocessed_depth_mip2: texture_storage_2d<r#{SSAO_DEPTH_FORMAT}float, write>;
@group(0) @binding(4) var preprocessed_depth_mip3: texture_storage_2d<r#{SSAO_DEPTH_FORMAT}float, write>;
@group(0) @binding(5) var preprocessed_depth_mip4: texture_storage_2d<r#{SSAO_DEPTH_FORMAT}float, write>;
Copy link
Contributor

Choose a reason for hiding this comment

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

This style of shader val formatting is going away with the incoming wesl migration, would you mind changing this to be a boolean style SSAO_USE_R32FLOAT_FALLBACK and duplicating the bindings instead? CC @tychedelia

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you @atlv24. In that case, I think I can just revert one of the commit and make it the original way of #idfef.

@alice-i-cecile alice-i-cecile 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 Oct 29, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Oct 29, 2025
Merged via the queue into bevyengine:main with commit ba04239 Oct 29, 2025
38 checks passed
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-Feature A new feature, making something new possible O-WebGPU Specific to the WebGPU 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

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants