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

Different icon for empty entity paths #5338

Merged
merged 7 commits into from
Mar 4, 2024
Merged

Different icon for empty entity paths #5338

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
90c40c8
Different icon for empty entity paths
abey79 Feb 28, 2024
a2bfa20
Merge branch 'main' into antoine/empty-entities-icon
abey79 Feb 29, 2024
685d569
Merge branch 'main' into antoine/empty-entities-icon
abey79 Feb 29, 2024
9cffc92
Now in blueprint tree as well
abey79 Feb 29, 2024
ca40ba2
Merge branch 'main' into antoine/empty-entities-icon
abey79 Mar 4, 2024
e29a9f2
Make streams icon also timeline-dependant and make some code less con…
abey79 Mar 4, 2024
33ddbf8
Fixed selection panel icon
abey79 Mar 4, 2024
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
55 changes: 54 additions & 1 deletion crates/re_data_ui/src/item_ui.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
//! TODO(andreas): This is not a `data_ui`, can this go somewhere else, shouldn't be in `re_data_ui`.

use re_entity_db::{EntityTree, InstancePath};
use re_log_types::external::re_types_core::components::InstanceKey;
use re_log_types::{ComponentPath, EntityPath, TimeInt, Timeline};
use re_ui::SyntaxHighlighting;
use re_viewer_context::{HoverHighlight, Item, SpaceViewId, UiVerbosity, ViewerContext};
Expand Down Expand Up @@ -124,6 +125,58 @@ pub fn instance_path_button(
)
}

pub fn instance_path_icon(
query: &re_data_store::LatestAtQuery,
store: &re_data_store::DataStore,
instance_path: &InstancePath,
) -> &'static re_ui::icons::Icon {
if instance_path.instance_key != InstanceKey::SPLAT {
return &re_ui::icons::ENTITY;
}

if store
.all_components(&query.timeline, &instance_path.entity_path)
.is_some()
abey79 marked this conversation as resolved.
Show resolved Hide resolved
{
&re_ui::icons::ENTITY
} else {
&re_ui::icons::ENTITY_EMPTY
}
}

/// The current time query, based on the current time control and an `entity_path`
///
/// If the user is inspecting the blueprint, and the `entity_path` is on the blueprint
/// timeline, then use the blueprint. Otherwise, use the recording.
// TODO(jleibs): Ideally this wouldn't be necessary and we could make the assessment
// directly from the entity_path.
pub fn guess_query_and_store_for_selected_entity<'a>(
ctx: &'a ViewerContext<'_>,
entity_path: &EntityPath,
) -> (re_data_store::LatestAtQuery, &'a re_data_store::DataStore) {
if ctx.app_options.inspect_blueprint_timeline
&& ctx.store_context.blueprint.is_logged_entity(entity_path)
{
(
ctx.blueprint_cfg.time_ctrl.read().current_query(),
ctx.store_context.blueprint.store(),
)
} else {
(
ctx.rec_cfg.time_ctrl.read().current_query(),
ctx.entity_db.store(),
)
}
}

pub fn guess_instance_path_icon(
ctx: &ViewerContext<'_>,
instance_path: &InstancePath,
) -> &'static re_ui::icons::Icon {
let (query, store) = guess_query_and_store_for_selected_entity(ctx, &instance_path.entity_path);
instance_path_icon(&query, store, instance_path)
}

/// Show an instance id and make it selectable.
pub fn instance_path_button_to(
ctx: &ViewerContext<'_>,
Expand All @@ -140,7 +193,7 @@ pub fn instance_path_button_to(
.re_ui
.selectable_label_with_icon(
ui,
&re_ui::icons::ENTITY,
instance_path_icon(query, store, instance_path),
text,
ctx.selection().contains_item(&item),
re_ui::LabelStyle::Normal,
Expand Down
6 changes: 5 additions & 1 deletion crates/re_time_panel/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,11 @@ impl TimePanel {
item_response: response,
body_response,
} = ListItem::new(ctx.re_ui, text)
.with_icon(&re_ui::icons::ENTITY)
.with_icon(if tree.entity.components.is_empty() {
&re_ui::icons::ENTITY_EMPTY
} else {
&re_ui::icons::ENTITY
})
Copy link
Member

Choose a reason for hiding this comment

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

There is an inconsistency here: the data UI uses DataStore::all_components which is timeful, while this uses the EntityTree which is timeless.

I.e. scrubbing the time cursor might change the icon that appears in the blueprint panel and elsewhere, while that won't happen in the time panel.

I think that's fine (unless we want both to be timeless), but deserves a comment to specify that this was intentional at least.

Copy link
Member Author

Choose a reason for hiding this comment

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

As discussed, both were actually timeless, but instance_path_icon() accepting a Query argument was very confusing. I changed it to accept a Timeline instead, which is all it needs.

Also, I made both the streams and the blueprint have their icons based on the current timeline (in my original commit, the streams icon was based on all timeline).

.width_allocation_mode(WidthAllocationMode::Compact)
.selected(is_selected)
.force_hovered(is_item_hovered)
Expand Down
35 changes: 7 additions & 28 deletions crates/re_viewer/src/ui/selection_panel.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
use egui::{NumExt as _, Ui};

use re_data_ui::{image_meaning_for_entity, item_ui, DataUi};
use re_data_ui::{
image_meaning_for_entity, item_ui,
item_ui::{guess_instance_path_icon, guess_query_and_store_for_selected_entity},
DataUi,
};
use re_entity_db::{
ColorMapper, Colormap, EditableAutoValue, EntityPath, EntityProperties, InstancePath,
};
Expand Down Expand Up @@ -38,31 +42,6 @@ pub(crate) struct SelectionPanel {
selection_state_ui: SelectionHistoryUi,
}

/// The current time query, based on the current time control and an `entity_path`
///
/// If the user is inspecting the blueprint, and the `entity_path` is on the blueprint
/// timeline, then use the blueprint. Otherwise use the recording.
// TODO(jleibs): Ideally this wouldn't be necessary and we could make the assessment
// directly from the entity_path.
fn guess_query_and_store_for_selected_entity<'a>(
ctx: &'a ViewerContext<'_>,
entity_path: &EntityPath,
) -> (re_data_store::LatestAtQuery, &'a re_data_store::DataStore) {
if ctx.app_options.inspect_blueprint_timeline
&& ctx.store_context.blueprint.is_logged_entity(entity_path)
{
(
ctx.blueprint_cfg.time_ctrl.read().current_query(),
ctx.store_context.blueprint.store(),
)
} else {
(
ctx.rec_cfg.time_ctrl.read().current_query(),
ctx.entity_db.store(),
)
}
}

impl SelectionPanel {
pub fn show_panel(
&mut self,
Expand Down Expand Up @@ -434,7 +413,7 @@ fn what_is_selected_ui(
ctx.re_ui,
ui,
name,
Some(&re_ui::icons::ENTITY),
Some(guess_instance_path_icon(ctx, instance_path)),
&format!(
"{typ} '{instance_path}' as shown in Space View {:?}",
space_view.display_name
Expand Down Expand Up @@ -467,7 +446,7 @@ fn what_is_selected_ui(
ctx.re_ui,
ui,
name,
Some(&re_ui::icons::ENTITY),
Some(guess_instance_path_icon(ctx, instance_path)),
&format!("{typ} '{instance_path}'"),
);

Expand Down
6 changes: 5 additions & 1 deletion crates/re_viewport/src/viewport_blueprint_ui.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use egui::{Response, Ui};

use itertools::Itertools;
use re_data_ui::item_ui::guess_instance_path_icon;

use re_entity_db::InstancePath;
use re_log_types::EntityPath;
Expand Down Expand Up @@ -400,7 +401,10 @@ impl Viewport<'_, '_> {

let list_item = ListItem::new(ctx.re_ui, item_label)
.selected(is_selected)
.with_icon(&re_ui::icons::ENTITY)
.with_icon(guess_instance_path_icon(
ctx,
&InstancePath::from(entity_path.clone()),
))
.subdued(subdued)
.force_hovered(is_item_hovered)
.with_buttons(|re_ui: &_, ui: &mut egui::Ui| {
Expand Down
Loading