Cluster RectLight#24827
Conversation
|
Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke! If it's expected, please add the M-Deliberate-Rendering-Change label. If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it. |
kfc35
added
A-Rendering
alice-i-cecile
added
C-Performance
pcwalton
left a comment
pcwalton
left a comment
There was a problem hiding this comment.
Looks good modulo a few nits, great cleanup.
| pub(crate) light_custom_data: Vec4, | ||
| // For rect lights: pack height in color_inverse_square_range.w. | ||
| pub(crate) color_inverse_square_range: Vec4, | ||
| // For rect lights: pack width in position_radius.w. |
There was a problem hiding this comment.
uber-nit: extra space before “width”
| // making rect lights clusterable. | ||
| // These are per-view because we have no mechanism for "non-clustered but non-view-specific" lights. | ||
| // We could introduce such a thing, but it may not be worth it for this fallback path that's only used | ||
| // when we have too few storage buffers to cluster area lights. |
There was a problem hiding this comment.
Agreed, especially since the “not enough storage buffers” case means WebGL 2, which is near end of life.
| let right = rect_light.transform.right().into(); | ||
| let up = rect_light.transform.up().into(); | ||
| gpu_lights.rect_lights[index] = GpuRectLight { | ||
| color: Vec4::from_slice(&rect_light.color.to_f32_array()) |
There was a problem hiding this comment.
This can just be color: rect_light.color.to_vec4() * rect_light.intensity(), no?
|
|
||
| fn rect_light( | ||
| light_id: u32, | ||
| light: RectLight, |
There was a problem hiding this comment.
Can you pass this as a ptr<function, …>? We’ve had severe performance problems in the past from passing large structs by value in shaders (the optimizer’s ability to optimize this pattern is kind of disappointing) and RectLight is pretty big.
| fn quat_rotate(q: vec4<f32>, dir: vec3<f32>) -> vec3<f32> { | ||
| let t = 2.0 * cross(q.xyz, dir); | ||
| return dir + q.w * t + cross(q.xyz, t); | ||
| } |
There was a problem hiding this comment.
Since you’re moving this function from environment_map.wgsl, do you want to delete the copy there and change environment_map.wgsl to import this function? That’d be cleaner.
4 participants
Objective
This clusters rectangular area lights, lifting the restriction on their number and making them behave more like point and spot lights. This should also improve performance.
On platforms without enough storage buffers (eg WebGL2), we fall back to the current non-clustered path.
Addresses the first item in #23700.
Testing
Flied around in the rect_light example with cpu clustrering, gpu clustering and
WGPU_SETTINGS_PRIO=webgl2.