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(())