Skip to content

Commit b8fdb08

Browse files
author
Ubuntu
committed
fix(viewer): restore click-to-nav via StartupOptionsPatch
PR #14's run_with_app_wrapper rewrote viewer.rs and dropped the on_event handler for click-to-navigate. This restores it properly: - Add StartupOptionsPatch struct to rerun entrypoint with on_event callback - Wire Ctrl+click -> PointStamped LCM on /clicked_point in viewer.rs - Arc<AtomicBool> for ctrl state sharing (Send required by AppWrapper) - 100ms debounce on nav goal publishing
1 parent 52038a4 commit b8fdb08

5 files changed

Lines changed: 122 additions & 52 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3110,7 +3110,7 @@ dependencies = [
31103110

31113111
[[package]]
31123112
name = "dimos-viewer"
3113-
version = "0.30.0-alpha.4"
3113+
version = "0.30.0-alpha.5"
31143114
dependencies = [
31153115
"bincode",
31163116
"clap",

crates/top/rerun/src/commands/entrypoint.rs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1708,6 +1708,13 @@ fn record_cli_command_analytics(args: &Args) {
17081708
/// Used by dimos-viewer to inject keyboard teleop and other behaviors.
17091709
pub type AppWrapper = Box<dyn FnOnce(re_viewer::App) -> Result<Box<dyn re_viewer::external::eframe::App>, Box<dyn std::error::Error + Send + Sync>> + Send>;
17101710

1711+
/// Optional patches to [`re_viewer::StartupOptions`] injected by the app wrapper.
1712+
#[derive(Default)]
1713+
pub struct StartupOptionsPatch {
1714+
/// Callback invoked on viewer events (e.g. SelectionChange for click-to-nav).
1715+
pub on_event: Option<std::rc::Rc<dyn Fn(re_viewer::ViewerEvent)>>,
1716+
}
1717+
17111718
/// Like [`run`], but accepts an optional `app_wrapper` callback that wraps the
17121719
/// viewer App before it is handed to eframe. When `app_wrapper` is `None`,
17131720
/// behavior is identical to stock Rerun.
@@ -1720,6 +1727,7 @@ pub fn run_with_app_wrapper<I, T>(
17201727
call_source: CallSource,
17211728
args: I,
17221729
app_wrapper: Option<AppWrapper>,
1730+
startup_patch: Option<StartupOptionsPatch>,
17231731
) -> anyhow::Result<u8>
17241732
where
17251733
I: IntoIterator<Item = T>,
@@ -1812,6 +1820,7 @@ where
18121820
#[cfg(feature = "native_viewer")]
18131821
profiler,
18141822
app_wrapper,
1823+
startup_patch,
18151824
)
18161825
};
18171826

@@ -1838,6 +1847,7 @@ fn run_impl_with_wrapper(
18381847
tokio_runtime_handle: &tokio::runtime::Handle,
18391848
#[cfg(feature = "native_viewer")] profiler: re_tracing::Profiler,
18401849
app_wrapper: Option<AppWrapper>,
1850+
startup_patch: Option<StartupOptionsPatch>,
18411851
) -> anyhow::Result<()> {
18421852
let connection_registry = re_redap_client::ConnectionRegistry::new_with_stored_credentials();
18431853

@@ -1959,6 +1969,7 @@ fn run_impl_with_wrapper(
19591969
#[cfg(feature = "server")]
19601970
server_options,
19611971
app_wrapper,
1972+
startup_patch,
19621973
)
19631974
} else {
19641975
Err(anyhow::anyhow!(
@@ -1985,11 +1996,17 @@ fn start_native_viewer_with_wrapper(
19851996
#[cfg(feature = "server")] server_addr: std::net::SocketAddr,
19861997
#[cfg(feature = "server")] server_options: re_sdk::ServerOptions,
19871998
app_wrapper: Option<AppWrapper>,
1999+
startup_patch: Option<StartupOptionsPatch>,
19882000
) -> anyhow::Result<()> {
19892001
use re_viewer::external::re_viewer_context;
19902002
use crate::external::re_ui::{UICommand, UICommandSender as _};
19912003

1992-
let startup_options = native_startup_options_from_args(args)?;
2004+
let mut startup_options = native_startup_options_from_args(args)?;
2005+
if let Some(patch) = startup_patch {
2006+
if patch.on_event.is_some() {
2007+
startup_options.on_event = patch.on_event;
2008+
}
2009+
}
19932010

19942011
let connect = args.connect.is_some();
19952012
let follow = args.follow;

crates/top/rerun/src/commands/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ mod analytics;
3535

3636
#[cfg(feature = "analytics")]
3737
pub(crate) use self::analytics::AnalyticsCommands;
38-
pub use self::entrypoint::{run, run_with_app_wrapper, AppWrapper, Args as RerunArgs, native_startup_options_from_args};
38+
pub use self::entrypoint::{run, run_with_app_wrapper, AppWrapper, StartupOptionsPatch, Args as RerunArgs, native_startup_options_from_args};
3939
#[cfg(feature = "data_loaders")]
4040
pub use self::mcap::McapCommands;
4141
pub use self::rrd::RrdCommands;

crates/top/rerun/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ pub mod demo_util;
124124
pub mod log_integration;
125125

126126
#[cfg(feature = "run")]
127-
pub use commands::{CallSource, run, run_with_app_wrapper, AppWrapper, RerunArgs, native_startup_options_from_args};
127+
pub use commands::{CallSource, run, run_with_app_wrapper, AppWrapper, StartupOptionsPatch, RerunArgs, native_startup_options_from_args};
128128
#[cfg(feature = "log")]
129129
pub use log_integration::Logger;
130130
#[cfg(feature = "log")]

dimos/src/viewer.rs

Lines changed: 101 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,83 +1,135 @@
11
//! DimOS Interactive Viewer — custom Rerun viewer with LCM click-to-navigate and WASD teleop.
22
//!
33
//! Accepts ALL stock Rerun CLI flags and adds DimOS-specific behavior:
4-
//! - Click-to-navigate: clicks publish PointStamped LCM on /clicked_point
5-
//! - WASD keyboard teleop: publishes Twist LCM on /cmd_vel
6-
//!
7-
//! ```bash
8-
//! dimos-viewer # standalone
9-
//! dimos-viewer --connect rerun+http://127.0.0.1:9876/proxy # connect to source
10-
//! dimos-viewer --port 9877 --memory-limit 2GB # custom port/memory
11-
//! dimos-viewer --serve-web # web viewer + gRPC
12-
//! dimos-viewer --serve-grpc # headless gRPC only
13-
//! dimos-viewer recording.rrd # open recording
14-
//! ```
15-
16-
use dimos_viewer::interaction::KeyboardHandler;
4+
//! - Click-to-navigate: click any entity with a 3D position → PointStamped LCM on /clicked_point
5+
//! - WASD keyboard teleop: click overlay to engage, then WASD publishes Twist on /cmd_vel
6+
7+
use std::rc::Rc;
8+
use std::cell::RefCell;
9+
use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH};
10+
11+
use dimos_viewer::interaction::{KeyboardHandler, LcmPublisher, click_event_from_ms};
1712
use rerun::external::{eframe, egui, re_memory, re_viewer};
1813

1914
#[global_allocator]
2015
static GLOBAL: re_memory::AccountingAllocator<mimalloc::MiMalloc> =
2116
re_memory::AccountingAllocator::new(mimalloc::MiMalloc);
2217

23-
/// Wraps re_viewer::App to add keyboard teleop and click-to-nav overlay.
18+
/// LCM channel for click events (follows RViz convention)
19+
const LCM_CHANNEL: &str = "/clicked_point#geometry_msgs.PointStamped";
20+
/// Minimum time between click events (debouncing)
21+
const CLICK_DEBOUNCE_MS: u64 = 100;
22+
/// Maximum rapid clicks before logging a warning
23+
const RAPID_CLICK_THRESHOLD: usize = 5;
24+
25+
/// Wraps re_viewer::App to add keyboard teleop overlay.
2426
struct DimosApp {
2527
inner: re_viewer::App,
2628
keyboard: KeyboardHandler,
2729
}
2830

29-
impl DimosApp {
30-
fn new(inner: re_viewer::App, keyboard: KeyboardHandler) -> Self {
31-
Self { inner, keyboard }
32-
}
33-
}
34-
3531
impl eframe::App for DimosApp {
3632
fn ui(&mut self, ui: &mut egui::Ui, frame: &mut eframe::Frame) {
3733
self.keyboard.process(ui.ctx());
3834
self.keyboard.draw_overlay(ui.ctx());
3935
self.inner.ui(ui, frame);
4036
}
4137

42-
fn save(&mut self, storage: &mut dyn eframe::Storage) {
43-
self.inner.save(storage);
44-
}
45-
46-
fn clear_color(&self, visuals: &egui::Visuals) -> [f32; 4] {
47-
self.inner.clear_color(visuals)
48-
}
49-
50-
fn persist_egui_memory(&self) -> bool {
51-
self.inner.persist_egui_memory()
52-
}
53-
54-
fn auto_save_interval(&self) -> std::time::Duration {
55-
self.inner.auto_save_interval()
56-
}
57-
38+
fn save(&mut self, storage: &mut dyn eframe::Storage) { self.inner.save(storage); }
39+
fn clear_color(&self, visuals: &egui::Visuals) -> [f32; 4] { self.inner.clear_color(visuals) }
40+
fn persist_egui_memory(&self) -> bool { self.inner.persist_egui_memory() }
41+
fn auto_save_interval(&self) -> Duration { self.inner.auto_save_interval() }
5842
fn raw_input_hook(&mut self, ctx: &egui::Context, raw_input: &mut egui::RawInput) {
5943
self.inner.raw_input_hook(ctx, raw_input);
6044
}
6145
}
6246

6347
fn main() -> Result<(), Box<dyn std::error::Error>> {
64-
// Delegate ALL CLI handling to Rerun's entrypoint with our DimosApp wrapper.
65-
//
66-
// `run_with_app_wrapper` handles:
67-
// - Full Rerun CLI arg parsing (--connect, --port, --memory-limit, etc.)
68-
// - --version, subcommands (reset, rrd, auth, etc.)
69-
// - Data source routing (--serve-grpc, --serve-web, .rrd files, etc.)
70-
// - Native viewer startup (where our wrapper injects DimosApp)
71-
//
72-
// The wrapper is ONLY called for the native viewer path. All other modes
73-
// (--serve-grpc, --serve-web, --save, etc.) work identically to stock Rerun.
7448
let main_thread_token = re_viewer::MainThreadToken::i_promise_i_am_on_the_main_thread();
7549
let build_info = re_viewer::build_info();
7650

77-
let wrapper: rerun::AppWrapper = Box::new(|app| {
51+
let lcm_publisher = LcmPublisher::new(LCM_CHANNEL.to_string())
52+
.expect("Failed to create LCM publisher");
53+
54+
let last_click_time = Rc::new(RefCell::new(
55+
Instant::now() - Duration::from_secs(10)
56+
));
57+
let rapid_click_count = Rc::new(RefCell::new(0usize));
58+
59+
// Plain click (no Ctrl required) fires nav goal on any entity with a 3D position
60+
let startup_patch = rerun::StartupOptionsPatch {
61+
on_event: Some(Rc::new(move |event: re_viewer::ViewerEvent| {
62+
if let re_viewer::ViewerEventKind::SelectionChange { items } = event.kind {
63+
let mut has_position = false;
64+
let mut no_position_count = 0;
65+
66+
for item in &items {
67+
match item {
68+
re_viewer::SelectionChangeItem::Entity {
69+
entity_path,
70+
position: Some(pos),
71+
..
72+
} => {
73+
has_position = true;
74+
75+
let now = Instant::now();
76+
let elapsed = now.duration_since(*last_click_time.borrow());
77+
78+
if elapsed < Duration::from_millis(CLICK_DEBOUNCE_MS) {
79+
let mut count = rapid_click_count.borrow_mut();
80+
*count += 1;
81+
if *count == RAPID_CLICK_THRESHOLD {
82+
rerun::external::re_log::warn!(
83+
"Rapid click detected ({RAPID_CLICK_THRESHOLD} clicks within {CLICK_DEBOUNCE_MS}ms)"
84+
);
85+
}
86+
continue;
87+
} else {
88+
*rapid_click_count.borrow_mut() = 0;
89+
}
90+
*last_click_time.borrow_mut() = now;
91+
92+
let ts = SystemTime::now()
93+
.duration_since(UNIX_EPOCH)
94+
.unwrap_or_default()
95+
.as_millis() as u64;
96+
97+
let click = click_event_from_ms(
98+
[pos.x, pos.y, pos.z],
99+
&entity_path.to_string(),
100+
ts,
101+
);
102+
103+
match lcm_publisher.publish(&click) {
104+
Ok(_) => rerun::external::re_log::debug!(
105+
"Nav goal: entity={}, pos=({:.2}, {:.2}, {:.2})",
106+
entity_path, pos.x, pos.y, pos.z
107+
),
108+
Err(e) => rerun::external::re_log::error!(
109+
"Failed to publish nav goal: {e:?}"
110+
),
111+
}
112+
}
113+
re_viewer::SelectionChangeItem::Entity { position: None, .. } => {
114+
no_position_count += 1;
115+
}
116+
_ => {}
117+
}
118+
}
119+
120+
if !has_position && no_position_count > 0 {
121+
rerun::external::re_log::trace!(
122+
"Selection change without position ({no_position_count} items) — normal for hover/keyboard nav."
123+
);
124+
}
125+
}
126+
})),
127+
};
128+
129+
let wrapper: rerun::AppWrapper = Box::new(move |app| {
78130
let keyboard = KeyboardHandler::new()
79131
.map_err(|e| -> Box<dyn std::error::Error + Send + Sync> { Box::new(e) })?;
80-
Ok(Box::new(DimosApp::new(app, keyboard)))
132+
Ok(Box::new(DimosApp { inner: app, keyboard }))
81133
});
82134

83135
let exit_code = rerun::run_with_app_wrapper(
@@ -86,6 +138,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
86138
rerun::CallSource::Cli,
87139
std::env::args(),
88140
Some(wrapper),
141+
Some(startup_patch),
89142
)?;
90143

91144
std::process::exit(exit_code.into());

0 commit comments

Comments
 (0)