Fix annotation images sometimes drawn in the background. (#1933)

This caused fairly ugly rendering whenever that happened. Also cleaned up redundant image/textured_rect defintions in spatial scene buildup

crates/re_renderer/src/renderer/rectangles.rs

@@ -33,15 +33,15 @@ use super::{
 };
 
 /// Texture filter setting for magnification (a texel covers several pixels).
-#[derive(Debug)]
+#[derive(Debug, Clone, Copy)]
 pub enum TextureFilterMag {
  Linear,
  Nearest,
  // TODO(andreas): Offer advanced (shader implemented) filters like cubic?
 }
 
 /// Texture filter setting for minification (several texels fall to one pixel).
-#[derive(Debug)]
+#[derive(Debug, Clone, Copy)]
 pub enum TextureFilterMin {
  Linear,
  Nearest,
@@ -98,6 +98,7 @@ impl ColormappedTexture {
  }
 }
 
+#[derive(Clone)]
 pub struct TexturedRect {
  /// Top left corner position in world space.
  pub top_left_corner_position: glam::Vec3,
@@ -114,6 +115,7 @@ pub struct TexturedRect {
  pub options: RectangleOptions,
 }
 
+#[derive(Clone)]
 pub struct RectangleOptions {
  pub texture_filter_magnification: TextureFilterMag,
  pub texture_filter_minification: TextureFilterMin,

crates/re_viewer/src/ui/view_spatial/scene/mod.rs

@@ -7,7 +7,7 @@ use re_log_types::{
  component_types::{ClassId, InstanceKey, KeypointId},
  DecodedTensor, MeshId,
 };
-use re_renderer::{Color32, OutlineMaskPreference, Size};
+use re_renderer::{renderer::TexturedRect, Color32, OutlineMaskPreference, Size};
 
 use crate::{
  misc::{mesh_loader::LoadedMesh, SpaceViewHighlights, TransformCache, ViewerContext},
@@ -63,15 +63,11 @@ pub struct Image {
 
  pub tensor: DecodedTensor,
 
- /// If this is a depth map, how long is a meter?
- ///
- /// For example, with a `u16` dtype one might have
- /// `meter == 1000.0` for millimeter precision
- /// up to a ~65m range.
- pub meter: Option<f32>,
-
  /// A thing that provides additional semantic context for your dtype.
  pub annotations: Arc<Annotations>,
+
+ /// Textured rectangle for the renderer.
+ pub textured_rect: TexturedRect,
 }
 
 pub enum UiLabelTarget {
@@ -104,9 +100,6 @@ pub struct SceneSpatialUiData {
  /// Picking any any of these rects cause the referred instance to be hovered.
  /// Only use this for 2d overlays!
  pub pickable_ui_rects: Vec<(egui::Rect, InstancePathHash)>,
-
- /// Images are a special case of rects where we're storing some extra information to allow miniature previews etc.
- pub images: Vec<Image>,
 }
 
 pub struct SceneSpatial {
@@ -231,7 +224,7 @@ impl SceneSpatial {
  return SpatialNavigationMode::ThreeD;
  }
 
- if !self.ui.images.is_empty() {
+ if !self.primitives.images.is_empty() {
  return SpatialNavigationMode::TwoD;
  }
  if self.num_logged_3d_objects == 0 {

crates/re_viewer/src/ui/view_spatial/scene/picking.rs

@@ -2,10 +2,10 @@
 
 use ahash::HashSet;
 use re_data_store::InstancePathHash;
-use re_log_types::{component_types::InstanceKey, EntityPathHash};
+use re_log_types::component_types::InstanceKey;
 use re_renderer::PickingLayerProcessor;
 
-use super::{SceneSpatialPrimitives, SceneSpatialUiData};
+use super::{Image, SceneSpatialPrimitives, SceneSpatialUiData};
 use crate::{
  misc::instance_hash_conversions::instance_path_hash_from_picking_layer_id,
  ui::view_spatial::eye::Eye,
@@ -116,11 +116,7 @@ impl PickingContext {
  self,
  previous_picking_result,
  );
- let mut rect_hits = picking_textured_rects(
- self,
- &primitives.textured_rectangles,
- &primitives.textured_rectangles_ids,
- );
+ let mut rect_hits = picking_textured_rects(self, &primitives.images);
  rect_hits.sort_by(|a, b| b.depth_offset.cmp(&a.depth_offset));
  let ui_rect_hits = picking_ui_rects(self, ui_data);
 
@@ -241,23 +237,13 @@ fn picking_gpu(
  }
 }
 
-fn picking_textured_rects(
- context: &PickingContext,
- textured_rectangles: &[re_renderer::renderer::TexturedRect],
- textured_rectangles_ids: &[EntityPathHash],
-) -> Vec<PickingRayHit> {
+fn picking_textured_rects(context: &PickingContext, images: &[Image]) -> Vec<PickingRayHit> {
  crate::profile_function!();
 
  let mut hits = Vec::new();
 
- for (rect, id) in textured_rectangles
- .iter()
- .zip(textured_rectangles_ids.iter())
- {
- if !id.is_some() {
- continue;
- }
-
+ for image in images {
+ let rect = &image.textured_rect;
  let rect_plane = macaw::Plane3::from_normal_point(
  rect.extent_u.cross(rect.extent_v).normalize(),
  rect.top_left_corner_position,
@@ -277,7 +263,7 @@ fn picking_textured_rects(
  if (0.0..=1.0).contains(&u) && (0.0..=1.0).contains(&v) {
  hits.push(PickingRayHit {
  instance_path_hash: InstancePathHash {
- entity_path_hash: *id,
+ entity_path_hash: image.ent_path.hash(),
  instance_key: InstanceKey::from_2d_image_coordinate(
  [
  (u * rect.colormapped_texture.texture.width() as f32) as u32,

crates/re_viewer/src/ui/view_spatial/scene/primitives.rs

@@ -1,6 +1,6 @@
 use egui::Color32;
 use re_data_store::EntityPath;
-use re_log_types::{component_types::InstanceKey, EntityPathHash};
+use re_log_types::component_types::InstanceKey;
 use re_renderer::{
  renderer::{DepthClouds, MeshInstance},
  LineStripSeriesBuilder, PointCloudBuilder,
@@ -20,11 +20,7 @@ pub struct SceneSpatialPrimitives {
  /// Estimated bounding box of all data in scene coordinates. Accumulated.
  pub(super) bounding_box: macaw::BoundingBox,
 
- // TODO(andreas): Storing extra data like so is unsafe and not future proof either
- // (see also above comment on the need to separate cpu-readable data)
- pub textured_rectangles_ids: Vec<EntityPathHash>,
- pub textured_rectangles: Vec<re_renderer::renderer::TexturedRect>,
-
+ pub images: Vec<super::Image>,
  pub line_strips: LineStripSeriesBuilder,
  pub points: PointCloudBuilder,
  pub meshes: Vec<MeshSource>,
@@ -44,8 +40,7 @@ impl SceneSpatialPrimitives {
  pub fn new(re_ctx: &mut re_renderer::RenderContext) -> Self {
  Self {
  bounding_box: macaw::BoundingBox::nothing(),
- textured_rectangles_ids: Default::default(),
- textured_rectangles: Default::default(),
+ images: Default::default(),
  line_strips: LineStripSeriesBuilder::new(re_ctx)
  .radius_boost_in_ui_points_for_outlines(SIZE_BOOST_IN_POINTS_FOR_LINE_OUTLINES),
  points: PointCloudBuilder::new(re_ctx)
@@ -68,16 +63,15 @@ impl SceneSpatialPrimitives {
  pub fn num_primitives(&self) -> usize {
  let Self {
  bounding_box: _,
- textured_rectangles,
- textured_rectangles_ids: _,
+ images,
  line_strips,
  points,
  meshes,
  depth_clouds,
  any_outlines: _,
  } = &self;
 
- textured_rectangles.len()
+ images.len()
  + line_strips.vertices.len()
  + points.vertices.len()
  + meshes.len()
@@ -89,8 +83,7 @@ impl SceneSpatialPrimitives {
 
  let Self {
  bounding_box,
- textured_rectangles_ids: _,
- textured_rectangles,
+ images,
  line_strips,
  points,
  meshes,
@@ -100,7 +93,8 @@ impl SceneSpatialPrimitives {
 
  *bounding_box = macaw::BoundingBox::nothing();
 
- for rect in textured_rectangles {
+ for image in images {
+ let rect = &image.textured_rect;
  bounding_box.extend(rect.top_left_corner_position);
  bounding_box.extend(rect.top_left_corner_position + rect.extent_u);
  bounding_box.extend(rect.top_left_corner_position + rect.extent_v);

crates/re_viewer/src/ui/view_spatial/scene/scene_part/images.rs

@@ -25,20 +25,18 @@ use crate::{
 
 use super::ScenePart;
 
-#[allow(clippy::too_many_arguments)]
-fn push_tensor_texture(
- scene: &mut SceneSpatial,
+fn to_textured_rect(
  ctx: &mut ViewerContext<'_>,
  annotations: &Annotations,
  world_from_obj: glam::Mat4,
  ent_path: &EntityPath,
  tensor: &DecodedTensor,
  multiplicative_tint: egui::Rgba,
  outline_mask: OutlineMaskPreference,
-) {
+) -> Option<re_renderer::renderer::TexturedRect> {
  crate::profile_function!();
 
- let Some([height, width, _]) = tensor.image_height_width_channels() else { return; };
+ let Some([height, width, _]) = tensor.image_height_width_channels() else { return None; };
 
  let debug_name = ent_path.to_string();
  let tensor_stats = ctx.cache.tensor_stats(tensor);
@@ -68,7 +66,7 @@ fn push_tensor_texture(
  re_renderer::renderer::TextureFilterMin::Linear
  };
 
- let textured_rect = re_renderer::renderer::TexturedRect {
+ Some(re_renderer::renderer::TexturedRect {
  top_left_corner_position: world_from_obj.transform_point3(glam::Vec3::ZERO),
  extent_u: world_from_obj.transform_vector3(glam::Vec3::X * width as f32),
  extent_v: world_from_obj.transform_vector3(glam::Vec3::Y * height as f32),
@@ -80,15 +78,11 @@ fn push_tensor_texture(
  depth_offset: -1, // Push to background. Mostly important for mouse picking order!
  outline_mask,
  },
- };
- scene.primitives.textured_rectangles.push(textured_rect);
- scene
- .primitives
- .textured_rectangles_ids
- .push(ent_path.hash());
+ })
  }
  Err(err) => {
  re_log::error_once!("Failed to create texture from tensor for {debug_name:?}: {err}");
+ None
  }
  }
 }
@@ -99,15 +93,17 @@ fn handle_image_layering(scene: &mut SceneSpatial) {
  // Handle layered rectangles that are on (roughly) the same plane and were logged in sequence.
  // First, group by similar plane.
  // TODO(andreas): Need planes later for picking as well!
- let rects_grouped_by_plane = {
+ let images_grouped_by_plane = {
  let mut cur_plane = macaw::Plane3::from_normal_dist(Vec3::NAN, std::f32::NAN);
  let mut rectangle_group = Vec::new();
  scene
  .primitives
- .textured_rectangles
- .iter_mut()
+ .images
+ .drain(..) // We rebuild the list as we might reorder as well!
  .batching(move |it| {
- for rect in it.by_ref() {
+ for image in it {
+ let rect = &image.textured_rect;
+
  let prev_plane = cur_plane;
  cur_plane = macaw::Plane3::from_normal_point(
  rect.extent_u.cross(rect.extent_v).normalize(),
@@ -119,25 +115,30 @@ fn handle_image_layering(scene: &mut SceneSpatial) {
  && prev_plane.normal.dot(cur_plane.normal) < 0.99
  && (prev_plane.d - cur_plane.d) < 0.01
  {
- let previous_group = std::mem::replace(&mut rectangle_group, vec![rect]);
+ let previous_group = std::mem::replace(&mut rectangle_group, vec![image]);
  return Some(previous_group);
  }
- rectangle_group.push(rect);
+ rectangle_group.push(image);
  }
  if !rectangle_group.is_empty() {
  Some(rectangle_group.drain(..).collect())
  } else {
  None
  }
  })
- };
- // Then, change opacity & transformation for planes within group except the base plane.
- for mut grouped_rects in rects_grouped_by_plane {
- let total_num_images = grouped_rects.len();
- for (idx, rect) in grouped_rects.iter_mut().enumerate() {
+ }
+ .collect_vec();
+
+ // Then, for each planar group do resorting and change transparency.
+ for mut grouped_images in images_grouped_by_plane {
+ // Class id images should generally come last as they typically have large areas being zeroed out (which maps to fully transparent).
+ grouped_images.sort_by_key(|image| image.tensor.meaning == TensorDataMeaning::ClassId);
+
+ let total_num_images = grouped_images.len();
+ for (idx, image) in grouped_images.iter_mut().enumerate() {
  // Set depth offset for correct order and avoid z fighting when there is a 3d camera.
  // Keep behind depth offset 0 for correct picking order.
- rect.options.depth_offset =
+ image.textured_rect.options.depth_offset =
  (idx as isize - total_num_images as isize) as re_renderer::DepthOffset;
 
  // make top images transparent
@@ -146,8 +147,14 @@ fn handle_image_layering(scene: &mut SceneSpatial) {
  } else {
  1.0 / total_num_images.at_most(20) as f32
  }; // avoid precision problems in framebuffer
- rect.options.multiplicative_tint = rect.options.multiplicative_tint.multiply(opacity);
+ image.textured_rect.options.multiplicative_tint = image
+ .textured_rect
+ .options
+ .multiplicative_tint
+ .multiply(opacity);
  }
+
+ scene.primitives.images.extend(grouped_images);
  }
 }
 
@@ -190,16 +197,6 @@ impl ImagesPart {
  };
 
  let annotations = scene.annotation_map.find(ent_path);
-
- // TODO(jleibs): Meter should really be its own component
- let meter = tensor.meter;
- scene.ui.images.push(Image {
- ent_path: ent_path.clone(),
- tensor: tensor.clone(),
- meter,
- annotations: annotations.clone(),
- });
-
  let entity_highlight = highlights.entity_outline_mask(ent_path.hash());
 
  if *properties.backproject_depth.get() && tensor.meaning == TensorDataMeaning::Depth {
@@ -234,16 +231,22 @@ impl ImagesPart {
  DefaultColor::OpaqueWhite,
  );
 
- push_tensor_texture(
- scene,
+ if let Some(textured_rect) = to_textured_rect(
  ctx,
  &annotations,
  world_from_obj,
  ent_path,
  &tensor,
  color.into(),
  entity_highlight.overall,
- );
+ ) {
+ scene.primitives.images.push(Image {
+ ent_path: ent_path.clone(),
+ tensor,
+ annotations,
+ textured_rect,
+ });
+ }
  }
 
  Ok(())