Skip to content

Commit

Permalink
Fix arrows requiring a radius to be visible (#1720)
Browse files Browse the repository at this point in the history
* Fix arrows requiring a radius to be visible.
.. that is the immediately user-facing problem this solves. But in fact it makes the line renderer a bit more predictable: Previously, line caps would extend the line. Now they stay within the bounds of the previously determined line positions!

* fix combination of round and arrow caps causing incorrect line length

* fix rectangles no longer being properly rounded

* spelling

* control outward extending for start/end caps separately. Make viewer arrows have a rounded base with outwards extension

* line shader simplifications
  • Loading branch information
Wumpf authored Mar 28, 2023
1 parent 58fde68 commit 0ff3191
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 57 deletions.
101 changes: 62 additions & 39 deletions crates/re_renderer/shader/lines.wgsl
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,11 @@ const POSITION_DATA_TEXTURE_SIZE: i32 = 256;
// See lines.rs#LineStripFlags
const CAP_END_TRIANGLE: u32 = 1u;
const CAP_END_ROUND: u32 = 2u;
const CAP_START_TRIANGLE: u32 = 4u;
const CAP_START_ROUND: u32 = 8u;
const NO_COLOR_GRADIENT: u32 = 16u;
const CAP_END_EXTEND_OUTWARDS: u32 = 4u;
const CAP_START_TRIANGLE: u32 = 8u;
const CAP_START_ROUND: u32 = 16u;
const CAP_START_EXTEND_OUTWARDS: u32 = 32u;
const NO_COLOR_GRADIENT: u32 = 64u;

// A lot of the attributes don't need to be interpolated across triangles.
// To document that and safe some time we mark them up with @interpolate(flat)
Expand All @@ -63,10 +65,10 @@ struct VertexOut {
active_radius: f32,

@location(4) @interpolate(perspective)
closest_strip_position: Vec3,
round_cap_circle_center: Vec3,

@location(5) @interpolate(flat)
currently_active_flags: u32,
fragment_flags: u32,
};

struct LineStripData {
Expand Down Expand Up @@ -140,14 +142,19 @@ fn vs_main(@builtin(vertex_index) vertex_idx: u32) -> VertexOut {
let local_idx = vertex_idx % 6u;
let top_bottom = select(-1.0, 1.0, (local_idx <= 1u || local_idx == 3u)); // 1 for a top vertex, -1 for a bottom vertex.

// There's a left and a right triangle in every quad, see sketch above.
// Right triangle can form end caps, left triangles can form start caps.
let is_right_triangle = local_idx >= 3u;

// Let's assume for starters this vertex is part of a regular quad in a line strip.
// Fetch position data at the beginning and the end of that quad.
// Fetch position data at the beginning and the end of that quad, as well as the position data before and after this quad.
let pos_data_quad_before = read_position_data(pos_data_idx - 1u);
let pos_data_quad_begin = read_position_data(pos_data_idx);
let pos_data_quad_end = read_position_data(pos_data_idx + 1u);
let pos_data_quad_after = read_position_data(pos_data_idx + 2u);

// If the strip indices don't match up for start/end, then we're in a cap triangle!
let is_cap_triangle = pos_data_quad_begin.strip_index != pos_data_quad_end.strip_index;
let is_end_cap = is_cap_triangle && local_idx >= 3u;

// Let's determine which one of the two position data is closer to our vertex.
// Which tells us things:
Expand All @@ -156,75 +163,90 @@ fn vs_main(@builtin(vertex_index) vertex_idx: u32) -> VertexOut {
//
// For caps, we determine the "only valid one" (as one of them belongs to another strip)
var pos_data_current: PositionData;
if is_end_cap || (!is_cap_triangle && !is_at_quad_end) {
if (is_cap_triangle && is_right_triangle) || (!is_cap_triangle && !is_at_quad_end) {
pos_data_current = pos_data_quad_begin;
} else {
pos_data_current = pos_data_quad_end;
}

// Note that for caps triangles, the pos_data_current.pos stays constant over the entire triangle!
// However, to handle things in the fragment shader we need to add a seceond position which is different
// for start/end of the cap triangle.
// The closest "line strip skeleton" position to the current vertex.
// Various things like end cap or radius boosting can cause adjustments to it.
var center_position = pos_data_current.pos;

// Data valid for the entire strip that this vertex belongs to.
let strip_data = read_strip_data(pos_data_current.strip_index);

// Active flags are all flags that we react to at the current vertex.
// I.e. cap flags are only active in the respective cap triangle.
var currently_active_flags = strip_data.flags & (~(CAP_START_TRIANGLE | CAP_END_TRIANGLE | CAP_START_ROUND | CAP_END_ROUND));

// Compute quad_dir and correct the currently_active_flags & correct center_position triangle caps.
// Compute quad_dir & correct center_position for triangle caps.
var quad_dir: Vec3;
var is_at_pointy_end = false;
if is_cap_triangle {
if is_end_cap && has_any_flag(strip_data.flags, CAP_END_TRIANGLE | CAP_END_ROUND) {
currently_active_flags |= strip_data.flags & (CAP_END_TRIANGLE | CAP_END_ROUND);
is_at_pointy_end = is_at_quad_end;
quad_dir = pos_data_quad_begin.pos - read_position_data(pos_data_idx - 1u).pos; // Go one pos data back
} else if !is_end_cap && has_any_flag(strip_data.flags, CAP_START_TRIANGLE | CAP_START_ROUND) {
currently_active_flags |= strip_data.flags & (CAP_START_TRIANGLE | CAP_START_ROUND);
is_at_pointy_end = !is_at_quad_end;
quad_dir = read_position_data(pos_data_idx + 2u).pos - pos_data_quad_end.pos; // Go one pos data forward
} else {
// Discard vertex.
center_position = Vec3(0.0/0.0, 0.0/0.0, 0.0/0.0);
}
quad_dir = normalize(quad_dir);
let is_end_cap_triangle = is_cap_triangle && is_right_triangle && has_any_flag(strip_data.flags, CAP_END_TRIANGLE | CAP_END_ROUND);
let is_start_cap_triangle = is_cap_triangle && !is_right_triangle && has_any_flag(strip_data.flags, CAP_START_TRIANGLE | CAP_START_ROUND);
if is_end_cap_triangle {
is_at_pointy_end = is_at_quad_end;
quad_dir = pos_data_quad_begin.pos - pos_data_quad_before.pos; // Go one pos data back.
} else if is_start_cap_triangle {
is_at_pointy_end = !is_at_quad_end;
quad_dir = pos_data_quad_after.pos - pos_data_quad_end.pos; // Go one pos data forward.
} else if is_cap_triangle {
// Discard vertex.
center_position = Vec3(0.0/0.0, 0.0/0.0, 0.0/0.0);
} else {
quad_dir = normalize(pos_data_quad_end.pos - pos_data_quad_begin.pos);
quad_dir = pos_data_quad_end.pos - pos_data_quad_begin.pos;
}
quad_dir = normalize(quad_dir);

// Resolve radius.
// (slight inaccuracy: End caps are going to adjust their center_position)
let camera_ray = camera_ray_to_world_pos(center_position);
let camera_distance = distance(camera_ray.origin, center_position);
var strip_radius = unresolved_size_to_world(strip_data.unresolved_radius, camera_distance, frame.auto_size_lines);

// Make space for the end cap if this is either the cap itself or the cap follows right after/before this quad.
if !has_any_flag(strip_data.flags, CAP_END_EXTEND_OUTWARDS) &&
(is_end_cap_triangle || (is_at_quad_end && pos_data_current.strip_index != pos_data_quad_after.strip_index)) {
var cap_length =
f32(has_any_flag(strip_data.flags, CAP_END_ROUND)) +
f32(has_any_flag(strip_data.flags, CAP_END_TRIANGLE)) * 4.0;
center_position -= quad_dir * (cap_length * strip_radius);
}
if !has_any_flag(strip_data.flags, CAP_START_EXTEND_OUTWARDS) &&
(is_start_cap_triangle || (!is_at_quad_end && pos_data_current.strip_index != pos_data_quad_before.strip_index)) {
var cap_length =
f32(has_any_flag(strip_data.flags, CAP_START_ROUND)) +
f32(has_any_flag(strip_data.flags, CAP_START_TRIANGLE)) * 4.0;
center_position += quad_dir * (cap_length * strip_radius);
}

// Boost radius only now that we subtracted/added the cap length.
// This way we don't get a gap for the outline at the cap.
if draw_data.radius_boost_in_ui_points > 0.0 {
let size_boost = world_size_from_point_size(draw_data.radius_boost_in_ui_points, camera_distance);
strip_radius += size_boost;
// Push out positions as well along the quad dir.
// This is especially important if there's no miters on a line-strip (TODO(#829)),
// as this would enhance gaps between lines otherwise.
center_position += quad_dir * (size_boost * select(-1.0, 1.0, is_at_quad_end));
center_position += quad_dir * (size_boost * select(-1.0, 1.0, is_right_triangle));
}

var active_radius = strip_radius;
// If this is a triangle cap, we blow up our ("virtual") quad by twice the size.
if has_any_flag(currently_active_flags, CAP_START_TRIANGLE | CAP_END_TRIANGLE) {
if (is_end_cap_triangle && has_any_flag(strip_data.flags, CAP_END_TRIANGLE)) ||
(is_start_cap_triangle && has_any_flag(strip_data.flags, CAP_START_TRIANGLE)) {
active_radius *= 2.0;
}

// Span up the vertex away from the line's axis, orthogonal to the direction to the camera
let dir_up = normalize(cross(camera_ray.direction, quad_dir));

let round_cap_circle_center = center_position;

var pos: Vec3;
if is_cap_triangle && is_at_pointy_end {
// We extend the cap triangle far enough to handle triangle caps,
// and far enough to do rounded caps without any visible clipping.
// There is _some_ clipping, but we can't see it ;)
// If we want to do it properly, we would extend the radius for rounded caps too.
center_position = pos_data_current.pos + quad_dir * (strip_radius * 4.0 * select(-1.0, 1.0, is_end_cap));
center_position += quad_dir * (strip_radius * 4.0 * select(-1.0, 1.0, is_right_triangle));
pos = center_position;
} else {
pos = center_position + (active_radius * top_bottom) * dir_up;
Expand All @@ -235,18 +257,19 @@ fn vs_main(@builtin(vertex_index) vertex_idx: u32) -> VertexOut {
out.position = frame.projection_from_world * Vec4(pos, 1.0);
out.position_world = pos;
out.center_position = center_position;
out.closest_strip_position = pos_data_current.pos;
out.round_cap_circle_center = round_cap_circle_center;
out.color = strip_data.color;
out.active_radius = active_radius;
out.currently_active_flags = currently_active_flags;
out.fragment_flags = strip_data.flags &
(NO_COLOR_GRADIENT | (u32(is_cap_triangle) * select(CAP_START_ROUND, CAP_END_ROUND, is_right_triangle)));

return out;
}

fn compute_coverage(in: VertexOut) -> f32 {
var coverage = 1.0;
if has_any_flag(in.currently_active_flags, CAP_START_ROUND | CAP_END_ROUND) {
let distance_to_skeleton = length(in.position_world - in.closest_strip_position);
if has_any_flag(in.fragment_flags, CAP_START_ROUND | CAP_END_ROUND) {
let distance_to_skeleton = length(in.position_world - in.round_cap_circle_center);
let pixel_world_size = approx_pixel_world_size_at(length(in.position_world - frame.camera_position));

// It's important that we do antialias both inwards and outwards of the exact border.
Expand All @@ -267,7 +290,7 @@ fn fs_main(in: VertexOut) -> @location(0) Vec4 {

// TODO(andreas): lighting setup
var shading = 1.0;
if !has_any_flag(in.currently_active_flags, NO_COLOR_GRADIENT) { // TODO(andreas): Flip flag meaning.
if !has_any_flag(in.fragment_flags, NO_COLOR_GRADIENT) { // TODO(andreas): Flip flag meaning.
let to_center = in.position_world - in.center_position;
let relative_distance_to_center_sq = dot(to_center, to_center) / (in.active_radius * in.active_radius);
shading = max(0.2, 1.0 - relative_distance_to_center_sq) * 0.9;
Expand Down
8 changes: 6 additions & 2 deletions crates/re_renderer/src/line_strip_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,9 @@ where
.flags(
LineStripFlags::CAP_END_ROUND
| LineStripFlags::CAP_START_ROUND
| LineStripFlags::NO_COLOR_GRADIENT,
| LineStripFlags::NO_COLOR_GRADIENT
| LineStripFlags::CAP_END_EXTEND_OUTWARDS
| LineStripFlags::CAP_START_EXTEND_OUTWARDS,
)
}

Expand Down Expand Up @@ -325,7 +327,9 @@ where
.flags(
LineStripFlags::CAP_END_ROUND
| LineStripFlags::CAP_START_ROUND
| LineStripFlags::NO_COLOR_GRADIENT,
| LineStripFlags::NO_COLOR_GRADIENT
| LineStripFlags::CAP_END_EXTEND_OUTWARDS
| LineStripFlags::CAP_START_EXTEND_OUTWARDS,
)
}

Expand Down
22 changes: 11 additions & 11 deletions crates/re_renderer/src/renderer/lines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,22 +213,22 @@ bitflags! {
/// Adds a round cap at the end of a line strip (excludes other end caps).
const CAP_END_ROUND = 0b0000_0010;

/// By default, line caps end at the last/first position of the the line strip.
/// This flag makes end caps extend outwards.
const CAP_END_EXTEND_OUTWARDS = 0b0000_0100;

/// Puts a equilateral triangle at the start of the line strip (excludes other start caps).
const CAP_START_TRIANGLE = 0b0000_0100;
const CAP_START_TRIANGLE = 0b0000_1000;

/// Adds a round cap at the start of a line strip (excludes other start caps).
const CAP_START_ROUND = 0b0000_1000;
const CAP_START_ROUND = 0b0001_0000;

/// Disable color gradient which is on by default
const NO_COLOR_GRADIENT = 0b0001_0000;
}
}
/// By default, line caps end at the last/first position of the the line strip.
/// This flag makes end caps extend outwards.
const CAP_START_EXTEND_OUTWARDS = 0b0010_0000;

impl LineStripFlags {
pub fn get_triangle_cap_tip_length(line_radius: f32) -> f32 {
// hardcoded in lines.wgsl
// Alternatively we could declare the entire last segment to be a tip, making the line length configurable!
line_radius * 4.0
/// Disable color gradient which is on by default
const NO_COLOR_GRADIENT = 0b0100_0000;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use re_log_types::{
Arrow3D, Component,
};
use re_query::{query_primary_with_history, EntityView, QueryError};
use re_renderer::{renderer::LineStripFlags, Size};
use re_renderer::Size;

use crate::{
misc::{SpaceViewHighlights, TransformCache, ViewerContext},
Expand Down Expand Up @@ -67,15 +67,17 @@ impl Arrows3DPart {
let origin = glam::Vec3::from(origin);

let radius = radius.map_or(Size::AUTO, |r| Size(r.0));
let tip_length = LineStripFlags::get_triangle_cap_tip_length(radius.0);
let vector_len = vector.length();
let end = origin + vector * ((vector_len - tip_length) / vector_len);
let end = origin + vector;

let segment = line_batch
.add_segment(origin, end)
.radius(radius)
.color(color)
.flags(re_renderer::renderer::LineStripFlags::CAP_END_TRIANGLE)
.flags(
re_renderer::renderer::LineStripFlags::CAP_END_TRIANGLE
| re_renderer::renderer::LineStripFlags::CAP_START_ROUND
| re_renderer::renderer::LineStripFlags::CAP_START_EXTEND_OUTWARDS,
)
.user_data(picking_instance_hash);

if let Some(outline_mask_ids) = entity_highlight.instances.get(&instance_key) {
Expand Down

2 comments on commit 0ff3191

@github-actions
Copy link

Choose a reason for hiding this comment

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

Rust Benchmark

Benchmark suite Current: 0ff3191 Previous: 58fde68 Ratio
datastore/insert/batch/rects/insert 608152 ns/iter (± 1760) 614660 ns/iter (± 2797) 0.99
datastore/latest_at/batch/rects/query 1845 ns/iter (± 3) 1860 ns/iter (± 2) 0.99
datastore/latest_at/missing_components/primary 287 ns/iter (± 3) 288 ns/iter (± 0) 1.00
datastore/latest_at/missing_components/secondaries 439 ns/iter (± 0) 462 ns/iter (± 0) 0.95
datastore/range/batch/rects/query 152510 ns/iter (± 269) 152737 ns/iter (± 316) 1.00
mono_points_arrow/generate_message_bundles 43451781 ns/iter (± 1105165) 47719562 ns/iter (± 1172319) 0.91
mono_points_arrow/generate_messages 122816272 ns/iter (± 1289870) 125221480 ns/iter (± 1085920) 0.98
mono_points_arrow/encode_log_msg 151781168 ns/iter (± 552829) 155424404 ns/iter (± 1007078) 0.98
mono_points_arrow/encode_total 323725799 ns/iter (± 1702107) 328100145 ns/iter (± 2593450) 0.99
mono_points_arrow/decode_log_msg 177567386 ns/iter (± 953469) 179814536 ns/iter (± 880070) 0.99
mono_points_arrow/decode_message_bundles 53587070 ns/iter (± 1103956) 54792433 ns/iter (± 768811) 0.98
mono_points_arrow/decode_total 231015505 ns/iter (± 1410548) 228898481 ns/iter (± 1395282) 1.01
batch_points_arrow/generate_message_bundles 288112 ns/iter (± 625) 285400 ns/iter (± 2281) 1.01
batch_points_arrow/generate_messages 6188 ns/iter (± 23) 6035 ns/iter (± 66) 1.03
batch_points_arrow/encode_log_msg 385152 ns/iter (± 1885) 380886 ns/iter (± 2348) 1.01
batch_points_arrow/encode_total 687040 ns/iter (± 1914) 688488 ns/iter (± 5164) 1.00
batch_points_arrow/decode_log_msg 352164 ns/iter (± 701) 358703 ns/iter (± 1400) 0.98
batch_points_arrow/decode_message_bundles 1607 ns/iter (± 9) 1609 ns/iter (± 20) 1.00
batch_points_arrow/decode_total 359563 ns/iter (± 1147) 363971 ns/iter (± 1491) 0.99
arrow_mono_points/insert 6078235757 ns/iter (± 11589970) 6181458622 ns/iter (± 33632406) 0.98
arrow_mono_points/query 1752430 ns/iter (± 15347) 1804337 ns/iter (± 23690) 0.97
arrow_batch_points/insert 3061032 ns/iter (± 8796) 3122613 ns/iter (± 101923) 0.98
arrow_batch_points/query 15449 ns/iter (± 263) 15453 ns/iter (± 222) 1.00
arrow_batch_vecs/insert 43639 ns/iter (± 76) 43589 ns/iter (± 154) 1.00
arrow_batch_vecs/query 479866 ns/iter (± 388) 480922 ns/iter (± 1102) 1.00
tuid/Tuid::random 34 ns/iter (± 0) 34 ns/iter (± 0) 1

This comment was automatically generated by workflow using github-action-benchmark.

@github-actions
Copy link

Choose a reason for hiding this comment

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

Rust Benchmark

Benchmark suite Current: 0ff3191 Previous: 58fde68 Ratio
datastore/insert/batch/rects/insert 612022 ns/iter (± 2517) 614660 ns/iter (± 2797) 1.00
datastore/latest_at/batch/rects/query 1850 ns/iter (± 4) 1860 ns/iter (± 2) 0.99
datastore/latest_at/missing_components/primary 292 ns/iter (± 0) 288 ns/iter (± 0) 1.01
datastore/latest_at/missing_components/secondaries 442 ns/iter (± 0) 462 ns/iter (± 0) 0.96
datastore/range/batch/rects/query 152890 ns/iter (± 108) 152737 ns/iter (± 316) 1.00
mono_points_arrow/generate_message_bundles 48635666 ns/iter (± 1324793) 47719562 ns/iter (± 1172319) 1.02
mono_points_arrow/generate_messages 134976653 ns/iter (± 1236588) 125221480 ns/iter (± 1085920) 1.08
mono_points_arrow/encode_log_msg 166405250 ns/iter (± 1061791) 155424404 ns/iter (± 1007078) 1.07
mono_points_arrow/encode_total 350408483 ns/iter (± 2080116) 328100145 ns/iter (± 2593450) 1.07
mono_points_arrow/decode_log_msg 188774152 ns/iter (± 1165284) 179814536 ns/iter (± 880070) 1.05
mono_points_arrow/decode_message_bundles 62299776 ns/iter (± 878320) 54792433 ns/iter (± 768811) 1.14
mono_points_arrow/decode_total 247162441 ns/iter (± 1574092) 228898481 ns/iter (± 1395282) 1.08
batch_points_arrow/generate_message_bundles 288290 ns/iter (± 490) 285400 ns/iter (± 2281) 1.01
batch_points_arrow/generate_messages 6105 ns/iter (± 12) 6035 ns/iter (± 66) 1.01
batch_points_arrow/encode_log_msg 384037 ns/iter (± 2590) 380886 ns/iter (± 2348) 1.01
batch_points_arrow/encode_total 700886 ns/iter (± 2246) 688488 ns/iter (± 5164) 1.02
batch_points_arrow/decode_log_msg 350984 ns/iter (± 1671) 358703 ns/iter (± 1400) 0.98
batch_points_arrow/decode_message_bundles 1609 ns/iter (± 10) 1609 ns/iter (± 20) 1
batch_points_arrow/decode_total 359916 ns/iter (± 1539) 363971 ns/iter (± 1491) 0.99
arrow_mono_points/insert 6900254250 ns/iter (± 16932732) 6181458622 ns/iter (± 33632406) 1.12
arrow_mono_points/query 1767107 ns/iter (± 26753) 1804337 ns/iter (± 23690) 0.98
arrow_batch_points/insert 3084449 ns/iter (± 19563) 3122613 ns/iter (± 101923) 0.99
arrow_batch_points/query 16067 ns/iter (± 248) 15453 ns/iter (± 222) 1.04
arrow_batch_vecs/insert 44048 ns/iter (± 96) 43589 ns/iter (± 154) 1.01
arrow_batch_vecs/query 480052 ns/iter (± 563) 480922 ns/iter (± 1102) 1.00
tuid/Tuid::random 34 ns/iter (± 0) 34 ns/iter (± 0) 1

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.