From 011b35716439af92e93bf11e07faec82488921b4 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Tue, 28 Feb 2023 14:46:38 +0100 Subject: [PATCH 1/2] heuristic for camera frustum length is now based on scene size Introduces `EditableAutoValue` and per-frame heuristic updates on properties. In the future we can build more properties on top of this and update them in a central place like done here --- .../re_data_store/src/editable_auto_value.rs | 43 +++++++++++++++++++ crates/re_data_store/src/entity_properties.rs | 35 +++------------ crates/re_data_store/src/lib.rs | 2 + crates/re_viewer/src/misc/transform_cache.rs | 6 +-- crates/re_viewer/src/ui/selection_panel.rs | 12 ++++-- crates/re_viewer/src/ui/space_view.rs | 3 ++ .../view_spatial/scene/scene_part/cameras.rs | 2 +- crates/re_viewer/src/ui/view_spatial/ui.rs | 42 +++++++++++++++++- 8 files changed, 106 insertions(+), 39 deletions(-) create mode 100644 crates/re_data_store/src/editable_auto_value.rs diff --git a/crates/re_data_store/src/editable_auto_value.rs b/crates/re_data_store/src/editable_auto_value.rs new file mode 100644 index 000000000000..c41409227894 --- /dev/null +++ b/crates/re_data_store/src/editable_auto_value.rs @@ -0,0 +1,43 @@ +/// A value that is either determined automatically by some heuristic, or specified by the user. +#[derive(Debug, Clone, serde::Deserialize, serde::Serialize, Eq, PartialEq)] +#[serde(bound = "T: serde::Serialize, for<'de2> T: serde::Deserialize<'de2>")] +pub enum EditableAutoValue +where + T: std::fmt::Debug + Eq + PartialEq + Clone + Default + serde::Serialize, + for<'de2> T: serde::Deserialize<'de2>, +{ + /// The user explicitly specified what they wanted + UserEdited(T), + + /// The value is determined automatically. + /// + /// We may update this at any time or interpret the value stored here differently under certain circumstances. + Auto(T), +} + +impl Default for EditableAutoValue +where + T: std::fmt::Debug + Eq + PartialEq + Clone + Default + serde::Serialize, + for<'de2> T: serde::Deserialize<'de2>, +{ + fn default() -> Self { + EditableAutoValue::Auto(T::default()) + } +} + +impl EditableAutoValue +where + T: std::fmt::Debug + Eq + PartialEq + Clone + Default + serde::Serialize, + for<'de2> T: serde::Deserialize<'de2>, +{ + pub fn is_auto(&self) -> bool { + matches!(self, EditableAutoValue::Auto(_)) + } + + /// Gets the value, disregarding if it was user edited or determined by a heuristic. + pub fn get(&self) -> &T { + match self { + EditableAutoValue::Auto(v) | EditableAutoValue::UserEdited(v) => v, + } + } +} diff --git a/crates/re_data_store/src/entity_properties.rs b/crates/re_data_store/src/entity_properties.rs index 606f97d1dd4c..2141236a0a54 100644 --- a/crates/re_data_store/src/entity_properties.rs +++ b/crates/re_data_store/src/entity_properties.rs @@ -4,7 +4,7 @@ use re_log_types::{ msg_bundle::DeserializableComponent, EntityPath, }; -use crate::log_db::EntityDb; +use crate::{log_db::EntityDb, EditableAutoValue}; // ---------------------------------------------------------------------------- @@ -39,44 +39,21 @@ pub struct EntityProperties { pub visible_history: ExtraQueryHistory, pub interactive: bool, - /// Distance of the projection plane. + /// Distance of the projection plane (frustum far plane). /// /// Only applies to pinhole cameras when in a spatial view, using 3D navigation. - pinhole_image_plane_distance: Option>, + pub pinhole_image_plane_distance: EditableAutoValue>, } impl EntityProperties { - /// If this has a pinhole camera transform, how far away is the image plane. - /// - /// Scale relative to the respective space the pinhole camera is in. - /// None indicates the user never edited this field (should use a meaningful default then). - /// - /// Method returns a pinhole camera specific default if the value hasn't been set yet. - pub fn pinhole_image_plane_distance(&self, pinhole: &re_log_types::Pinhole) -> f32 { - self.pinhole_image_plane_distance - .unwrap_or_else(|| { - let distance = pinhole - .focal_length() - .unwrap_or_else(|| pinhole.focal_length_in_pixels().y()); - ordered_float::NotNan::new(distance).unwrap_or_default() - }) - .into() - } - - /// see `pinhole_image_plane_distance()` - pub fn set_pinhole_image_plane_distance(&mut self, distance: f32) { - self.pinhole_image_plane_distance = ordered_float::NotNan::new(distance).ok(); - } - /// Multiply/and these together. pub fn with_child(&self, child: &Self) -> Self { Self { visible: self.visible && child.visible, visible_history: self.visible_history.with_child(&child.visible_history), interactive: self.interactive && child.interactive, - pinhole_image_plane_distance: child - .pinhole_image_plane_distance - .or(self.pinhole_image_plane_distance), + // TODO(andreas): Inheriting doesn't work properly since parents don't set this value usually + pinhole_image_plane_distance: child.pinhole_image_plane_distance.clone(), } } } @@ -87,7 +64,7 @@ impl Default for EntityProperties { visible: true, visible_history: ExtraQueryHistory::default(), interactive: true, - pinhole_image_plane_distance: None, + pinhole_image_plane_distance: EditableAutoValue::default(), } } } diff --git a/crates/re_data_store/src/lib.rs b/crates/re_data_store/src/lib.rs index cf7ebd6667c5..e31943e08411 100644 --- a/crates/re_data_store/src/lib.rs +++ b/crates/re_data_store/src/lib.rs @@ -4,6 +4,7 @@ #![doc = document_features::document_features!()] //! +mod editable_auto_value; pub mod entity_properties; pub mod entity_tree; mod instance_path; @@ -16,6 +17,7 @@ pub use log_db::LogDb; use re_log_types::msg_bundle; +pub use editable_auto_value::EditableAutoValue; pub use re_log_types::{ComponentName, EntityPath, EntityPathPart, Index, TimeInt, Timeline}; // ---------------------------------------------------------------------------- diff --git a/crates/re_viewer/src/misc/transform_cache.rs b/crates/re_viewer/src/misc/transform_cache.rs index 87d4925b9e31..8b83e84104d3 100644 --- a/crates/re_viewer/src/misc/transform_cache.rs +++ b/crates/re_viewer/src/misc/transform_cache.rs @@ -239,10 +239,8 @@ fn transform_at( // A pinhole camera means that we're looking at an image. // Images are spanned in their local x/y space. // Center it and move it along z, scaling the further we move. - - let distance = entity_properties - .get(entity_path) - .pinhole_image_plane_distance(&pinhole); + let props = entity_properties.get(entity_path); + let distance: f32 = (*props.pinhole_image_plane_distance.get()).into(); let focal_length = pinhole.focal_length_in_pixels(); let focal_length = glam::vec2(focal_length.x(), focal_length.y()); diff --git a/crates/re_viewer/src/ui/selection_panel.rs b/crates/re_viewer/src/ui/selection_panel.rs index aef84d37d014..62d094414cc1 100644 --- a/crates/re_viewer/src/ui/selection_panel.rs +++ b/crates/re_viewer/src/ui/selection_panel.rs @@ -1,4 +1,4 @@ -use re_data_store::{query_latest_single, EntityPath, EntityProperties}; +use re_data_store::{query_latest_single, EditableAutoValue, EntityPath, EntityProperties}; use re_log_types::{TimeType, Transform}; use crate::{ @@ -411,11 +411,12 @@ fn entity_props_ui( if view_state.state_spatial.nav_mode == SpatialNavigationMode::ThreeD { if let Some(entity_path) = entity_path { let query = ctx.current_query(); - if let Some(re_log_types::Transform::Pinhole(pinhole)) = + if let Some(re_log_types::Transform::Pinhole(_)) = query_latest_single::(&ctx.log_db.entity_db, entity_path, &query) { ui.label("Image plane distance"); - let mut distance = entity_props.pinhole_image_plane_distance(&pinhole); + let mut distance: f32 = + (*entity_props.pinhole_image_plane_distance.get()).into(); let speed = (distance * 0.05).at_least(0.01); if ui .add( @@ -426,7 +427,10 @@ fn entity_props_ui( .on_hover_text("Controls how far away the image plane is.") .changed() { - entity_props.set_pinhole_image_plane_distance(distance); + entity_props.pinhole_image_plane_distance = + EditableAutoValue::UserEdited( + ordered_float::NotNan::new(distance).unwrap(), + ); } ui.end_row(); } diff --git a/crates/re_viewer/src/ui/space_view.rs b/crates/re_viewer/src/ui/space_view.rs index 3e9c7d693ff1..c24689c34782 100644 --- a/crates/re_viewer/src/ui/space_view.rs +++ b/crates/re_viewer/src/ui/space_view.rs @@ -178,6 +178,9 @@ impl SpaceView { ); let mut scene = view_spatial::SceneSpatial::new(ctx.render_ctx); scene.load(ctx, &query, &transforms, highlights); + self.view_state + .state_spatial + .update_object_property_heuristics(ctx, &mut self.data_blueprint); self.view_state .ui_spatial(ctx, ui, &self.space_path, scene, self.id, highlights); } diff --git a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/cameras.rs b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/cameras.rs index bdccfc79b0fc..0d4edd620af4 100644 --- a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/cameras.rs +++ b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/cameras.rs @@ -86,7 +86,7 @@ impl CamerasPart { return; }; - let frustum_length = props.pinhole_image_plane_distance(&pinhole); + let frustum_length: f32 = (*props.pinhole_image_plane_distance.get()).into(); scene.space_cameras.push(SpaceCamera3D { entity_path: entity_path.clone(), diff --git a/crates/re_viewer/src/ui/view_spatial/ui.rs b/crates/re_viewer/src/ui/view_spatial/ui.rs index 81d5678e6f91..71cb6070858f 100644 --- a/crates/re_viewer/src/ui/view_spatial/ui.rs +++ b/crates/re_viewer/src/ui/view_spatial/ui.rs @@ -1,4 +1,4 @@ -use re_data_store::EntityPath; +use re_data_store::{query_latest_single, EditableAutoValue, EntityPath}; use re_format::format_f32; use egui::{NumExt, WidgetText}; @@ -147,6 +147,46 @@ impl ViewSpatialState { heuristic0.min(heuristic1) } + pub fn update_object_property_heuristics( + &self, + ctx: &mut ViewerContext<'_>, + data_blueprint: &mut DataBlueprintTree, + ) { + crate::profile_function!(); + + let scene_size = self.scene_bbox_accum.size().length(); + let default_image_plane_distance = if scene_size.is_finite() && scene_size > 0.0 { + scene_size * 0.05 + } else { + 1.0 + }; + + let query = ctx.current_query(); + + let entity_paths = data_blueprint.entity_paths().clone(); // TODO(andreas): Workaround borrow checker + for entity_path in entity_paths { + if let Some(re_log_types::Transform::Pinhole(_)) = + query_latest_single::( + &ctx.log_db.entity_db, + &entity_path, + &query, + ) + { + let mut properties = data_blueprint + .data_blueprints_individual() + .get(&entity_path); + if properties.pinhole_image_plane_distance.is_auto() { + properties.pinhole_image_plane_distance = EditableAutoValue::Auto( + ordered_float::NotNan::new(default_image_plane_distance).unwrap(), + ); + data_blueprint + .data_blueprints_individual() + .set(entity_path, properties); + } + } + } + } + pub fn selection_ui( &mut self, ctx: &mut ViewerContext<'_>, From 8e2be745008cbe420693a219c2adb832a3b4d774 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Tue, 28 Feb 2023 16:07:40 +0100 Subject: [PATCH 2/2] review feedback --- crates/re_data_store/src/editable_auto_value.rs | 9 +++++++++ crates/re_data_store/src/entity_properties.rs | 6 ++++-- crates/re_viewer/src/misc/transform_cache.rs | 2 +- crates/re_viewer/src/ui/selection_panel.rs | 2 +- .../src/ui/view_spatial/scene/scene_part/cameras.rs | 2 +- 5 files changed, 16 insertions(+), 5 deletions(-) diff --git a/crates/re_data_store/src/editable_auto_value.rs b/crates/re_data_store/src/editable_auto_value.rs index c41409227894..f3a798268ad2 100644 --- a/crates/re_data_store/src/editable_auto_value.rs +++ b/crates/re_data_store/src/editable_auto_value.rs @@ -40,4 +40,13 @@ where EditableAutoValue::Auto(v) | EditableAutoValue::UserEdited(v) => v, } } + + /// Returns other if self is auto, self otherwise. + pub fn or<'a>(&'a self, other: &'a EditableAutoValue) -> &'a EditableAutoValue { + if self.is_auto() { + other + } else { + self + } + } } diff --git a/crates/re_data_store/src/entity_properties.rs b/crates/re_data_store/src/entity_properties.rs index 2141236a0a54..c470bca755e2 100644 --- a/crates/re_data_store/src/entity_properties.rs +++ b/crates/re_data_store/src/entity_properties.rs @@ -52,8 +52,10 @@ impl EntityProperties { visible: self.visible && child.visible, visible_history: self.visible_history.with_child(&child.visible_history), interactive: self.interactive && child.interactive, - // TODO(andreas): Inheriting doesn't work properly since parents don't set this value usually - pinhole_image_plane_distance: child.pinhole_image_plane_distance.clone(), + pinhole_image_plane_distance: self + .pinhole_image_plane_distance + .or(&child.pinhole_image_plane_distance) + .clone(), } } } diff --git a/crates/re_viewer/src/misc/transform_cache.rs b/crates/re_viewer/src/misc/transform_cache.rs index 8b83e84104d3..1a87ea3686cb 100644 --- a/crates/re_viewer/src/misc/transform_cache.rs +++ b/crates/re_viewer/src/misc/transform_cache.rs @@ -240,7 +240,7 @@ fn transform_at( // Images are spanned in their local x/y space. // Center it and move it along z, scaling the further we move. let props = entity_properties.get(entity_path); - let distance: f32 = (*props.pinhole_image_plane_distance.get()).into(); + let distance: f32 = props.pinhole_image_plane_distance.get().into_inner(); let focal_length = pinhole.focal_length_in_pixels(); let focal_length = glam::vec2(focal_length.x(), focal_length.y()); diff --git a/crates/re_viewer/src/ui/selection_panel.rs b/crates/re_viewer/src/ui/selection_panel.rs index 62d094414cc1..7f77c6a9fcf2 100644 --- a/crates/re_viewer/src/ui/selection_panel.rs +++ b/crates/re_viewer/src/ui/selection_panel.rs @@ -416,7 +416,7 @@ fn entity_props_ui( { ui.label("Image plane distance"); let mut distance: f32 = - (*entity_props.pinhole_image_plane_distance.get()).into(); + entity_props.pinhole_image_plane_distance.get().into_inner(); let speed = (distance * 0.05).at_least(0.01); if ui .add( diff --git a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/cameras.rs b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/cameras.rs index 0d4edd620af4..35475d872819 100644 --- a/crates/re_viewer/src/ui/view_spatial/scene/scene_part/cameras.rs +++ b/crates/re_viewer/src/ui/view_spatial/scene/scene_part/cameras.rs @@ -86,7 +86,7 @@ impl CamerasPart { return; }; - let frustum_length: f32 = (*props.pinhole_image_plane_distance.get()).into(); + let frustum_length: f32 = props.pinhole_image_plane_distance.get().into_inner(); scene.space_cameras.push(SpaceCamera3D { entity_path: entity_path.clone(),