From 1ca66369f63d90ef3bcb86f7fc0e91390ee375d2 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 20 Dec 2023 08:30:49 +0100 Subject: [PATCH 01/10] Python API: add helpers for constructing an entity path Added `rr.escape_entity_path`, which constructs an entity path from a list of strings. `rr.log` also accepts such a list of unescaped strings directly. --- crates/re_sdk/src/recording_stream.rs | 19 +++++++--- docs/code-examples/Cargo.toml | 4 +++ docs/code-examples/entity_path.cpp | 18 ++++++++++ docs/code-examples/entity_path.py | 6 ++++ docs/code-examples/entity_path.rs | 16 +++++++++ docs/code-examples/roundtrips.py | 1 + rerun_py/rerun_sdk/rerun/_log.py | 50 ++++++++++++++++++++++++--- rerun_py/src/python_bridge.rs | 18 +++++----- 8 files changed, 116 insertions(+), 16 deletions(-) create mode 100644 docs/code-examples/entity_path.cpp create mode 100644 docs/code-examples/entity_path.py create mode 100644 docs/code-examples/entity_path.rs diff --git a/crates/re_sdk/src/recording_stream.rs b/crates/re_sdk/src/recording_stream.rs index e8ed47a6c568..4e1c4743d4ab 100644 --- a/crates/re_sdk/src/recording_stream.rs +++ b/crates/re_sdk/src/recording_stream.rs @@ -719,16 +719,17 @@ impl RecordingStream { /// The data will be timestamped automatically based on the [`RecordingStream`]'s internal clock. /// See [`RecordingStream::set_time_sequence`] etc for more information. /// + /// The entity path can either be a string + /// (with special characters escaped, split on unescaped slashes) + /// or an [`EntityPath`] constructed with [`crate::entity_path`]. + /// See for more on entity paths. + /// /// See also: [`Self::log_timeless`] for logging timeless data. /// /// Internally, the stream will automatically micro-batch multiple log calls to optimize /// transport. /// See [SDK Micro Batching] for more information. /// - /// The entity path can either be a string - /// (with special characters escaped, split on unescaped slashes) - /// or an [`EntityPath`] constructed with [`crate::entity_path`]. - /// /// # Example: /// ```ignore /// # use rerun; @@ -791,6 +792,11 @@ impl RecordingStream { /// internal clock. /// See `RecordingStream::set_time_*` family of methods for more information. /// + /// The entity path can either be a string + /// (with special characters escaped, split on unescaped slashes) + /// or an [`EntityPath`] constructed with [`crate::entity_path`]. + /// See for more on entity paths. + /// /// Internally, the stream will automatically micro-batch multiple log calls to optimize /// transport. /// See [SDK Micro Batching] for more information. @@ -830,6 +836,11 @@ impl RecordingStream { /// All of the batches should have the same number of instances, or length 1 if the component is /// a splat, or 0 if the component is being cleared. /// + /// The entity path can either be a string + /// (with special characters escaped, split on unescaped slashes) + /// or an [`EntityPath`] constructed with [`crate::entity_path`]. + /// See for more on entity paths. + /// /// Internally, the stream will automatically micro-batch multiple log calls to optimize /// transport. /// See [SDK Micro Batching] for more information. diff --git a/docs/code-examples/Cargo.toml b/docs/code-examples/Cargo.toml index 01ec3942f236..5307d953b889 100644 --- a/docs/code-examples/Cargo.toml +++ b/docs/code-examples/Cargo.toml @@ -71,6 +71,10 @@ path = "depth_image_simple.rs" name = "disconnected_space" path = "disconnected_space.rs" +[[bin]] +name = "entity_path" +path = "entity_path.rs" + [[bin]] name = "image_simple" path = "image_simple.rs" diff --git a/docs/code-examples/entity_path.cpp b/docs/code-examples/entity_path.cpp new file mode 100644 index 000000000000..0a374c649ef0 --- /dev/null +++ b/docs/code-examples/entity_path.cpp @@ -0,0 +1,18 @@ +// Log a `TextDocument` + +#include + +int main() { + const auto rec = rerun::RecordingStream("rerun_example_text_document"); + rec.spawn().exit_on_failure(); + + rec.log( + R"(world/escaped\ string\!)", + rerun::TextDocument("This entity path was escaped manually") + ); + // TODO: figure this one out + // rec.log( + // rerun::entity_path ![ "world", "unescaped string!" ], + // rerun::TextDocument("This entity path was provided as a list of unescaped strings") + // ); +} diff --git a/docs/code-examples/entity_path.py b/docs/code-examples/entity_path.py new file mode 100644 index 000000000000..7a0713fefa15 --- /dev/null +++ b/docs/code-examples/entity_path.py @@ -0,0 +1,6 @@ +import rerun as rr + +rr.init("rerun_example_entity_path", spawn=True) + +rr.log(r"world/escaped\ string\!", rr.TextDocument("This entity path was escaped manually")) +rr.log(["world", "unescaped string!"], rr.TextDocument("This entity path was provided as a list of unescaped strings")) diff --git a/docs/code-examples/entity_path.rs b/docs/code-examples/entity_path.rs new file mode 100644 index 000000000000..53b2347dc455 --- /dev/null +++ b/docs/code-examples/entity_path.rs @@ -0,0 +1,16 @@ +//! Log a `TextDocument` + +fn main() -> Result<(), Box> { + let rec = rerun::RecordingStreamBuilder::new("rerun_example_text_document").spawn()?; + + rec.log( + r"world/escaped\ string\!", + &rerun::TextDocument::new("This entity path was escaped manually"), + )?; + rec.log( + rerun::entity_path!["world", "unescaped string!"], + &rerun::TextDocument::new("This entity path was provided as a list of unescaped strings"), + )?; + + Ok(()) +} diff --git a/docs/code-examples/roundtrips.py b/docs/code-examples/roundtrips.py index 10b32ecfd4ef..6208cdcef3ae 100755 --- a/docs/code-examples/roundtrips.py +++ b/docs/code-examples/roundtrips.py @@ -41,6 +41,7 @@ opt_out_compare = { "arrow3d_simple": ["cpp", "py", "rust"], # TODO(#3206): examples use different RNGs "asset3d_out_of_tree": ["cpp", "py", "rust"], # float issues since calculation is done slightly differently (also, Python uses doubles) + "entity_path": ["cpp"], # C++ doesn't have helpers for escaping an entity path yet "mesh3d_partial_updates": ["cpp", "py", "rust"], # float precision issues "pinhole_simple": ["cpp", "py", "rust"], # TODO(#3206): examples use different RNGs "point2d_random": ["cpp", "py", "rust"], # TODO(#3206): examples use different RNGs diff --git a/rerun_py/rerun_sdk/rerun/_log.py b/rerun_py/rerun_sdk/rerun/_log.py index 5848c6f8401b..efcef469585d 100644 --- a/rerun_py/rerun_sdk/rerun/_log.py +++ b/rerun_py/rerun_sdk/rerun/_log.py @@ -54,14 +54,14 @@ def _splat() -> cmp.InstanceKeyBatch: @catch_and_log_exceptions() def log( - entity_path: str, + entity_path: str | list[str], entity: AsComponents | Iterable[ComponentBatchLike], *extra: AsComponents | Iterable[ComponentBatchLike], timeless: bool = False, recording: RecordingStream | None = None, strict: bool | None = None, ) -> None: - """ + r""" Log data to Rerun. This is the main entry point for logging data to rerun. It can be used to log anything @@ -98,6 +98,15 @@ def log( ---------- entity_path: Path to the entity in the space hierarchy. + + The entity path can either be a string + (with special characters escaped, split on unescaped slashes) + or a list of unescaped strings. + This means that logging to `"world/my\ image\!"` is the same as logging + to ["world", "my image!"]. + + See for more on entity paths. + entity: Anything that implements the [`rerun.AsComponents`][] interface, usually an archetype. *extra: @@ -163,7 +172,7 @@ def log( @catch_and_log_exceptions() def log_components( - entity_path: str, + entity_path: str | list[str], components: Iterable[ComponentBatchLike], *, num_instances: int | None = None, @@ -171,7 +180,7 @@ def log_components( recording: RecordingStream | None = None, strict: bool | None = None, ) -> None: - """ + r""" Log an entity from a collection of `ComponentBatchLike` objects. All of the batches should have the same length as the value of @@ -182,6 +191,15 @@ def log_components( ---------- entity_path: Path to the entity in the space hierarchy. + + The entity path can either be a string + (with special characters escaped, split on unescaped slashes) + or a list of unescaped strings. + This means that logging to `"world/my\ image\!"` is the same as logging + to ["world", "my image!"]. + + See for more on entity paths. + components: A collection of `ComponentBatchLike` objects that num_instances: @@ -211,6 +229,9 @@ def log_components( if num_instances is None: num_instances = max(len(arr) for arr in arrow_arrays) + if isinstance(entity_path, list): + entity_path = bindings.escape_entity_path([str(part) for part in entity_path]) + added = set() for name, array in zip(names, arrow_arrays): @@ -256,3 +277,24 @@ def log_components( timeless=timeless, recording=recording, ) + + +def escape_entity_path(entity_path: list[str]) -> str: + r""" + Construct an entity path, defined by a list of (unescaped) parts. + + For instance, `escape_entity_path(["world", "my image!"])` will return `"world/my\ image\!"`. + + See for more on entity paths. + + Parameters + ---------- + entity_path: + A list of strings to escape and join with slash. + + Returns + ------- + str: + The escaped entity path. + """ + return bindings.escape_entity_path([str(part) for part in entity_path]) diff --git a/rerun_py/src/python_bridge.rs b/rerun_py/src/python_bridge.rs index 5410c0382851..9f8febccecd0 100644 --- a/rerun_py/src/python_bridge.rs +++ b/rerun_py/src/python_bridge.rs @@ -15,7 +15,7 @@ use pyo3::{ //use re_viewport::{SpaceViewBlueprint, VIEWPORT_PATH}; use re_viewport::VIEWPORT_PATH; -use re_log_types::{DataRow, StoreKind}; +use re_log_types::{DataRow, EntityPathPart, StoreKind}; use rerun::{ log::RowId, sink::MemorySinkStorage, time::TimePoint, EntityPath, RecordingStream, RecordingStreamBuilder, StoreId, @@ -152,6 +152,7 @@ fn rerun_bindings(_py: Python<'_>, m: &PyModule) -> PyResult<()> { m.add_function(wrap_pyfunction!(version, m)?)?; m.add_function(wrap_pyfunction!(get_app_url, m)?)?; m.add_function(wrap_pyfunction!(start_web_viewer_server, m)?)?; + m.add_function(wrap_pyfunction!(escape_entity_path, m)?)?; // blueprint m.add_function(wrap_pyfunction!(set_panels, m)?)?; @@ -760,7 +761,7 @@ fn set_panel(entity_path: &str, is_expanded: bool, blueprint: Option<&PyRecordin use re_viewer::blueprint::components::PanelView; // TODO(jleibs): Validation this is a valid blueprint path? - let entity_path = parse_entity_path(entity_path); + let entity_path = EntityPath::parse_forgiving(entity_path); let panel_state = PanelView(is_expanded); @@ -867,7 +868,7 @@ fn log_arrow_msg( return Ok(()); }; - let entity_path = parse_entity_path(entity_path); + let entity_path = EntityPath::parse_forgiving(entity_path); // It's important that we don't hold the session lock while building our arrow component. // the API we call to back through pyarrow temporarily releases the GIL, which can cause @@ -938,6 +939,12 @@ fn start_web_viewer_server(port: u16) -> PyResult<()> { } } +#[pyfunction] +fn escape_entity_path(parts: Vec<&str>) -> String { + let path = EntityPath::from(parts.into_iter().map(EntityPathPart::from).collect_vec()); + path.to_string() +} + // --- Helpers --- fn python_version(py: Python<'_>) -> re_log_types::PythonVersion { @@ -1013,8 +1020,3 @@ authkey = multiprocessing.current_process().authkey }) .map(|authkey: &PyBytes| authkey.as_bytes().to_vec()) } - -fn parse_entity_path(entity_path: &str) -> EntityPath { - // We accept anything! - EntityPath::parse_forgiving(entity_path) -} From 1efc0f4bd47fea5e7d3d1b82c6e7c781412a9eb3 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 20 Dec 2023 08:36:00 +0100 Subject: [PATCH 02/10] Allow integers and other things that can be turned into strings as parts --- crates/re_log_types/src/path/mod.rs | 11 +++++++---- docs/code-examples/entity_path.cpp | 4 ++-- docs/code-examples/entity_path.py | 6 ++++-- docs/code-examples/entity_path.rs | 4 ++-- rerun_py/rerun_sdk/rerun/_log.py | 10 ++++++---- 5 files changed, 21 insertions(+), 14 deletions(-) diff --git a/crates/re_log_types/src/path/mod.rs b/crates/re_log_types/src/path/mod.rs index 166e8413996f..0284354ca24b 100644 --- a/crates/re_log_types/src/path/mod.rs +++ b/crates/re_log_types/src/path/mod.rs @@ -25,7 +25,7 @@ pub use parse_path::PathParseError; /// Build a `Vec`: /// ``` /// # use re_log_types::*; -/// let parts: Vec = entity_path_vec!("foo", "bar"); +/// let parts: Vec = entity_path_vec!("foo", 42, "my image!"); /// ``` #[macro_export] macro_rules! entity_path_vec { @@ -34,7 +34,10 @@ macro_rules! entity_path_vec { ::std::vec::Vec::<$crate::EntityPathPart>::new() }; ($($part: expr),* $(,)?) => { - vec![ $($crate::EntityPathPart::from($part),)+ ] + vec![ $($crate::EntityPathPart::from( + #[allow(clippy::str_to_string, clippy::string_to_string)] + $part.to_string() + ),)+ ] }; } @@ -42,8 +45,8 @@ macro_rules! entity_path_vec { /// /// ``` /// # use re_log_types::*; -/// let path: EntityPath = entity_path!("world", "my image!"); -/// assert_eq!(path, EntityPath::parse_strict(r"world/my\ image\!").unwrap()); +/// let path: EntityPath = entity_path!("world", 42, "my image!"); +/// assert_eq!(path, EntityPath::parse_strict(r"world/42/my\ image\!").unwrap()); /// ``` #[macro_export] macro_rules! entity_path { diff --git a/docs/code-examples/entity_path.cpp b/docs/code-examples/entity_path.cpp index 0a374c649ef0..2a25f7533359 100644 --- a/docs/code-examples/entity_path.cpp +++ b/docs/code-examples/entity_path.cpp @@ -7,12 +7,12 @@ int main() { rec.spawn().exit_on_failure(); rec.log( - R"(world/escaped\ string\!)", + R"(world/42/escaped\ string\!)", rerun::TextDocument("This entity path was escaped manually") ); // TODO: figure this one out // rec.log( - // rerun::entity_path ![ "world", "unescaped string!" ], + // {"world", 42, "unescaped string!"}, // rerun::TextDocument("This entity path was provided as a list of unescaped strings") // ); } diff --git a/docs/code-examples/entity_path.py b/docs/code-examples/entity_path.py index 7a0713fefa15..82a27102b24e 100644 --- a/docs/code-examples/entity_path.py +++ b/docs/code-examples/entity_path.py @@ -2,5 +2,7 @@ rr.init("rerun_example_entity_path", spawn=True) -rr.log(r"world/escaped\ string\!", rr.TextDocument("This entity path was escaped manually")) -rr.log(["world", "unescaped string!"], rr.TextDocument("This entity path was provided as a list of unescaped strings")) +rr.log(r"world/42/escaped\ string\!", rr.TextDocument("This entity path was escaped manually")) +rr.log( + ["world", 42, "unescaped string!"], rr.TextDocument("This entity path was provided as a list of unescaped strings") +) diff --git a/docs/code-examples/entity_path.rs b/docs/code-examples/entity_path.rs index 53b2347dc455..89b63bf2fd72 100644 --- a/docs/code-examples/entity_path.rs +++ b/docs/code-examples/entity_path.rs @@ -4,11 +4,11 @@ fn main() -> Result<(), Box> { let rec = rerun::RecordingStreamBuilder::new("rerun_example_text_document").spawn()?; rec.log( - r"world/escaped\ string\!", + r"world/42/escaped\ string\!", &rerun::TextDocument::new("This entity path was escaped manually"), )?; rec.log( - rerun::entity_path!["world", "unescaped string!"], + rerun::entity_path!["world", 42, "unescaped string!"], &rerun::TextDocument::new("This entity path was provided as a list of unescaped strings"), )?; diff --git a/rerun_py/rerun_sdk/rerun/_log.py b/rerun_py/rerun_sdk/rerun/_log.py index efcef469585d..2b5248db2f74 100644 --- a/rerun_py/rerun_sdk/rerun/_log.py +++ b/rerun_py/rerun_sdk/rerun/_log.py @@ -1,6 +1,6 @@ from __future__ import annotations -from typing import Iterable +from typing import Any, Iterable import pyarrow as pa import rerun_bindings as bindings @@ -279,11 +279,13 @@ def log_components( ) -def escape_entity_path(entity_path: list[str]) -> str: +def escape_entity_path(entity_path: list[Any]) -> str: r""" Construct an entity path, defined by a list of (unescaped) parts. - For instance, `escape_entity_path(["world", "my image!"])` will return `"world/my\ image\!"`. + If any part if not a string, it will be converted to a string using `str()`. + + For instance, `escape_entity_path(["world", 42, "my image!"])` will return `"world/42/my\ image\!"`. See for more on entity paths. @@ -297,4 +299,4 @@ def escape_entity_path(entity_path: list[str]) -> str: str: The escaped entity path. """ - return bindings.escape_entity_path([str(part) for part in entity_path]) + return str(bindings.escape_entity_path([str(part) for part in entity_path])) From ef73085303f7c5d614097f711f69b64e075053c5 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 20 Dec 2023 08:44:55 +0100 Subject: [PATCH 03/10] Cleanup --- docs/code-examples/entity_path.cpp | 2 +- docs/code-examples/entity_path.rs | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/docs/code-examples/entity_path.cpp b/docs/code-examples/entity_path.cpp index 2a25f7533359..140bcfdfa13f 100644 --- a/docs/code-examples/entity_path.cpp +++ b/docs/code-examples/entity_path.cpp @@ -10,7 +10,7 @@ int main() { R"(world/42/escaped\ string\!)", rerun::TextDocument("This entity path was escaped manually") ); - // TODO: figure this one out + // TODO(emilk): implement entity path escaping in C++ // rec.log( // {"world", 42, "unescaped string!"}, // rerun::TextDocument("This entity path was provided as a list of unescaped strings") diff --git a/docs/code-examples/entity_path.rs b/docs/code-examples/entity_path.rs index 89b63bf2fd72..cdfa860c7690 100644 --- a/docs/code-examples/entity_path.rs +++ b/docs/code-examples/entity_path.rs @@ -1,5 +1,3 @@ -//! Log a `TextDocument` - fn main() -> Result<(), Box> { let rec = rerun::RecordingStreamBuilder::new("rerun_example_text_document").spawn()?; From bb67c24da8901c0dfd12b080c6e1a03ea3f9f034 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 20 Dec 2023 09:16:33 +0100 Subject: [PATCH 04/10] Add easy entity path construction to C++ too --- crates/re_sdk/src/lib.rs | 4 +++- crates/rerun_c/src/lib.rs | 33 ++++++++++++++++++++++++++++ crates/rerun_c/src/rerun.h | 17 +++++++++++++++ docs/code-examples/entity_path.cpp | 9 ++++---- rerun_cpp/src/rerun.hpp | 1 + rerun_cpp/src/rerun/c/rerun.h | 17 +++++++++++++++ rerun_cpp/src/rerun/entity_path.cpp | 34 +++++++++++++++++++++++++++++ rerun_cpp/src/rerun/entity_path.hpp | 13 +++++++++++ 8 files changed, 122 insertions(+), 6 deletions(-) create mode 100644 rerun_cpp/src/rerun/entity_path.cpp create mode 100644 rerun_cpp/src/rerun/entity_path.hpp diff --git a/crates/re_sdk/src/lib.rs b/crates/re_sdk/src/lib.rs index ab677ba59510..4010d71b00cb 100644 --- a/crates/re_sdk/src/lib.rs +++ b/crates/re_sdk/src/lib.rs @@ -31,7 +31,9 @@ pub use self::recording_stream::{ pub use re_sdk_comms::{default_flush_timeout, default_server_addr}; -pub use re_log_types::{entity_path, ApplicationId, EntityPath, StoreId, StoreKind}; +pub use re_log_types::{ + entity_path, ApplicationId, EntityPath, EntityPathPart, StoreId, StoreKind, +}; pub use global::cleanup_if_forked_child; diff --git a/crates/rerun_c/src/lib.rs b/crates/rerun_c/src/lib.rs index fcbc4b9e9734..fa59f60590fa 100644 --- a/crates/rerun_c/src/lib.rs +++ b/crates/rerun_c/src/lib.rs @@ -703,6 +703,39 @@ pub unsafe extern "C" fn rr_recording_stream_log( } } +// ---------------------------------------------------------------------------- +// Private functions + +#[allow(unsafe_code)] +#[no_mangle] +pub unsafe extern "C" fn _rr_escape_entity_path_part(part: CStringView) -> *const c_char { + let Ok(part) = part.as_str("entity_path_part") else { + return std::ptr::null(); + }; + + let part = re_sdk::EntityPathPart::from(part).to_string(); // escape the part + + let Ok(part) = CString::new(part) else { + return std::ptr::null(); + }; + + part.into_raw() +} + +#[allow(unsafe_code)] +#[no_mangle] +pub unsafe extern "C" fn _rr_free_string(str: *mut c_char) { + if str.is_null() { + return; + } + + // Free the string: + unsafe { + // SAFETY: `_rr_free_string` should + let _ = CString::from_raw(str); + } +} + // ---------------------------------------------------------------------------- // Helper functions: diff --git a/crates/rerun_c/src/rerun.h b/crates/rerun_c/src/rerun.h index 34f72b1d3840..b1c3a9c4d586 100644 --- a/crates/rerun_c/src/rerun.h +++ b/crates/rerun_c/src/rerun.h @@ -412,6 +412,23 @@ extern void rr_recording_stream_log( rr_recording_stream stream, rr_data_row data_row, bool inject_time, rr_error* error ); +// ---------------------------------------------------------------------------- +// Private functions + +/// PRIVATE FUNCTION: do not use. +/// +/// Escape a single part of an entity path, returning an new null-terminated string. +/// +/// The returned string must be freed with `_rr_free_string`. +/// +/// Returns `nullptr` on failure (e.g. invalid UTF8, ore null bytes in the string). +extern char* _rr_escape_entity_path_part(rr_string part); + +/// PRIVATE FUNCTION: do not use. +/// +/// Must only be called with the results from `_rr_escape_entity_path_part`. +extern void _rr_free_string(char* string); + // ---------------------------------------------------------------------------- #ifdef __cplusplus diff --git a/docs/code-examples/entity_path.cpp b/docs/code-examples/entity_path.cpp index 140bcfdfa13f..e76a32df921a 100644 --- a/docs/code-examples/entity_path.cpp +++ b/docs/code-examples/entity_path.cpp @@ -10,9 +10,8 @@ int main() { R"(world/42/escaped\ string\!)", rerun::TextDocument("This entity path was escaped manually") ); - // TODO(emilk): implement entity path escaping in C++ - // rec.log( - // {"world", 42, "unescaped string!"}, - // rerun::TextDocument("This entity path was provided as a list of unescaped strings") - // ); + rec.log( + rerun::escape_entity_path({"world", std::to_string(42), "unescaped string!"}), + rerun::TextDocument("This entity path was provided as a list of unescaped strings") + ); } diff --git a/rerun_cpp/src/rerun.hpp b/rerun_cpp/src/rerun.hpp index 7ebd2635ffbc..abd7a6fc091d 100644 --- a/rerun_cpp/src/rerun.hpp +++ b/rerun_cpp/src/rerun.hpp @@ -10,6 +10,7 @@ #include "rerun/collection_adapter.hpp" #include "rerun/collection_adapter_builtins.hpp" #include "rerun/config.hpp" +#include "rerun/entity_path.hpp" #include "rerun/error.hpp" #include "rerun/recording_stream.hpp" #include "rerun/result.hpp" diff --git a/rerun_cpp/src/rerun/c/rerun.h b/rerun_cpp/src/rerun/c/rerun.h index 93a072b9c3df..51d8aa79aeea 100644 --- a/rerun_cpp/src/rerun/c/rerun.h +++ b/rerun_cpp/src/rerun/c/rerun.h @@ -412,6 +412,23 @@ extern void rr_recording_stream_log( rr_recording_stream stream, rr_data_row data_row, bool inject_time, rr_error* error ); +// ---------------------------------------------------------------------------- +// Private functions + +/// PRIVATE FUNCTION: do not use. +/// +/// Escape a single part of an entity path, returning an new null-terminated string. +/// +/// The returned string must be freed with `_rr_free_string`. +/// +/// Returns `nullptr` on failure (e.g. invalid UTF8, ore null bytes in the string). +extern char* _rr_escape_entity_path_part(rr_string part); + +/// PRIVATE FUNCTION: do not use. +/// +/// Must only be called with the results from `_rr_escape_entity_path_part`. +extern void _rr_free_string(char* string); + // ---------------------------------------------------------------------------- #ifdef __cplusplus diff --git a/rerun_cpp/src/rerun/entity_path.cpp b/rerun_cpp/src/rerun/entity_path.cpp new file mode 100644 index 000000000000..3e37ecf5b617 --- /dev/null +++ b/rerun_cpp/src/rerun/entity_path.cpp @@ -0,0 +1,34 @@ +#include "entity_path.hpp" + +#include "c/rerun.h" +#include "error.hpp" +#include "string_utils.hpp" + +namespace rerun { + std::string escape_entity_path(const std::vector& path) { + if (path.empty()) { + return "/"; + } + + std::string result; + + for (const auto& part : path) { + auto escaped_c_str = _rr_escape_entity_path_part(detail::to_rr_string(part)); + + if (escaped_c_str == nullptr) { + Error(ErrorCode::InvalidStringArgument, "Failed to escape entity path part") + .handle(); + } else { + if (!result.empty()) { + result += "/"; // leading slash would also have be fine + } + + result += escaped_c_str; + _rr_free_string(escaped_c_str); + } + } + + return result; + } + +} // namespace rerun diff --git a/rerun_cpp/src/rerun/entity_path.hpp b/rerun_cpp/src/rerun/entity_path.hpp new file mode 100644 index 000000000000..41a59c4c851f --- /dev/null +++ b/rerun_cpp/src/rerun/entity_path.hpp @@ -0,0 +1,13 @@ +#pragma once + +#include +#include + +namespace rerun { + /// Construct an entity path by escaping each part of the path. + /// + /// For instance, `rerun::escape_entity_path({"world", 42, "unescaped string!"})` will return + /// `"world/42/escaped\ string\!"`. + std::string escape_entity_path(const std::vector& path); + +} // namespace rerun From 38535a3e1e3c58c19feb9aa1584f8f8d55ca9181 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 20 Dec 2023 09:18:58 +0100 Subject: [PATCH 05/10] Rename `escape_entity_path` -> `new_entity_path` --- docs/code-examples/entity_path.cpp | 2 +- rerun_cpp/src/rerun/entity_path.cpp | 3 +-- rerun_cpp/src/rerun/entity_path.hpp | 4 ++-- rerun_py/rerun_sdk/rerun/_log.py | 8 ++++---- rerun_py/src/python_bridge.rs | 4 ++-- 5 files changed, 10 insertions(+), 11 deletions(-) diff --git a/docs/code-examples/entity_path.cpp b/docs/code-examples/entity_path.cpp index e76a32df921a..e0b05528fa87 100644 --- a/docs/code-examples/entity_path.cpp +++ b/docs/code-examples/entity_path.cpp @@ -11,7 +11,7 @@ int main() { rerun::TextDocument("This entity path was escaped manually") ); rec.log( - rerun::escape_entity_path({"world", std::to_string(42), "unescaped string!"}), + rerun::new_entity_path({"world", std::to_string(42), "unescaped string!"}), rerun::TextDocument("This entity path was provided as a list of unescaped strings") ); } diff --git a/rerun_cpp/src/rerun/entity_path.cpp b/rerun_cpp/src/rerun/entity_path.cpp index 3e37ecf5b617..567b5121f8f7 100644 --- a/rerun_cpp/src/rerun/entity_path.cpp +++ b/rerun_cpp/src/rerun/entity_path.cpp @@ -5,7 +5,7 @@ #include "string_utils.hpp" namespace rerun { - std::string escape_entity_path(const std::vector& path) { + std::string new_entity_path(const std::vector& path) { if (path.empty()) { return "/"; } @@ -22,7 +22,6 @@ namespace rerun { if (!result.empty()) { result += "/"; // leading slash would also have be fine } - result += escaped_c_str; _rr_free_string(escaped_c_str); } diff --git a/rerun_cpp/src/rerun/entity_path.hpp b/rerun_cpp/src/rerun/entity_path.hpp index 41a59c4c851f..38f33ae8bad0 100644 --- a/rerun_cpp/src/rerun/entity_path.hpp +++ b/rerun_cpp/src/rerun/entity_path.hpp @@ -6,8 +6,8 @@ namespace rerun { /// Construct an entity path by escaping each part of the path. /// - /// For instance, `rerun::escape_entity_path({"world", 42, "unescaped string!"})` will return + /// For instance, `rerun::new_entity_path({"world", 42, "unescaped string!"})` will return /// `"world/42/escaped\ string\!"`. - std::string escape_entity_path(const std::vector& path); + std::string new_entity_path(const std::vector& path); } // namespace rerun diff --git a/rerun_py/rerun_sdk/rerun/_log.py b/rerun_py/rerun_sdk/rerun/_log.py index 2b5248db2f74..c9e10bc27e8d 100644 --- a/rerun_py/rerun_sdk/rerun/_log.py +++ b/rerun_py/rerun_sdk/rerun/_log.py @@ -230,7 +230,7 @@ def log_components( num_instances = max(len(arr) for arr in arrow_arrays) if isinstance(entity_path, list): - entity_path = bindings.escape_entity_path([str(part) for part in entity_path]) + entity_path = bindings.new_entity_path([str(part) for part in entity_path]) added = set() @@ -279,13 +279,13 @@ def log_components( ) -def escape_entity_path(entity_path: list[Any]) -> str: +def new_entity_path(entity_path: list[Any]) -> str: r""" Construct an entity path, defined by a list of (unescaped) parts. If any part if not a string, it will be converted to a string using `str()`. - For instance, `escape_entity_path(["world", 42, "my image!"])` will return `"world/42/my\ image\!"`. + For instance, `new_entity_path(["world", 42, "my image!"])` will return `"world/42/my\ image\!"`. See for more on entity paths. @@ -299,4 +299,4 @@ def escape_entity_path(entity_path: list[Any]) -> str: str: The escaped entity path. """ - return str(bindings.escape_entity_path([str(part) for part in entity_path])) + return str(bindings.new_entity_path([str(part) for part in entity_path])) diff --git a/rerun_py/src/python_bridge.rs b/rerun_py/src/python_bridge.rs index 9f8febccecd0..e270e5e045f9 100644 --- a/rerun_py/src/python_bridge.rs +++ b/rerun_py/src/python_bridge.rs @@ -152,7 +152,7 @@ fn rerun_bindings(_py: Python<'_>, m: &PyModule) -> PyResult<()> { m.add_function(wrap_pyfunction!(version, m)?)?; m.add_function(wrap_pyfunction!(get_app_url, m)?)?; m.add_function(wrap_pyfunction!(start_web_viewer_server, m)?)?; - m.add_function(wrap_pyfunction!(escape_entity_path, m)?)?; + m.add_function(wrap_pyfunction!(new_entity_path, m)?)?; // blueprint m.add_function(wrap_pyfunction!(set_panels, m)?)?; @@ -940,7 +940,7 @@ fn start_web_viewer_server(port: u16) -> PyResult<()> { } #[pyfunction] -fn escape_entity_path(parts: Vec<&str>) -> String { +fn new_entity_path(parts: Vec<&str>) -> String { let path = EntityPath::from(parts.into_iter().map(EntityPathPart::from).collect_vec()); path.to_string() } From b14f7d2fcbb4e7e3857ffceee0c9889d7be9d80f Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 20 Dec 2023 11:16:22 +0100 Subject: [PATCH 06/10] finish comment --- crates/rerun_c/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/rerun_c/src/lib.rs b/crates/rerun_c/src/lib.rs index fa59f60590fa..124120841ad8 100644 --- a/crates/rerun_c/src/lib.rs +++ b/crates/rerun_c/src/lib.rs @@ -731,7 +731,7 @@ pub unsafe extern "C" fn _rr_free_string(str: *mut c_char) { // Free the string: unsafe { - // SAFETY: `_rr_free_string` should + // SAFETY: `_rr_free_string` should only be called on strings allocated by `_rr_escape_entity_path_part`. let _ = CString::from_raw(str); } } From e519509e1cbbdcf3424c4a80f77c33047a846139 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 20 Dec 2023 11:19:26 +0100 Subject: [PATCH 07/10] The C++ roundtrip is implemented --- docs/code-examples/roundtrips.py | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/code-examples/roundtrips.py b/docs/code-examples/roundtrips.py index 6208cdcef3ae..10b32ecfd4ef 100755 --- a/docs/code-examples/roundtrips.py +++ b/docs/code-examples/roundtrips.py @@ -41,7 +41,6 @@ opt_out_compare = { "arrow3d_simple": ["cpp", "py", "rust"], # TODO(#3206): examples use different RNGs "asset3d_out_of_tree": ["cpp", "py", "rust"], # float issues since calculation is done slightly differently (also, Python uses doubles) - "entity_path": ["cpp"], # C++ doesn't have helpers for escaping an entity path yet "mesh3d_partial_updates": ["cpp", "py", "rust"], # float precision issues "pinhole_simple": ["cpp", "py", "rust"], # TODO(#3206): examples use different RNGs "point2d_random": ["cpp", "py", "rust"], # TODO(#3206): examples use different RNGs From 2fd7fcbf88b9603cae6b599ccde925b23b8ca02c Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 20 Dec 2023 13:03:25 +0100 Subject: [PATCH 08/10] Add docstring --- docs/code-examples/entity_path.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/code-examples/entity_path.rs b/docs/code-examples/entity_path.rs index cdfa860c7690..49bde42e2bc3 100644 --- a/docs/code-examples/entity_path.rs +++ b/docs/code-examples/entity_path.rs @@ -1,3 +1,5 @@ +//! Example of different ways of constructing an entity path. + fn main() -> Result<(), Box> { let rec = rerun::RecordingStreamBuilder::new("rerun_example_text_document").spawn()?; From 2e0279917bf4953294933085c3f62d5ea7bafe8a Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 20 Dec 2023 13:24:02 +0100 Subject: [PATCH 09/10] Add and test `escape_entity_path_part` and `new_entity_path` --- docs/code-examples/entity_path.cpp | 3 +++ docs/code-examples/entity_path.py | 3 +++ rerun_cpp/src/rerun/entity_path.cpp | 27 +++++++++++++++------------ rerun_cpp/src/rerun/entity_path.hpp | 6 ++++++ rerun_py/docs/gen_common_index.py | 2 ++ rerun_py/rerun_sdk/rerun/__init__.py | 14 ++++++++++++-- rerun_py/rerun_sdk/rerun/_log.py | 21 +++++++++++++++++++++ rerun_py/src/python_bridge.rs | 6 ++++++ 8 files changed, 68 insertions(+), 14 deletions(-) diff --git a/docs/code-examples/entity_path.cpp b/docs/code-examples/entity_path.cpp index e0b05528fa87..c16377e5d5b9 100644 --- a/docs/code-examples/entity_path.cpp +++ b/docs/code-examples/entity_path.cpp @@ -14,4 +14,7 @@ int main() { rerun::new_entity_path({"world", std::to_string(42), "unescaped string!"}), rerun::TextDocument("This entity path was provided as a list of unescaped strings") ); + + assert(rerun::escape_entity_path_part("my string!") == R"(my\ string\!)"); + assert(rerun::new_entity_path({"world", "42", "my string!"}) == R"(/world/42/my\ string\!)"); } diff --git a/docs/code-examples/entity_path.py b/docs/code-examples/entity_path.py index 82a27102b24e..233329ef45d3 100644 --- a/docs/code-examples/entity_path.py +++ b/docs/code-examples/entity_path.py @@ -6,3 +6,6 @@ rr.log( ["world", 42, "unescaped string!"], rr.TextDocument("This entity path was provided as a list of unescaped strings") ) + +assert rr.escape_entity_path_part("my string!") == r"my\ string\!" +assert rr.new_entity_path(["world", 42, "my string!"]) == r"/world/42/my\ string\!" diff --git a/rerun_cpp/src/rerun/entity_path.cpp b/rerun_cpp/src/rerun/entity_path.cpp index 567b5121f8f7..bcf7add0c38d 100644 --- a/rerun_cpp/src/rerun/entity_path.cpp +++ b/rerun_cpp/src/rerun/entity_path.cpp @@ -5,6 +5,19 @@ #include "string_utils.hpp" namespace rerun { + std::string escape_entity_path_part(std::string_view unescaped) { + auto escaped_c_str = _rr_escape_entity_path_part(detail::to_rr_string(unescaped)); + + if (escaped_c_str == nullptr) { + Error(ErrorCode::InvalidStringArgument, "Failed to escape entity path part").handle(); + return std::string(unescaped); + } else { + std::string result = escaped_c_str; + _rr_free_string(escaped_c_str); + return result; + } + } + std::string new_entity_path(const std::vector& path) { if (path.empty()) { return "/"; @@ -13,18 +26,8 @@ namespace rerun { std::string result; for (const auto& part : path) { - auto escaped_c_str = _rr_escape_entity_path_part(detail::to_rr_string(part)); - - if (escaped_c_str == nullptr) { - Error(ErrorCode::InvalidStringArgument, "Failed to escape entity path part") - .handle(); - } else { - if (!result.empty()) { - result += "/"; // leading slash would also have be fine - } - result += escaped_c_str; - _rr_free_string(escaped_c_str); - } + result += "/"; + result += escape_entity_path_part(part); } return result; diff --git a/rerun_cpp/src/rerun/entity_path.hpp b/rerun_cpp/src/rerun/entity_path.hpp index 38f33ae8bad0..b660bb81c83e 100644 --- a/rerun_cpp/src/rerun/entity_path.hpp +++ b/rerun_cpp/src/rerun/entity_path.hpp @@ -4,6 +4,12 @@ #include namespace rerun { + + /// Escape an individual part of an entity path. + /// + /// For instance, `escape_entity_path_parth("my image!")` will return `"my\ image\!"`. + std::string escape_entity_path_part(std::string_view str); + /// Construct an entity path by escaping each part of the path. /// /// For instance, `rerun::new_entity_path({"world", 42, "unescaped string!"})` will return diff --git a/rerun_py/docs/gen_common_index.py b/rerun_py/docs/gen_common_index.py index fc406e00cf29..6b0b88f7d2e1 100755 --- a/rerun_py/docs/gen_common_index.py +++ b/rerun_py/docs/gen_common_index.py @@ -233,6 +233,8 @@ class Section: "set_global_data_recording", "set_thread_local_data_recording", "start_web_viewer_server", + "escape_entity_path_part", + "new_entity_path", ], class_list=["RecordingStream", "LoggingHandler", "MemoryRecording"], ), diff --git a/rerun_py/rerun_sdk/rerun/__init__.py b/rerun_py/rerun_sdk/rerun/__init__.py index c6244024af2e..458dd17b6495 100644 --- a/rerun_py/rerun_sdk/rerun/__init__.py +++ b/rerun_py/rerun_sdk/rerun/__init__.py @@ -65,6 +65,7 @@ "datatypes", "disable_timeline", "disconnect", + "escape_entity_path_part", "experimental", "get_application_id", "get_data_recording", @@ -72,9 +73,10 @@ "get_recording_id", "get_thread_local_data_recording", "is_enabled", - "log", "log_components", + "log", "memory_recording", + "new_entity_path", "reset_time", "save", "script_add_args", @@ -92,7 +94,15 @@ import rerun_bindings as bindings # type: ignore[attr-defined] from ._image import ImageEncoded, ImageFormat -from ._log import AsComponents, ComponentBatchLike, IndicatorComponentBatch, log, log_components +from ._log import ( + AsComponents, + ComponentBatchLike, + IndicatorComponentBatch, + escape_entity_path_part, + log, + log_components, + new_entity_path, +) from .any_value import AnyValues from .archetypes import ( AnnotationContext, diff --git a/rerun_py/rerun_sdk/rerun/_log.py b/rerun_py/rerun_sdk/rerun/_log.py index c9e10bc27e8d..ccd274c8e18b 100644 --- a/rerun_py/rerun_sdk/rerun/_log.py +++ b/rerun_py/rerun_sdk/rerun/_log.py @@ -279,6 +279,27 @@ def log_components( ) +def escape_entity_path_part(part: str) -> str: + r""" + Escape an individual part of an entity path. + + For instance, `escape_entity_path_parth("my image!")` will return `"my\ image\!"`. + + See for more on entity paths. + + Parameters + ---------- + part: + An unescaped string + + Returns + ------- + str: + The escaped entity path. + """ + return str(bindings.escape_entity_path_part(part)) + + def new_entity_path(entity_path: list[Any]) -> str: r""" Construct an entity path, defined by a list of (unescaped) parts. diff --git a/rerun_py/src/python_bridge.rs b/rerun_py/src/python_bridge.rs index e270e5e045f9..8a7021294338 100644 --- a/rerun_py/src/python_bridge.rs +++ b/rerun_py/src/python_bridge.rs @@ -152,6 +152,7 @@ fn rerun_bindings(_py: Python<'_>, m: &PyModule) -> PyResult<()> { m.add_function(wrap_pyfunction!(version, m)?)?; m.add_function(wrap_pyfunction!(get_app_url, m)?)?; m.add_function(wrap_pyfunction!(start_web_viewer_server, m)?)?; + m.add_function(wrap_pyfunction!(escape_entity_path_part, m)?)?; m.add_function(wrap_pyfunction!(new_entity_path, m)?)?; // blueprint @@ -939,6 +940,11 @@ fn start_web_viewer_server(port: u16) -> PyResult<()> { } } +#[pyfunction] +fn escape_entity_path_part(part: &str) -> String { + EntityPathPart::from(part).to_string() +} + #[pyfunction] fn new_entity_path(parts: Vec<&str>) -> String { let path = EntityPath::from(parts.into_iter().map(EntityPathPart::from).collect_vec()); From a5103b62304217c31d9c410aa435a54406e24161 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 20 Dec 2023 13:34:40 +0100 Subject: [PATCH 10/10] fix typo --- rerun_cpp/src/rerun/entity_path.hpp | 2 +- rerun_py/rerun_sdk/rerun/_log.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rerun_cpp/src/rerun/entity_path.hpp b/rerun_cpp/src/rerun/entity_path.hpp index b660bb81c83e..87eb633c4fe8 100644 --- a/rerun_cpp/src/rerun/entity_path.hpp +++ b/rerun_cpp/src/rerun/entity_path.hpp @@ -7,7 +7,7 @@ namespace rerun { /// Escape an individual part of an entity path. /// - /// For instance, `escape_entity_path_parth("my image!")` will return `"my\ image\!"`. + /// For instance, `escape_entity_path_path("my image!")` will return `"my\ image\!"`. std::string escape_entity_path_part(std::string_view str); /// Construct an entity path by escaping each part of the path. diff --git a/rerun_py/rerun_sdk/rerun/_log.py b/rerun_py/rerun_sdk/rerun/_log.py index ccd274c8e18b..a637ed5ab9a0 100644 --- a/rerun_py/rerun_sdk/rerun/_log.py +++ b/rerun_py/rerun_sdk/rerun/_log.py @@ -283,7 +283,7 @@ def escape_entity_path_part(part: str) -> str: r""" Escape an individual part of an entity path. - For instance, `escape_entity_path_parth("my image!")` will return `"my\ image\!"`. + For instance, `escape_entity_path_path("my image!")` will return `"my\ image\!"`. See for more on entity paths.