Skip to content

Commit 9d199eb

Browse files
committed
implement consistent open/import behavior for all cases
1 parent 3886323 commit 9d199eb

File tree

8 files changed

+96
-23
lines changed

8 files changed

+96
-23
lines changed

Cargo.lock

+1
Original file line numberDiff line numberDiff line change
@@ -6415,6 +6415,7 @@ dependencies = [
64156415
"strum",
64166416
"strum_macros",
64176417
"thiserror",
6418+
"uuid",
64186419
"wasm-bindgen",
64196420
"wasm-bindgen-futures",
64206421
"web-sys",

crates/store/re_data_loader/src/lib.rs

+8
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,12 @@ pub struct DataLoaderSettings {
6464
/// The [`re_log_types::StoreId`] that is currently opened in the viewer, if any.
6565
pub opened_store_id: Option<re_log_types::StoreId>,
6666

67+
/// Whether `SetStoreInfo`s should be sent, regardless of the surrounding context.
68+
///
69+
/// Only useful when creating a recording just-in-time directly in the viewer (which is what
70+
/// happens when importing things into the welcome screen).
71+
pub force_store_info: bool,
72+
6773
/// What should the logged entity paths be prefixed with?
6874
pub entity_path_prefix: Option<EntityPath>,
6975

@@ -79,6 +85,7 @@ impl DataLoaderSettings {
7985
opened_application_id: Default::default(),
8086
store_id: store_id.into(),
8187
opened_store_id: Default::default(),
88+
force_store_info: false,
8289
entity_path_prefix: Default::default(),
8390
timepoint: Default::default(),
8491
}
@@ -91,6 +98,7 @@ impl DataLoaderSettings {
9198
opened_application_id,
9299
store_id,
93100
opened_store_id,
101+
force_store_info: _,
94102
entity_path_prefix,
95103
timepoint,
96104
} = self;

crates/store/re_data_loader/src/load_file.rs

+11-6
Original file line numberDiff line numberDiff line change
@@ -73,21 +73,20 @@ pub fn load_from_file_contents(
7373

7474
/// Prepares an adequate [`re_log_types::StoreInfo`] [`LogMsg`] given the input.
7575
pub(crate) fn prepare_store_info(
76+
application_id: re_log_types::ApplicationId,
7677
store_id: &re_log_types::StoreId,
7778
file_source: FileSource,
78-
path: &std::path::Path,
7979
) -> LogMsg {
80-
re_tracing::profile_function!(path.display().to_string());
80+
re_tracing::profile_function!();
8181

8282
use re_log_types::SetStoreInfo;
8383

84-
let app_id = re_log_types::ApplicationId(path.display().to_string());
8584
let store_source = re_log_types::StoreSource::File { file_source };
8685

8786
LogMsg::SetStoreInfo(SetStoreInfo {
8887
row_id: *re_chunk::RowId::new(),
8988
info: re_log_types::StoreInfo {
90-
application_id: app_id.clone(),
89+
application_id,
9190
store_id: store_id.clone(),
9291
cloned_from: None,
9392
is_official_example: false,
@@ -310,11 +309,17 @@ pub(crate) fn send(
310309
let is_a_preexisting_recording =
311310
Some(&store_id) == settings.opened_store_id.as_ref();
312311

313-
if store_info_already_created || is_a_preexisting_recording {
312+
if !settings.force_store_info
313+
&& (store_info_already_created || is_a_preexisting_recording)
314+
{
314315
continue;
315316
}
316317

317-
let store_info = prepare_store_info(&store_id, file_source.clone(), &path);
318+
let app_id = settings
319+
.opened_application_id
320+
.clone()
321+
.unwrap_or_else(|| re_log_types::ApplicationId(path.display().to_string()));
322+
let store_info = prepare_store_info(app_id, &store_id, file_source.clone());
318323
tx.send(store_info).ok();
319324
}
320325

crates/store/re_data_source/src/data_source.rs

+3
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,7 @@ impl DataSource {
178178
let settings = re_data_loader::DataLoaderSettings {
179179
opened_application_id: file_source.recommended_application_id().cloned(),
180180
opened_store_id: file_source.recommended_recording_id().cloned(),
181+
force_store_info: file_source.force_store_info(),
181182
..re_data_loader::DataLoaderSettings::recommended(shared_store_id)
182183
};
183184
re_data_loader::load_from_path(&settings, file_source, &path, &tx)
@@ -206,6 +207,7 @@ impl DataSource {
206207
let settings = re_data_loader::DataLoaderSettings {
207208
opened_application_id: file_source.recommended_application_id().cloned(),
208209
opened_store_id: file_source.recommended_recording_id().cloned(),
210+
force_store_info: file_source.force_store_info(),
209211
..re_data_loader::DataLoaderSettings::recommended(shared_store_id)
210212
};
211213
re_data_loader::load_from_file_contents(
@@ -275,6 +277,7 @@ fn test_data_source_from_uri() {
275277
let file_source = FileSource::DragAndDrop {
276278
recommended_application_id: None,
277279
recommended_recording_id: None,
280+
force_store_info: false,
278281
};
279282

280283
for uri in file {

crates/store/re_log_types/src/lib.rs

+25
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,12 @@ pub enum FileSource {
418418
/// The [`StoreId`] that the viewer heuristically recommends should be used when loading
419419
/// this data source, based on the surrounding context.
420420
recommended_recording_id: Option<StoreId>,
421+
422+
/// Whether `SetStoreInfo`s should be sent, regardless of the surrounding context.
423+
///
424+
/// Only useful when creating a recording just-in-time directly in the viewer (which is what
425+
/// happens when importing things into the welcome screen).
426+
force_store_info: bool,
421427
},
422428

423429
FileDialog {
@@ -428,6 +434,12 @@ pub enum FileSource {
428434
/// The [`StoreId`] that the viewer heuristically recommends should be used when loading
429435
/// this data source, based on the surrounding context.
430436
recommended_recording_id: Option<StoreId>,
437+
438+
/// Whether `SetStoreInfo`s should be sent, regardless of the surrounding context.
439+
///
440+
/// Only useful when creating a recording just-in-time directly in the viewer (which is what
441+
/// happens when importing things into the welcome screen).
442+
force_store_info: bool,
431443
},
432444

433445
Sdk,
@@ -463,6 +475,19 @@ impl FileSource {
463475
Self::Cli | Self::Sdk => None,
464476
}
465477
}
478+
479+
#[inline]
480+
pub fn force_store_info(&self) -> bool {
481+
match self {
482+
Self::FileDialog {
483+
force_store_info, ..
484+
}
485+
| Self::DragAndDrop {
486+
force_store_info, ..
487+
} => *force_store_info,
488+
Self::Cli | Self::Sdk => false,
489+
}
490+
}
466491
}
467492

468493
/// The source of a recording or blueprint.

crates/top/re_sdk/src/recording_stream.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1259,6 +1259,7 @@ impl RecordingStream {
12591259
opened_application_id: None,
12601260
store_id: store_info.store_id.clone(),
12611261
opened_store_id: None,
1262+
force_store_info: false,
12621263
entity_path_prefix,
12631264
timepoint: (!static_).then(|| {
12641265
self.with(|inner| {

crates/viewer/re_viewer/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ serde = { workspace = true, features = ["derive"] }
120120
serde_json.workspace = true
121121
serde-wasm-bindgen.workspace = true
122122
thiserror.workspace = true
123+
uuid.workspace = true
123124
web-time.workspace = true
124125
wgpu.workspace = true
125126

crates/viewer/re_viewer/src/app.rs

+46-17
Original file line numberDiff line numberDiff line change
@@ -573,17 +573,33 @@ impl App {
573573
store_context: Option<&StoreContext<'_>>,
574574
cmd: UICommand,
575575
) {
576+
let mut force_store_info = false;
576577
let active_application_id = store_context
577578
.and_then(|ctx| {
578579
ctx.hub
579580
.active_app()
580581
// Don't redirect data to the welcome screen.
581582
.filter(|&app_id| app_id != &StoreHub::welcome_screen_app_id())
583+
.cloned()
582584
})
583-
.cloned();
585+
// If we don't have any application ID to recommend (which means we are on the welcome screen),
586+
// then just generate a new one using a UUID.
587+
.or_else(|| Some(uuid::Uuid::new_v4().to_string().into()));
584588
let active_recording_id = store_context
585-
.and_then(|ctx| ctx.hub.active_recording_id())
586-
.cloned();
589+
.and_then(|ctx| ctx.hub.active_recording_id().cloned())
590+
.or_else(|| {
591+
// When we're on the welcome screen, there is no recording ID to recommend.
592+
// But we want one, otherwise multiple things being dropped simultaneously on the
593+
// welcome screen would end up in different recordings!
594+
595+
// We're creating a recording just-in-time, directly from the viewer.
596+
// We need those store infos or the data will just be silently ignored.
597+
force_store_info = true;
598+
599+
// NOTE: We don't override blueprints' store IDs anyhow, so it is sound to assume that
600+
// this can only be a recording.
601+
Some(re_log_types::StoreId::random(StoreKind::Recording))
602+
});
587603

588604
match cmd {
589605
UICommand::SaveRecording => {
@@ -615,6 +631,7 @@ impl App {
615631
FileSource::FileDialog {
616632
recommended_application_id: None,
617633
recommended_recording_id: None,
634+
force_store_info,
618635
},
619636
file_path,
620637
)));
@@ -624,18 +641,15 @@ impl App {
624641
UICommand::Open => {
625642
let egui_ctx = egui_ctx.clone();
626643

627-
// Open: we want to try and load into a new dedicated recording.
628-
let recommended_application_id = None;
629-
let recommended_recording_id = None;
630644
let promise = poll_promise::Promise::spawn_local(async move {
631645
let file = async_open_rrd_dialog().await;
632646
egui_ctx.request_repaint(); // Wake ui thread
633647
file
634648
});
635649

636650
self.open_files_promise = Some(PendingFilePromise {
637-
recommended_application_id,
638-
recommended_recording_id,
651+
recommended_application_id: None,
652+
recommended_recording_id: None,
639653
promise,
640654
});
641655
}
@@ -648,6 +662,7 @@ impl App {
648662
FileSource::FileDialog {
649663
recommended_application_id: active_application_id.clone(),
650664
recommended_recording_id: active_recording_id.clone(),
665+
force_store_info,
651666
},
652667
file_path,
653668
)));
@@ -657,19 +672,15 @@ impl App {
657672
UICommand::Import => {
658673
let egui_ctx = egui_ctx.clone();
659674

660-
// Import: we want to try and load into the current recording.
661-
let recommended_application_id = active_application_id;
662-
let recommended_recording_id = active_recording_id;
663-
664675
let promise = poll_promise::Promise::spawn_local(async move {
665676
let file = async_open_rrd_dialog().await;
666677
egui_ctx.request_repaint(); // Wake ui thread
667678
file
668679
});
669680

670681
self.open_files_promise = Some(PendingFilePromise {
671-
recommended_application_id,
672-
recommended_recording_id,
682+
recommended_application_id: active_application_id.clone(),
683+
recommended_recording_id: active_recording_id.clone(),
673684
promise,
674685
});
675686
}
@@ -1364,17 +1375,33 @@ impl App {
13641375
return;
13651376
}
13661377

1378+
let mut force_store_info = false;
13671379
let active_application_id = store_ctx
13681380
.and_then(|ctx| {
13691381
ctx.hub
13701382
.active_app()
13711383
// Don't redirect data to the welcome screen.
13721384
.filter(|&app_id| app_id != &StoreHub::welcome_screen_app_id())
1385+
.cloned()
13731386
})
1374-
.cloned();
1387+
// If we don't have any application ID to recommend (which means we are on the welcome screen),
1388+
// then just generate a new one using a UUID.
1389+
.or_else(|| Some(uuid::Uuid::new_v4().to_string().into()));
13751390
let active_recording_id = store_ctx
1376-
.and_then(|ctx| ctx.hub.active_recording_id())
1377-
.cloned();
1391+
.and_then(|ctx| ctx.hub.active_recording_id().cloned())
1392+
.or_else(|| {
1393+
// When we're on the welcome screen, there is no recording ID to recommend.
1394+
// But we want one, otherwise multiple things being dropped simultaneously on the
1395+
// welcome screen would end up in different recordings!
1396+
1397+
// We're creating a recording just-in-time, directly from the viewer.
1398+
// We need those store infos or the data will just be silently ignored.
1399+
force_store_info = true;
1400+
1401+
// NOTE: We don't override blueprints' store IDs anyhow, so it is sound to assume that
1402+
// this can only be a recording.
1403+
Some(re_log_types::StoreId::random(StoreKind::Recording))
1404+
});
13781405

13791406
for file in dropped_files {
13801407
if let Some(bytes) = file.bytes {
@@ -1384,6 +1411,7 @@ impl App {
13841411
FileSource::DragAndDrop {
13851412
recommended_application_id: active_application_id.clone(),
13861413
recommended_recording_id: active_recording_id.clone(),
1414+
force_store_info,
13871415
},
13881416
FileContents {
13891417
name: file.name.clone(),
@@ -1400,6 +1428,7 @@ impl App {
14001428
FileSource::DragAndDrop {
14011429
recommended_application_id: active_application_id.clone(),
14021430
recommended_recording_id: active_recording_id.clone(),
1431+
force_store_info,
14031432
},
14041433
path,
14051434
)));

0 commit comments

Comments
 (0)