Skip to content

Commit

Permalink
Merge pull request #86 from luftkode/fix85-wasm-broken
Browse files Browse the repository at this point in the history
fix integer overflow on wasm
  • Loading branch information
CramBL authored Oct 18, 2024
2 parents 81a01a8 + f5490b9 commit d506ec2
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 55 deletions.
7 changes: 6 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Unreleased

## [0.24.1]

### Fix

- Web version of `logviewer-rs` was broken due an integer overflow. When determining down sample level, a cast from 64-bit float to pointer size caused integer overflow on wasm due to wasm having a 32-bit pointer size.

## [0.24.0]

### Added

- Initial support for `HDF` files, starting with bifrost (TX) loop current. The feature is currently guarded behind a feature flag, enabling it is tracked at: https://github.com/luftkode/logviewer-rs/issues/84.
-

### Changed

Expand Down
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
name = "logviewer-rs"
description = "Log viewer app for viewing plots of data from projects such as motor and generator control"
authors = ["SkyTEM Surveys", "Marc Beck König"]
version = "0.24.0"
version = "0.24.1"
edition = "2021"
repository = "https://github.com/luftkode/logviewer-rs"
homepage = "https://github.com/luftkode/logviewer-rs"
Expand Down
6 changes: 2 additions & 4 deletions crates/plot_util/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,8 @@ pub fn plot_lines<'pv>(
match mipmap_cfg {
MipMapConfiguration::Disabled => plot_raw(plot_ui, plot_vals, (x_lower, x_higher)),
MipMapConfiguration::Auto => {
let (level, idx_range) = plot_vals.get_scaled_mipmap_levels(
plots_width_pixels,
(x_lower as usize, x_higher as usize),
);
let (level, idx_range) =
plot_vals.get_scaled_mipmap_levels(plots_width_pixels, (x_lower, x_higher));

plot_with_mipmapping(
plot_ui,
Expand Down
66 changes: 19 additions & 47 deletions crates/plot_util/src/mipmap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,29 +67,29 @@ pub enum MipMapStrategy {
Max,
}

#[derive(Debug, Default, PartialEq, Eq, Serialize, Deserialize)]
struct LevelLookupCached {
#[derive(Debug, Default, Clone, PartialEq, Serialize, Deserialize)]
struct LevelLookupCached<T: Num + ToPrimitive + FromPrimitive + PartialOrd> {
pixel_width: usize,
x_bounds: (usize, usize),
x_bounds: (T, T),
result_span: (usize, usize),
result_idx: usize,
}

impl LevelLookupCached {
pub fn is_equal(&self, pixel_width: usize, x_bounds: (usize, usize)) -> bool {
impl<T: Num + ToPrimitive + FromPrimitive + Copy + PartialOrd> LevelLookupCached<T> {
pub fn is_equal(&self, pixel_width: usize, x_bounds: (T, T)) -> bool {
// Compare bounds first because pixel_width is much more stable
self.x_bounds == x_bounds && self.pixel_width == pixel_width
}
}

#[derive(Debug, PartialEq, Eq, Serialize, Deserialize)]
#[derive(Debug, PartialEq, Serialize, Deserialize)]
pub struct MipMap2D<T: Num + ToPrimitive + FromPrimitive + PartialOrd> {
strategy: MipMapStrategy,
data: Vec<Vec<[T; 2]>>,
most_recent_lookup: RefCell<LevelLookupCached>,
most_recent_lookup: RefCell<LevelLookupCached<T>>,
}

impl<T: Num + ToPrimitive + FromPrimitive + Copy + PartialOrd> MipMap2D<T> {
impl<T: Num + ToPrimitive + FromPrimitive + Copy + PartialOrd + Default> MipMap2D<T> {
pub fn new(source: Vec<[T; 2]>, strategy: MipMapStrategy, min_elements: usize) -> Self {
let mut data = vec![source.clone()];
let mut current = source;
Expand Down Expand Up @@ -223,7 +223,7 @@ impl<T: Num + ToPrimitive + FromPrimitive + Copy + PartialOrd> MipMap2D<T> {
pub fn get_level_match(
&self,
pixel_width: usize,
x_bounds: (usize, usize),
x_bounds: (T, T),
) -> (usize, Option<(usize, usize)>) {
if self
.most_recent_lookup
Expand All @@ -250,8 +250,8 @@ impl<T: Num + ToPrimitive + FromPrimitive + Copy + PartialOrd> MipMap2D<T> {
continue;
}

let start_idx = lvl.partition_point(|x| fast_unix_ns_to_usize(x[0]) < x_min);
let end_idx = lvl.partition_point(|x| fast_unix_ns_to_usize(x[0]) < x_max);
let start_idx = lvl.partition_point(|x| x[0] < x_min);
let end_idx = lvl.partition_point(|x| x[0] < x_max);

// Use saturating_sub for safety and to avoid potential panic
let count_within_bounds = end_idx.saturating_sub(start_idx);
Expand All @@ -271,40 +271,12 @@ impl<T: Num + ToPrimitive + FromPrimitive + Copy + PartialOrd> MipMap2D<T> {
}
}

/// Converts a unix timestamp in nanoseconds to `usize`.
///
/// This function is highly optimized for performance.
#[inline(always)]
#[allow(
clippy::needless_pass_by_value,
reason = "It's a numeric primitive, we don't mind passing by value"
)]
pub fn fast_unix_ns_to_usize<T: Num + ToPrimitive + FromPrimitive + PartialOrd>(
unix_ts_ns: T,
) -> usize {
// On 64-bit platforms, we can assume that `unix_ts_ns` fits in usize, so we just cast it directly.
#[cfg(not(target_pointer_width = "32"))]
#[allow(unsafe_code)]
// SAFETY:
// Assumes:
// - That `usize` is at least 64 bits.
// - That the argument is a unix timestamp that is less than the year ~2554
unsafe {
unix_ts_ns.to_usize().unwrap_unchecked()
}
#[cfg(target_pointer_width = "32")]
unix_ts_ns.to_usize().expect("Doesn't fit in usize")
}

#[cfg(test)]
mod tests {
use super::*;
use pretty_assertions::assert_eq;

#[cfg(not(target_pointer_width = "32"))]
const UNIX_TS_NS: usize = 1_728_470_564_000_000_000;
#[cfg(target_pointer_width = "32")]
const UNIX_TS_NS: usize = 1_728_470_564;
const UNIX_TS_NS: f64 = 1_728_470_564_000_000_000.0;

#[test]
fn test_mipmap_strategy_max() {
Expand Down Expand Up @@ -404,7 +376,7 @@ mod tests {
(8, 0, Some((0, 15))),
(16, 0, None),
] {
let (lvl, range_res) = mipmap.get_level_match(pixel_width, (0, 15));
let (lvl, range_res) = mipmap.get_level_match(pixel_width, (0., 15.));
assert_eq!(
lvl, expected_lvl,
"Expected lvl {expected_lvl} for width: {pixel_width}"
Expand All @@ -421,17 +393,17 @@ mod tests {
fn test_level_match_large_timestamps() {
let source_len = 1600;
let source: Vec<[f64; 2]> = (0..source_len)
.map(|i| [(i + UNIX_TS_NS) as f64, (i + UNIX_TS_NS) as f64])
.map(|i| [i as f64 + UNIX_TS_NS, i as f64 + UNIX_TS_NS])
.collect();
let mipmap = MipMap2D::new(source, MipMapStrategy::Min, 1);

let x_bounds = (UNIX_TS_NS + 300, UNIX_TS_NS + 5000);
let x_bounds = (UNIX_TS_NS + 300., UNIX_TS_NS + 1500.);

for (pixel_width, expected_lvl, expected_range) in [
(100usize, 3usize, Some((48, 200))),
(200, 2, Some((96, 400))),
(400, 1, Some((192, 800))),
(800, 0, Some((384, 1600))),
(100usize, 3usize, Some((17, 176))),
(200, 2, Some((33, 352))),
(400, 1, Some((65, 704))),
(800, 0, Some((129, 1408))),
(1600, 0, None),
] {
let (lvl, range_res) = mipmap.get_level_match(pixel_width, x_bounds);
Expand Down
2 changes: 1 addition & 1 deletion crates/plot_util/src/plots/plot_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ impl PlotValues {
pub fn get_scaled_mipmap_levels(
&self,
pixel_width: usize,
x_bounds: (usize, usize),
x_bounds: (f64, f64),
) -> (usize, Option<(usize, usize)>) {
self.mipmap_min.get_level_match(pixel_width, x_bounds)
}
Expand Down

0 comments on commit d506ec2

Please sign in to comment.