Skip to content

Commit

Permalink
Remove depth backproject depth option and express it as override
Browse files Browse the repository at this point in the history 
Fixes wrong value on picking
  • Loading branch information
@Wumpf
Wumpf committed Jun 13, 2024
1 parent 0a93d7b commit 7865fef
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 66 deletions.
16 changes: 0 additions & 16 deletions crates/re_entity_db/src/entity_properties.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,6 @@ pub struct EntityProperties {
// TODO(#5067): Test property used so we don't have to continuously adjust existing tests while we're dismantling `EntityProperties`.
pub test_property: bool,

/// How many depth units per world-space unit. e.g. 1000 for millimeters.
///
/// This corresponds to `re_components::Tensor::meter`.
pub depth_from_world_scale: EditableAutoValue<f32>, // TODO(andreas): Just remove once we can edit meter & be able to set semi-clever defaults per visualizer.

/// Used to scale the radii of the points in the resulting point cloud.
pub backproject_radius_scale: EditableAutoValue<f32>, // TODO(andreas): should be a component on the DepthImage archetype.

Expand All @@ -114,7 +109,6 @@ impl Default for EntityProperties {
fn default() -> Self {
Self {
test_property: true,
depth_from_world_scale: EditableAutoValue::Auto(1.0),
backproject_radius_scale: EditableAutoValue::Auto(1.0),
time_series_aggregator: EditableAutoValue::Auto(TimeSeriesAggregator::default()),
}
Expand All @@ -128,10 +122,6 @@ impl EntityProperties {
Self {
test_property: self.test_property && child.test_property,

depth_from_world_scale: self
.depth_from_world_scale
.or(&child.depth_from_world_scale)
.clone(),
backproject_radius_scale: self
.backproject_radius_scale
.or(&child.backproject_radius_scale)
Expand All @@ -155,10 +145,6 @@ impl EntityProperties {
Self {
test_property: other.test_property,

depth_from_world_scale: other
.depth_from_world_scale
.or(&self.depth_from_world_scale)
.clone(),
backproject_radius_scale: other
.backproject_radius_scale
.or(&self.backproject_radius_scale)
Expand All @@ -175,13 +161,11 @@ impl EntityProperties {
pub fn has_edits(&self, other: &Self) -> bool {
let Self {
test_property,
depth_from_world_scale,
backproject_radius_scale,
time_series_aggregator,
} = self;

test_property != &other.test_property
|| depth_from_world_scale.has_edits(&other.depth_from_world_scale)
|| backproject_radius_scale.has_edits(&other.backproject_radius_scale)
|| time_series_aggregator.has_edits(&other.time_series_aggregator)
}
Expand Down
21 changes: 14 additions & 7 deletions crates/re_selection_panel/src/selection_panel.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use re_types::{
archetypes::{Axes3D, DepthImage, Pinhole},
blueprint::components::Interactive,
components::{
AxisLength, Colormap, ImagePlaneDistance, PinholeProjection, Transform3D,
AxisLength, Colormap, DepthMeter, ImagePlaneDistance, PinholeProjection, Transform3D,
VisualizerOverrides,
},
tensor_data::TensorDataMeaning,
Expand Down Expand Up @@ -1428,16 +1428,23 @@ fn depth_props_ui(
.on_hover_text("The entity path of the pinhole transform being used to do the backprojection.");
ui.end_row();

depth_from_world_scale_ui(ui, &mut entity_props.depth_from_world_scale);
depth_from_world_scale_ui(ctx, ui, data_result);
backproject_radius_scale_ui(ui, &mut entity_props.backproject_radius_scale);
colormap_props_ui(ctx, ui, data_result);

Some(())
}

fn depth_from_world_scale_ui(ui: &mut egui::Ui, property: &mut EditableAutoValue<f32>) {
fn depth_from_world_scale_ui(ctx: &ViewContext<'_>, ui: &mut egui::Ui, data_result: &DataResult) {
let (query, _store) =
guess_query_and_db_for_selected_entity(ctx.viewer_ctx, &data_result.entity_path);

// TODO(andreas): Queries the entire image archetype for no good reason, but all of this ui is a hack anyways.
let results = data_result.latest_at_with_overrides::<DepthImage>(ctx, &query);
let depth_meter = results.get_mono_with_fallback::<DepthMeter>();

ui.label("Backproject meter");
let mut value = *property.get();
let mut value = *depth_meter;
let speed = (value * 0.05).at_least(0.01);
let response = ui
.add(
Expand All @@ -1448,12 +1455,12 @@ fn depth_from_world_scale_ui(ui: &mut egui::Ui, property: &mut EditableAutoValue
.on_hover_text("How many steps in the depth image correspond to one world-space unit. For instance, 1000 means millimeters.\n\
Double-click to reset.");
if response.double_clicked() {
// reset to auto - the exact value will be restored somewhere else
*property = EditableAutoValue::Auto(value);
data_result.clear_individual_override::<DepthMeter>(ctx.viewer_ctx);
response.surrender_focus();
} else if response.changed() {
*property = EditableAutoValue::UserEdited(value);
data_result.save_individual_override(ctx.viewer_ctx, &DepthMeter(value));
}

ui.end_row();
}

Expand Down
28 changes: 1 addition & 27 deletions crates/re_space_view_spatial/src/heuristics.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,7 @@ use nohash_hasher::IntSet;
use re_data_ui::image_meaning_for_entity;
use re_entity_db::EditableAutoValue;
use re_log_types::EntityPath;
use re_types::{
components::{DepthMeter, TensorData},
tensor_data::TensorDataMeaning,
SpaceViewClassIdentifier,
};
use re_types::{tensor_data::TensorDataMeaning, SpaceViewClassIdentifier};
use re_viewer_context::{IdentifiedViewSystem, PerSystemEntities, ViewerContext};

use crate::{
Expand Down Expand Up @@ -67,34 +63,12 @@ fn update_depth_cloud_property_heuristics(
.get(&ImageVisualizer::identifier())
.unwrap_or(&BTreeSet::new())
{
// TODO(#5607): what should happen if the promise is still pending?
let Some(tensor) = ctx
.recording()
.latest_at_component::<TensorData>(ent_path, &ctx.current_query())
else {
continue;
};

let meaning =
image_meaning_for_entity(ent_path, &ctx.current_query(), ctx.recording().store());

// TODO(#5607): what should happen if the promise is still pending?
let meter = ctx
.recording()
.latest_at_component::<DepthMeter>(ent_path, &ctx.current_query())
.map(|meter| meter.value.0);

let mut properties = auto_properties.get(ent_path);

if meaning == TensorDataMeaning::Depth {
let auto = meter.unwrap_or_else(|| {
if tensor.dtype().is_integer() {
1000.0
} else {
1.0
}
});
properties.depth_from_world_scale = EditableAutoValue::Auto(auto);
properties.backproject_radius_scale = EditableAutoValue::Auto(1.0);

auto_properties.overwrite_properties(ent_path.clone(), properties);
Expand Down
54 changes: 38 additions & 16 deletions crates/re_space_view_spatial/src/visualizers/images.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@ use nohash_hasher::IntSet;

use re_entity_db::{EntityPath, EntityProperties};
use re_log_types::{EntityPathHash, RowId, TimeInt};
use re_query::range_zip_1x3;
use re_query::range_zip_1x4;
use re_renderer::{
renderer::{DepthCloud, DepthClouds, RectangleOptions, TexturedRect},
RenderContext,
};
use re_space_view::diff_component_filter;
use re_types::{
archetypes::{DepthImage, Image, SegmentationImage},
components::{Color, Colormap, DrawOrder, TensorData, ViewCoordinates},
components::{Color, Colormap, DepthMeter, DrawOrder, TensorData, ViewCoordinates},
tensor_data::{DecodedTensor, TensorDataMeaning},
Archetype, ComponentNameSet,
};
Expand Down Expand Up @@ -159,6 +159,7 @@ struct ImageComponentData<'a> {
color: Option<&'a Color>,
draw_order: Option<&'a DrawOrder>,
colormap: Option<&'a Colormap>,
depth_meter: Option<&'a DepthMeter>,
}

// NOTE: Do not put profile scopes in these methods. They are called for all entities and all
Expand Down Expand Up @@ -466,6 +467,11 @@ impl ImageVisualizer {

if is_3d_view {
if let Some(parent_pinhole_path) = transforms.parent_pinhole(entity_path) {
let depth_meter = data
.depth_meter
.copied()
.unwrap_or_else(|| self.fallback_for(ctx));

// NOTE: we don't pass in `world_from_obj` because this corresponds to the
// transform of the projection plane, which is of no use to us here.
// What we want are the extrinsics of the depth camera!
Expand All @@ -480,6 +486,7 @@ impl ImageVisualizer {
entity_path,
parent_pinhole_path,
colormap,
depth_meter,
) {
Ok(cloud) => {
self.data.add_bounding_box(
Expand Down Expand Up @@ -555,6 +562,7 @@ impl ImageVisualizer {
ent_path: &EntityPath,
parent_pinhole_path: &EntityPath,
colormap: Colormap,
depth_meter: DepthMeter,
) -> anyhow::Result<DepthCloud> {
re_tracing::profile_function!();

Expand Down Expand Up @@ -602,9 +610,7 @@ impl ImageVisualizer {
Some(colormap),
)?;

let depth_from_world_scale = *properties.depth_from_world_scale;

let world_depth_from_texture_depth = 1.0 / depth_from_world_scale;
let world_depth_from_texture_depth = 1.0 / depth_meter.0;

// We want point radius to be defined in a scale where the radius of a point
// is a factor (`backproject_radius_scale`) of the diameter of a pixel projected
Expand Down Expand Up @@ -899,22 +905,27 @@ impl ImageVisualizer {
let colors = results.get_or_empty_dense(resolver)?;
let draw_orders = results.get_or_empty_dense(resolver)?;
let colormap = results.get_or_empty_dense(resolver)?;
let depth_meter = results.get_or_empty_dense(resolver)?;

let mut data = range_zip_1x3(
let mut data = range_zip_1x4(
tensors.range_indexed(),
draw_orders.range_indexed(),
colors.range_indexed(),
colormap.range_indexed(),
depth_meter.range_indexed(),
)
.filter_map(|(&index, tensors, draw_orders, colors, colormap)| {
tensors.first().map(|tensor| ImageComponentData {
index,
tensor,
color: colors.and_then(|colors| colors.first()),
draw_order: draw_orders.and_then(|draw_orders| draw_orders.first()),
colormap: colormap.and_then(|colormap| colormap.first()),
})
});
.filter_map(
|(&index, tensors, draw_orders, colors, colormap, depth_meter)| {
tensors.first().map(|tensor| ImageComponentData {
index,
tensor,
color: colors.and_then(|colors| colors.first()),
draw_order: draw_orders.and_then(|draw_orders| draw_orders.first()),
colormap: colormap.and_then(|colormap| colormap.first()),
depth_meter: depth_meter.and_then(|depth_meter| depth_meter.first()),
})
},
);

f(
self,
Expand Down Expand Up @@ -942,4 +953,15 @@ impl TypedComponentFallbackProvider<Colormap> for ImageVisualizer {
}
}

re_viewer_context::impl_component_fallback_provider!(ImageVisualizer => [Colormap]);
impl TypedComponentFallbackProvider<DepthMeter> for ImageVisualizer {
fn fallback_for(&self, ctx: &re_viewer_context::QueryContext<'_>) -> DepthMeter {
let is_integer_tensor = ctx
.recording()
.latest_at_component::<TensorData>(ctx.target_entity_path, ctx.query)
.map_or(false, |tensor| tensor.dtype().is_integer());

if is_integer_tensor { 1000.0 } else { 1.0 }.into()
}
}

re_viewer_context::impl_component_fallback_provider!(ImageVisualizer => [Colormap, DepthMeter]);

0 comments on commit 7865fef

Please sign in to comment.