From 356991983668bfe086dc4c9cb1b753f14d039910 Mon Sep 17 00:00:00 2001 From: Robert Swain Date: Sat, 23 Sep 2023 12:59:10 +0200 Subject: [PATCH 1/3] skybox.wgsl: Fix precision issues --- .../bevy_core_pipeline/src/skybox/skybox.wgsl | 37 ++++++++++++------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/crates/bevy_core_pipeline/src/skybox/skybox.wgsl b/crates/bevy_core_pipeline/src/skybox/skybox.wgsl index d3bc9c5faa571..ef1c876a8f066 100644 --- a/crates/bevy_core_pipeline/src/skybox/skybox.wgsl +++ b/crates/bevy_core_pipeline/src/skybox/skybox.wgsl @@ -6,7 +6,6 @@ struct VertexOutput { @builtin(position) clip_position: vec4, - @location(0) world_position: vec3, }; // 3 | 2. @@ -29,21 +28,33 @@ fn skybox_vertex(@builtin(vertex_index) vertex_index: u32) -> VertexOutput { 0.25, 0.5 ) * 4.0 - vec4(1.0); - // Use the position on the near clipping plane to avoid -inf world position - // because the far plane of an infinite reverse projection is at infinity. - // NOTE: The clip position has a w component equal to 1.0 so we don't need - // to apply a perspective divide to it before inverse-projecting it. - let world_position_homogeneous = view.inverse_view_proj * vec4(clip_position.xy, 1.0, 1.0); - let world_position = world_position_homogeneous.xyz / world_position_homogeneous.w; - return VertexOutput(clip_position, world_position); + return VertexOutput(clip_position); } @fragment fn skybox_fragment(in: VertexOutput) -> @location(0) vec4 { - // The skybox cubemap is sampled along the direction from the camera world - // position, to the fragment world position on the near clipping plane - let ray_direction = in.world_position - view.world_position; - // cube maps are left-handed so we negate the z coordinate - return textureSample(skybox, skybox_sampler, ray_direction * vec3(1.0, 1.0, -1.0)); + // Using world positions of the fragment and camera to calculate a ray direction + // break down at large translations. This code only needs to know the ray direction. + // The ray direction is along the direction from the camera to the fragment position. + // In view space, the camera is at the origin, so the view space ray direction is + // along the direction of the fragment position - (0,0,0) which is just the + // fragment position. + // Use the position on the near clipping plane to avoid -inf world position + // because the far plane of an infinite reverse projection is at infinity. + let view_position_homogeneous = view.inverse_projection * vec4( + in.clip_position.xy / view.viewport.zw * vec2(2.0, -2.0) + vec2(-1.0, 1.0), + 1.0, + 1.0 + ); + let view_position = view_position_homogeneous.xyz / view_position_homogeneous.w; + let view_ray_direction = normalize(view_position); + // Transforming the view space ray direction by the view matrix, transforms the + // direction to world space. Note that the w element is set to 0.0, as this is a + // vector direction, not a position, That causes the matrix multiplication to ignore + // the translations from the view matrix. + let ray_direction = (view.view * vec4(view_ray_direction, 0.0)).xyz; + + // Cube maps are left-handed so we negate the z coordinate. + return textureSample(skybox, skybox_sampler, normalize(ray_direction) * vec3(1.0, 1.0, -1.0)); } From 3695ff1a4073f4296b71508af9a0aaca1056259c Mon Sep 17 00:00:00 2001 From: Robert Swain Date: Sat, 23 Sep 2023 20:48:55 +0200 Subject: [PATCH 2/3] Cleanup, and reuse coords_to_viewport_uv --- .../bevy_core_pipeline/src/skybox/skybox.wgsl | 51 +++++++++++-------- 1 file changed, 29 insertions(+), 22 deletions(-) diff --git a/crates/bevy_core_pipeline/src/skybox/skybox.wgsl b/crates/bevy_core_pipeline/src/skybox/skybox.wgsl index ef1c876a8f066..607416a88c496 100644 --- a/crates/bevy_core_pipeline/src/skybox/skybox.wgsl +++ b/crates/bevy_core_pipeline/src/skybox/skybox.wgsl @@ -1,11 +1,37 @@ #import bevy_render::view View +#import bevy_pbr::utils coords_to_viewport_uv @group(0) @binding(0) var skybox: texture_cube; @group(0) @binding(1) var skybox_sampler: sampler; @group(0) @binding(2) var view: View; +fn coords_to_ray_direction(position: vec2, viewport: vec4) -> vec3 { + // Using world positions of the fragment and camera to calculate a ray direction + // break down at large translations. This code only needs to know the ray direction. + // The ray direction is along the direction from the camera to the fragment position. + // In view space, the camera is at the origin, so the view space ray direction is + // along the direction of the fragment position - (0,0,0) which is just the + // fragment position. + // Use the position on the near clipping plane to avoid -inf world position + // because the far plane of an infinite reverse projection is at infinity. + let view_position_homogeneous = view.inverse_projection * vec4( + coords_to_viewport_uv(position, viewport) * vec2(2.0, -2.0) + vec2(-1.0, 1.0), + 1.0, + 1.0, + ); + let view_position = view_position_homogeneous.xyz / view_position_homogeneous.w; + let view_ray_direction = normalize(view_position); + // Transforming the view space ray direction by the view matrix, transforms the + // direction to world space. Note that the w element is set to 0.0, as this is a + // vector direction, not a position, That causes the matrix multiplication to ignore + // the translations from the view matrix. + let ray_direction = (view.view * vec4(view_ray_direction, 0.0)).xyz; + + return normalize(ray_direction); +} + struct VertexOutput { - @builtin(position) clip_position: vec4, + @builtin(position) position: vec4, }; // 3 | 2. @@ -34,27 +60,8 @@ fn skybox_vertex(@builtin(vertex_index) vertex_index: u32) -> VertexOutput { @fragment fn skybox_fragment(in: VertexOutput) -> @location(0) vec4 { - // Using world positions of the fragment and camera to calculate a ray direction - // break down at large translations. This code only needs to know the ray direction. - // The ray direction is along the direction from the camera to the fragment position. - // In view space, the camera is at the origin, so the view space ray direction is - // along the direction of the fragment position - (0,0,0) which is just the - // fragment position. - // Use the position on the near clipping plane to avoid -inf world position - // because the far plane of an infinite reverse projection is at infinity. - let view_position_homogeneous = view.inverse_projection * vec4( - in.clip_position.xy / view.viewport.zw * vec2(2.0, -2.0) + vec2(-1.0, 1.0), - 1.0, - 1.0 - ); - let view_position = view_position_homogeneous.xyz / view_position_homogeneous.w; - let view_ray_direction = normalize(view_position); - // Transforming the view space ray direction by the view matrix, transforms the - // direction to world space. Note that the w element is set to 0.0, as this is a - // vector direction, not a position, That causes the matrix multiplication to ignore - // the translations from the view matrix. - let ray_direction = (view.view * vec4(view_ray_direction, 0.0)).xyz; + let ray_direction = coords_to_ray_direction(in.position.xy, view.viewport); // Cube maps are left-handed so we negate the z coordinate. - return textureSample(skybox, skybox_sampler, normalize(ray_direction) * vec3(1.0, 1.0, -1.0)); + return textureSample(skybox, skybox_sampler, ray_direction * vec3(1.0, 1.0, -1.0)); } From 906e282152fe58c58ec8140d7a76596a928aa911 Mon Sep 17 00:00:00 2001 From: Robert Swain Date: Sat, 23 Sep 2023 20:58:17 +0200 Subject: [PATCH 3/3] Remove probably unnecessary normalize() --- crates/bevy_core_pipeline/src/skybox/skybox.wgsl | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/bevy_core_pipeline/src/skybox/skybox.wgsl b/crates/bevy_core_pipeline/src/skybox/skybox.wgsl index 607416a88c496..856cde7d96eaf 100644 --- a/crates/bevy_core_pipeline/src/skybox/skybox.wgsl +++ b/crates/bevy_core_pipeline/src/skybox/skybox.wgsl @@ -19,8 +19,7 @@ fn coords_to_ray_direction(position: vec2, viewport: vec4) -> vec3