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.

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

Make object hover & selection colors brighter and more pronounced #6596

Merged
merged 10 commits into from
Jun 19, 2024
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);
}
25 changes: 16 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,23 @@ 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);

// TODO(emilk): We need to double this value here to get correct widths, and I don't understand why.
// It doesn't have to do with `pixels_per_point`, so maybe a bug in radius vs width somewhere?
let outline_radius_pixel = 2.0 * outline_radius_pixel;

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
Loading