Skip to content

Commit

Permalink
Make object hover & selection colors brighter and more pronounced (#6596
Browse files Browse the repository at this point in the history
)

### What

#### Problem
The outline colors of things that are hovered and/or selected in spatial
space views has always been way too dark.
Can you quickly spot what is hovered and what is selected in this scene?


![image](https://github.com/rerun-io/rerun/assets/1148717/27e8579b-a5cb-4e88-bbf3-f379bfd54866)

The problem with the selection color is that it is the dark blue used
for selection of other things, e.g. `ListItem`s:


![image](https://github.com/rerun-io/rerun/assets/1148717/391a8bf2-23cf-4360-aee5-a263ee05f80e)

Using the same color for selections everywhere makes sense, except one
cannot really use the same color for a thin stroke as one uses for a
massive background.

#### Solution
This PR introduces two new `ContextExt` methods: `hover_stroke` and
`selection_stroke`.

The hover stroke is very bright, making it directly obvious what is
being hovered.
The selection stroke is a brighter version of the blue background
selection color. The result:


![image](https://github.com/rerun-io/rerun/assets/1148717/b2cd3d6a-991f-4bc5-9c5d-a72ce8893c3d)

This is a vast improvement in practice (though the anti-aliasing leaves
a little something to be desired…)

#### Other
I also chose to reuse the same colors for other thing things, like the
rays show out in the 2D->3D projection:


![image](https://github.com/rerun-io/rerun/assets/1148717/0d2ea310-6731-4410-bc27-d9f82221ac14)

### 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/6596?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/6596?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/6596)
- [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
emilk authored Jun 19, 2024
1 parent 87db88a commit 19efe41
Show file tree
Hide file tree
Showing 8 changed files with 73 additions and 33 deletions.
19 changes: 14 additions & 5 deletions crates/re_renderer/shader/composite.wgsl
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ fn main(in: FragmentInput) -> @location(0) vec4f {
// The issue is that positions provided by @builtin(position) are not dependent on the set viewport,
// but are about the location of the texel in the target texture.
var color = textureSample(color_texture, nearest_sampler, in.texcoord).rgb;
// TODO(andreas): Do something meaningful with values above 1
color = clamp(color, vec3f(0.0), vec3f(1.0));

// Outlines
{
Expand All @@ -43,9 +41,18 @@ fn main(in: FragmentInput) -> @location(0) vec4f {
let outline_color_a = outline_a * uniforms.outline_color_layer_a;
let outline_color_b = outline_b * uniforms.outline_color_layer_b;

// Blend outlines with screen color.
color = color * (1.0 - outline_color_a.a) + outline_color_a.rgb;
color = color * (1.0 - outline_color_b.a) + outline_color_b.rgb;
// Blend outlines with screen color:
if false {
// Normal blending with premul alpha.
// Problem: things that are both hovered and selected will get double outlines,
// which can look really ugly if e.g. the selection is dark blue and the hover is bright white.
color = color * (1.0 - outline_color_a.a) + outline_color_a.rgb;
color = color * (1.0 - outline_color_b.a) + outline_color_b.rgb;
} else {
// Add the two outline colors, then blend that in:
let outline_color_sum = saturate(outline_color_a + outline_color_b);
color = color * (1.0 - outline_color_sum.a) + outline_color_sum.rgb;
}

// Show only the outline. Useful for debugging.
//color = outline_color_a.rgb;
Expand All @@ -54,6 +61,8 @@ fn main(in: FragmentInput) -> @location(0) vec4f {
//color = vec3f(closest_positions.xy / resolution, 0.0);
}

color = saturate(color); // TODO(andreas): Do something meaningful with values above 1

// Apply srgb gamma curve - this is necessary since the final eframe output does *not* have an srgb format.
return vec4f(srgb_from_linear(color), 1.0);
}
21 changes: 12 additions & 9 deletions crates/re_space_view_spatial/src/ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use re_types::{
tensor_data::TensorDataMeaning,
Loggable as _,
};
use re_ui::UiExt as _;
use re_ui::{ContextExt as _, UiExt as _};
use re_viewer_context::{
HoverHighlight, Item, ItemSpaceContext, SelectionHighlight, SpaceViewHighlights,
SpaceViewState, SpaceViewSystemExecutionError, TensorDecodeCache, TensorStatsCache, UiLayout,
Expand Down Expand Up @@ -368,16 +368,19 @@ pub fn create_labels(
}

pub fn outline_config(gui_ctx: &egui::Context) -> OutlineConfig {
// Take the exact same colors we have in the ui!
let selection_outline_color =
re_renderer::Rgba::from(gui_ctx.style().visuals.selection.bg_fill);
let hover_outline_color =
re_renderer::Rgba::from(gui_ctx.style().visuals.widgets.hovered.bg_fill);
// Use the exact same colors we have in the ui!
let hover_outline = gui_ctx.hover_stroke();
let selection_outline = gui_ctx.selection_stroke();

// See also: SIZE_BOOST_IN_POINTS_FOR_LINE_OUTLINES

let outline_radius_ui_pts = 0.5 * f32::max(hover_outline.width, selection_outline.width);
let outline_radius_pixel = (gui_ctx.pixels_per_point() * outline_radius_ui_pts).at_least(0.5);

OutlineConfig {
outline_radius_pixel: (gui_ctx.pixels_per_point() * 1.5).at_least(0.5),
color_layer_a: hover_outline_color,
color_layer_b: selection_outline_color,
outline_radius_pixel,
color_layer_a: re_renderer::Rgba::from(hover_outline.color),
color_layer_b: re_renderer::Rgba::from(selection_outline.color),
}
}

Expand Down
9 changes: 5 additions & 4 deletions crates/re_space_view_spatial/src/ui_2d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use re_types::{
},
components::ViewCoordinates,
};
use re_ui::ContextExt as _;
use re_viewer_context::{
gpu_bridge, ItemSpaceContext, SpaceViewId, SpaceViewSystemExecutionError,
SystemExecutionOutput, ViewQuery, ViewerContext,
Expand Down Expand Up @@ -274,7 +275,7 @@ impl SpatialSpaceView2D {
query.space_origin,
&ui_from_scene,
selected_context,
ui.style().visuals.selection.bg_fill,
ui.ctx().selection_stroke().color,
));
}
if let Some(hovered_context) = ctx.selection_state().hovered_space_context() {
Expand All @@ -283,7 +284,7 @@ impl SpatialSpaceView2D {
query.space_origin,
&ui_from_scene,
hovered_context,
egui::Color32::WHITE,
ui.ctx().hover_stroke().color,
));
}

Expand Down Expand Up @@ -426,7 +427,7 @@ fn show_projections_from_3d_space(
space: &EntityPath,
ui_from_scene: &RectTransform,
space_context: &ItemSpaceContext,
color: egui::Color32,
circle_fill_color: egui::Color32,
) -> Vec<Shape> {
let mut shapes = Vec::new();
if let ItemSpaceContext::ThreeD {
Expand All @@ -445,7 +446,7 @@ fn show_projections_from_3d_space(
radius + 2.0,
Color32::BLACK,
));
shapes.push(Shape::circle_filled(pos_in_ui, radius, color));
shapes.push(Shape::circle_filled(pos_in_ui, radius, circle_fill_color));

let text_color = Color32::WHITE;
let text = format!("Depth: {:.3} m", pos_2d.z);
Expand Down
17 changes: 9 additions & 8 deletions crates/re_space_view_spatial/src/ui_3d.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use egui::{emath::RectTransform, NumExt as _};
use glam::Affine3A;
use macaw::{BoundingBox, Quat, Vec3};
use re_ui::ContextExt;
use re_viewport_blueprint::ViewProperty;
use web_time::Instant;

Expand Down Expand Up @@ -634,7 +635,7 @@ impl SpatialSpaceView3D {
space_cameras,
state,
selected_context,
ui.style().visuals.selection.bg_fill,
ui.ctx().selection_stroke().color,
);
}
if let Some(hovered_context) = ctx.selection_state().hovered_space_context() {
Expand All @@ -643,7 +644,7 @@ impl SpatialSpaceView3D {
space_cameras,
state,
hovered_context,
egui::Color32::WHITE,
ui.ctx().hover_stroke().color,
);
}

Expand Down Expand Up @@ -809,7 +810,7 @@ fn show_projections_from_2d_space(
space_cameras: &[SpaceCamera3D],
state: &SpatialSpaceViewState,
space_context: &ItemSpaceContext,
color: egui::Color32,
ray_color: egui::Color32,
) {
match space_context {
ItemSpaceContext::TwoD { space_2d, pos } => {
Expand Down Expand Up @@ -842,7 +843,7 @@ fn show_projections_from_2d_space(
ray,
&state.bounding_boxes.accumulated,
thick_ray_length,
color,
ray_color,
);
}
}
Expand Down Expand Up @@ -870,7 +871,7 @@ fn show_projections_from_2d_space(
ray,
&state.bounding_boxes.accumulated,
distance,
color,
ray_color,
);
}
}
Expand All @@ -884,7 +885,7 @@ fn add_picking_ray(
ray: macaw::Ray3,
scene_bbox_accum: &BoundingBox,
thick_ray_length: f32,
color: egui::Color32,
ray_color: egui::Color32,
) {
let mut line_batch = line_builder.batch("picking ray");

Expand All @@ -895,11 +896,11 @@ fn add_picking_ray(

line_batch
.add_segment(origin, main_ray_end)
.color(color)
.color(ray_color)
.radius(Size::new_points(1.0));
line_batch
.add_segment(main_ray_end, fallback_ray_end)
.color(color.gamma_multiply(0.7))
.color(ray_color.gamma_multiply(0.7))
// TODO(andreas): Make this dashed.
.radius(Size::new_points(0.5));
}
Expand Down
6 changes: 5 additions & 1 deletion crates/re_space_view_spatial/src/visualizers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,11 @@ use super::contexts::SpatialSceneEntityContext;
pub type Keypoints = HashMap<(re_types::components::ClassId, i64), HashMap<KeypointId, glam::Vec3>>;

// TODO(andreas): It would be nice if these wouldn't need to be set on every single line/point builder.
pub const SIZE_BOOST_IN_POINTS_FOR_LINE_OUTLINES: f32 = 1.5;

/// Gap between lines and their outline.
pub const SIZE_BOOST_IN_POINTS_FOR_LINE_OUTLINES: f32 = 1.0;

/// Gap between points and their outline.
pub const SIZE_BOOST_IN_POINTS_FOR_POINT_OUTLINES: f32 = 2.5;

pub fn register_2d_spatial_visualizers(
Expand Down
18 changes: 18 additions & 0 deletions crates/re_ui/src/context_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,24 @@ pub trait ContextExt {
}
}

/// Hovered UI and spatial primitives should have this outline.
fn hover_stroke(&self) -> egui::Stroke {
// We want something bright here.
self.ctx().style().visuals.widgets.active.fg_stroke
}

/// Selected UI and spatial primitives should have this outline.
fn selection_stroke(&self) -> egui::Stroke {
self.ctx().style().visuals.selection.stroke

// It is tempting to use the background selection color for outlines,
// but in practice it is way too dark for spatial views (you can't tell what is selected).
// Also: background colors should not be used as stroke colors.
// let color = self.ctx().style().visuals.selection.bg_fill;
// let stroke_width = self.ctx().style().visuals.selection.stroke.width;
// egui::Stroke::new(stroke_width, color)
}

#[must_use]
fn warning_text(&self, text: impl Into<String>) -> egui::RichText {
let style = self.ctx().style();
Expand Down
5 changes: 5 additions & 0 deletions crates/re_ui/src/design_tokens.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ impl DesignTokens {

egui_style.visuals.selection.bg_fill =
get_aliased_color(&self.json, "{Alias.Color.Highlight.Default.value}");
egui_style.visuals.selection.stroke.color = egui::Color32::from_rgb(173, 184, 255); // Brighter version of the above

egui_style.visuals.widgets.noninteractive.bg_stroke.color = Color32::from_gray(30); // from figma. separator lines, panel lines, etc

Expand All @@ -171,6 +172,10 @@ impl DesignTokens {
egui_style.visuals.widgets.inactive.fg_stroke.color = default; // button text
egui_style.visuals.widgets.active.fg_stroke.color = strong; // strong text and active button text

let wide_stroke_width = 1.5; // Make it a bit more visible, especially important for spatial primitives.
egui_style.visuals.widgets.active.fg_stroke.width = wide_stroke_width;
egui_style.visuals.selection.stroke.width = wide_stroke_width;

// From figma
let shadow = egui::epaint::Shadow {
offset: egui::vec2(0.0, 15.0),
Expand Down
11 changes: 5 additions & 6 deletions crates/re_ui/src/list_item/list_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@
use egui::{NumExt, Response, Shape, Ui};

use crate::list_item::{ContentContext, DesiredWidth, LayoutInfoStack, ListItemContent};
use crate::{
list_item::{ContentContext, DesiredWidth, LayoutInfoStack, ListItemContent},
ContextExt as _,
};
use crate::{DesignTokens, UiExt as _};

struct ListItemResponse {
Expand Down Expand Up @@ -331,11 +334,7 @@ impl ListItem {
if drag_target {
ui.painter().set(
background_frame,
Shape::rect_stroke(
bg_rect_to_paint,
0.0,
(1.0, ui.visuals().selection.bg_fill),
),
Shape::rect_stroke(bg_rect_to_paint, 0.0, ui.ctx().hover_stroke()),
);
} else if interactive {
let bg_fill = if !response.hovered() && ui.rect_contains_pointer(bg_rect) {
Expand Down

0 comments on commit 19efe41

Please sign in to comment.