Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve the depth backprojection feature #1690

Merged
merged 29 commits into from
Mar 24, 2023
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
4df37f1
Replace the backproject scale with the familiar "meter"
emilk Mar 23, 2023
61faf43
Assume depth images usually start at zero
emilk Mar 23, 2023
e18a1c5
Make the depth point radius scale more sane
emilk Mar 23, 2023
f783c0f
Fix an off-by-half error, giving all points an extra 0.5 pixels in ra…
emilk Mar 23, 2023
89faaf8
Add a fudge-factor
emilk Mar 23, 2023
8594073
Improve tooltip
emilk Mar 23, 2023
9034c5c
Fix typos
emilk Mar 23, 2023
5cb4e2d
Fix doclink
emilk Mar 23, 2023
0317372
Fix the math
emilk Mar 23, 2023
a7fbbfc
fix typo
emilk Mar 23, 2023
3deb14c
#[inline]
emilk Mar 23, 2023
ed32db8
Another typo
emilk Mar 23, 2023
13d4844
typo
emilk Mar 23, 2023
2b103f2
Adjust the feathering margins
emilk Mar 23, 2023
f661a25
Clean up the depth texture uploading
emilk Mar 24, 2023
5603fa0
Better docs
emilk Mar 24, 2023
37d8725
fix typos
emilk Mar 24, 2023
4964a54
Use world depth values in the shader and simplify user code
emilk Mar 24, 2023
0e72422
Better naming
emilk Mar 24, 2023
d5ee3c1
Fix bug in the image-hover code
emilk Mar 24, 2023
51a21ac
Use same depth range for the color map in the depth projection shader
emilk Mar 24, 2023
2c72031
Fix the gamma of the wgsl color map
emilk Mar 24, 2023
8134e8a
Tweak default point radius to avoid Moiré patterns
emilk Mar 24, 2023
f489b2a
Final touches
emilk Mar 24, 2023
e321037
spell fix
emilk Mar 24, 2023
f2e1484
more typos
emilk Mar 24, 2023
21cfdcf
Fix comment about why we use Depth16Unorm
emilk Mar 24, 2023
ea8c05e
Add lines to changelog
emilk Mar 24, 2023
ca8b777
max_depth -> max_depth_in_world
emilk Mar 24, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions crates/re_data_store/src/entity_properties.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,14 @@ pub struct EntityProperties {
/// `None` means backprojection is disabled.
pub backproject_pinhole_ent_path: Option<EntityPath>,

/// Used to scale the resulting point cloud.
pub backproject_scale: EditableAutoValue<f32>,
/// How many depth units per world-space unit. e.g. 1000 for millimeters.
///
/// This corresponds to [`re_log_types::component_types::Tensor::meter`].
pub backproject_depth_meter: EditableAutoValue<f32>,
emilk marked this conversation as resolved.
Show resolved Hide resolved

/// Used to scale the radii of the points in the resulting point cloud.
///
/// Default is 1.0.
pub backproject_radius_scale: EditableAutoValue<f32>,
}

Expand All @@ -94,7 +98,10 @@ impl EntityProperties {
.backproject_pinhole_ent_path
.clone()
.or(child.backproject_pinhole_ent_path.clone()),
backproject_scale: self.backproject_scale.or(&child.backproject_scale).clone(),
backproject_depth_meter: self
.backproject_depth_meter
.or(&child.backproject_depth_meter)
.clone(),
backproject_radius_scale: self
.backproject_radius_scale
.or(&child.backproject_radius_scale)
Expand All @@ -114,8 +121,8 @@ impl Default for EntityProperties {
pinhole_image_plane_distance: EditableAutoValue::default(),
backproject_depth: false,
backproject_pinhole_ent_path: None,
backproject_scale: EditableAutoValue::default(),
backproject_radius_scale: EditableAutoValue::default(),
backproject_depth_meter: EditableAutoValue::default(),
backproject_radius_scale: EditableAutoValue::Auto(1.0),
}
}
}
Expand Down
4 changes: 4 additions & 0 deletions crates/re_log_types/src/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ impl TensorDataType {
}
}

pub fn is_integer(&self) -> bool {
!self.is_float()
}

pub fn is_float(&self) -> bool {
emilk marked this conversation as resolved.
Show resolved Hide resolved
match self {
Self::U8
Expand Down
2 changes: 1 addition & 1 deletion crates/re_renderer/examples/depth_cloud.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ impl RenderDepthClouds {
&[DepthCloud {
depth_camera_extrinsics: world_from_obj,
depth_camera_intrinsics: *intrinsics,
radius_scale: *radius_scale,
point_radius_from_normalized_depth: *radius_scale,
depth_dimensions: depth.dimensions,
depth_data: depth.data.clone(),
colormap: re_renderer::ColorMap::ColorMapTurbo,
Expand Down
7 changes: 4 additions & 3 deletions crates/re_renderer/shader/depth_cloud.wgsl
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ fn compute_point_data(quad_idx: i32) -> PointData {

var data: PointData;
data.pos_in_world = pos_in_world.xyz;
data.unresolved_radius = norm_linear_depth * depth_cloud_info.radius_scale;
data.unresolved_radius = depth_cloud_info.point_radius_from_normalized_depth * norm_linear_depth;
data.color = color;

return data;
Expand All @@ -61,15 +61,16 @@ struct DepthCloudInfo {
/// Only supports pinhole cameras at the moment.
depth_camera_intrinsics: Mat3,

/// The scale to apply to the radii of the backprojected points.
radius_scale: f32,
/// Point radius is calculated as normalized depth times this value.
point_radius_from_normalized_depth: f32,

/// Configures color mapping mode, see `colormap.wgsl`.
colormap: u32,

/// Outline mask id for the outline mask pass.
outline_mask_id: UVec2,
};

@group(1) @binding(0)
var<uniform> depth_cloud_info: DepthCloudInfo;

Expand Down
8 changes: 6 additions & 2 deletions crates/re_renderer/shader/utils/sphere_quad.wgsl
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,13 @@ fn sphere_quad_coverage(world_position: Vec3, radius: f32, point_center: Vec3) -
// https://www.shadertoy.com/view/MsSSWV
// (but rearranged and labeled to it's easier to understand!)
let d = ray_sphere_distance(ray, point_center, radius);
let smallest_distance_to_sphere = d.x;
let distance_to_sphere_surface = d.x;
let closest_ray_dist = d.y;
let pixel_world_size = approx_pixel_world_size_at(closest_ray_dist);

return 1.0 - saturate(smallest_distance_to_sphere / pixel_world_size);
let distance_to_surface_in_pixels = distance_to_sphere_surface / pixel_world_size;

// At the surface we have 50% coverage, and it decreases with distance.
// Not that we have signed distances to the sphere surface.
emilk marked this conversation as resolved.
Show resolved Hide resolved
return saturate(0.5 - distance_to_surface_in_pixels);
emilk marked this conversation as resolved.
Show resolved Hide resolved
}
11 changes: 6 additions & 5 deletions crates/re_renderer/src/renderer/depth_cloud.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ mod gpu_data {
pub depth_camera_extrinsics: crate::wgpu_buffer_types::Mat4,
pub depth_camera_intrinsics: crate::wgpu_buffer_types::Mat3,

pub radius_scale: f32,
/// Point radius is calculated as depth times this value.
pub point_radius_from_normalized_depth: f32,
pub colormap: u32,
pub outline_mask_id: crate::wgpu_buffer_types::UVec2,

Expand Down Expand Up @@ -84,8 +85,8 @@ pub struct DepthCloud {
/// Only supports pinhole cameras at the moment.
pub depth_camera_intrinsics: glam::Mat3,

/// The scale to apply to the radii of the backprojected points.
pub radius_scale: f32,
/// Point radius is calculated as depth times this value.
pub point_radius_from_normalized_depth: f32,

/// The dimensions of the depth texture in pixels.
pub depth_dimensions: glam::UVec2,
Expand All @@ -107,7 +108,7 @@ impl Default for DepthCloud {
Self {
depth_camera_extrinsics: glam::Mat4::IDENTITY,
depth_camera_intrinsics: glam::Mat3::IDENTITY,
radius_scale: 1.0,
point_radius_from_normalized_depth: 1e-3,
depth_dimensions: glam::UVec2::ZERO,
depth_data: DepthCloudDepthData::default(),
colormap: ColorMap::ColorMapTurbo,
Expand Down Expand Up @@ -162,7 +163,7 @@ impl DepthCloudDrawData {
depth_clouds.iter().map(|info| gpu_data::DepthCloudInfoUBO {
depth_camera_extrinsics: info.depth_camera_extrinsics.into(),
depth_camera_intrinsics: info.depth_camera_intrinsics.into(),
radius_scale: info.radius_scale,
point_radius_from_normalized_depth: info.point_radius_from_normalized_depth,
colormap: info.colormap as u32,
outline_mask_id: info.outline_mask_id.0.unwrap_or_default().into(),
end_padding: Default::default(),
Expand Down
2 changes: 2 additions & 0 deletions crates/re_viewer/src/misc/caches/tensor_image_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,8 @@ fn depth_tensor_as_color_image(tensor: &Tensor) -> anyhow::Result<ColorImage> {
"Depth image had non-finite values"
);

min = min.min(0.0); // Depth usually start at zero.

if min == max {
// Uniform image. We can't remap it to a 0-1 range, so do whatever:
min = 0.0;
Expand Down
61 changes: 38 additions & 23 deletions crates/re_viewer/src/ui/selection_panel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -525,37 +525,52 @@ fn depth_props_ui(
);
ui.end_row();

ui.label("Backproject scale");
let mut scale = *entity_props.backproject_scale.get();
let speed = (scale * 0.05).at_least(0.01);
if ui
{
ui.label("Backproject meter");
let mut meter = *entity_props.backproject_depth_meter.get();
let speed = (meter * 0.05).at_least(0.01);
let response = ui
.add(
egui::DragValue::new(&mut scale)
egui::DragValue::new(&mut meter)
.clamp_range(0.0..=1.0e8)
.speed(speed),
)
.on_hover_text("Scales the backprojected point cloud")
.changed()
{
entity_props.backproject_scale = EditableAutoValue::UserEdited(scale);
.on_hover_text("How many depth values correspond to one work-space unit. For instance, 1000 means millimeters.\
emilk marked this conversation as resolved.
Show resolved Hide resolved
\nDouble-click to reset.");
if response.double_clicked() {
// reset to auto - the exacy value will be restored somewhere else
emilk marked this conversation as resolved.
Show resolved Hide resolved
entity_props.backproject_depth_meter = EditableAutoValue::Auto(meter);
response.surrender_focus();
} else if response.changed() {
entity_props.backproject_depth_meter = EditableAutoValue::UserEdited(meter);
}
ui.end_row();
}
ui.end_row();

ui.label("Backproject radius scale");
let mut radius_scale = *entity_props.backproject_radius_scale.get();
let speed = (radius_scale * 0.001).at_least(0.001);
if ui
.add(
egui::DragValue::new(&mut radius_scale)
.clamp_range(0.0..=1.0e8)
.speed(speed),
)
.on_hover_text("Scales the radii of the points in the backprojected point cloud")
.changed()
{
entity_props.backproject_radius_scale = EditableAutoValue::UserEdited(radius_scale);
ui.label("Backproject radius scale");
let mut radius_scale = *entity_props.backproject_radius_scale.get();
let speed = (radius_scale * 0.01).at_least(0.001);
let response = ui
.add(
egui::DragValue::new(&mut radius_scale)
.clamp_range(0.0..=1.0e8)
.speed(speed),
)
.on_hover_text(
"Scales the radii of the points in the backprojected point cloud.\n\
With a scale of one, diagonally adjacent pixels at the same depth are sized so that they are just touching, leaving no gaps.\
\nDouble-click to reset.",
);
if response.double_clicked() {
Copy link
Member

Choose a reason for hiding this comment

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

huh. yeah! I like that. Goes on the growing pile of "how to deal with defaults in a uniform manner everywhere" :)

entity_props.backproject_radius_scale = EditableAutoValue::Auto(1.0);
response.surrender_focus();
} else if response.changed() {
entity_props.backproject_radius_scale =
EditableAutoValue::UserEdited(radius_scale);
}
ui.end_row();
}
ui.end_row();

// TODO(cmc): This should apply to the depth map entity as a whole, but for that we
// need to get the current hardcoded colormapping out of the image cache first.
Expand Down
33 changes: 25 additions & 8 deletions crates/re_viewer/src/ui/view_spatial/scene/scene_part/images.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,9 +278,10 @@ impl ImagesPart {
};

// TODO(cmc): getting to those extrinsics is no easy task :|
let Some(extrinsics) = pinhole_ent_path
.parent()
.and_then(|ent_path| transforms.reference_from_entity(&ent_path)) else {
let extrinsics = pinhole_ent_path
.parent()
.and_then(|ent_path| transforms.reference_from_entity(&ent_path));
let Some(extrinsics) = extrinsics else {
re_log::warn_once!("Couldn't fetch pinhole extrinsics at {pinhole_ent_path:?}");
return;
};
Expand All @@ -291,16 +292,21 @@ impl ImagesPart {
TensorData::U16(data) => DepthCloudDepthData::U16(data.clone()),
TensorData::F32(data) => DepthCloudDepthData::F32(data.clone()),
_ => {
let discriminant = std::mem::discriminant(&tensor.data);
re_log::warn_once!(
"Tensor datatype is not supported for backprojection ({discriminant:?})"
"Tensor datatype {} is not supported for backprojection",
tensor.dtype()
);
return;
}
};

let scale = *properties.backproject_scale.get();
let radius_scale = *properties.backproject_radius_scale.get();
let meter = *properties.backproject_depth_meter.get();
let scale = match data {
// the GPU normalizes the u16 to [0, 1] during texture smapler:
emilk marked this conversation as resolved.
Show resolved Hide resolved
DepthCloudDepthData::U16(_) => u16::MAX as f32 / meter,

DepthCloudDepthData::F32(_) => 1.0 / meter,
};

let (h, w) = (tensor.shape()[0].size, tensor.shape()[1].size);
let dimensions = glam::UVec2::new(w as _, h as _);
Expand All @@ -318,10 +324,21 @@ impl ImagesPart {
},
};

// By default, we assign a radius to each point so that each point becomes a ball
// that just barely touches its diagonally-neighboring pixel-point.
// This means the point radius increases with distance.
// It also increases with the field of view, and decreases with the resolution.
let radius_scale = *properties.backproject_radius_scale.get();
let point_radius_from_depth = 0.5_f32.sqrt() * intrinsics.fov_y().unwrap_or(1.0) / h as f32;
let point_radius_from_normalized_depth = radius_scale * point_radius_from_depth * scale;

// TODO(emilk): we get gaps between diagonally adjacent points without this hack. WHY???
let point_radius_from_normalized_depth = 1.1 * point_radius_from_normalized_depth;
emilk marked this conversation as resolved.
Show resolved Hide resolved

scene.primitives.depth_clouds.push(DepthCloud {
depth_camera_extrinsics: world_from_obj,
depth_camera_intrinsics: intrinsics.image_from_cam.into(),
radius_scale,
point_radius_from_normalized_depth,
depth_dimensions: dimensions,
depth_data: data,
colormap,
Expand Down
40 changes: 13 additions & 27 deletions crates/re_viewer/src/ui/view_spatial/ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use re_format::format_f32;

use egui::{NumExt, WidgetText};
use macaw::BoundingBox;
use re_log_types::component_types::{Tensor, TensorData, TensorDataMeaning};
use re_log_types::component_types::{Tensor, TensorDataMeaning};
use re_renderer::renderer::OutlineConfig;

use crate::{
Expand Down Expand Up @@ -175,13 +175,7 @@ impl ViewSpatialState {
&entity_path,
scene_size,
);
Self::update_depth_cloud_property_heuristics(
ctx,
data_blueprint,
&query,
&entity_path,
scene_size,
);
Self::update_depth_cloud_property_heuristics(ctx, data_blueprint, &query, &entity_path);
}
}

Expand Down Expand Up @@ -221,34 +215,26 @@ impl ViewSpatialState {
data_blueprint: &mut DataBlueprintTree,
query: &re_arrow_store::LatestAtQuery,
entity_path: &EntityPath,
scene_size: f32,
) {
let tensor = query_latest_single::<Tensor>(&ctx.log_db.entity_db, entity_path, query);
if tensor.as_ref().map(|t| t.meaning) == Some(TensorDataMeaning::Depth) {
let tensor = tensor.as_ref().unwrap();

let mut properties = data_blueprint.data_blueprints_individual().get(entity_path);
if properties.backproject_scale.is_auto() {
let auto = tensor.meter.map_or_else(
|| match &tensor.data {
TensorData::U16(_) => 1.0 / u16::MAX as f32,
_ => 1.0,
},
|meter| match &tensor.data {
TensorData::U16(_) => 1.0 / meter * u16::MAX as f32,
_ => meter,
},
);
properties.backproject_scale = EditableAutoValue::Auto(auto);
if properties.backproject_depth_meter.is_auto() {
let auto = tensor.meter.unwrap_or_else(|| {
use re_log_types::component_types::TensorTrait as _;
if tensor.dtype().is_integer() {
1000.0
} else {
1.0
}
});
properties.backproject_depth_meter = EditableAutoValue::Auto(auto);
}

if properties.backproject_radius_scale.is_auto() {
let auto = if scene_size.is_finite() && scene_size > 0.0 {
f32::max(0.02, scene_size * 0.001)
} else {
0.02
};
properties.backproject_radius_scale = EditableAutoValue::Auto(auto);
properties.backproject_radius_scale = EditableAutoValue::Auto(1.0);
}

data_blueprint
Expand Down