From e4e1ae7adc0a6f952971b209371332a2efb688a0 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 25 Sep 2023 11:51:21 +0200 Subject: [PATCH] Click `recording://entity/path` links in markdown (#3442) ### What You can now embed links to entities in markdown documents using `recording://entity/path`, and link to components using `recording://entity/path.Color`. In order to support this, a new `DataPath` is introduced, with a stricter parsing of entity paths. Previously `foo bar` was a valid path, but now you are only allowed ASCII, numbers, underscore and dash as names (outside of quotes). A (uncompleted) description of the SfM example as markdown to - Test how that feels as documentation - Try out the ability to link directly to entities and components from within the markdown ![image](https://github.com/rerun-io/rerun/assets/1148717/3883afe7-4c7b-42da-995d-3214fc773b52) ![image](https://github.com/rerun-io/rerun/assets/1148717/f55ddd85-297f-44ff-9dde-a0309931ade2) ### Checklist * [x] I have read and agree to [Contributor Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and the [Code of Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md) * [x] I've included a screenshot or gif (if applicable) * [x] I have tested [demo.rerun.io](https://demo.rerun.io/pr/3389) (if applicable) - [PR Build Summary](https://build.rerun.io/pr/3389) - [Docs preview](https://rerun.io/preview/e14be611c235231fa3e91a7f106cf594bf245e51/docs) - [Examples preview](https://rerun.io/preview/e14be611c235231fa3e91a7f106cf594bf245e51/examples) - [Recent benchmark results](https://ref.rerun.io/dev/bench/) - [Wasm size tracking](https://ref.rerun.io/dev/sizes/) --------- Co-authored-by: Nikolaus West --- Cargo.lock | 2 +- Cargo.toml | 2 +- crates/re_data_store/src/instance_path.rs | 45 +- crates/re_data_store/src/store_db.rs | 5 + crates/re_data_ui/src/component_path.rs | 27 +- .../re_data_ui/src/component_ui_registry.rs | 72 ++- crates/re_data_ui/src/instance_path.rs | 39 +- crates/re_log_types/src/index.rs | 4 - crates/re_log_types/src/path/data_path.rs | 42 ++ crates/re_log_types/src/path/entity_path.rs | 17 +- .../re_log_types/src/path/entity_path_impl.rs | 5 + crates/re_log_types/src/path/mod.rs | 38 +- crates/re_log_types/src/path/parse_path.rs | 461 +++++++++++++++++- .../src/space_view_class.rs | 8 + crates/re_types/source_hash.txt | 2 +- crates/re_viewer/src/app_state.rs | 37 +- crates/re_viewer_context/src/item.rs | 66 ++- crates/rerun_c/src/lib.rs | 20 +- docs/code-examples/text_document.cpp | 47 +- docs/code-examples/text_document.py | 48 +- docs/code-examples/text_document.rs | 49 +- examples/python/structure_from_motion/main.py | 30 ++ examples/python/text_logging/main.py | 2 +- .../rerun/datatypes/tensor_buffer.py | 2 +- rerun_py/src/python_bridge.rs | 5 +- .../test_types/datatypes/affix_fuzzer3.py | 2 +- scripts/lint.py | 4 +- tests/python/roundtrips/text_document/main.py | 2 +- tests/python/test_api/main.py | 4 +- tests/rust/test_api/src/main.rs | 2 +- 30 files changed, 969 insertions(+), 120 deletions(-) create mode 100644 crates/re_log_types/src/path/data_path.rs diff --git a/Cargo.lock b/Cargo.lock index 94fa7e694e65..d1639f4df674 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1456,7 +1456,7 @@ dependencies = [ [[package]] name = "egui_commonmark" version = "0.7.4" -source = "git+https://github.com/lampsitter/egui_commonmark.git?rev=a133564f26a95672e756079ac5583817e0cdaa1f#a133564f26a95672e756079ac5583817e0cdaa1f" +source = "git+https://github.com/lampsitter/egui_commonmark.git?rev=a4470c253b06a4a350e17418a7c6cbc413ade644#a4470c253b06a4a350e17418a7c6cbc413ade644" dependencies = [ "egui", "egui_extras", diff --git a/Cargo.toml b/Cargo.toml index 6680fbbc65c0..08c1c8050955 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -179,7 +179,7 @@ emath = { git = "https://github.com/emilk/egui", rev = "d949eaf" } epaint = { git = "https://github.com/emilk/egui", rev = "d949eaf" } # Temporary patch until next egui_commonmark release -egui_commonmark = { git = "https://github.com/lampsitter/egui_commonmark.git", rev = "a133564f26a95672e756079ac5583817e0cdaa1f" } +egui_commonmark = { git = "https://github.com/lampsitter/egui_commonmark.git", rev = "a4470c253b06a4a350e17418a7c6cbc413ade644" } # Temporary patch until next egui_tiles release egui_tiles = { git = "https://github.com/rerun-io/egui_tiles", rev = "c66d6cba7ddb5b236be614d1816be4561260274e" } diff --git a/crates/re_data_store/src/instance_path.rs b/crates/re_data_store/src/instance_path.rs index 4cec42900773..c0453bb3af7a 100644 --- a/crates/re_data_store/src/instance_path.rs +++ b/crates/re_data_store/src/instance_path.rs @@ -1,6 +1,6 @@ -use std::hash::Hash; +use std::{hash::Hash, str::FromStr}; -use re_log_types::{EntityPath, EntityPathHash, RowId}; +use re_log_types::{DataPath, EntityPath, EntityPathHash, PathParseError, RowId}; use re_types::components::InstanceKey; use crate::{store_db::EntityDb, VersionedInstancePath, VersionedInstancePathHash}; @@ -19,6 +19,13 @@ pub struct InstancePath { pub instance_key: InstanceKey, } +impl From for InstancePath { + #[inline] + fn from(entity_path: EntityPath) -> Self { + Self::entity_splat(entity_path) + } +} + impl InstancePath { /// Indicate the whole entity (all instances of it) - i.e. a splat. /// @@ -77,6 +84,40 @@ impl std::fmt::Display for InstancePath { } } +impl FromStr for InstancePath { + type Err = PathParseError; + + fn from_str(s: &str) -> Result { + let DataPath { + entity_path, + instance_key, + component_name, + } = DataPath::from_str(s)?; + + if let Some(component_name) = component_name { + return Err(PathParseError::UnexpectedComponentName(component_name)); + } + + let instance_key = instance_key.unwrap_or(InstanceKey::SPLAT); + + Ok(InstancePath { + entity_path, + instance_key, + }) + } +} + +#[test] +fn test_parse_instance_path() { + assert_eq!( + InstancePath::from_str("world/points[#123]"), + Ok(InstancePath { + entity_path: EntityPath::from_str("world/points").unwrap(), + instance_key: InstanceKey(123) + }) + ); +} + // ---------------------------------------------------------------------------- /// Hashes of the components of an [`InstancePath`]. diff --git a/crates/re_data_store/src/store_db.rs b/crates/re_data_store/src/store_db.rs index dbfc678a6e9c..832467402494 100644 --- a/crates/re_data_store/src/store_db.rs +++ b/crates/re_data_store/src/store_db.rs @@ -55,6 +55,11 @@ impl EntityDb { self.entity_path_from_hash.get(entity_path_hash) } + #[inline] + pub fn knows_of_entity(&self, entity_path: &EntityPath) -> bool { + self.entity_path_from_hash.contains_key(&entity_path.hash()) + } + fn register_entity_path(&mut self, entity_path: &EntityPath) { self.entity_path_from_hash .entry(entity_path.hash()) diff --git a/crates/re_data_ui/src/component_path.rs b/crates/re_data_ui/src/component_path.rs index c9ba8e8395ac..fc67e056f929 100644 --- a/crates/re_data_ui/src/component_path.rs +++ b/crates/re_data_ui/src/component_path.rs @@ -11,21 +11,34 @@ impl DataUi for ComponentPath { verbosity: UiVerbosity, query: &re_arrow_store::LatestAtQuery, ) { + let Self { + entity_path, + component_name, + } = self; + let store = &ctx.store_db.entity_db.data_store; - if let Some((_, component_data)) = re_query::get_component_with_instances( - store, - query, - self.entity_path(), - self.component_name, - ) { + if let Some((_, component_data)) = + re_query::get_component_with_instances(store, query, entity_path, *component_name) + { super::component::EntityComponentWithInstances { entity_path: self.entity_path.clone(), component_data, } .data_ui(ctx, ui, verbosity, query); + } else if let Some(entity_tree) = ctx.store_db.entity_db.tree.subtree(entity_path) { + if entity_tree.components.contains_key(component_name) { + ui.label(""); + } else { + ui.label(format!( + "Entity {entity_path:?} has no component {component_name:?}" + )); + } } else { - ui.label(""); + ui.label( + ctx.re_ui + .error_text(format!("Unknown entity: {entity_path:?}")), + ); } } } diff --git a/crates/re_data_ui/src/component_ui_registry.rs b/crates/re_data_ui/src/component_ui_registry.rs index ef07e7a21e3e..0fe125351c3b 100644 --- a/crates/re_data_ui/src/component_ui_registry.rs +++ b/crates/re_data_ui/src/component_ui_registry.rs @@ -1,6 +1,7 @@ use re_arrow_store::LatestAtQuery; use re_log_types::{external::arrow2, EntityPath}; use re_query::ComponentWithInstances; +use re_types::external::arrow2::array::Utf8Array; use re_viewer_context::{ComponentUiRegistry, UiVerbosity, ViewerContext}; use super::EntityDataUi; @@ -54,7 +55,7 @@ pub fn create_component_ui_registry() -> ComponentUiRegistry { fn fallback_component_ui( _ctx: &mut ViewerContext<'_>, ui: &mut egui::Ui, - _verbosity: UiVerbosity, + verbosity: UiVerbosity, _query: &LatestAtQuery, _entity_path: &EntityPath, component: &ComponentWithInstances, @@ -62,25 +63,80 @@ fn fallback_component_ui( ) { // No special ui implementation - use a generic one: if let Some(value) = component.lookup_arrow(instance_key) { - ui.label(format_arrow(&*value)); + arrow_ui(ui, verbosity, &*value); } else { ui.weak("(null)"); } } -fn format_arrow(value: &dyn arrow2::array::Array) -> String { +fn arrow_ui(ui: &mut egui::Ui, verbosity: UiVerbosity, array: &dyn arrow2::array::Array) { use re_log_types::SizeBytes as _; - let bytes = value.total_size_bytes(); - if bytes < 256 { + // Special-treat text. + // Note: we match on the raw data here, so this works for any component containing text. + if let Some(utf8) = array.as_any().downcast_ref::>() { + if utf8.len() == 1 { + let string = utf8.value(0); + text_ui(string, ui, verbosity); + return; + } + } + if let Some(utf8) = array.as_any().downcast_ref::>() { + if utf8.len() == 1 { + let string = utf8.value(0); + text_ui(string, ui, verbosity); + return; + } + } + + let num_bytes = array.total_size_bytes(); + if num_bytes < 256 { // Print small items: let mut string = String::new(); - let display = arrow2::array::get_display(value, "null"); + let display = arrow2::array::get_display(array, "null"); if display(&mut string, 0).is_ok() { - return string; + ui.label(string); + return; } } // Fallback: - format!("{bytes} bytes") + ui.label(format!( + "{} of {:?}", + re_format::format_bytes(num_bytes as _), + array.data_type() + )); +} + +fn text_ui(string: &str, ui: &mut egui::Ui, verbosity: UiVerbosity) { + let font_id = egui::TextStyle::Monospace.resolve(ui.style()); + let color = ui.visuals().text_color(); + let wrap_width = ui.available_width(); + let mut layout_job = + egui::text::LayoutJob::simple(string.to_owned(), font_id, color, wrap_width); + + let mut needs_scroll_area = false; + + match verbosity { + UiVerbosity::Small => { + // Elide + layout_job.wrap.max_rows = 1; + layout_job.wrap.break_anywhere = true; + } + UiVerbosity::Reduced => { + layout_job.wrap.max_rows = 3; + } + UiVerbosity::All => { + let num_newlines = string.chars().filter(|&c| c == '\n').count(); + needs_scroll_area = 10 < num_newlines || 300 < string.len(); + } + } + + if needs_scroll_area { + egui::ScrollArea::vertical().show(ui, |ui| { + ui.label(layout_job); + }); + } else { + ui.label(layout_job); + } } diff --git a/crates/re_data_ui/src/instance_path.rs b/crates/re_data_ui/src/instance_path.rs index 7656aac26574..52afbd0aa7db 100644 --- a/crates/re_data_ui/src/instance_path.rs +++ b/crates/re_data_ui/src/instance_path.rs @@ -17,10 +17,26 @@ impl DataUi for InstancePath { verbosity: UiVerbosity, query: &re_arrow_store::LatestAtQuery, ) { + let Self { + entity_path, + instance_key, + } = self; + let store = &ctx.store_db.entity_db.data_store; - let Some(mut components) = store.all_components(&query.timeline, &self.entity_path) else { - ui.label(format!("No components in entity {}", self.entity_path)); + let Some(mut components) = store.all_components(&query.timeline, entity_path) else { + if ctx.store_db.entity_db.knows_of_entity(entity_path) { + ui.label(format!( + "No components in entity {:?} on timeline {:?}", + entity_path, + query.timeline.name() + )); + } else { + ui.label( + ctx.re_ui + .error_text(format!("Unknown entity: {entity_path:?}")), + ); + } return; }; components.sort(); @@ -29,12 +45,9 @@ impl DataUi for InstancePath { .num_columns(2) .show(ui, |ui| { for component_name in components { - let Some((_, component_data)) = get_component_with_instances( - store, - query, - &self.entity_path, - component_name, - ) else { + let Some((_, component_data)) = + get_component_with_instances(store, query, entity_path, component_name) + else { continue; // no need to show components that are unset at this point in time }; @@ -56,12 +69,12 @@ impl DataUi for InstancePath { item_ui::component_path_button( ctx, ui, - &ComponentPath::new(self.entity_path.clone(), component_name), + &ComponentPath::new(entity_path.clone(), component_name), ); - if self.instance_key.is_splat() { + if instance_key.is_splat() { super::component::EntityComponentWithInstances { - entity_path: self.entity_path.clone(), + entity_path: entity_path.clone(), component_data, } .data_ui(ctx, ui, UiVerbosity::Small, query); @@ -71,9 +84,9 @@ impl DataUi for InstancePath { ui, UiVerbosity::Small, query, - &self.entity_path, + entity_path, &component_data, - &self.instance_key, + instance_key, ); } diff --git a/crates/re_log_types/src/index.rs b/crates/re_log_types/src/index.rs index e1d4ae376ff5..dcb60450726c 100644 --- a/crates/re_log_types/src/index.rs +++ b/crates/re_log_types/src/index.rs @@ -9,9 +9,6 @@ pub enum Index { /// For arrays, assumed to be dense (0, 1, 2, …). Sequence(u64), - /// X,Y pixel coordinates, from top left. - Pixel([u64; 2]), - /// Any integer, e.g. a hash or an arbitrary identifier. Integer(i128), @@ -56,7 +53,6 @@ impl std::fmt::Display for Index { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { Self::Sequence(seq) => format!("#{seq}").fmt(f), - Self::Pixel([x, y]) => format!("[{x}, {y}]").fmt(f), Self::Integer(value) => value.fmt(f), Self::Uuid(value) => value.fmt(f), Self::String(value) => format!("{value:?}").fmt(f), // put it in quotes diff --git a/crates/re_log_types/src/path/data_path.rs b/crates/re_log_types/src/path/data_path.rs new file mode 100644 index 000000000000..5041c2a387ec --- /dev/null +++ b/crates/re_log_types/src/path/data_path.rs @@ -0,0 +1,42 @@ +use re_types::{components::InstanceKey, ComponentName}; + +use crate::EntityPath; + +/// A general path to some data. +/// +/// This always starts with an [`EntityPath`], followed +/// by an optional [`InstanceKey`], followed by an optional [`ComponentName`]. +/// +/// For instance: +/// +/// * `points` +/// * `points.Color` +/// * `points[#42]` +/// * `points[#42].Color` +#[derive(Clone, Eq, PartialEq, Hash)] +pub struct DataPath { + pub entity_path: EntityPath, + + pub instance_key: Option, + + pub component_name: Option, +} + +impl std::fmt::Display for DataPath { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.entity_path.fmt(f)?; + if let Some(instance_key) = &self.instance_key { + write!(f, "[#{}]", instance_key.0)?; + } + if let Some(component_name) = &self.component_name { + write!(f, ".{component_name:?}")?; + } + Ok(()) + } +} + +impl std::fmt::Debug for DataPath { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.to_string().fmt(f) + } +} diff --git a/crates/re_log_types/src/path/entity_path.rs b/crates/re_log_types/src/path/entity_path.rs index 248b844d257a..21f8ae9bd4b1 100644 --- a/crates/re_log_types/src/path/entity_path.rs +++ b/crates/re_log_types/src/path/entity_path.rs @@ -1,9 +1,6 @@ use std::sync::Arc; -use crate::{ - hash::Hash64, parse_entity_path, path::entity_path_impl::EntityPathImpl, EntityPathPart, - SizeBytes, -}; +use crate::{hash::Hash64, path::entity_path_impl::EntityPathImpl, EntityPathPart, SizeBytes}; // ---------------------------------------------------------------------------- @@ -125,6 +122,11 @@ impl EntityPath { self.path.as_slice() } + #[inline] + pub fn to_vec(&self) -> Vec { + self.path.to_vec() + } + #[inline] pub fn is_root(&self) -> bool { self.path.is_root() @@ -208,18 +210,19 @@ impl From<&[EntityPathPart]> for EntityPath { } } -#[allow(clippy::fallible_impl_from)] +#[allow(clippy::fallible_impl_from)] // TODO(#3393): we should force users to handle errors instead, and have a nice macro for constructing entity path impl From<&str> for EntityPath { #[inline] fn from(path: &str) -> Self { - Self::from(parse_entity_path(path).unwrap()) + path.parse().unwrap() } } +#[allow(clippy::fallible_impl_from)] // TODO(#3393): we should force users to handle errors instead, and have a nice macro for constructing entity path impl From for EntityPath { #[inline] fn from(path: String) -> Self { - Self::from(path.as_str()) + path.parse().unwrap() } } diff --git a/crates/re_log_types/src/path/entity_path_impl.rs b/crates/re_log_types/src/path/entity_path_impl.rs index e7e418d5a209..d07eefde4af8 100644 --- a/crates/re_log_types/src/path/entity_path_impl.rs +++ b/crates/re_log_types/src/path/entity_path_impl.rs @@ -25,6 +25,11 @@ impl EntityPathImpl { self.parts.as_slice() } + #[inline] + pub fn to_vec(&self) -> Vec { + self.parts.clone() + } + #[inline] pub fn is_root(&self) -> bool { self.parts.is_empty() diff --git a/crates/re_log_types/src/path/mod.rs b/crates/re_log_types/src/path/mod.rs index 022ad6037473..dc672d5d12eb 100644 --- a/crates/re_log_types/src/path/mod.rs +++ b/crates/re_log_types/src/path/mod.rs @@ -7,14 +7,16 @@ //! The [`Index`]es are for tables, arrays etc. mod component_path; +mod data_path; mod entity_path; mod entity_path_impl; mod parse_path; pub use component_path::ComponentPath; +pub use data_path::DataPath; pub use entity_path::{EntityPath, EntityPathHash}; pub use entity_path_impl::EntityPathImpl; -pub use parse_path::{parse_entity_path, PathParseError}; +pub use parse_path::PathParseError; use re_string_interner::InternedString; @@ -26,10 +28,12 @@ use crate::Index; #[derive(Clone, Debug, Hash, PartialEq, Eq, PartialOrd, Ord)] #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] pub enum EntityPathPart { - /// Struct member. Each member can have a different type. + /// Corresponds to the name of a struct field. + /// + /// Only a limited set of characters are allowed in a name. Name(InternedString), - /// Array/table/map member. Each member must be of the same type (homogeneous). + /// Array/table/map member. Index(Index), } @@ -72,13 +76,13 @@ impl From for EntityPathPart { /// ``` #[macro_export] macro_rules! entity_path_vec { - () => { - vec![] - }; - ($($part: expr),* $(,)?) => { - vec![ $($crate::EntityPathPart::from($part),)+ ] - }; - } + () => { + vec![] + }; + ($($part: expr),* $(,)?) => { + vec![ $($crate::EntityPathPart::from($part),)+ ] + }; +} /// Build a `EntityPath`: /// ``` @@ -87,10 +91,10 @@ macro_rules! entity_path_vec { /// ``` #[macro_export] macro_rules! entity_path { - () => { - vec![] - }; - ($($part: expr),* $(,)?) => { - $crate::EntityPath::from(vec![ $($crate::EntityPathPart::from($part),)+ ]) - }; - } + () => { + vec![] + }; + ($($part: expr),* $(,)?) => { + $crate::EntityPath::from(vec![ $($crate::EntityPathPart::from($part),)+ ]) + }; +} diff --git a/crates/re_log_types/src/path/parse_path.rs b/crates/re_log_types/src/path/parse_path.rs index 5f81081afbbd..9712595cedce 100644 --- a/crates/re_log_types/src/path/parse_path.rs +++ b/crates/re_log_types/src/path/parse_path.rs @@ -1,10 +1,28 @@ -use crate::{EntityPathPart, Index}; +use std::str::FromStr; + +use re_types::{components::InstanceKey, ComponentName}; + +use crate::{ComponentPath, DataPath, EntityPath, EntityPathPart, Index}; + +/// When parsing a [`DataPath`], it is important that we can distinguish the +/// component and index from the actual entity path. This requires +/// us to forbid certain characters in an entity part name. +/// For instance, in `foo/bar.baz`, is `baz` a component name, or part of the entity path? +/// So, when parsing a full [`DataPath`]s we are quite strict with what we allow. +/// But when parsing [`EntityPath`]s we want to be a bit more forgiving, so we +/// can accept things like `foo/bar.baz` and transform it into `foo/"bar.baz"`. +/// This allows user to do things like `log(f"foo/{filename}", my_mesh)` without +/// Rerun throwing a fit. +const STRICT_ENTITY_PATH_PARSING: bool = false; #[derive(thiserror::Error, Debug, PartialEq, Eq)] pub enum PathParseError { #[error("Expected path, found empty string")] EmptyString, + #[error("No entity path found")] + MissingPath, + #[error("Path had leading slash")] LeadingSlash, @@ -22,10 +40,187 @@ pub enum PathParseError { #[error("Missing slash (/)")] MissingSlash, + + #[error("Extra trailing slash (/)")] + TrailingSlash, + + #[error("Empty part")] + EmptyPart, + + #[error("Invalid character: {character:?} in entity path identifier {part:?}. Only ASCII characters, numbers, underscore, and dash are allowed. To put wild text in an entity path, surround it with double-quotes.")] + InvalidCharacterInPart { part: String, character: char }, + + #[error("Invalid instance key: {0:?} (expected '[#1234]')")] + BadInstanceKey(String), + + #[error("Found an unexpected instance key: [#{}]", 0.0)] + UnexpectedInstanceKey(InstanceKey), + + #[error("Found an unexpected trailing component name: {0:?}")] + UnexpectedComponentName(ComponentName), + + #[error("Found no component name")] + MissingComponentName, + + #[error("Found trailing dot (.)")] + TrailingDot, } -/// Parses an entity path, e.g. `foo/bar/#1234/5678/"string index"/a6a5e96c-fd52-4d21-a394-ffbb6e5def1d` -pub fn parse_entity_path(path: &str) -> Result, PathParseError> { +type Result = std::result::Result; + +impl std::str::FromStr for DataPath { + type Err = PathParseError; + + /// For instance: + /// + /// * `world/points` + /// * `world/points.Color` + /// * `world/points[#42]` + /// * `world/points[#42].rerun.components.Color` + fn from_str(path: &str) -> Result { + if path.is_empty() { + return Err(PathParseError::EmptyString); + } + + // Start by looking for a component + + let mut tokens = tokenize(path)?; + + let mut component_name = None; + let mut instance_key = None; + + // Parse `.rerun.components.Color` suffix: + if let Some(dot) = tokens.iter().position(|&token| token == ".") { + let component_tokens = &tokens[dot + 1..]; + + if component_tokens.is_empty() { + return Err(PathParseError::TrailingDot); + } else if component_tokens.len() == 1 { + component_name = Some(ComponentName::from(format!( + "rerun.components.{}", + join(component_tokens) + ))); + } else { + component_name = Some(ComponentName::from(join(component_tokens))); + } + tokens.truncate(dot); + } + + // Parse `[#1234]` suffix: + if let Some(bracket) = tokens.iter().position(|&token| token == "[") { + let instance_key_tokens = &tokens[bracket..]; + if instance_key_tokens.len() != 3 || instance_key_tokens.last() != Some(&"]") { + return Err(PathParseError::BadInstanceKey(join(instance_key_tokens))); + } + let instance_key_token = instance_key_tokens[1]; + if let Some(nr) = instance_key_token.strip_prefix('#') { + if let Ok(nr) = u64::from_str(nr) { + instance_key = Some(InstanceKey(nr)); + } else { + return Err(PathParseError::BadInstanceKey( + instance_key_token.to_owned(), + )); + } + } else { + return Err(PathParseError::BadInstanceKey( + instance_key_token.to_owned(), + )); + } + tokens.truncate(bracket); + } + + // The remaining tokens should all be separated with `/`: + + let parts = entity_path_parts_from_tokens(&tokens)?; + + // Validate names: + for part in &parts { + if let EntityPathPart::Name(name) = part { + validate_name(name)?; + } + } + + let entity_path = EntityPath::from(parts); + + Ok(Self { + entity_path, + instance_key, + component_name, + }) + } +} + +impl FromStr for EntityPath { + type Err = PathParseError; + + fn from_str(s: &str) -> Result { + if STRICT_ENTITY_PATH_PARSING { + let DataPath { + entity_path, + instance_key, + component_name, + } = DataPath::from_str(s)?; + + if let Some(instance_key) = instance_key { + return Err(PathParseError::UnexpectedInstanceKey(instance_key)); + } + if let Some(component_name) = component_name { + return Err(PathParseError::UnexpectedComponentName(component_name)); + } + Ok(entity_path) + } else { + let mut parts = parse_entity_path_forgiving(s)?; + + for part in &mut parts { + if let EntityPathPart::Name(name) = part { + if validate_name(name).is_err() { + // Quote this, e.g. `foo/invalid.name` -> `foo/"invalid.name"` + *part = EntityPathPart::Index(Index::String(name.to_string())); + } + } + } + + let path = EntityPath::from(parts); + + if path.to_string() != s { + re_log::warn_once!("Found an invalid entity path '{s}' that was interpreted as '{path}'. Only ASCII characters, numbers, underscore, and dash are allowed in identifiers. To put wild text in an entity path, surround it with double-quotes."); + } + + Ok(path) + } + } +} + +impl FromStr for ComponentPath { + type Err = PathParseError; + + fn from_str(s: &str) -> Result { + let DataPath { + entity_path, + instance_key, + component_name, + } = DataPath::from_str(s)?; + + if let Some(instance_key) = instance_key { + return Err(PathParseError::UnexpectedInstanceKey(instance_key)); + } + + let Some(component_name) = component_name else { + return Err(PathParseError::MissingComponentName); + }; + + Ok(ComponentPath { + entity_path, + component_name, + }) + } +} + +/// A very forgiving parsing of the given entity path. +/// +/// Things like `foo/Hallå Där!` will be accepted, and transformed into +/// the path `foo/"Hallå Där!"`. +fn parse_entity_path_forgiving(path: &str) -> Result, PathParseError> { if path.is_empty() { return Err(PathParseError::EmptyString); } @@ -90,7 +285,114 @@ pub fn parse_entity_path(path: &str) -> Result, PathParseErr Ok(parts) } -fn parse_part(s: &str) -> Result { +fn entity_path_parts_from_tokens(mut tokens: &[&str]) -> Result> { + if tokens.is_empty() { + return Err(PathParseError::MissingPath); + } + + if tokens == ["/"] { + return Ok(vec![]); // special-case root entity + } + + if tokens[0] == "/" { + return Err(PathParseError::LeadingSlash); + } + + let mut parts = vec![]; + + loop { + let token = tokens[0]; + tokens = &tokens[1..]; + + if token == "/" { + return Err(PathParseError::DoubleSlash); + } else if token.starts_with('"') { + assert!(token.ends_with('"')); + let unescaped = unescape_string(&token[1..token.len() - 1]) + .map_err(|details| PathParseError::BadEscape { details })?; + parts.push(EntityPathPart::Index(Index::String(unescaped))); + } else { + parts.push(parse_part(token)?); + } + + if let Some(next_token) = tokens.first() { + if *next_token == "/" { + tokens = &tokens[1..]; + if tokens.is_empty() { + return Err(PathParseError::TrailingSlash); + } + } else { + return Err(PathParseError::MissingSlash); + } + } else { + break; + } + } + + Ok(parts) +} + +fn join(tokens: &[&str]) -> String { + let mut s = String::default(); + for token in tokens { + s.push_str(token); + } + s +} + +fn tokenize(path: &str) -> Result> { + let mut bytes = path.as_bytes(); + + fn is_special_character(c: u8) -> bool { + matches!(c, b'[' | b']' | b'.' | b'/') + } + + let mut tokens = vec![]; + + while let Some(c) = bytes.first() { + if *c == b'"' { + // Look for the terminating quote ignoring escaped quotes (\"): + let mut i = 1; + loop { + if i == bytes.len() { + return Err(PathParseError::UnterminatedString); + } else if bytes[i] == b'\\' && i + 1 < bytes.len() { + i += 2; // consume escape and what was escaped + } else if bytes[i] == b'"' { + break; + } else { + i += 1; + } + } + + let token = &bytes[..i + 1]; // Include the closing quote + tokens.push(token); + bytes = &bytes[i + 1..]; // skip the closing quote + } else if is_special_character(*c) { + tokens.push(&bytes[..1]); + bytes = &bytes[1..]; + } else { + let mut i = 0; + while i < bytes.len() { + if bytes[i] == b'"' || is_special_character(bytes[i]) { + break; + } + i += 1; + } + assert!(0 < i); + tokens.push(&bytes[..i]); + bytes = &bytes[i..]; + } + } + + // Safety: we split at proper character boundaries + Ok(tokens + .iter() + .map(|token| std::str::from_utf8(token).unwrap()) + .collect()) +} + +fn parse_part(s: &str) -> Result { use std::str::FromStr as _; if s.is_empty() { @@ -110,6 +412,23 @@ fn parse_part(s: &str) -> Result { } } +fn validate_name(name: &str) -> Result<()> { + if name.is_empty() { + return Err(PathParseError::EmptyPart); + } + + for c in name.chars() { + if !c.is_ascii_alphanumeric() && c != '_' && c != '-' { + return Err(PathParseError::InvalidCharacterInPart { + part: name.to_owned(), + character: c, + }); + } + } + + Ok(()) +} + fn unescape_string(input: &str) -> Result { let mut output = String::with_capacity(input.len()); let mut chars = input.chars(); @@ -143,23 +462,21 @@ fn test_unescape_string() { } #[test] -fn test_parse_path() { +fn test_parse_entity_path() { use crate::entity_path_vec; - assert_eq!(parse_entity_path(""), Err(PathParseError::EmptyString)); - assert_eq!(parse_entity_path("/"), Ok(entity_path_vec!())); - assert_eq!(parse_entity_path("foo"), Ok(entity_path_vec!("foo"))); - assert_eq!(parse_entity_path("/foo"), Err(PathParseError::LeadingSlash)); - assert_eq!( - parse_entity_path("foo/bar"), - Ok(entity_path_vec!("foo", "bar")) - ); - assert_eq!( - parse_entity_path("foo//bar"), - Err(PathParseError::DoubleSlash) - ); + fn parse(s: &str) -> Result> { + EntityPath::from_str(s).map(|path| path.to_vec()) + } + + assert_eq!(parse(""), Err(PathParseError::EmptyString)); + assert_eq!(parse("/"), Ok(entity_path_vec!())); + assert_eq!(parse("foo"), Ok(entity_path_vec!("foo"))); + assert_eq!(parse("/foo"), Err(PathParseError::LeadingSlash)); + assert_eq!(parse("foo/bar"), Ok(entity_path_vec!("foo", "bar"))); + assert_eq!(parse("foo//bar"), Err(PathParseError::DoubleSlash)); assert_eq!( - parse_entity_path(r#"foo/"bar"/#123/-1234/6d046bf4-e5d3-4599-9153-85dd97218cb3"#), + parse(r#"foo/"bar"/#123/-1234/6d046bf4-e5d3-4599-9153-85dd97218cb3"#), Ok(entity_path_vec!( "foo", Index::String("bar".into()), @@ -169,7 +486,113 @@ fn test_parse_path() { )) ); assert_eq!( - parse_entity_path(r#"foo/"bar""baz""#), + parse(r#"foo/"bar""baz""#), Err(PathParseError::MissingSlash) ); + + if STRICT_ENTITY_PATH_PARSING { + assert_eq!(parse("foo/bar/"), Err(PathParseError::TrailingSlash)); + assert!(matches!( + parse(r#"entity.component"#), + Err(PathParseError::UnexpectedComponentName { .. }) + )); + assert!(matches!( + parse(r#"entity[#123]"#), + Err(PathParseError::UnexpectedInstanceKey(InstanceKey(123))) + )); + } else { + assert_eq!( + EntityPath::from_str("foo/bar/").unwrap().to_string(), + "foo/bar" + ); + assert_eq!( + EntityPath::from_str("foo/bar.baz").unwrap().to_string(), + r#"foo/"bar.baz""# + ); + } +} + +#[test] +fn test_parse_component_path() { + assert_eq!( + ComponentPath::from_str("world/points.rerun.components.Color"), + Ok(ComponentPath { + entity_path: EntityPath::from_str("world/points").unwrap(), + component_name: "rerun.components.Color".into(), + }) + ); + assert_eq!( + ComponentPath::from_str("world/points.Color"), + Ok(ComponentPath { + entity_path: EntityPath::from_str("world/points").unwrap(), + component_name: "rerun.components.Color".into(), + }) + ); + assert_eq!( + ComponentPath::from_str("world/points.my.custom.color"), + Ok(ComponentPath { + entity_path: EntityPath::from_str("world/points").unwrap(), + component_name: "my.custom.color".into(), + }) + ); + assert_eq!( + ComponentPath::from_str("world/points."), + Err(PathParseError::TrailingDot) + ); + assert_eq!( + ComponentPath::from_str("world/points"), + Err(PathParseError::MissingComponentName) + ); + assert_eq!( + ComponentPath::from_str("world/points[#42].rerun.components.Color"), + Err(PathParseError::UnexpectedInstanceKey(InstanceKey(42))) + ); +} + +#[test] +fn test_parse_data_path() { + assert_eq!( + DataPath::from_str("world/points[#42].rerun.components.Color"), + Ok(DataPath { + entity_path: EntityPath::from_str("world/points").unwrap(), + instance_key: Some(InstanceKey(42)), + component_name: Some("rerun.components.Color".into()), + }) + ); + assert_eq!( + DataPath::from_str("world/points.rerun.components.Color"), + Ok(DataPath { + entity_path: EntityPath::from_str("world/points").unwrap(), + instance_key: None, + component_name: Some("rerun.components.Color".into()), + }) + ); + assert_eq!( + DataPath::from_str("world/points[#42]"), + Ok(DataPath { + entity_path: EntityPath::from_str("world/points").unwrap(), + instance_key: Some(InstanceKey(42)), + component_name: None, + }) + ); + assert_eq!( + DataPath::from_str("world/points"), + Ok(DataPath { + entity_path: EntityPath::from_str("world/points").unwrap(), + instance_key: None, + component_name: None, + }) + ); + + // Check that we catch invalid characters in identifiers/names: + assert!(matches!( + DataPath::from_str(r#"hello there"#), + Err(PathParseError::InvalidCharacterInPart { .. }) + )); + assert!(matches!( + DataPath::from_str(r#"hallådär"#), + Err(PathParseError::InvalidCharacterInPart { .. }) + )); + assert!(DataPath::from_str(r#"hello_there"#).is_ok()); + assert!(DataPath::from_str(r#"hello-there"#).is_ok()); } diff --git a/crates/re_space_view_text_document/src/space_view_class.rs b/crates/re_space_view_text_document/src/space_view_class.rs index f4b9d71cbf8a..4c23003ee2c5 100644 --- a/crates/re_space_view_text_document/src/space_view_class.rs +++ b/crates/re_space_view_text_document/src/space_view_class.rs @@ -124,6 +124,14 @@ impl SpaceViewClass for TextDocumentSpaceView { { if media_type == &re_types::components::MediaType::markdown() { re_tracing::profile_scope!("egui_commonmark"); + + // Make sure headers are big: + ui.style_mut() + .text_styles + .entry(egui::TextStyle::Heading) + .or_insert(egui::FontId::proportional(32.0)) + .size = 24.0; + egui_commonmark::CommonMarkViewer::new("markdown_viewer") .max_image_width(Some(ui.available_width().floor() as _)) .show(ui, &mut state.commonmark_cache, body); diff --git a/crates/re_types/source_hash.txt b/crates/re_types/source_hash.txt index 970b44a9f39d..fdcbe75668cd 100644 --- a/crates/re_types/source_hash.txt +++ b/crates/re_types/source_hash.txt @@ -1,4 +1,4 @@ # This is a sha256 hash for all direct and indirect dependencies of this crate's build script. # It can be safely removed at anytime to force the build script to run again. # Check out build.rs to see how it's computed. -879d33df3fbb99178624e96a6ac1c6f05ee14b4683be2cb6c9b48b7469efb257 +3cd3f97b282ac8bc7e678f9f18108a67dfdd1337f65f8113d09ea39573a963ad diff --git a/crates/re_viewer/src/app_state.rs b/crates/re_viewer/src/app_state.rs index e3485efd9513..2cad00b3e48d 100644 --- a/crates/re_viewer/src/app_state.rs +++ b/crates/re_viewer/src/app_state.rs @@ -5,7 +5,7 @@ use re_log_types::{LogMsg, StoreId, TimeRangeF}; use re_smart_channel::ReceiveSet; use re_viewer_context::{ AppOptions, Caches, CommandSender, ComponentUiRegistry, PlayState, RecordingConfig, - SpaceViewClassRegistry, StoreContext, ViewerContext, + SelectionState, SpaceViewClassRegistry, StoreContext, ViewerContext, }; use re_viewport::{SpaceInfoCollection, Viewport, ViewportState}; @@ -232,6 +232,9 @@ impl AppState { if WATERMARK { re_ui.paint_watermark(); } + + // This must run after any ui code, or other code that tells egui to open an url: + check_for_clicked_hyperlinks(&re_ui.egui_ctx, &mut rec_cfg.selection_state); } pub fn recording_config_mut(&mut self, rec_id: &StoreId) -> Option<&mut RecordingConfig> { @@ -282,3 +285,35 @@ fn recording_config_entry<'cfgs>( .entry(id) .or_insert_with(|| new_recording_config(store_db)) } + +/// We allow linking to entities and components via hyperlinks, +/// e.g. in embedded markdown. +/// +/// Detect and handle that here. +/// +/// Must run after any ui code, or other code that tells egui to open an url. +fn check_for_clicked_hyperlinks(egui_ctx: &egui::Context, selection_state: &mut SelectionState) { + let recording_scheme = "recording://"; + + let mut path = None; + + egui_ctx.output_mut(|o| { + if let Some(open_url) = &o.open_url { + if let Some(path_str) = open_url.url.strip_prefix(recording_scheme) { + path = Some(path_str.to_owned()); + o.open_url = None; + } + } + }); + + if let Some(path) = path { + match path.parse() { + Ok(item) => { + selection_state.set_single_selection(item); + } + Err(err) => { + re_log::warn!("Failed to parse entity path {path:?}: {err}"); + } + } + } +} diff --git a/crates/re_viewer_context/src/item.rs b/crates/re_viewer_context/src/item.rs index 7852c5074720..090675291815 100644 --- a/crates/re_viewer_context/src/item.rs +++ b/crates/re_viewer_context/src/item.rs @@ -1,6 +1,7 @@ -use itertools::Itertools; +use itertools::Itertools as _; + use re_data_store::InstancePath; -use re_log_types::ComponentPath; +use re_log_types::{ComponentPath, DataPath, EntityPath}; use super::{DataBlueprintGroupHandle, SpaceViewId}; @@ -17,6 +18,67 @@ pub enum Item { DataBlueprintGroup(SpaceViewId, DataBlueprintGroupHandle), } +impl From for Item { + #[inline] + fn from(space_view_id: SpaceViewId) -> Self { + Self::SpaceView(space_view_id) + } +} + +impl From for Item { + #[inline] + fn from(component_path: ComponentPath) -> Self { + Self::ComponentPath(component_path) + } +} + +impl From for Item { + #[inline] + fn from(instance_path: InstancePath) -> Self { + Self::InstancePath(None, instance_path) + } +} + +impl From for Item { + #[inline] + fn from(entity_path: EntityPath) -> Self { + Self::InstancePath(None, InstancePath::from(entity_path)) + } +} + +impl std::str::FromStr for Item { + type Err = re_log_types::PathParseError; + + fn from_str(s: &str) -> Result { + let DataPath { + entity_path, + instance_key, + component_name, + } = DataPath::from_str(s)?; + + match (instance_key, component_name) { + (Some(instance_key), Some(_component_name)) => { + // TODO(emilk): support selecting a specific component of a specific instance. + Err(re_log_types::PathParseError::UnexpectedInstanceKey( + instance_key, + )) + } + (Some(instance_key), None) => Ok(Item::InstancePath( + None, + InstancePath::instance(entity_path, instance_key), + )), + (None, Some(component_name)) => Ok(Item::ComponentPath(ComponentPath { + entity_path, + component_name, + })), + (None, None) => Ok(Item::InstancePath( + None, + InstancePath::entity_splat(entity_path), + )), + } + } +} + impl std::fmt::Debug for Item { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { diff --git a/crates/rerun_c/src/lib.rs b/crates/rerun_c/src/lib.rs index 782d34c19bdf..e70ea7812032 100644 --- a/crates/rerun_c/src/lib.rs +++ b/crates/rerun_c/src/lib.rs @@ -9,7 +9,10 @@ mod error; mod ptr; -use std::ffi::{c_char, CString}; +use std::{ + ffi::{c_char, CString}, + str::FromStr, +}; use once_cell::sync::Lazy; use parking_lot::Mutex; @@ -324,15 +327,12 @@ fn rr_log_impl( } = *data_row; let entity_path = ptr::try_char_ptr_as_str(entity_path, "entity_path")?; - let entity_path = match re_log_types::parse_entity_path(entity_path) { - Ok(entity_path) => EntityPath::from(entity_path), - Err(err) => { - return Err(CError::new( - CErrorCode::InvalidEntityPath, - &format!("Failed to parse entity path {entity_path:?}: {err}"), - )) - } - }; + let entity_path = EntityPath::from_str(entity_path).map_err(|err| { + CError::new( + CErrorCode::InvalidEntityPath, + &format!("Failed to parse entity path {entity_path:?}: {err}"), + ) + })?; re_log::debug!( "rerun_log {entity_path:?}, num_instances: {num_instances}, num_data_cells: {num_data_cells}", diff --git a/docs/code-examples/text_document.cpp b/docs/code-examples/text_document.cpp index 12009bbad956..c6c0631aa095 100644 --- a/docs/code-examples/text_document.cpp +++ b/docs/code-examples/text_document.cpp @@ -12,14 +12,49 @@ int main() { rr_stream.connect("127.0.0.1:9876").throw_on_failure(); rr_stream.log("text_document", rr::archetypes::TextDocument("Hello, TextDocument!")); + rr_stream.log( "markdown", - rr::archetypes::TextDocument("# Hello\n" - "Markdown with `code`!\n" - "\n" - "A random image:\n" - "\n" - "![A random image](https://picsum.photos/640/480)") + rr::archetypes::TextDocument(R"#(# Hello Markdown! +[Click here to see the raw text](recording://markdown.Text). + +Basic formatting: + +| **Feature** | **Alternative** | +| ----------------- | --------------- | +| Plain | | +| *italics* | _italics_ | +| **bold** | __bold__ | +| ~~strikethrough~~ | | +| `inline code` | | + +---------------------------------- + +Some code: +```rs +fn main() { + println!("Hello, world!"); +} +``` + +## Support +- [x] [Commonmark](https://commonmark.org/help/) support +- [x] GitHub-style strikethrough, tables, and checkboxes +- Basic syntax highlighting for: + - [x] C and C++ + - [x] Python + - [x] Rust + - [ ] Other languages + +## Links +You can link to [an entity](recording://markdown), +a [specific instance of an entity](recording://markdown[#0]), +or a [specific component](recording://markdown.Text). + +Of course you can also have [normal https links](https://github.com/rerun-io/rerun), e.g. . + +## Image +![A random image](https://picsum.photos/640/480))#") .with_media_type(rr::components::MediaType::markdown()) ); } diff --git a/docs/code-examples/text_document.py b/docs/code-examples/text_document.py index b85449752bd1..2d74fb1f90af 100755 --- a/docs/code-examples/text_document.py +++ b/docs/code-examples/text_document.py @@ -6,11 +6,53 @@ rr.init("rerun_example_text_document", spawn=True) -rr2.log("text_document", rr2.TextDocument(body="Hello, TextDocument!")) +rr2.log("text_document", rr2.TextDocument("Hello, TextDocument!")) + rr2.log( "markdown", rr2.TextDocument( - body="# Hello\nMarkdown with `code`!\n\nA random image:\n\n![A random image](https://picsum.photos/640/480)", - media_type=rr2.cmp.MediaType.markdown(), + """ +# Hello Markdown! +[Click here to see the raw text](recording://markdown.Text). + +Basic formatting: + +| **Feature** | **Alternative** | +| ----------------- | --------------- | +| Plain | | +| *italics* | _italics_ | +| **bold** | __bold__ | +| ~~strikethrough~~ | | +| `inline code` | | + +---------------------------------- + +Some code: +```rs +fn main() { + println!("Hello, world!"); +} +``` + +## Support +- [x] [Commonmark](https://commonmark.org/help/) support +- [x] GitHub-style strikethrough, tables, and checkboxes +- Basic syntax highlighting for: + - [x] C and C++ + - [x] Python + - [x] Rust + - [ ] Other languages + +## Links +You can link to [an entity](recording://markdown), +a [specific instance of an entity](recording://markdown[#0]), +or a [specific component](recording://markdown.Text). + +Of course you can also have [normal https links](https://github.com/rerun-io/rerun), e.g. . + +## Image +![A random image](https://picsum.photos/640/480) +""".strip(), + media_type="text/markdown", ), ) diff --git a/docs/code-examples/text_document.rs b/docs/code-examples/text_document.rs index ce5fdc932a04..4165b7e264c1 100644 --- a/docs/code-examples/text_document.rs +++ b/docs/code-examples/text_document.rs @@ -8,15 +8,52 @@ fn main() -> Result<(), Box> { let (rec, storage) = RecordingStreamBuilder::new("rerun_example_text_document").memory()?; rec.log("text_document", &TextDocument::new("Hello, TextDocument!"))?; + rec.log( "markdown", &TextDocument::new( - "# Hello\n\ - Markdown with `code`!\n\ - \n\ - A random image:\n\ - \n\ - ![A random image](https://picsum.photos/640/480)", + r#" +# Hello Markdown! +[Click here to see the raw text](recording://markdown.Text). + +Basic formatting: + +| **Feature** | **Alternative** | +| ----------------- | --------------- | +| Plain | | +| *italics* | _italics_ | +| **bold** | __bold__ | +| ~~strikethrough~~ | | +| `inline code` | | + +---------------------------------- + +Some code: +```rs +fn main() { + println!("Hello, world!"); +} +``` + +## Support +- [x] [Commonmark](https://commonmark.org/help/) support +- [x] GitHub-style strikethrough, tables, and checkboxes +- Basic syntax highlighting for: + - [x] C and C++ + - [x] Python + - [x] Rust + - [ ] Other languages + +## Links +You can link to [an entity](recording://markdown), +a [specific instance of an entity](recording://markdown[#0]), +or a [specific component](recording://markdown.Text). + +Of course you can also have [normal https links](https://github.com/rerun-io/rerun), e.g. . + +## Image +![A random image](https://picsum.photos/640/480) +"#.trim(), ) .with_media_type(MediaType::markdown()), )?; diff --git a/examples/python/structure_from_motion/main.py b/examples/python/structure_from_motion/main.py index 6673054dce95..0515146bdc55 100755 --- a/examples/python/structure_from_motion/main.py +++ b/examples/python/structure_from_motion/main.py @@ -15,6 +15,7 @@ import numpy.typing as npt import requests import rerun as rr # pip install rerun-sdk +import rerun.experimental as rr2 from read_write_model import Camera, read_model from tqdm import tqdm @@ -23,6 +24,34 @@ # When dataset filtering is turned on, drop views with less than this many valid points. FILTER_MIN_VISIBLE: Final = 500 +DESCRIPTION = """ +# Sparse Reconstruction by COLMAP + +This example was generated from the output of a sparse reconstruction +done with COLMAP. + +[COLMAP](https://colmap.github.io/index.html) is a general-purpose +Structure-from-Motion (SfM) and Multi-View Stereo (MVS) pipeline +with a graphical and command-line interface. + +In this example a short video clip has been processed offline by the +COLMAP pipeline, and we use Rerun to visualize the individual +camera frames, estimated camera poses, and resulting point clouds over time. + +## How it was made +The full source code for this example is available [on GitHub](https://github.com/rerun-io/rerun/blob/latest/examples/python/structure_from_motion/main.py). + +### Colored 3D Points +The colored 3D points were added to the scene by logging the +[rr.Points3D archetype](https://www.rerun.io/docs/reference/data_types/point3d) +to the [points entity](recording://points): +```python +rr.log_points("points", points, colors=point_colors, ext={"error": point_errors}) +``` +**Note:** we added some [custom per-point errors](recording://points.ext.error) that you can see when you +hover over the points in the 3D view. +""".strip() + def scale_camera(camera: Camera, resize: tuple[int, int]) -> tuple[Camera, npt.NDArray[np.float_]]: """Scale the camera intrinsics to match the resized image.""" @@ -81,6 +110,7 @@ def read_and_log_sparse_reconstruction(dataset_path: Path, filter_output: bool, # Filter out noisy points points3D = {id: point for id, point in points3D.items() if point.rgb.any() and len(point.image_ids) > 4} + rr2.log("description", rr2.TextDocument(DESCRIPTION, media_type="text/markdown"), timeless=True) rr.log_view_coordinates("/", up="-Y", timeless=True) # Iterate through images (video frames) logging data related to each frame. diff --git a/examples/python/text_logging/main.py b/examples/python/text_logging/main.py index 0a1c598e0579..7de9595a7889 100755 --- a/examples/python/text_logging/main.py +++ b/examples/python/text_logging/main.py @@ -82,7 +82,7 @@ def main() -> None: for frame_offset in range(args.repeat): log_stuff(frame_offset) - rr2.log("text_document", rr2.TextDocument(body="This is to show the difference between TextLog and TextDocument")) + rr2.log("text_document", rr2.TextDocument("This is to show the difference between TextLog and TextDocument")) rr.script_teardown(args) diff --git a/rerun_py/rerun_sdk/rerun/datatypes/tensor_buffer.py b/rerun_py/rerun_sdk/rerun/datatypes/tensor_buffer.py index 32f82bfe9982..c9f2943ada7b 100644 --- a/rerun_py/rerun_sdk/rerun/datatypes/tensor_buffer.py +++ b/rerun_py/rerun_sdk/rerun/datatypes/tensor_buffer.py @@ -28,7 +28,7 @@ class TensorBuffer(TensorBufferExt): # You can define your own __init__ function as a member of TensorBufferExt in tensor_buffer_ext.py - inner: npt.NDArray[np.float16] | (npt.NDArray[np.float32] | (npt.NDArray[np.float64] | (npt.NDArray[np.int16] | (npt.NDArray[np.int32] | (npt.NDArray[np.int64] | (npt.NDArray[np.int8] | (npt.NDArray[np.uint16] | (npt.NDArray[np.uint32] | (npt.NDArray[np.uint64] | npt.NDArray[np.uint8]))))))))) = field(converter=TensorBufferExt.inner__field_converter_override) # type: ignore[misc] + inner: npt.NDArray[np.float16] | npt.NDArray[np.float32] | npt.NDArray[np.float64] | npt.NDArray[np.int16] | npt.NDArray[np.int32] | npt.NDArray[np.int64] | npt.NDArray[np.int8] | npt.NDArray[np.uint16] | npt.NDArray[np.uint32] | npt.NDArray[np.uint64] | npt.NDArray[np.uint8] = field(converter=TensorBufferExt.inner__field_converter_override) # type: ignore[misc] """ U8 (npt.NDArray[np.uint8]): diff --git a/rerun_py/src/python_bridge.rs b/rerun_py/src/python_bridge.rs index 13257a89bfcc..cebfe4fa42e4 100644 --- a/rerun_py/src/python_bridge.rs +++ b/rerun_py/src/python_bridge.rs @@ -1050,7 +1050,6 @@ fn convert_color(color: Vec) -> PyResult<[u8; 4]> { } fn parse_entity_path(entity_path: &str) -> PyResult { - let components = re_log_types::parse_entity_path(entity_path) - .map_err(|err| PyTypeError::new_err(err.to_string()))?; - Ok(EntityPath::from(components)) + use std::str::FromStr as _; + EntityPath::from_str(entity_path).map_err(|err| PyTypeError::new_err(err.to_string())) } diff --git a/rerun_py/tests/test_types/datatypes/affix_fuzzer3.py b/rerun_py/tests/test_types/datatypes/affix_fuzzer3.py index 5b5fa40ee36e..8f065c9e76fe 100644 --- a/rerun_py/tests/test_types/datatypes/affix_fuzzer3.py +++ b/rerun_py/tests/test_types/datatypes/affix_fuzzer3.py @@ -22,7 +22,7 @@ class AffixFuzzer3: # You can define your own __init__ function as a member of AffixFuzzer3Ext in affix_fuzzer3_ext.py - inner: float | (list[datatypes.AffixFuzzer1] | npt.NDArray[np.float32]) = field() + inner: float | list[datatypes.AffixFuzzer1] | npt.NDArray[np.float32] = field() """ degrees (float): diff --git a/scripts/lint.py b/scripts/lint.py index a6de8316c639..fa99ff331b97 100755 --- a/scripts/lint.py +++ b/scripts/lint.py @@ -44,7 +44,7 @@ def lint_line(line: str, file_extension: str = "rs") -> str | None: if "NOLINT" in line: return None # NOLINT ignores the linter - if file_extension == "md": + if file_extension not in ("py", "txt", "yml"): if "Github" in line: return "It's 'GitHub', not 'Github'" @@ -201,7 +201,7 @@ def is_empty(line: str) -> bool: line = line.strip() prev_line = prev_line.strip() - if is_empty(prev_line): + if is_empty(prev_line) or prev_line.strip().startswith("```"): return False if line.startswith("fn ") and line.endswith(";"): diff --git a/tests/python/roundtrips/text_document/main.py b/tests/python/roundtrips/text_document/main.py index 8e60258fb7e3..b15292ac6f6f 100755 --- a/tests/python/roundtrips/text_document/main.py +++ b/tests/python/roundtrips/text_document/main.py @@ -17,7 +17,7 @@ def main() -> None: rr.script_setup(args, "rerun_example_roundtrip_text_document") - rr2.log("text_document", rr2.TextDocument(body="Hello, TextDocument!")) + rr2.log("text_document", rr2.TextDocument("Hello, TextDocument!")) rr2.log( "markdown", rr2.TextDocument( diff --git a/tests/python/test_api/main.py b/tests/python/test_api/main.py index c81fe5fa87b8..217a83769042 100755 --- a/tests/python/test_api/main.py +++ b/tests/python/test_api/main.py @@ -46,7 +46,7 @@ def run_segmentation(experimental_api: bool) -> None: rr2.Points2D([[40, 50], [120, 70], [80, 30]], class_ids=np.array([13, 42, 99], dtype=np.uint8)), ) rr2.log( - "seg_test/many points", + "seg_test/many_points", rr2.Points2D( [[100 + (int(i / 5)) * 2, 100 + (i % 5) * 2] for i in range(25)], class_ids=np.array([42], dtype=np.uint8), @@ -62,7 +62,7 @@ def run_segmentation(experimental_api: bool) -> None: class_ids=np.array([13, 42, 99], dtype=np.uint8), ) rr.log_points( - "seg_test/many points", + "seg_test/many_points", np.array([[100 + (int(i / 5)) * 2, 100 + (i % 5) * 2] for i in range(25)]), class_ids=np.array([42], dtype=np.uint8), ) diff --git a/tests/rust/test_api/src/main.rs b/tests/rust/test_api/src/main.rs index dfb73267a742..47e9be835578 100644 --- a/tests/rust/test_api/src/main.rs +++ b/tests/rust/test_api/src/main.rs @@ -332,7 +332,7 @@ fn test_segmentation(rec: &RecordingStream) -> anyhow::Result<()> { &Points2D::new([(40.0, 50.0), (120.0, 70.0), (80.0, 30.0)]).with_class_ids([13, 42, 99]), )?; rec.log( - "seg_test/many points", + "seg_test/many_points", &Points2D::new( (0..25).map(|i| (100.0 + (i / 5) as f32 * 2.0, 100.0 + (i % 5) as f32 * 2.0)), )