Skip to content

Commit

Permalink
fix temporal gaps in gpu picking report
Browse files Browse the repository at this point in the history
  • Loading branch information
Wumpf committed Mar 30, 2023
1 parent a1669ac commit 0a38698
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 4 deletions.
3 changes: 3 additions & 0 deletions crates/re_viewer/src/ui/view_spatial/scene/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,10 +247,12 @@ impl SceneSpatial {
SpatialNavigationMode::ThreeD
}

#[allow(clippy::too_many_arguments)]
pub fn picking(
&self,
render_ctx: &re_renderer::RenderContext,
gpu_readback_identifier: re_renderer::GpuReadbackIdentifier,
previous_picking_result: &Option<PickingResult>,
pointer_in_ui: glam::Vec2,
ui_rect: &egui::Rect,
eye: &Eye,
Expand All @@ -259,6 +261,7 @@ impl SceneSpatial {
picking::picking(
render_ctx,
gpu_readback_identifier,
previous_picking_result,
pointer_in_ui,
ui_rect,
eye,
Expand Down
25 changes: 23 additions & 2 deletions crates/re_viewer/src/ui/view_spatial/scene/picking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use crate::{
ui::view_spatial::eye::Eye,
};

#[derive(Clone)]
pub enum AdditionalPickingInfo {
/// No additional picking information.
None,
Expand All @@ -21,6 +22,7 @@ pub enum AdditionalPickingInfo {
GuiOverlay,
}

#[derive(Clone)]
pub struct PickingRayHit {
/// What entity or instance got hit by the picking ray.
///
Expand All @@ -34,6 +36,9 @@ pub struct PickingRayHit {

/// Any additional information about the picking hit.
pub info: AdditionalPickingInfo,

/// True if this picking result came from a GPU picking pass.
pub used_gpu_picking: bool,
}

impl PickingRayHit {
Expand All @@ -43,10 +48,12 @@ impl PickingRayHit {
ray_t: t,
info: AdditionalPickingInfo::None,
depth_offset: 0,
used_gpu_picking: false,
}
}
}

#[derive(Clone)]
pub struct PickingResult {
/// Picking ray hit for an opaque object (if any).
pub opaque_hit: Option<PickingRayHit>,
Expand Down Expand Up @@ -131,11 +138,11 @@ impl PickingState {
}
}

// TODO(andreas): This should be temporary. As we switch over (almost) everything over to gpu based picking this should go away.
#[allow(clippy::too_many_arguments)]
pub fn picking(
render_ctx: &re_renderer::RenderContext,
gpu_readback_identifier: re_renderer::GpuReadbackIdentifier,
previous_picking_result: &Option<PickingResult>,
pointer_in_ui: glam::Vec2,
ui_rect: &egui::Rect,
eye: &Eye,
Expand All @@ -160,6 +167,7 @@ pub fn picking(
ray_t: f32::INFINITY,
info: AdditionalPickingInfo::None,
depth_offset: 0,
used_gpu_picking: false,
},
// Combined, sorted (and partially "hidden") by opaque results later.
transparent_hits: Vec::new(),
Expand Down Expand Up @@ -206,9 +214,20 @@ pub fn picking(

// TODO(andreas): We're lacking depth information!
state.closest_opaque_pick.instance_path_hash = picked_object;
state.closest_opaque_pick.used_gpu_picking = true;
} else {
// TODO(andreas): It is *possible* that some frames we don't get a picking result and the frame after we get several.
// It is possible that some frames we don't get a picking result and the frame after we get several.
// We need to cache the last picking result and use it until we get a new one or the mouse leaves the screen.
// (Andreas: On my mac this *actually* happens in very simple scenes, I get occasional frames with 0 and then with 2 picking results!)
if let Some(PickingResult {
opaque_hit: Some(previous_opaque_hit),
..
}) = previous_picking_result
{
if previous_opaque_hit.used_gpu_picking {
state.closest_opaque_pick = previous_opaque_hit.clone();
}
}
}
}

Expand Down Expand Up @@ -368,6 +387,7 @@ fn picking_textured_rects(
ray_t: t,
info: AdditionalPickingInfo::TexturedRect(glam::vec2(u, v)),
depth_offset: rect.depth_offset,
used_gpu_picking: false,
};
state.check_hit(0.0, picking_hit, rect.multiplicative_tint.a() < 1.0);
}
Expand All @@ -391,6 +411,7 @@ fn picking_ui_rects(
ray_t: 0.0,
info: AdditionalPickingInfo::GuiOverlay,
depth_offset: 0,
used_gpu_picking: false,
},
false,
);
Expand Down
12 changes: 10 additions & 2 deletions crates/re_viewer/src/ui/view_spatial/ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@ use crate::{
};

use super::{
eye::Eye, scene::SceneSpatialUiData, ui_2d::View2DState, ui_3d::View3DState, SceneSpatial,
SpaceSpecs,
eye::Eye,
scene::{PickingResult, SceneSpatialUiData},
ui_2d::View2DState,
ui_3d::View3DState,
SceneSpatial, SpaceSpecs,
};

/// Describes how the scene is navigated, determining if it is a 2D or 3D experience.
Expand Down Expand Up @@ -77,6 +80,10 @@ pub struct ViewSpatialState {
#[serde(skip)]
pub scene_num_primitives: usize,

/// Last frame's picking result.
#[serde(skip)]
pub previous_picking_result: Option<PickingResult>,

pub(super) state_2d: View2DState,
pub(super) state_3d: View3DState,

Expand All @@ -97,6 +104,7 @@ impl Default for ViewSpatialState {
point_radius: re_renderer::Size::AUTO, // let re_renderer decide
line_radius: re_renderer::Size::AUTO, // let re_renderer decide
},
previous_picking_result: None,
}
}
}
Expand Down
4 changes: 4 additions & 0 deletions crates/re_viewer/src/ui/view_spatial/ui_2d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -367,11 +367,13 @@ fn view_2d_scrollable(
let picking_result = scene.picking(
ctx.render_ctx,
space_view_id.gpu_readback_id(),
&state.previous_picking_result,
glam::vec2(pointer_pos_space.x, pointer_pos_space.y),
&scene_rect_accum,
&eye,
hover_radius,
);
state.previous_picking_result = Some(picking_result.clone());

for hit in picking_result.iter_hits() {
let Some(instance_path) = hit.instance_path_hash.resolve(&ctx.log_db.entity_db)
Expand Down Expand Up @@ -462,6 +464,8 @@ fn view_2d_scrollable(
.map(|instance_path| Item::InstancePath(Some(space_view_id), instance_path))
}));
}
} else {
state.previous_picking_result = None;
}

ctx.select_hovered_on_click(&response);
Expand Down
4 changes: 4 additions & 0 deletions crates/re_viewer/src/ui/view_spatial/ui_3d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,11 +377,13 @@ pub fn view_3d(
let picking_result = scene.picking(
ctx.render_ctx,
space_view_id.gpu_readback_id(),
&state.previous_picking_result,
glam::vec2(pointer_pos.x, pointer_pos.y),
&rect,
&eye,
5.0,
);
state.previous_picking_result = Some(picking_result.clone());

for hit in picking_result.iter_hits() {
let Some(instance_path) = hit.instance_path_hash.resolve(&ctx.log_db.entity_db)
Expand Down Expand Up @@ -459,6 +461,8 @@ pub fn view_3d(
.map(|hit| picking_result.space_position(hit));

project_onto_other_spaces(ctx, &scene.space_cameras, &mut state.state_3d, space);
} else {
state.previous_picking_result = None;
}

ctx.select_hovered_on_click(&response);
Expand Down

0 comments on commit 0a38698

Please sign in to comment.