Move view transformations to bevy_render#20313
Conversation
…ransformations.wgsl
|
It looks like your PR is a breaking change, but you didn't provide a migration guide. Please review the instructions for writing migration guides, then expand or revise the content in the migration guides directory to reflect your changes. |
| fn depth_ndc_to_view_z(ndc_depth: f32, clip_from_view: mat4x4<f32>, view_from_clip: mat4x4<f32>) -> f32 { | ||
| #ifdef VIEW_PROJECTION_PERSPECTIVE | ||
| return -perspective_camera_near(clip_from_view) / ndc_depth; | ||
| #else ifdef VIEW_PROJECTION_ORTHOGRAPHIC | ||
| return -(clip_from_view[3][2] - ndc_depth) / clip_from_view[2][2]; | ||
| #else | ||
| let view_pos = view_from_clip * vec4(0.0, 0.0, ndc_depth, 1.0); | ||
| return view_pos.z / view_pos.w; | ||
| #endif |
There was a problem hiding this comment.
note: the need for two matrices here smells funny to me. why do we need this? is there a way to write it in terms of one matrix?
There was a problem hiding this comment.
It's because of optimisations. Using values from the forward transform (clip from view) to invert it.
| let world_pos = world_from_view * vec4(view_pos, 1.0); | ||
| return world_pos.xyz; |
There was a problem hiding this comment.
note: most of these functions are the same thing with different names. "augment point with w = 1.0, mul, return xyz" or same thing but with w = 0.0 for directions. Do we want to unify them?
There was a problem hiding this comment.
Not sure it's worth it. These functions are mostly so people don't need to understand exactly what the correct maths should be, I guess. Convenience. But extracting functions for... transform_position() and transform_direction() seems unnecessary to me.
tychedelia
left a comment
There was a problem hiding this comment.
This makes a lot of sense to me.
I think in the future, we may be able to better play with code generation, such that these functions link to a module that is provided at runtime by the WESL loader and contains the resources, but we can experiment with that later.
superdump
left a comment
There was a problem hiding this comment.
I'm fine with this but it's probably too late for 0.17.
|
Turns out it wasn't too late for 0.17. |
# Objective - add migration guide for #20313
# Objective - split off bevy_shader from bevy_render to allow for defining shader libraries and materials for scenes without depending on a renderer ## Solution - a bunch of small refactor commits, then bevy_shader! (review commit by commit) ## Testing - please help with this, especially with spirv passthrough and decoupled_naga testing: i only ran 3d_scene at every step to make sure i didnt break anything. EDIT: this has been tested now, decoupled_naga works, and I also fixed a bug that was on main when wesl and decoupled_naga were both present it didnt compile, now it does # Future Work - Split ShaderCacheError out of PipelineCacheError (or just rename) (punted because breaking change) - Consider renaming PipelineCacheId to ShaderCacheId ? (punted because breaking change) - bevy_material - bevy_bsdf (bindingless pbr shader library) - move view.wgsl into bevy_camera after #20313 lands ?
# Objective - having view_transformations in bevy_pbr just because they need the bindings sucks, it hinders code reuse and means we often can't use these functions on custom view bindings. ## Solution - Add binding-less version of view transformations and deprecate view_transformations.wgsl ## Testing - 3d_scene runs # The Plan: this is a "trial run" first step: it includes no migration of the existing code-base, and does no breaking changes, its just a minimal example of what going in this direction will look like. If we like this direction i'll sink more time into this, writing a migration guide and some followup prs: - migrations for the codebase - removal of the deprecated functions (in 0.18, probably) - similar PRs for PBR functions
…0338) # Objective - add migration guide for bevyengine#20313
# Objective - split off bevy_shader from bevy_render to allow for defining shader libraries and materials for scenes without depending on a renderer ## Solution - a bunch of small refactor commits, then bevy_shader! (review commit by commit) ## Testing - please help with this, especially with spirv passthrough and decoupled_naga testing: i only ran 3d_scene at every step to make sure i didnt break anything. EDIT: this has been tested now, decoupled_naga works, and I also fixed a bug that was on main when wesl and decoupled_naga were both present it didnt compile, now it does # Future Work - Split ShaderCacheError out of PipelineCacheError (or just rename) (punted because breaking change) - Consider renaming PipelineCacheId to ShaderCacheId ? (punted because breaking change) - bevy_material - bevy_bsdf (bindingless pbr shader library) - move view.wgsl into bevy_camera after bevyengine#20313 lands ?
# 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 -
# 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 -
# 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](#7402). ## 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](https://caniuse.com/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 #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)) <img width="353" height="327" alt="chrome_ssao" src="https://github.com/user-attachments/assets/0666673f-508e-4b57-a152-19327ffdb6f3" /> - 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' . ---
Objective
Solution
Testing
The Plan:
this is a "trial run" first step: it includes no migration of the existing code-base, and does no breaking changes, its just a minimal example of what going in this direction will look like. If we like this direction i'll sink more time into this, writing a migration guide and some followup prs: