Skip to content

Commit

Permalink
Improve the Selection Panel UI for components when a single item is s…
Browse files Browse the repository at this point in the history 
…elected (#4416)

### What

This PR:
- adds a new UI verbosity level to distinguish between single- and
multi-selection in the Selection Panel;
- adjusts `DataUi` impls accordingly;
- update the UI of for `AnnotationContext` to use collapsible headers
instead of inner scroll areas (there can be *many* tables for one
instance, so inner scroll bars are really annoying);
- adds a script to log very long `AnnotationContext` for UI test
purposes.


* Is affected by (and doesn't fix):
#4367
* Follow-up to #4370
* Fixes #4375

### Screenshot

New collapsible-header-based UI for annotation context:


https://github.com/rerun-io/rerun/assets/49431240/435566a0-420b-48d7-8ea4-026d02d903a2

Also fix this spurious separator (and the related sizing issue) at the
top of the hover box:

<img width="662" alt="image"
src="https://github.com/rerun-io/rerun/assets/49431240/077a3af4-2a5d-423a-8609-46bbc4f66221">


### 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):
  * Full build: [app.rerun.io](https://app.rerun.io/pr/4416/index.html)
* Partial build:
[app.rerun.io](https://app.rerun.io/pr/4416/index.html?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
- Useful for quick testing when changes do not affect examples in any
way
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/4416)
- [Docs
preview](https://rerun.io/preview/83f1bdcdcd055dd46658343a44f9ae7022c64f45/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/83f1bdcdcd055dd46658343a44f9ae7022c64f45/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

---------

Co-authored-by: Emil Ernerfeldt <[email protected]>
  • Loading branch information
@abey79 @emilk
abey79 and emilk authored Dec 6, 2023
1 parent c849b74 commit 9ad5a0d
Show file tree
Hide file tree
Showing 14 changed files with 285 additions and 172 deletions.
12 changes: 5 additions & 7 deletions clippy.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,15 @@ disallowed-macros = [

# https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_methods
disallowed-methods = [
"std::env::temp_dir", # Use the tempdir crate instead
{ path = "egui_extras::TableBody::row", reason = "`row` doesn't scale. Use `rows` instead." },
{ path = "sha1::Digest::new", reason = "SHA1 is cryptographically broken" },
{ path = "std::env::temp_dir", reason = "Use the tempdir crate instead" },
{ path = "std::panic::catch_unwind", reason = "We compile with `panic = 'abort'`" },
{ path = "std::thread::spawn", reason = "Use `std::thread::Builder` and name the thread" },

# There are many things that aren't allowed on wasm,
# but we cannot disable them all here (because of e.g. https://github.com/rust-lang/rust-clippy/issues/10406)
# so we do that in `clipppy_wasm.toml` instead.

"std::thread::spawn", # Use `std::thread::Builder` and name the thread

"sha1::Digest::new", # SHA1 is cryptographically broken

"std::panic::catch_unwind", # We compile with `panic = "abort"`
]

# https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_names
Expand Down
229 changes: 127 additions & 102 deletions crates/re_data_ui/src/annotation_context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@ use re_types::datatypes::{
};
use re_viewer_context::{auto_color, UiVerbosity, ViewerContext};

use super::DataUi;

const TABLE_SCROLL_AREA_HEIGHT: f32 = 500.0; // add scroll-bars when we get to this height
use super::{table_for_verbosity, DataUi};

impl crate::EntityDataUi for re_types::components::ClassId {
fn entity_data_ui(
Expand Down Expand Up @@ -41,12 +39,13 @@ impl crate::EntityDataUi for re_types::components::ClassId {
|| !class.keypoint_annotations.is_empty()
{
response.response.on_hover_ui(|ui| {
class_description_ui(ui, class, id);
class_description_ui(ctx, ui, verbosity, class, id);
});
}
}
UiVerbosity::Reduced | UiVerbosity::All => {
class_description_ui(ui, class, id);
UiVerbosity::Reduced | UiVerbosity::Full | UiVerbosity::LimitHeight => {
ui.separator();
class_description_ui(ctx, ui, verbosity, class, id);
}
}
} else {
Expand Down Expand Up @@ -97,7 +96,7 @@ fn annotation_info(
impl DataUi for AnnotationContext {
fn data_ui(
&self,
_ctx: &ViewerContext<'_>,
ctx: &ViewerContext<'_>,
ui: &mut egui::Ui,
verbosity: UiVerbosity,
_query: &re_arrow_store::LatestAtQuery,
Expand All @@ -111,22 +110,25 @@ impl DataUi for AnnotationContext {
ui.label(format!("AnnotationContext with {} classes", self.0.len()));
}
}
UiVerbosity::All => {
UiVerbosity::LimitHeight | UiVerbosity::Full => {
ui.vertical(|ui| {
annotation_info_table_ui(
ui,
self.0
.iter()
.map(|class| &class.class_description.info)
.sorted_by_key(|info| info.id),
);
ctx.re_ui
.maybe_collapsing_header(ui, true, "Classes", true, |ui| {
let annotation_infos = self
.0
.iter()
.map(|class| &class.class_description.info)
.sorted_by_key(|info| info.id)
.collect_vec();
annotation_info_table_ui(ui, verbosity, &annotation_infos);
});

for ClassDescriptionMapElem {
class_id,
class_description,
} in &self.0
{
class_description_ui(ui, class_description, *class_id);
class_description_ui(ctx, ui, verbosity, class_description, *class_id);
}
});
}
Expand All @@ -135,105 +137,129 @@ impl DataUi for AnnotationContext {
}

fn class_description_ui(
ctx: &re_viewer_context::ViewerContext<'_>,
ui: &mut egui::Ui,
mut verbosity: UiVerbosity,
class: &ClassDescription,
id: re_types::datatypes::ClassId,
) {
if class.keypoint_connections.is_empty() && class.keypoint_annotations.is_empty() {
return;
}

re_tracing::profile_function!();

let use_collapsible = verbosity == UiVerbosity::LimitHeight || verbosity == UiVerbosity::Full;

// We use collapsible header as a means for the user to limit the height, so the annotation info
// tables can be fully unrolled.
if verbosity == UiVerbosity::LimitHeight {
verbosity = UiVerbosity::Full;
}

let row_height = re_ui::ReUi::table_line_height();
ui.separator();
ui.strong(format!("Keypoints for Class {}", id.0));
if !class.keypoint_annotations.is_empty() {
ui.add_space(8.0);
ui.strong("Keypoints Annotations");
ui.push_id(format!("keypoint_annotations_{}", id.0), |ui| {
annotation_info_table_ui(
ui,
class
ctx.re_ui.maybe_collapsing_header(
ui,
use_collapsible,
&format!("Keypoints Annotation for Class {}", id.0),
true,
|ui| {
let annotation_infos = class
.keypoint_annotations
.iter()
.sorted_by_key(|annotation| annotation.id),
);
});
.sorted_by_key(|annotation| annotation.id)
.collect_vec();
ui.push_id(format!("keypoint_annotations_{}", id.0), |ui| {
annotation_info_table_ui(ui, verbosity, &annotation_infos);
});
},
);
}

if !class.keypoint_connections.is_empty() {
ui.add_space(8.0);
ui.strong("Keypoint Connections");
ui.push_id(format!("keypoints_connections_{}", id.0), |ui| {
use egui_extras::{Column, TableBuilder};
ctx.re_ui.maybe_collapsing_header(
ui,
use_collapsible,
&format!("Keypoint Connections for Class {}", id.0),
true,
|ui| {
ui.push_id(format!("keypoints_connections_{}", id.0), |ui| {
use egui_extras::Column;

let table = TableBuilder::new(ui)
.min_scrolled_height(TABLE_SCROLL_AREA_HEIGHT)
.max_scroll_height(TABLE_SCROLL_AREA_HEIGHT)
.cell_layout(egui::Layout::left_to_right(egui::Align::Center))
.column(Column::auto().clip(true).at_least(40.0))
.column(Column::auto().clip(true).at_least(40.0));
table
.header(re_ui::ReUi::table_header_height(), |mut header| {
re_ui::ReUi::setup_table_header(&mut header);
header.col(|ui| {
ui.strong("From");
});
header.col(|ui| {
ui.strong("To");
});
})
.body(|mut body| {
re_ui::ReUi::setup_table_body(&mut body);
let table = table_for_verbosity(verbosity, ui)
.cell_layout(egui::Layout::left_to_right(egui::Align::Center))
.column(Column::auto().clip(true).at_least(40.0))
.column(Column::auto().clip(true).at_least(40.0));
table
.header(re_ui::ReUi::table_header_height(), |mut header| {
re_ui::ReUi::setup_table_header(&mut header);
header.col(|ui| {
ui.strong("From");
});
header.col(|ui| {
ui.strong("To");
});
})
.body(|mut body| {
re_ui::ReUi::setup_table_body(&mut body);

// TODO(jleibs): Helper to do this with caching somewhere
let keypoint_map: ahash::HashMap<KeypointId, AnnotationInfo> = {
re_tracing::profile_scope!("build_annotation_map");
class
.keypoint_annotations
.iter()
.map(|kp| (kp.id.into(), kp.clone()))
.collect()
};
// TODO(jleibs): Helper to do this with caching somewhere
let keypoint_map: ahash::HashMap<KeypointId, AnnotationInfo> = {
re_tracing::profile_scope!("build_annotation_map");
class
.keypoint_annotations
.iter()
.map(|kp| (kp.id.into(), kp.clone()))
.collect()
};

for KeypointPair {
keypoint0: from,
keypoint1: to,
} in &class.keypoint_connections
{
body.row(row_height, |mut row| {
for id in [from, to] {
row.col(|ui| {
ui.label(
keypoint_map
.get(id)
.and_then(|info| info.label.as_ref())
.map_or_else(
|| format!("id {}", id.0),
|label| label.to_string(),
),
);
});
}
body.rows(
row_height,
class.keypoint_connections.len(),
|row_idx, mut row| {
let pair = &class.keypoint_connections[row_idx];
let KeypointPair {
keypoint0,
keypoint1,
} = pair;

for id in [keypoint0, keypoint1] {
row.col(|ui| {
ui.label(
keypoint_map
.get(id)
.and_then(|info| info.label.as_ref())
.map_or_else(
|| format!("id {}", id.0),
|label| label.to_string(),
),
);
});
}
},
);
});
}
});
});
},
);
}
}

fn annotation_info_table_ui<'a>(
fn annotation_info_table_ui(
ui: &mut egui::Ui,
annotation_infos: impl Iterator<Item = &'a AnnotationInfo>,
verbosity: UiVerbosity,
annotation_infos: &[&AnnotationInfo],
) {
re_tracing::profile_function!();

let row_height = re_ui::ReUi::table_line_height();

ui.spacing_mut().item_spacing.x = 20.0; // column spacing.

use egui_extras::{Column, TableBuilder};
use egui_extras::Column;

let table = TableBuilder::new(ui)
.min_scrolled_height(TABLE_SCROLL_AREA_HEIGHT)
.max_scroll_height(TABLE_SCROLL_AREA_HEIGHT)
let table = table_for_verbosity(verbosity, ui)
.cell_layout(egui::Layout::left_to_right(egui::Align::Center))
.column(Column::auto()) // id
.column(Column::auto().clip(true).at_least(40.0)) // label
Expand All @@ -255,24 +281,23 @@ fn annotation_info_table_ui<'a>(
.body(|mut body| {
re_ui::ReUi::setup_table_body(&mut body);

for info in annotation_infos {
body.row(row_height, |mut row| {
row.col(|ui| {
ui.label(info.id.to_string());
});
row.col(|ui| {
let label = if let Some(label) = &info.label {
label.as_str()
} else {
""
};
ui.label(label);
});
row.col(|ui| {
color_ui(ui, info, Vec2::new(64.0, row_height));
});
body.rows(row_height, annotation_infos.len(), |row_idx, mut row| {
let info = &annotation_infos[row_idx];
row.col(|ui| {
ui.label(info.id.to_string());
});
}
row.col(|ui| {
let label = if let Some(label) = &info.label {
label.as_str()
} else {
""
};
ui.label(label);
});
row.col(|ui| {
color_ui(ui, info, Vec2::new(64.0, row_height));
});
});
});
}

Expand Down
28 changes: 5 additions & 23 deletions crates/re_data_ui/src/component.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use re_query::ComponentWithInstances;
use re_types::ComponentName;
use re_viewer_context::{UiVerbosity, ViewerContext};

use super::DataUi;
use super::{table_for_verbosity, DataUi};
use crate::item_ui;

// We do NOT implement `DataUi` for just `ComponentWithInstances`
Expand Down Expand Up @@ -41,14 +41,14 @@ impl DataUi for EntityComponentWithInstances {

let one_line = match verbosity {
UiVerbosity::Small => true,
UiVerbosity::Reduced | UiVerbosity::All => false,
UiVerbosity::Reduced | UiVerbosity::LimitHeight | UiVerbosity::Full => false,
};

// in some cases, we don't want to display all instances
let max_row = match verbosity {
UiVerbosity::Small => 0,
UiVerbosity::Reduced => num_instances.at_most(4), // includes "…x more" if any
UiVerbosity::All => num_instances,
UiVerbosity::LimitHeight | UiVerbosity::Full => num_instances,
};

// Here we enforce that exactly `max_row` rows are displayed, which means that:
Expand Down Expand Up @@ -97,28 +97,10 @@ impl DataUi for EntityComponentWithInstances {
re_format::format_number(num_instances)
));
} else {
let mut table = egui_extras::TableBuilder::new(ui)
table_for_verbosity(verbosity, ui)
.resizable(false)
.cell_layout(egui::Layout::left_to_right(egui::Align::Center))
.columns(egui_extras::Column::auto(), 2);

if verbosity == UiVerbosity::All {
// Use full width in the selection panel.
table = table.auto_shrink([false, true]);
} else {
// Be as small as possible in the hover tooltips.
table = table.auto_shrink([true, true]);
}

if verbosity == UiVerbosity::All && ctx.selection().len() <= 1 {
// We're alone in the selection panel. Let the outer ScrollArea do the work.
table = table.vscroll(false);
} else {
// Don't take too much vertical space to leave room for other selected items.
table = table.vscroll(true).max_scroll_height(100.0);
}

table
.columns(egui_extras::Column::auto(), 2)
.header(re_ui::ReUi::table_header_height(), |mut header| {
re_ui::ReUi::setup_table_header(&mut header);
header.col(|ui| {
Expand Down
Loading

0 comments on commit 9ad5a0d

Please sign in to comment.