bevy_solari ReSTIR DI #19790
bevy_solari ReSTIR DI #19790
Conversation
| let tangent_plane_distance = abs(dot(normal, other_world_position - world_position)); | ||
| let view_z = -depth_ndc_to_view_z(depth); | ||
|
|
||
| return tangent_plane_distance / view_z > 0.003 || dot(normal, other_normal) < 0.906; |
There was a problem hiding this comment.
maybe write as cos(radians(25.0))?
| size: Extent3d { | ||
| width: view_size.x, | ||
| height: view_size.y, | ||
| depth_or_array_layers: 1, | ||
| }, |
There was a problem hiding this comment.
this seems like logic that would be used in a lot of places in bevy, perhaps extracting a function is in order
There was a problem hiding this comment.
UVec2 -> Extent3d. its used 4 times in this file alone, probably more in bevy
There was a problem hiding this comment.
probably better for a different pr
| fn depth_ndc_to_view_z(ndc_depth: f32) -> f32 { | ||
| #ifdef VIEW_PROJECTION_PERSPECTIVE | ||
| return -view.clip_from_view[3][2]() / ndc_depth; | ||
| #else ifdef VIEW_PROJECTION_ORTHOGRAPHIC | ||
| return -(view.clip_from_view[3][2] - ndc_depth) / view.clip_from_view[2][2]; | ||
| #else | ||
| let view_pos = view.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.
Its really unfortunate that the one exported from view transformations cant be used because it references a different binding. I believe the wpgu 25 upgrade will force them to coincide again, so deduplicating this in the future should be kept in mind
|
|
||
| var phi: f32; | ||
| var r: f32; | ||
| if (a * a > b * b) { |
There was a problem hiding this comment.
| if (a * a > b * b) { | |
| if (abs(a) > abs(b)) { |
| if (a * a > b * b) { | ||
| r = disk_radius * a; | ||
| phi = (PI / 4.0) * (b / a); | ||
| } else { | ||
| r = disk_radius * b; | ||
| phi = (PI / 2.0) - (PI / 4.0) * (a / b); | ||
| } |
There was a problem hiding this comment.
this whole thing could probably be rewritten to collapse the branches using min/max
| if rand_f(rng) < other_resampling_weight / combined_reservoir.weight_sum { | ||
| combined_reservoir.sample = other_reservoir.sample; | ||
|
|
||
| let inverse_target_function = select(0.0, 1.0 / other_target_function.a, other_target_function.a > 0.0); |
There was a problem hiding this comment.
maybe extract a safe_invert function?
alice-i-cecile
added
C-Feature
| return world_pos.xyz / world_pos.w; | ||
| } | ||
|
|
||
| // Reject if tangent plane difference difference more than 0.3% or angle between normals more than 25 degrees |
There was a problem hiding this comment.
How were these numbers chosen?
There was a problem hiding this comment.
25 degrees is what's suggested in the original ReSTIR paper, and 0.3% was what was suggested when I asked what thresholds people use in discord.
| @@ -1092,7 +1093,8 @@ pub fn prepare_prepass_textures( | |||
| dimension: TextureDimension::D2, | |||
| format: DEFERRED_PREPASS_FORMAT, | |||
| usage: TextureUsages::RENDER_ATTACHMENT | |||
| | TextureUsages::TEXTURE_BINDING, | |||
| | TextureUsages::TEXTURE_BINDING | |||
| | TextureUsages::COPY_SRC, // TODO: Remove COPY_SRC, double buffer instead (for bevy_solari) | |||
There was a problem hiding this comment.
I know you want to double buffer, but we should also just make these more easily configurable in general to avoid this kind of thing
There was a problem hiding this comment.
Yeah. The problem is that it's configured per-render target, and not per camera, but all our components live on cameras. For now I don't have any better idea :/
| @@ -5,6 +5,9 @@ | |||
| struct PreviousViewUniforms { | |||
| view_from_world: mat4x4<f32>, | |||
| clip_from_world: mat4x4<f32>, | |||
| clip_from_view: mat4x4<f32>, | |||
There was a problem hiding this comment.
Was this just missed? Not sure why it's here.
There was a problem hiding this comment.
Yeah it was missed in a previous PR.
tychedelia
added
S-Ready-For-Final-Review
Objective
Showcase
ReSTIR:
Previous RIS:
