Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show all entities/components in the Streams UI, even if empty for the selected timeline #3779

Merged
merged 2 commits into from
Oct 11, 2023
Merged

Show all entities/components in the Streams UI, even if empty for the selected timeline #3779

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f8edf1b
Show all entities/components in the Stream Tree, even if empty in the…
abey79 Oct 10, 2023
017c367
Review comments
abey79 Oct 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions crates/re_data_ui/src/instance_path.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,13 @@ impl DataUi for InstancePath {

let Some(components) = store.all_components(&query.timeline, entity_path) else {
if ctx.store_db.entity_db.is_known_entity(entity_path) {
ui.label(format!(
"No components in entity {:?} on timeline {:?}",
entity_path,
query.timeline.name()
));
ui.label(
egui::RichText::new(format!(
"No components logged on timeline {:?}",
query.timeline.name()
))
.color(egui::Color32::KHAKI),
abey79 marked this conversation as resolved.
Show resolved Hide resolved
);
} else {
ui.label(
ctx.re_ui
Expand Down
63 changes: 33 additions & 30 deletions crates/re_time_panel/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,13 +417,10 @@ impl TimePanel {
tree: &EntityTree,
ui: &mut egui::Ui,
) {
if !tree
let tree_has_data_in_current_timeline = tree
.prefix_times
.has_timeline(ctx.rec_cfg.time_ctrl.timeline())
&& tree.num_timeless_messages() == 0
{
return; // ignore entities that have no data for the current timeline, nor any timeless data.
}
|| tree.num_timeless_messages() > 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's great that you made a readable variable out of this, but let's take it to the logical conclusion and make helper function for this: tree.has_data_in_current_timeline or similar

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also note that this will return true for world if data is logged to world/point, since world is a prefix of world/point

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also note that this will return true for world if data is logged to world/point, since world is a prefix of world/point

Yeah, that's the desired behaviour for reporting "emptiness" I think.


// The last part of the path component
let text = if let Some(last_path_part) = last_path_part {
Expand Down Expand Up @@ -478,7 +475,7 @@ impl TimePanel {
let response_rect = response.rect;
self.next_col_right = self.next_col_right.max(response_rect.right());

// From the left of the label, all the way to the rightmost of the time panel
// From the left of the label, all the way to the right-most of the time panel
let full_width_rect = Rect::from_x_y_ranges(
response_rect.left()..=ui.max_rect().right(),
response_rect.y_range(),
Expand All @@ -489,7 +486,7 @@ impl TimePanel {
// ----------------------------------------------

// show the data in the time area:
if is_visible {
if is_visible && tree_has_data_in_current_timeline {
let row_rect =
Rect::from_x_y_ranges(time_area_response.rect.x_range(), response_rect.y_range());

Expand Down Expand Up @@ -547,12 +544,9 @@ impl TimePanel {
for component_name in re_data_ui::ui_visible_components(tree.components.keys()) {
let data = &tree.components[component_name];

if !data.times.has_timeline(ctx.rec_cfg.time_ctrl.timeline())
&& data.num_timeless_messages() == 0
{
continue; // ignore fields that have no data for the current timeline
}

let component_has_data_in_current_timeline =
data.times.has_timeline(ctx.rec_cfg.time_ctrl.timeline())
|| data.num_timeless_messages() > 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, this could be turned into a helper function for even more increased readability (even though this is already an improvement)

let component_path = ComponentPath::new(tree.path.clone(), *component_name);
let short_component_name = component_path.component_name.short_name();
let item = Item::ComponentPath(component_path);
Expand Down Expand Up @@ -587,29 +581,38 @@ impl TimePanel {

let response_rect = response.rect;

let empty_messages_over_time = TimeHistogram::default();
let messages_over_time = data
.times
.get(ctx.rec_cfg.time_ctrl.timeline())
.unwrap_or(&empty_messages_over_time);

// `data.times` does not contain timeless. Need to add those manually:
let total_num_messages =
messages_over_time.total_count() + data.num_timeless_messages() as u64;
response.on_hover_ui(|ui| {
if total_num_messages == 0 {
ui.label(
egui::RichText::new(format!(
"No event logged on timeline {:?}",
ctx.rec_cfg.time_ctrl.timeline().name()
))
.color(Color32::KHAKI),
);
} else {
ui.label(format!("Number of events: {total_num_messages}"));
}
});

self.next_col_right = self.next_col_right.max(response_rect.right());

// From the left of the label, all the way to the rightmost of the time panel
// From the left of the label, all the way to the right-most of the time panel
let full_width_rect = Rect::from_x_y_ranges(
response_rect.left()..=ui.max_rect().right(),
response_rect.y_range(),
);
let is_visible = ui.is_rect_visible(full_width_rect);

if is_visible {
let empty_messages_over_time = TimeHistogram::default();
let messages_over_time = data
.times
.get(ctx.rec_cfg.time_ctrl.timeline())
.unwrap_or(&empty_messages_over_time);

// `data.times` does not contain timeless. Need to add those manually:
let total_num_messages =
messages_over_time.total_count() + data.num_timeless_messages() as u64;
response.on_hover_ui(|ui| {
ui.label(format!("Number of events: {total_num_messages}"));
});

if is_visible && component_has_data_in_current_timeline {
// show the data in the time area:
let row_rect = Rect::from_x_y_ranges(
time_area_response.rect.x_range(),
Expand Down Expand Up @@ -859,7 +862,7 @@ fn view_everything(x_range: &Rangef, timeline_axis: &TimelineAxis) -> TimeView {
let factor = if width_sans_gaps > 0.0 {
width / width_sans_gaps
} else {
1.0 // too narrow to fit everything anyways
1.0 // too narrow to fit everything anyway
};

let min = timeline_axis.min();
Expand Down
Loading