From 9af6550bc5c3b919b78649d6a652b8b41d784676 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 15 Jun 2022 14:01:26 +0200 Subject: [PATCH 01/17] Split out Data::Batch to a new enum, LoggedData --- data_store/src/log_store.rs | 61 +++++++++++++-------------------- log_types/src/data.rs | 44 ++++++++++++++++++------ log_types/src/lib.rs | 57 ++++++++++++++++++++++++++++-- nyud/src/lib.rs | 6 ++-- viewer/src/misc/log_db.rs | 2 +- viewer/src/ui/context_panel.rs | 11 +++--- viewer/src/ui/log_table_view.rs | 4 +-- viewer/src/ui/space_view.rs | 17 +++++++-- 8 files changed, 138 insertions(+), 64 deletions(-) diff --git a/data_store/src/log_store.rs b/data_store/src/log_store.rs index 62843ec60214..9fef54583727 100644 --- a/data_store/src/log_store.rs +++ b/data_store/src/log_store.rs @@ -1,7 +1,8 @@ use nohash_hasher::IntMap; use log_types::{ - Data, DataMsg, DataPath, DataVec, IndexKey, ObjPath, ObjPathHash, TimeSource, TimeType, + Data, DataMsg, DataPath, DataVec, IndexKey, LoggedData, ObjPath, ObjPathHash, TimeSource, + TimeType, }; use crate::{Batch, TypePathDataStore}; @@ -58,33 +59,16 @@ impl LogDataStore { } = data_msg.data_path.clone(); match data_msg.data.clone() { - Data::Batch { indices, data } => { + LoggedData::Batch { indices, data } => { log_types::data_vec_map!(data, |vec| { let batch = batcher.batch(indices, vec); store.insert_batch(&op, fname, time, id, batch) }) } - - Data::I32(data) => store.insert_individual(op, fname, time, id, data), - Data::F32(data) => store.insert_individual(op, fname, time, id, data), - Data::Color(data) => store.insert_individual(op, fname, time, id, data), - Data::String(data) => store.insert_individual(op, fname, time, id, data), - - Data::Vec2(data) => store.insert_individual(op, fname, time, id, data), - Data::BBox2D(data) => store.insert_individual(op, fname, time, id, data), - Data::LineSegments2D(data) => store.insert_individual(op, fname, time, id, data), - Data::Image(data) => store.insert_individual(op, fname, time, id, data), - - Data::Vec3(data) => store.insert_individual(op, fname, time, id, data), - Data::Box3(data) => store.insert_individual(op, fname, time, id, data), - Data::Path3D(data) => store.insert_individual(op, fname, time, id, data), - Data::LineSegments3D(data) => store.insert_individual(op, fname, time, id, data), - Data::Mesh3D(data) => store.insert_individual(op, fname, time, id, data), - Data::Camera(data) => store.insert_individual(op, fname, time, id, data), - - Data::Vecf32(data) => store.insert_individual(op, fname, time, id, data), - - Data::Space(data) => store.insert_individual(op, fname, time, id, data), + LoggedData::Single(data) => { + log_types::data_map!(data, |data| store + .insert_individual(op, fname, time, id, data)) + } }?; } @@ -94,21 +78,24 @@ impl LogDataStore { fn register_obj_path(&mut self, data_msg: &DataMsg) { let obj_path = data_msg.data_path.obj_path(); - if let Data::Batch { indices, .. } = &data_msg.data { - for index_path_suffix in indices { - crate::profile_scope!("Register batch obj paths"); - let (obj_type_path, index_path_prefix) = - obj_path.clone().into_type_path_and_index_path(); - // TODO: speed this up. A lot. Please. - let mut index_path = index_path_prefix.clone(); - index_path.replace_last_placeholder_with(index_path_suffix.clone().into()); - let obj_path = ObjPath::new(obj_type_path.clone(), index_path); - self.obj_path_from_hash.insert(*obj_path.hash(), obj_path); + match &data_msg.data { + LoggedData::Batch { indices, .. } => { + for index_path_suffix in indices { + crate::profile_scope!("Register batch obj paths"); + let (obj_type_path, index_path_prefix) = + obj_path.clone().into_type_path_and_index_path(); + // TODO: speed this up. A lot. Please. + let mut index_path = index_path_prefix.clone(); + index_path.replace_last_placeholder_with(index_path_suffix.clone().into()); + let obj_path = ObjPath::new(obj_type_path.clone(), index_path); + self.obj_path_from_hash.insert(*obj_path.hash(), obj_path); + } + } + LoggedData::Single(_) => { + self.obj_path_from_hash + .entry(*obj_path.hash()) + .or_insert_with(|| obj_path.clone()); } - } else { - self.obj_path_from_hash - .entry(*obj_path.hash()) - .or_insert_with(|| obj_path.clone()); } } } diff --git a/log_types/src/data.rs b/log_types/src/data.rs index 0c17e93960d1..1cf230342bc8 100644 --- a/log_types/src/data.rs +++ b/log_types/src/data.rs @@ -1,4 +1,4 @@ -use crate::{impl_into_enum, Index, ObjPath}; +use crate::{impl_into_enum, ObjPath}; // ---------------------------------------------------------------------------- @@ -160,13 +160,6 @@ pub mod data_types { #[derive(Clone, Debug, PartialEq)] #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] pub enum Data { - Batch { - indices: Vec, - data: DataVec, - }, - - // ---------------------------- - // 1D: I32(i32), F32(f32), @@ -201,10 +194,8 @@ pub enum Data { impl Data { #[inline] - pub fn typ(&self) -> DataType { + pub fn data_type(&self) -> DataType { match self { - Self::Batch { data, .. } => data.data_type(), - Self::I32(_) => DataType::I32, Self::F32(_) => DataType::F32, Self::Color(_) => DataType::Color, @@ -267,6 +258,37 @@ pub enum DataVec { Space(Vec), } +/// Do the same thing with all members of a [`Data`]. +/// +/// ``` +/// # use log_types::{Data, data_map}; +/// # let data: Data = Data::F32(0.0); +/// data_map!(data, |data| dbg!(data)); +/// ``` +#[macro_export] +macro_rules! data_map( + ($data: expr, |$value: pat_param| $action: expr) => ({ + match $data { + Data::I32($value) => $action, + Data::F32($value) => $action, + Data::Color($value) => $action, + Data::String($value) => $action, + Data::Vec2($value) => $action, + Data::BBox2D($value) => $action, + Data::LineSegments2D($value) => $action, + Data::Image($value) => $action, + Data::Vec3($value) => $action, + Data::Box3($value) => $action, + Data::Path3D($value) => $action, + Data::LineSegments3D($value) => $action, + Data::Mesh3D($value) => $action, + Data::Camera($value) => $action, + Data::Vecf32($value) => $action, + Data::Space($value) => $action, + } + }); +); + /// Do the same thing with all members of a [`DataVec`]. /// /// ``` diff --git a/log_types/src/lib.rs b/log_types/src/lib.rs index 69ec22184b71..d93e0ec8b9b1 100644 --- a/log_types/src/lib.rs +++ b/log_types/src/lib.rs @@ -122,7 +122,7 @@ pub struct DataMsg { pub data_path: DataPath, /// The value of this. - pub data: Data, + pub data: LoggedData, } #[inline] @@ -130,7 +130,7 @@ pub fn data_msg( time_point: &TimePoint, obj_path: impl Into, field_name: impl Into, - data: impl Into, + data: impl Into, ) -> DataMsg { DataMsg { time_point: time_point.clone(), @@ -142,6 +142,59 @@ pub fn data_msg( // ---------------------------------------------------------------------------- +#[derive(Clone, Debug, PartialEq)] +#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] +pub enum LoggedData { + /// A single data value + Single(Data), + + /// Log multiple values at once. + /// + /// The index replaces the last index in [`DataMsg.data_path`], which should be [`Index::Placeholder]`. + Batch { indices: Vec, data: DataVec }, +} + +impl LoggedData { + #[inline] + pub fn data_type(&self) -> DataType { + match self { + Self::Single(data) => data.data_type(), + Self::Batch { data, .. } => data.data_type(), + } + } +} + +impl From for LoggedData { + #[inline] + fn from(data: Data) -> Self { + Self::Single(data) + } +} + +#[macro_export] +macro_rules! impl_into_logged_data { + ($from_ty: ty, $data_enum_variant: ident) => { + impl From<$from_ty> for LoggedData { + #[inline] + fn from(value: $from_ty) -> Self { + Self::Single(Data::$data_enum_variant(value)) + } + } + }; +} + +impl_into_logged_data!(i32, I32); +impl_into_logged_data!(f32, F32); +impl_into_logged_data!(BBox2D, BBox2D); +impl_into_logged_data!(Image, Image); +impl_into_logged_data!(Box3, Box3); +impl_into_logged_data!(Mesh3D, Mesh3D); +impl_into_logged_data!(Camera, Camera); +impl_into_logged_data!(Vec, Vecf32); +impl_into_logged_data!(ObjPath, Space); + +// ---------------------------------------------------------------------------- + rr_string_interner::declare_new_type!( /// The name of a time source. Often something like `"time"` or `"frame"`. pub struct TimeSource; diff --git a/nyud/src/lib.rs b/nyud/src/lib.rs index 500422f5a370..b4e738be2b6d 100644 --- a/nyud/src/lib.rs +++ b/nyud/src/lib.rs @@ -155,7 +155,7 @@ fn log_dataset_zip(path: &Path, logger: &Logger<'_>) -> anyhow::Result<()> { &time_point, &obj_path, "pos", - Data::Batch { + LoggedData::Batch { indices: indices.clone(), data: DataVec::Vec3(positions), }, @@ -166,7 +166,7 @@ fn log_dataset_zip(path: &Path, logger: &Logger<'_>) -> anyhow::Result<()> { &time_point, &obj_path, "space", - Data::Batch { + LoggedData::Batch { indices: indices.clone(), data: DataVec::Space(spaces), }, @@ -178,7 +178,7 @@ fn log_dataset_zip(path: &Path, logger: &Logger<'_>) -> anyhow::Result<()> { &time_point, &obj_path, "color", - Data::Batch { + LoggedData::Batch { indices, data: DataVec::Color(colors), }, diff --git a/viewer/src/misc/log_db.rs b/viewer/src/misc/log_db.rs index 4bf8830a0fe0..2dee6b0e021d 100644 --- a/viewer/src/misc/log_db.rs +++ b/viewer/src/misc/log_db.rs @@ -189,7 +189,7 @@ impl DataColumns { .insert(msg.id); self.per_type - .entry(msg.data.typ()) + .entry(msg.data.data_type()) .or_default() .insert(msg.id); } diff --git a/viewer/src/ui/context_panel.rs b/viewer/src/ui/context_panel.rs index 6d17e97c6a55..46e55ab19a2f 100644 --- a/viewer/src/ui/context_panel.rs +++ b/viewer/src/ui/context_panel.rs @@ -1,6 +1,7 @@ -use data_store::ObjPath; use itertools::Itertools; -use log_types::{Data, DataMsg, DataPath, LogId}; + +use data_store::ObjPath; +use log_types::{Data, DataMsg, DataPath, LogId, LoggedData}; use crate::{LogDb, Preview, Selection, ViewerContext}; @@ -166,7 +167,7 @@ pub(crate) fn show_detailed_data_msg( data, } = msg; - let is_image = matches!(msg.data, Data::Image(_)); + let is_image = matches!(msg.data, LoggedData::Single(Data::Image(_))); egui::Grid::new("fields") .striped(true) @@ -185,12 +186,12 @@ pub(crate) fn show_detailed_data_msg( if !is_image { ui.monospace("data:"); - crate::space_view::ui_data(context, ui, id, data, Preview::Medium); + crate::space_view::ui_logged_data(context, ui, id, data, Preview::Medium); ui.end_row(); } }); - if let Data::Image(image) = &msg.data { + if let LoggedData::Single(Data::Image(image)) = &msg.data { show_image(context, ui, id, image); } } diff --git a/viewer/src/ui/log_table_view.rs b/viewer/src/ui/log_table_view.rs index f195fe259a30..e70e7e922073 100644 --- a/viewer/src/ui/log_table_view.rs +++ b/viewer/src/ui/log_table_view.rs @@ -82,7 +82,7 @@ pub(crate) fn message_table( } fn row_height(msg: &DataMsg) -> f32 { - if matches!(&msg.data, log_types::Data::Image(_)) { + if matches!(msg.data.data_type(), DataType::Image) { 48.0 } else { 18.0 @@ -114,6 +114,6 @@ fn table_row( context.data_path_button(ui, data_path); }); row.col(|ui| { - crate::space_view::ui_data(context, ui, id, data, Preview::Specific(row_height)); + crate::space_view::ui_logged_data(context, ui, id, data, Preview::Specific(row_height)); }); } diff --git a/viewer/src/ui/space_view.rs b/viewer/src/ui/space_view.rs index 1a7a289ea706..24703b79e15e 100644 --- a/viewer/src/ui/space_view.rs +++ b/viewer/src/ui/space_view.rs @@ -330,7 +330,7 @@ pub(crate) fn show_log_msg( ui.end_row(); ui.monospace("data:"); - ui_data(context, ui, id, data, preview); + ui_logged_data(context, ui, id, data, preview); ui.end_row(); }); } @@ -351,6 +351,19 @@ pub(crate) fn ui_time_point( }); } +pub(crate) fn ui_logged_data( + context: &mut ViewerContext, + ui: &mut egui::Ui, + id: &LogId, + data: &LoggedData, + preview: Preview, +) -> egui::Response { + match data { + LoggedData::Batch { data, .. } => ui.label(format!("batch: {:?}", data)), + LoggedData::Single(data) => ui_data(context, ui, id, data, preview), + } +} + pub(crate) fn ui_data( context: &mut ViewerContext, ui: &mut egui::Ui, @@ -359,8 +372,6 @@ pub(crate) fn ui_data( preview: Preview, ) -> egui::Response { match data { - Data::Batch { data, .. } => ui.label(format!("batch: {:?}", data)), - Data::I32(value) => ui.label(value.to_string()), Data::F32(value) => ui.label(value.to_string()), Data::Color([r, g, b, a]) => { From 21a975e53d7e7cc87ec5b95033723abe0f1937a5 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 15 Jun 2022 14:35:31 +0200 Subject: [PATCH 02/17] Add "batch splatting" --- data_store/src/log_store.rs | 21 ++-- data_store/src/obj_path_query.rs | 12 +++ data_store/src/storage.rs | 161 ++++++++++++++++++++++++++++-- data_store/src/type_path_query.rs | 12 +++ log_types/src/lib.rs | 10 +- nyud/src/lib.rs | 53 +++++----- viewer/src/ui/space_view.rs | 7 ++ 7 files changed, 233 insertions(+), 43 deletions(-) diff --git a/data_store/src/log_store.rs b/data_store/src/log_store.rs index 9fef54583727..b66feda55444 100644 --- a/data_store/src/log_store.rs +++ b/data_store/src/log_store.rs @@ -59,15 +59,19 @@ impl LogDataStore { } = data_msg.data_path.clone(); match data_msg.data.clone() { + LoggedData::Single(data) => { + log_types::data_map!(data, |data| store + .insert_individual(op, fname, time, id, data)) + } LoggedData::Batch { indices, data } => { log_types::data_vec_map!(data, |vec| { let batch = batcher.batch(indices, vec); store.insert_batch(&op, fname, time, id, batch) }) } - LoggedData::Single(data) => { + LoggedData::BatchSplat(data) => { log_types::data_map!(data, |data| store - .insert_individual(op, fname, time, id, data)) + .insert_batch_splat(op, fname, time, id, data)) } }?; } @@ -79,6 +83,11 @@ impl LogDataStore { let obj_path = data_msg.data_path.obj_path(); match &data_msg.data { + LoggedData::Single(_) => { + self.obj_path_from_hash + .entry(*obj_path.hash()) + .or_insert_with(|| obj_path.clone()); + } LoggedData::Batch { indices, .. } => { for index_path_suffix in indices { crate::profile_scope!("Register batch obj paths"); @@ -91,10 +100,10 @@ impl LogDataStore { self.obj_path_from_hash.insert(*obj_path.hash(), obj_path); } } - LoggedData::Single(_) => { - self.obj_path_from_hash - .entry(*obj_path.hash()) - .or_insert_with(|| obj_path.clone()); + LoggedData::BatchSplat(_) => { + // We don't have a way to know which indices might be used, but that fine. + // BatchSplat may not be used on primary fields (e.g. `pos`) anyway, + // so all indices that will actually be used will be handled by the primary fields. } } } diff --git a/data_store/src/obj_path_query.rs b/data_store/src/obj_path_query.rs index e9a29de51a00..d9c99f7187ae 100644 --- a/data_store/src/obj_path_query.rs +++ b/data_store/src/obj_path_query.rs @@ -48,6 +48,9 @@ pub fn visit_obj_data<'s, Time: 'static + Copy + Ord, T: DataTrait>( ); } } + DataStore::BatchSplat(_) => { + tracing::error!("Used BatchSplat for a primary field {field_name:?}"); + } } } @@ -112,6 +115,9 @@ pub fn visit_obj_data_1<'s, Time: 'static + Copy + Ord, T: DataTrait, S1: DataTr ); } } + DataStore::BatchSplat(_) => { + tracing::error!("Used BatchSplat for a primary field {field_name:?}"); + } } } @@ -189,6 +195,9 @@ pub fn visit_obj_data_2< ); } } + DataStore::BatchSplat(_) => { + tracing::error!("Used BatchSplat for a primary field {field_name:?}"); + } } } @@ -274,6 +283,9 @@ pub fn visit_obj_data_3< ); } } + DataStore::BatchSplat(_) => { + tracing::error!("Used BatchSplat for a primary field {field_name:?}"); + } } } diff --git a/data_store/src/storage.rs b/data_store/src/storage.rs index dd919745ae4c..ce731f077141 100644 --- a/data_store/src/storage.rs +++ b/data_store/src/storage.rs @@ -12,11 +12,8 @@ use crate::{ObjTypePath, TimeQuery}; #[derive(Clone, Copy, Debug)] pub enum Error { - /// First stored as a batch, then individually. Not supported. - BatchFollowedByIndividual, - - /// First stored individually, then followed by a batch. Not supported. - IndividualFollowedByBatch, + /// The same type batch must be either individual, batch, or a batch splat. + MixingIndividualBatchesOrSplats, /// One type was first logged, then another. WrongType, @@ -63,7 +60,7 @@ impl TypePathDataStore