Skip to content

Commit

Permalink
Fix out of bounds access in data density graph (#8441)
Browse files Browse the repository at this point in the history
Regressed this somewhat recently during perf optimizations in the
data_density_graph: it was possible to get into an out of bounds access
when zooming in.
Cleaned it up a bit overall and fixed a subtle bug for when there's only
fractional buckets filled in the density graph
  • Loading branch information
Wumpf authored Dec 13, 2024
1 parent 084a1e2 commit 98152a7
Showing 1 changed file with 24 additions and 10 deletions.
34 changes: 24 additions & 10 deletions crates/viewer/re_time_panel/src/data_density_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,10 @@ impl DensityGraph {
pub fn add_range(&mut self, (min_x, max_x): (f32, f32), count: f32) {
debug_assert!(min_x <= max_x);

if max_x < self.min_x || self.max_x < min_x {
return;
}

if min_x == max_x {
let center_x = lerp(min_x..=max_x, 0.5);
self.add_point(center_x, count);
Expand All @@ -162,29 +166,39 @@ impl DensityGraph {
// We then want to add to the buckets [3, 4, 5, 6],
// but not in equal amounts.

let first_bucket_factor = 1.0 - (min_bucket - min_bucket.floor());
let num_full_buckets = 1.0 + max_bucket.floor() - min_bucket.ceil();
let last_bucket_factor = 1.0 - (max_bucket.ceil() - max_bucket);
let min_full_bucket = min_bucket.ceil();
let first_bucket = min_bucket.floor();
let max_full_bucket = max_bucket.floor();
let last_bucket = max_bucket.ceil();
let first_bucket_factor = 1.0 - (min_bucket - first_bucket);
let num_full_buckets = 1.0 + max_full_bucket - min_full_bucket;
let last_bucket_factor = 1.0 - (last_bucket - max_bucket);
let count_per_bucket =
count / (first_bucket_factor + num_full_buckets + last_bucket_factor);

// For filling self.buckets, we need to account for min_bucket/max_bucket being out of range!
// (everything before & beyond can be seen as a "virtual" bucket that we can't fill)

// first bucket, partially filled:
if let Ok(i) = usize::try_from(min_bucket.floor() as i64) {
if let Ok(i) = usize::try_from(first_bucket as i64) {
if let Some(bucket) = self.buckets.get_mut(i) {
*bucket += first_bucket_factor * count_per_bucket;
}
}

// full buckets:
let min_full_bucket_idx = (min_bucket.ceil() as i64).at_least(0) as usize;
let max_full_bucket_idx =
(max_bucket.floor() as i64).at_most(self.buckets.len() as i64 - 1) as usize;
for bucket in &mut self.buckets[min_full_bucket_idx..=max_full_bucket_idx] {
*bucket += count_per_bucket;
if min_full_bucket != max_full_bucket {
let min_full_bucket_idx =
(min_full_bucket as i64).clamp(0, self.buckets.len() as i64 - 1) as usize;
let max_full_bucket_idx =
(max_full_bucket as i64).clamp(0, self.buckets.len() as i64 - 1) as usize;
for bucket in &mut self.buckets[min_full_bucket_idx..=max_full_bucket_idx] {
*bucket += count_per_bucket;
}
}

// last bucket, partially filled:
if let Ok(i) = usize::try_from(max_bucket.ceil() as i64) {
if let Ok(i) = usize::try_from(last_bucket as i64) {
if let Some(bucket) = self.buckets.get_mut(i) {
*bucket += last_bucket_factor * count_per_bucket;
}
Expand Down

0 comments on commit 98152a7

Please sign in to comment.