Skip to content

Commit

Permalink
Update UI for static components (#6101)
Browse files Browse the repository at this point in the history
### What

This PR updates the UI to better display static components, including:
- Components logged as static now have a different icon.
- Components logged as static do not have any event displayed in the
timeline (see below for possible follow-up work).
- The "timeless" gutter in the timeline is gone.
- The hover tooltip in the streams for static component now displays
their content (timeful components have their content displayed on the
corresponding event in the timeline).
- All components now have a "logged time" displayed along their data
(showing the actually logging time that was taken into consideration by
the current LatestAt query). It shows `<static>` for static components.
This appears both in the selection panel and in the hover tooltips.
- In addition, data consistency information is displayed for static
component (both tooltip and selection panel):
- Warning (yellow) if multiple static logs were recorded (the data of
previous logging event is overwritten and not recoverable from the
datastore).
- Error (red) if the static component has timeful stuff logged as well
(albeit that data is not loss, it's not queryable and extremely likely
an indication of user error).

Note: the included release checklist is nice to try the error/warning
cases.

- Fixes #6074
- Possible follow-up #6102

### Screenshots

static + timeful error:
<img width="368" alt="image"
src="https://github.com/rerun-io/rerun/assets/49431240/98c7f1c8-8cf0-4604-95b8-ab7291bd1c20">

multi-static warning:
<img width="334" alt="image"
src="https://github.com/rerun-io/rerun/assets/49431240/4d8e7612-bb3e-4295-bbd1-f9ba0554ac2a">

selection panel:
<img width="295" alt="image"
src="https://github.com/rerun-io/rerun/assets/49431240/a63178ed-adcc-434f-8425-3b22ba88d003">


### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6101?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6101?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!

- [PR Build Summary](https://build.rerun.io/pr/6101)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.
  • Loading branch information
abey79 committed Apr 25, 2024
1 parent 17e646e commit b6a5072
Show file tree
Hide file tree
Showing 17 changed files with 235 additions and 80 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

46 changes: 46 additions & 0 deletions crates/re_data_ui/src/component.rs
Expand Up @@ -52,6 +52,52 @@ impl DataUi for EntityLatestAtResults {
UiVerbosity::LimitHeight | UiVerbosity::Full => num_instances,
};

// Display data time and additional diagnostic information for static components.
if verbosity != UiVerbosity::Small {
ui.label(format!(
"Data time: {}",
query
.timeline()
.typ()
.format(self.results.index().0, ctx.app_options.time_zone),
));

// if the component is static, we display extra diagnostic information
if self.results.is_static() {
if let Some(histogram) = db
.tree()
.subtree(&self.entity_path)
.and_then(|tree| tree.entity.components.get(&self.component_name))
{
if histogram.num_static_messages() > 1 {
ui.label(ctx.re_ui.warning_text(format!(
"Static component value was overridden {} times",
histogram.num_static_messages().saturating_sub(1),
)))
.on_hover_text(
"When a static component is logged multiple times, only the last value \
is stored. Previously logged values are overwritten and not \
recoverable.",
);
}

let timeline_message_count = histogram.num_temporal_messages();
if timeline_message_count > 0 {
ui.label(ctx.re_ui.error_text(format!(
"Static component has {} event{} logged on timelines",
timeline_message_count,
if timeline_message_count > 1 { "s" } else { "" }
)))
.on_hover_text(
"Components should be logged either as static or on timelines, but \
never both. Values for static components logged to timelines cannot be \
displayed.",
);
}
}
}
}

// Here we enforce that exactly `max_row` rows are displayed, which means that:
// - For `num_instances == max_row`, then `max_row` rows are displayed.
// - For `num_instances == max_row + 1`, then `max_row-1` rows are displayed and "…2 more"
Expand Down
2 changes: 2 additions & 0 deletions crates/re_data_ui/src/instance_path.rs
Expand Up @@ -65,6 +65,7 @@ impl DataUi for InstancePath {
ctx,
ui,
&ComponentPath::new(entity_path.clone(), component_name),
db,
);
}
}
Expand All @@ -89,6 +90,7 @@ impl DataUi for InstancePath {
ctx,
ui,
&ComponentPath::new(entity_path.clone(), component_name),
db,
);

if instance_key.is_splat() {
Expand Down
10 changes: 9 additions & 1 deletion crates/re_data_ui/src/item_ui.rs
Expand Up @@ -374,12 +374,14 @@ pub fn component_path_button(
ctx: &ViewerContext<'_>,
ui: &mut egui::Ui,
component_path: &ComponentPath,
db: &re_entity_db::EntityDb,
) -> egui::Response {
component_path_button_to(
ctx,
ui,
component_path.component_name.short_name(),
component_path,
db,
)
.on_hover_text(component_path.component_name.full_name()) // we should show the full name somewhere
}
Expand All @@ -390,11 +392,17 @@ pub fn component_path_button_to(
ui: &mut egui::Ui,
text: impl Into<egui::WidgetText>,
component_path: &ComponentPath,
db: &re_entity_db::EntityDb,
) -> egui::Response {
let item = Item::ComponentPath(component_path.clone());
let is_component_static = db.is_component_static(component_path).unwrap_or_default();
let response = ctx.re_ui.selectable_label_with_icon(
ui,
&icons::COMPONENT,
if is_component_static {
&icons::COMPONENT_STATIC
} else {
&icons::COMPONENT
},
text,
ctx.selection().contains_item(&item),
re_ui::LabelStyle::Normal,
Expand Down
25 changes: 19 additions & 6 deletions crates/re_entity_db/src/entity_db.rs
Expand Up @@ -8,9 +8,9 @@ use re_data_store::{
DataStore, DataStoreConfig, GarbageCollectionOptions, StoreEvent, StoreSubscriber,
};
use re_log_types::{
ApplicationId, DataCell, DataRow, DataTable, DataTableResult, EntityPath, EntityPathHash,
LogMsg, RowId, SetStoreInfo, StoreId, StoreInfo, StoreKind, TimePoint, TimeRange, TimeRangeF,
Timeline,
ApplicationId, ComponentPath, DataCell, DataRow, DataTable, DataTableResult, EntityPath,
EntityPathHash, LogMsg, RowId, SetStoreInfo, StoreId, StoreInfo, StoreKind, TimePoint,
TimeRange, TimeRangeF, Timeline,
};
use re_query2::PromiseResult;
use re_types_core::{components::InstanceKey, Archetype, Loggable};
Expand Down Expand Up @@ -327,9 +327,22 @@ impl EntityDb {
self.tree().subtree.time_histogram.get(timeline)
}

/// Total number of timeless messages for any entity.
pub fn num_timeless_messages(&self) -> u64 {
self.tree.num_timeless_messages_recursive()
/// Total number of static messages for any entity.
pub fn num_static_messages(&self) -> u64 {
self.tree.num_static_messages_recursive()
}

/// Returns whether a component is static.
pub fn is_component_static(&self, component_path: &ComponentPath) -> Option<bool> {
if let Some(entity_tree) = self.tree().subtree(component_path.entity_path()) {
entity_tree
.entity
.components
.get(&component_path.component_name)
.map(|component_histogram| component_histogram.is_static())
} else {
None
}
}

pub fn num_rows(&self) -> usize {
Expand Down
4 changes: 2 additions & 2 deletions crates/re_entity_db/src/entity_tree.rs
Expand Up @@ -245,8 +245,8 @@ impl EntityTree {
}

/// Number of timeless messages in this tree, or any child, recursively.
pub fn num_timeless_messages_recursive(&self) -> u64 {
self.subtree.time_histogram.num_timeless_messages()
pub fn num_static_messages_recursive(&self) -> u64 {
self.subtree.time_histogram.num_static_messages()
}

pub fn time_histogram_for_component(
Expand Down
36 changes: 23 additions & 13 deletions crates/re_entity_db/src/time_histogram_per_timeline.rs
Expand Up @@ -16,14 +16,19 @@ pub struct TimeHistogramPerTimeline {
/// When do we have data? Ignores timeless.
times: BTreeMap<Timeline, TimeHistogram>,

/// Extra book-keeping used to seed any timelines that include timeless msgs.
num_timeless_messages: u64,
/// Extra bookkeeping used to seed any timelines that include static msgs.
num_static_messages: u64,
}

impl TimeHistogramPerTimeline {
#[inline]
pub fn is_empty(&self) -> bool {
self.times.is_empty() && self.num_timeless_messages == 0
self.times.is_empty() && self.num_static_messages == 0
}

#[inline]
pub fn is_static(&self) -> bool {
self.num_static_messages > 0
}

#[inline]
Expand All @@ -47,20 +52,25 @@ impl TimeHistogramPerTimeline {
}

#[inline]
pub fn num_timeless_messages(&self) -> u64 {
self.num_timeless_messages
pub fn num_static_messages(&self) -> u64 {
self.num_static_messages
}

/// Total number of temporal messages over all timelines.
pub fn num_temporal_messages(&self) -> u64 {
self.times.values().map(|hist| hist.total_count()).sum()
}

pub fn add(&mut self, times: &[(Timeline, TimeInt)], n: u32) {
if times.is_empty() {
self.num_timeless_messages = self
.num_timeless_messages
self.num_static_messages = self
.num_static_messages
.checked_add(n as u64)
.unwrap_or_else(|| {
re_log::debug!(
current = self.num_timeless_messages,
current = self.num_static_messages,
added = n,
"book keeping overflowed"
"bookkeeping overflowed"
);
u64::MAX
});
Expand All @@ -76,15 +86,15 @@ impl TimeHistogramPerTimeline {

pub fn remove(&mut self, timepoint: &TimePoint, n: u32) {
if timepoint.is_static() {
self.num_timeless_messages = self
.num_timeless_messages
self.num_static_messages = self
.num_static_messages
.checked_sub(n as u64)
.unwrap_or_else(|| {
// TODO(#4355): hitting this on plots demo
re_log::debug!(
current = self.num_timeless_messages,
current = self.num_static_messages,
removed = n,
"book keeping underflowed"
"bookkeeping underflowed"
);
u64::MIN
});
Expand Down
6 changes: 6 additions & 0 deletions crates/re_query_cache2/src/latest_at/results.rs
Expand Up @@ -157,6 +157,12 @@ impl CachedLatestAtComponentResults {
_ => None,
}
}

/// Returns whether the resolved data is static.
#[inline]
pub fn is_static(&self) -> bool {
self.index.0 == TimeInt::STATIC
}
}

impl SizeBytes for CachedLatestAtComponentResults {
Expand Down
1 change: 1 addition & 0 deletions crates/re_time_panel/Cargo.toml
Expand Up @@ -23,6 +23,7 @@ re_data_store.workspace = true
re_data_ui.workspace = true
re_entity_db.workspace = true
re_format.workspace = true
re_log.workspace = true
re_log_types.workspace = true
re_tracing.workspace = true
re_ui.workspace = true
Expand Down
14 changes: 4 additions & 10 deletions crates/re_time_panel/src/data_density_graph.rs
Expand Up @@ -10,7 +10,7 @@ use egui::{epaint::Vertex, lerp, pos2, remap, Color32, NumExt as _, Rect, Shape}

use re_data_ui::item_ui;
use re_entity_db::TimeHistogram;
use re_log_types::{ComponentPath, TimeInt, TimeRange, TimeReal};
use re_log_types::{ComponentPath, TimeRange, TimeReal};
use re_viewer_context::{Item, TimeControl, UiVerbosity, ViewerContext};

use crate::TimePanelItem;
Expand Down Expand Up @@ -368,14 +368,13 @@ fn smooth(density: &[f32]) -> Vec<f32> {

#[allow(clippy::too_many_arguments)]
pub fn data_density_graph_ui(
data_dentity_graph_painter: &mut DataDensityGraphPainter,
data_density_graph_painter: &mut DataDensityGraphPainter,
ctx: &ViewerContext<'_>,
time_ctrl: &mut TimeControl,
db: &re_entity_db::EntityDb,
time_area_response: &egui::Response,
time_area_painter: &egui::Painter,
ui: &egui::Ui,
num_timeless_messages: usize,
time_histogram: &TimeHistogram,
row_rect: Rect,
time_ranges_ui: &TimeRangesUi,
Expand Down Expand Up @@ -435,11 +434,6 @@ pub fn data_density_graph_ui(
}
};

add_data_point(
TimeRange::point(TimeInt::MIN_TIME_PANEL),
num_timeless_messages,
);

let visible_time_range = time_ranges_ui
.time_range_from_x_range((row_rect.left() - MARGIN_X)..=(row_rect.right() + MARGIN_X));

Expand Down Expand Up @@ -479,7 +473,7 @@ pub fn data_density_graph_ui(
density_graph.buckets = smooth(&density_graph.buckets);

density_graph.paint(
data_dentity_graph_painter,
data_density_graph_painter,
row_rect.y_range(),
time_area_painter,
graph_color(ctx, &item.to_item(), ui),
Expand Down Expand Up @@ -559,7 +553,7 @@ fn show_row_ids_tooltip(

if let Some(component_name) = component_name {
let component_path = ComponentPath::new(entity_path.clone(), *component_name);
item_ui::component_path_button(ctx, ui, &component_path);
item_ui::component_path_button(ctx, ui, &component_path, db);
ui.add_space(8.0);
component_path.data_ui(ctx, ui, verbosity, &query, db);
} else {
Expand Down

0 comments on commit b6a5072

Please sign in to comment.