Skip to content

Commit

Permalink
Stop using infinities in wgsl shaders (#1594)
Browse files Browse the repository at this point in the history
* Stop using infinities in wgsl shaders
This fixes rendering issues on Windows browsers (likely due to a bug in ANGLE?). However this operation is undefined by spec anyways. Instead we're now using  float min/max instead. Wgsl float constants are now following wgsl spec to the letter.

* spelling

* Fix assertion & point-size conversion issues in size.rs
  • Loading branch information
Wumpf authored Mar 15, 2023
1 parent 51bd75c commit 38f4fd6
Show file tree
Hide file tree
Showing 10 changed files with 53 additions and 33 deletions.
2 changes: 1 addition & 1 deletion crates/re_renderer/shader/global_bindings.wgsl
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ struct FrameUniformBuffer {
pixels_from_point: f32,

/// (tan(fov_y / 2) * aspect_ratio, tan(fov_y /2)), i.e. half ratio of screen dimension to screen distance in x & y.
/// Both values are set to positive infinity for orthographic projection
/// Both values are set to f32max for orthographic projection
tan_half_fov: Vec2,

// Size used for all point radii given with Size::AUTO.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@ fn compute_pixel_coords(center_coord: IVec2, unnormalized_edge_pos_a_and_b: Vec4

var pixel_coord_a: Vec2;
if num_edges_a_and_b.x == 0.0 {
pixel_coord_a = Vec2(inf());
pixel_coord_a = Vec2(f32max);
} else {
pixel_coord_a = Vec2(center_coord) + edge_pos_a_and_b.xy;
}
var pixel_coord_b: Vec2;
if num_edges_a_and_b.y == 0.0 {
pixel_coord_b = Vec2(inf());
pixel_coord_b = Vec2(f32max);
} else {
pixel_coord_b = Vec2(center_coord) + edge_pos_a_and_b.zw;
}
Expand Down
8 changes: 4 additions & 4 deletions crates/re_renderer/shader/outlines/jumpflooding_step.wgsl
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ fn main(in: FragmentInput) -> @location(0) Vec4 {
let pixel_step = Vec2(f32(uniforms.step_width), f32(uniforms.step_width)) / resolution;
let pixel_coordinates = resolution * in.texcoord;

var closest_positions_a = Vec2(-inf());
var closest_distance_sq_a = inf();
var closest_positions_b = Vec2(-inf());
var closest_distance_sq_b = inf();
var closest_positions_a = Vec2(f32min);
var closest_distance_sq_a = f32max;
var closest_positions_b = Vec2(f32min);
var closest_distance_sq_b = f32max;

for (var y: i32 = -1; y <= 1; y += 1) {
for (var x: i32 = -1; x <= 1; x += 1) {
Expand Down
35 changes: 27 additions & 8 deletions crates/re_renderer/shader/types.wgsl
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,23 @@ type Mat3 = mat3x3<f32>;
type Mat4x3 = mat4x3<f32>;
type Mat4 = mat4x4<f32>;

const f32min = -3.4028235e38;
const f32max = 3.4028235e38;
const f32eps = 0.00000011920928955078125;

// Extreme values as documented by the spec:
// https://www.w3.org/TR/WGSL/#floating-point-types
const f32max = 0x1.fffffep+127f; // Largest positive float value.
const f32min = -0x0.fffffep+127f; // Smallest negative float value.
const f32min_normal = 0x1p-126f; // Smallest positive normal float value.
// F16 is not implemented yet in Naga https://github.com/gfx-rs/naga/issues/1884
//const f16min = 0x0.ffcp+15h; // Smallest negative float value.
//const f16max = 0x1.ffcp+15h; // Largest positive float value.
//const f16min_normal = 0x1p-14h; // Smallest positive normal float value.
// https://www.w3.org/TR/WGSL/#integer-types
const i32min = -0x80000000i;
const i32max = 0x7fffffffi;
const u32min = 0u;
const u32max = 0xFFFFFFFFu;
const u32max = 0xffffffffu;

// Difference between `1.0` and the next larger representable number.
const f32eps = 0.00000011920928955078125;

const X = Vec3(1.0, 0.0, 0.0);
const Y = Vec3(0.0, 1.0, 0.0);
Expand All @@ -26,6 +37,14 @@ const Z = Vec3(0.0, 0.0, 1.0);
const ZERO = Vec4(0.0, 0.0, 0.0, 0.0);
const ONE = Vec4(1.0, 1.0, 1.0, 1.0);

fn inf() -> f32 {
return 1.0 / 0.0;
}

// Do NOT use inf() or nan() in your WGSL shaders. Ever.
// The WGSL spec allows implementations to assume that neither Inf or NaN are ever occurring:
// https://www.w3.org/TR/WGSL/#floating-point-evaluation
//
// It will work most of the time, but there are rare cases where this will break.
// (Notably, we had a case where the following commented inf function would silently break shaders when using ANGLE, i.e. in browsers on Windows!)
//
// fn inf() -> f32 {
// return 1.0 / 0.0;
// }
4 changes: 2 additions & 2 deletions crates/re_renderer/shader/utils/camera.wgsl
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@

// True if the camera is orthographic
fn is_camera_orthographic() -> bool {
return frame.tan_half_fov.x == inf();
return frame.tan_half_fov.x >= f32max;
}

// True if the camera is perspective
fn is_camera_perspective() -> bool {
return frame.tan_half_fov.x != inf();
return frame.tan_half_fov.x < f32max;
}

struct Ray {
Expand Down
8 changes: 4 additions & 4 deletions crates/re_renderer/shader/utils/size.wgsl
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@
fn unresolved_size_to_world(_unresolved_size: f32, camera_distance: f32, auto_size: f32) -> f32 {
// Resolve auto size.
var unresolved_size: f32;
if _unresolved_size == inf() {
// positive inf for small auto size
if _unresolved_size >= f32max {
// positive max for small auto size
unresolved_size = auto_size;
} else if _unresolved_size == -inf() {
// negative inf for large auto size
} else if _unresolved_size <= f32min {
// negative max for large auto size
let large_factor = 1.33;
unresolved_size = auto_size * large_factor;
} else {
Expand Down
2 changes: 1 addition & 1 deletion crates/re_renderer/src/global_bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ pub(crate) struct FrameUniformBuffer {
pub pixels_from_point: f32,

/// (tan(fov_y / 2) * aspect_ratio, tan(fov_y /2)), i.e. half ratio of screen dimension to screen distance in x & y.
/// Both values are set to positive infinity for orthographic projection
/// Both values are set to f32max for orthographic projection
pub tan_half_fov: wgpu_buffer_types::Vec2,

// Size used for all point radii given with Size::AUTO.
Expand Down
4 changes: 2 additions & 2 deletions crates/re_renderer/src/renderer/lines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -424,13 +424,13 @@ impl LineDrawData {
Vec::with_capacity(wgpu::util::align_to(num_segments, POSITION_TEXTURE_SIZE) as usize);
// sentinel at the beginning to facilitate caps.
position_data_staging.push(LineVertex {
position: glam::vec3(f32::INFINITY, f32::INFINITY, f32::INFINITY),
position: glam::vec3(f32::MAX, f32::MAX, f32::MAX),
strip_index: u32::MAX,
});
position_data_staging.extend(vertices.iter());
// placeholder at the end to facilitate caps.
position_data_staging.push(LineVertex {
position: glam::vec3(f32::INFINITY, f32::INFINITY, f32::INFINITY),
position: glam::vec3(f32::MAX, f32::MAX, f32::MAX),
strip_index: u32::MAX,
});
position_data_staging.extend(std::iter::repeat(gpu_data::LineVertex::zeroed()).take(
Expand Down
17 changes: 9 additions & 8 deletions crates/re_renderer/src/size.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,17 @@ pub struct Size(pub f32);

impl Size {
/// Automatically sized, based on a view builder setting.
pub const AUTO: Self = Self(f32::INFINITY);
pub const AUTO: Self = Self(f32::MAX);

/// Like [`Size::AUTO`], but larger by some small factor (~2).
pub const AUTO_LARGE: Self = Self(-f32::INFINITY);
pub const AUTO_LARGE: Self = Self(f32::MIN);

/// Creates a new size in scene units.
///
/// Values passed must be finite positive.
#[inline]
pub fn new_scene(size: f32) -> Self {
debug_assert!(size.is_finite() && size >= 0.0, "Bad size: {size}");
debug_assert!((0.0..f32::MAX).contains(&size), "Bad size: {size}");
Self(size)
}

Expand All @@ -32,27 +32,26 @@ impl Size {
/// Values passed must be finite positive.
#[inline]
pub fn new_points(size: f32) -> Self {
debug_assert!(size.is_finite() && size >= 0.0, "Bad size: {size}");
debug_assert!((0.0..f32::MAX).contains(&size), "Bad size: {size}");
Self(-size)
}

/// Returns true if the size is an automatically determined size ([`Self::AUTO`] or [`Self::AUTO_LARGE`]).
#[inline]
pub fn is_auto(&self) -> bool {
self.0.is_infinite()
self.0 <= f32::MIN || self.0 >= f32::MAX
}

/// Get the scene-size of this, if stored as a scene size.
#[inline]
#[allow(unused)] // wgpu is not yet using this
pub fn scene(&self) -> Option<f32> {
(self.0.is_finite() && self.0 >= 0.0).then_some(self.0)
(0.0..f32::MAX).contains(&self.0).then_some(self.0)
}

/// Get the point size of this, if stored as a point size.
#[inline]
pub fn points(&self) -> Option<f32> {
(self.0.is_finite() && self.0 <= 0.0).then_some(-self.0)
(self.0 > f32::MIN && self.0 <= 0.0).then_some(-self.0)
}
}

Expand All @@ -69,6 +68,7 @@ impl std::ops::Mul<f32> for Size {
#[inline]
fn mul(self, rhs: f32) -> Self::Output {
debug_assert!(rhs.is_finite() && rhs >= 0.0);
debug_assert!((self.0 * rhs).is_finite());
Self(self.0 * rhs)
}
}
Expand All @@ -77,6 +77,7 @@ impl std::ops::MulAssign<f32> for Size {
#[inline]
fn mul_assign(&mut self, rhs: f32) {
debug_assert!(rhs.is_finite() && rhs >= 0.0);
debug_assert!((self.0 * rhs).is_finite());
self.0 *= rhs;
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/re_renderer/src/view_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ impl ViewBuilder {
}
};

let tan_half_fov = glam::vec2(f32::INFINITY, f32::INFINITY);
let tan_half_fov = glam::vec2(f32::MAX, f32::MAX);
let pixel_world_size_from_camera_distance =
vertical_world_size / config.resolution_in_pixel[1] as f32;

Expand Down

1 comment on commit 38f4fd6

@github-actions
Copy link

Choose a reason for hiding this comment

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

Rust Benchmark

Benchmark suite Current: 38f4fd6 Previous: 51bd75c Ratio
datastore/insert/batch/rects/insert 559261 ns/iter (± 1861) 545489 ns/iter (± 6848) 1.03
datastore/latest_at/batch/rects/query 1840 ns/iter (± 30) 1792 ns/iter (± 20) 1.03
datastore/latest_at/missing_components/primary 286 ns/iter (± 0) 282 ns/iter (± 2) 1.01
datastore/latest_at/missing_components/secondaries 438 ns/iter (± 0) 429 ns/iter (± 11) 1.02
datastore/range/batch/rects/query 151520 ns/iter (± 519) 146009 ns/iter (± 1912) 1.04
mono_points_arrow/generate_message_bundles 48318016 ns/iter (± 494422) 45169265 ns/iter (± 1621915) 1.07
mono_points_arrow/generate_messages 127474339 ns/iter (± 1508294) 136978867 ns/iter (± 1646771) 0.93
mono_points_arrow/encode_log_msg 159549179 ns/iter (± 1325016) 161184495 ns/iter (± 1393974) 0.99
mono_points_arrow/encode_total 337112136 ns/iter (± 1931763) 348332023 ns/iter (± 4291123) 0.97
mono_points_arrow/decode_log_msg 181004618 ns/iter (± 744958) 183910047 ns/iter (± 1890395) 0.98
mono_points_arrow/decode_message_bundles 65470475 ns/iter (± 794309) 72775733 ns/iter (± 1679365) 0.90
mono_points_arrow/decode_total 243039709 ns/iter (± 1379454) 250687161 ns/iter (± 2350687) 0.97
batch_points_arrow/generate_message_bundles 323386 ns/iter (± 480) 313646 ns/iter (± 3750) 1.03
batch_points_arrow/generate_messages 6458 ns/iter (± 20) 6160 ns/iter (± 83) 1.05
batch_points_arrow/encode_log_msg 356799 ns/iter (± 984) 352657 ns/iter (± 3461) 1.01
batch_points_arrow/encode_total 706689 ns/iter (± 2127) 708725 ns/iter (± 8754) 1.00
batch_points_arrow/decode_log_msg 347642 ns/iter (± 1114) 344364 ns/iter (± 2590) 1.01
batch_points_arrow/decode_message_bundles 2072 ns/iter (± 12) 1969 ns/iter (± 24) 1.05
batch_points_arrow/decode_total 356898 ns/iter (± 1454) 351139 ns/iter (± 2251) 1.02
arrow_mono_points/insert 6126182762 ns/iter (± 35168800) 7026781500 ns/iter (± 20179326) 0.87
arrow_mono_points/query 1823165 ns/iter (± 16496) 1775722 ns/iter (± 18401) 1.03
arrow_batch_points/insert 2768566 ns/iter (± 73357) 2636509 ns/iter (± 28407) 1.05
arrow_batch_points/query 17005 ns/iter (± 39) 16554 ns/iter (± 259) 1.03
arrow_batch_vecs/insert 42357 ns/iter (± 101) 41419 ns/iter (± 499) 1.02
arrow_batch_vecs/query 507025 ns/iter (± 1072) 492091 ns/iter (± 6243) 1.03
tuid/Tuid::random 34 ns/iter (± 0) 34 ns/iter (± 0) 1

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.