Stable image order, fixing flickering

crates/re_log_types/src/component_types/draw_order.rs

@@ -17,7 +17,7 @@ use crate::Component;
 ///
 /// assert_eq!(DrawOrder::data_type(), DataType::Float32);
 /// ```
-#[derive(Debug, Clone, ArrowField, ArrowSerialize, ArrowDeserialize)]
+#[derive(Debug, Clone, Copy, ArrowField, ArrowSerialize, ArrowDeserialize)]
 #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
 #[arrow_field(transparent)]
 pub struct DrawOrder(pub f32);

crates/re_log_types/src/hash.rs

@@ -8,7 +8,7 @@ use std::hash::BuildHasher;
 /// 10^-9  collision risk with 190k values.
 /// 10^-6  collision risk with   6M values.
 /// 10^-3  collision risk with 200M values.
-#[derive(Copy, Clone, Eq)]
+#[derive(Copy, Clone, Eq, PartialOrd, Ord)]
 pub struct Hash64(u64);
 
 impl Hash64 {

crates/re_log_types/src/path/entity_path.rs

@@ -8,7 +8,7 @@ use crate::{
 // ----------------------------------------------------------------------------
 
 /// A 64 bit hash of [`EntityPath`] with very small risk of collision.
-#[derive(Copy, Clone, Eq)]
+#[derive(Copy, Clone, Eq, PartialOrd, Ord)]
 pub struct EntityPathHash(Hash64);
 
 impl EntityPathHash {

crates/re_viewer/src/misc/transform_cache.rs

@@ -1,8 +1,21 @@
 use nohash_hasher::IntMap;
 use re_arrow_store::LatestAtQuery;
 use re_data_store::{log_db::EntityDb, EntityPath, EntityPropertyMap, EntityTree};
+use re_log_types::EntityPathHash;
 use re_viewer_context::TimeControl;
 
+#[derive(Clone)]
+struct TransformInfo {
+    /// The transform from the entity to the reference space.
+    pub reference_from_entity: glam::Affine3A,
+
+    /// The pinhole camera ancestor of this entity if any.
+    ///
+    /// None indicates that this entity is under the eye camera with no Pinhole camera in-between.
+    /// Some indicates that the entity is under a pinhole camera at the given entity path that is not at the root of the space view.
+    pub parent_pinhole: Option<EntityPathHash>,
+}
+
 /// Provides transforms from an entity to a chosen reference space for all elements in the scene
 /// for the currently selected time & timeline.
 ///
@@ -16,7 +29,7 @@ pub struct TransformCache {
     reference_path: EntityPath,
 
     /// All reachable entities.
-    reference_from_entity_per_entity: IntMap<EntityPath, glam::Affine3A>,
+    transform_per_entity: IntMap<EntityPath, TransformInfo>,
 
     /// All unreachable descendant paths of `reference_path`.
     unreachable_descendants: Vec<(EntityPath, UnreachableTransform)>,
@@ -73,7 +86,7 @@ impl TransformCache {
 
         let mut transforms = TransformCache {
             reference_path: space_path.clone(),
-            reference_from_entity_per_entity: Default::default(),
+            transform_per_entity: Default::default(),
             unreachable_descendants: Default::default(),
             first_unreachable_parent: None,
         };
@@ -95,11 +108,11 @@ impl TransformCache {
             &query,
             entity_prop_map,
             glam::Affine3A::IDENTITY,
-            false,
+            None, // Ignore potential pinhole camera at the root of the space view, since it regarded as being "above" this root.
         );
 
         // Walk up from the reference to the highest reachable parent.
-        let mut encountered_pinhole = false;
+        let mut encountered_pinhole = None;
         let mut reference_from_ancestor = glam::Affine3A::IDENTITY;
         while let Some(parent_path) = current_tree.path.parent() {
             let Some(parent_tree) = &entity_db.tree.subtree(&parent_path) else {
@@ -156,17 +169,17 @@ impl TransformCache {
         query: &LatestAtQuery,
         entity_properties: &EntityPropertyMap,
         reference_from_entity: glam::Affine3A,
-        encountered_pinhole: bool,
+        encountered_pinhole: Option<EntityPathHash>,
     ) {
-        match self
-            .reference_from_entity_per_entity
-            .entry(tree.path.clone())
-        {
+        match self.transform_per_entity.entry(tree.path.clone()) {
             std::collections::hash_map::Entry::Occupied(_) => {
                 return;
             }
             std::collections::hash_map::Entry::Vacant(e) => {
-                e.insert(reference_from_entity);
+                e.insert(TransformInfo {
+                    reference_from_entity,
+                    parent_pinhole: encountered_pinhole,
+                });
             }
         }
 
@@ -205,10 +218,20 @@ impl TransformCache {
     /// Retrieves the transform of on entity from its local system to the space of the reference.
     ///
     /// Returns None if the path is not reachable.
-    pub fn reference_from_entity(&self, entity_path: &EntityPath) -> Option<macaw::Affine3A> {
-        self.reference_from_entity_per_entity
-            .get(entity_path)
-            .cloned()
+    pub fn reference_from_entity(&self, ent_path: &EntityPath) -> Option<macaw::Affine3A> {
+        self.transform_per_entity
+            .get(ent_path)
+            .map(|i| i.reference_from_entity)
+    }
+
+    /// Retrieves the ancestor (or self) pinhole under which this entity sits.
+    ///
+    /// None indicates either that the entity does not exist in this hierarchy or that this entity is under the eye camera with no Pinhole camera in-between.
+    /// Some indicates that the entity is under a pinhole camera at the given entity path that is not at the root of the space view.
+    pub fn parent_pinhole(&self, ent_path: &EntityPath) -> Option<EntityPathHash> {
+        self.transform_per_entity
+            .get(ent_path)
+            .and_then(|i| i.parent_pinhole)
     }
 
     // This method isn't currently implemented, but we might need it in the future.
@@ -225,7 +248,7 @@ fn transform_at(
     store: &re_arrow_store::DataStore,
     query: &LatestAtQuery,
     pinhole_image_plane_distance: impl Fn(&EntityPath) -> f32,
-    encountered_pinhole: &mut bool,
+    encountered_pinhole: &mut Option<EntityPathHash>,
 ) -> Result<Option<glam::Affine3A>, UnreachableTransform> {
     if let Some(transform) = store.query_latest_component(entity_path, query) {
         match transform {
@@ -234,10 +257,10 @@ fn transform_at(
             re_log_types::Transform::Unknown => Err(UnreachableTransform::UnknownTransform),
 
             re_log_types::Transform::Pinhole(pinhole) => {
-                if *encountered_pinhole {
+                if encountered_pinhole.is_some() {
                     Err(UnreachableTransform::NestedPinholeCameras)
                 } else {
-                    *encountered_pinhole = true;
+                    *encountered_pinhole = Some(entity_path.hash());
 
                     // A pinhole camera means that we're looking at some 2D image plane from a single point (the pinhole).
                     // Center the image plane and move it along z, scaling the further the image plane is.

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

@@ -1,4 +1,7 @@
-use std::{collections::BTreeMap, sync::Arc};
+use std::{
+    collections::{BTreeMap, BTreeSet},
+    sync::Arc,
+};
 
 use ahash::HashMap;
 
@@ -10,8 +13,6 @@ use re_log_types::{
 };
 use re_renderer::{renderer::TexturedRect, Color32, OutlineMaskPreference, Size};
 use re_viewer_context::{auto_color, AnnotationMap, Annotations, SceneQuery, ViewerContext};
-use smallvec::smallvec;
-use smallvec::SmallVec;
 
 use crate::misc::{mesh_loader::LoadedMesh, SpaceViewHighlights, TransformCache};
 
@@ -43,6 +44,9 @@ pub struct Image {
     /// Textured rectangle for the renderer.
     pub textured_rect: TexturedRect,
 
+    /// Pinhole camera this image is under.
+    pub parent_pinhole: Option<EntityPathHash>,
+
     /// Draw order value used.
     pub draw_order: DrawOrder,
 }
@@ -130,6 +134,7 @@ impl SceneSpatial {
     ) -> EntityDepthOffsets {
         crate::profile_function!();
 
+        #[derive(PartialEq, PartialOrd, Eq, Ord)]
         enum DrawOrderTarget {
             Entity(EntityPathHash),
             DefaultBox2D,
@@ -140,7 +145,8 @@ impl SceneSpatial {
 
         let store = &ctx.log_db.entity_db.data_store;
 
-        let mut entities_per_draw_order = BTreeMap::<DrawOrder, SmallVec<[_; 4]>>::new();
+        // Use a BTreeSet for entity hashes to get a stable order.
+        let mut entities_per_draw_order = BTreeMap::<DrawOrder, BTreeSet<DrawOrderTarget>>::new();
         for (ent_path, _) in query.iter_entities() {
             if let Some(draw_order) = store.query_latest_component::<DrawOrder>(
                 ent_path,
@@ -149,26 +155,26 @@ impl SceneSpatial {
                 entities_per_draw_order
                     .entry(draw_order)
                     .or_default()
-                    .push(DrawOrderTarget::Entity(ent_path.hash()));
+                    .insert(DrawOrderTarget::Entity(ent_path.hash()));
             }
         }
 
         // Push in default draw orders. All of them using the none hash.
         entities_per_draw_order.insert(
             DrawOrder::DEFAULT_BOX2D,
-            smallvec![DrawOrderTarget::DefaultBox2D],
+            [DrawOrderTarget::DefaultBox2D].into(),
         );
         entities_per_draw_order.insert(
             DrawOrder::DEFAULT_IMAGE,
-            smallvec![DrawOrderTarget::DefaultImage],
+            [DrawOrderTarget::DefaultImage].into(),
         );
         entities_per_draw_order.insert(
             DrawOrder::DEFAULT_LINES2D,
-            smallvec![DrawOrderTarget::DefaultLines2D],
+            [DrawOrderTarget::DefaultLines2D].into(),
         );
         entities_per_draw_order.insert(
             DrawOrder::DEFAULT_POINTS2D,
-            smallvec![DrawOrderTarget::DefaultPoints],
+            [DrawOrderTarget::DefaultPoints].into(),
         );
 
         // Determine re_renderer draw order from this.
@@ -211,7 +217,7 @@ impl SceneSpatial {
                             }
                         }
                     })
-                    .collect::<SmallVec<[_; 4]>>()
+                    .collect::<Vec<_>>()
             })
             .collect();
 

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

@@ -1,11 +1,11 @@
+use std::collections::BTreeMap;
+
 use egui::NumExt;
-use glam::Vec3;
-use itertools::Itertools;
 
 use re_data_store::{EntityPath, EntityProperties};
 use re_log_types::{
     component_types::{ColorRGBA, InstanceKey, Tensor, TensorData, TensorDataMeaning},
-    Component, DecodedTensor, DrawOrder, Transform,
+    Component, DecodedTensor, DrawOrder, EntityPathHash, Transform,
 };
 use re_query::{query_primary_with_history, EntityView, QueryError};
 use re_renderer::{
@@ -89,63 +89,40 @@ fn to_textured_rect(
     }
 }
 
+#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
+struct ImageGrouping {
+    parent_pinhole: Option<EntityPathHash>,
+    draw_order: DrawOrder,
+}
+
 fn handle_image_layering(scene: &mut SceneSpatial) {
     crate::profile_function!();
 
-    // Handle layered rectangles that are on (roughly) the same plane with the same depth offset and were logged in sequence.
-    // First, group by similar plane.
-    // TODO(andreas): Need planes later for picking as well!
-    let images_grouped_by_plane = {
-        let mut cur_plane = macaw::Plane3::from_normal_dist(Vec3::NAN, std::f32::NAN);
-        let mut rectangle_group: Vec<Image> = Vec::new();
-        scene
-            .primitives
-            .images
-            .drain(..) // We rebuild the list as we might reorder as well!
-            .batching(move |it| {
-                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(),
-                        rect.top_left_corner_position,
-                    );
-
-                    fn is_plane_similar(a: macaw::Plane3, b: macaw::Plane3) -> bool {
-                        a.normal.dot(b.normal) > 0.99 && (a.d - b.d).abs() < 0.01
-                    }
-
-                    // Use draw order, not depth offset since depth offset might change when draw order does not.
-                    let has_same_draw_order = rectangle_group
-                        .last()
-                        .map_or(true, |last_image| last_image.draw_order == image.draw_order);
-
-                    // If the planes are similar, add them to the same group, otherwise start a new group.
-                    if has_same_draw_order && is_plane_similar(prev_plane, cur_plane) {
-                        rectangle_group.push(image);
-                    } else {
-                        let previous_group = std::mem::replace(&mut rectangle_group, vec![image]);
-                        return Some(previous_group);
-                    }
-                }
-                if !rectangle_group.is_empty() {
-                    Some(rectangle_group.drain(..).collect())
-                } else {
-                    None
-                }
+    // Rebuild the image list, grouped by "shared plane", identified with camera & draw order.
+    let mut image_groups: BTreeMap<ImageGrouping, Vec<Image>> = BTreeMap::new();
+    for image in scene.primitives.images.drain(..) {
+        image_groups
+            .entry(ImageGrouping {
+                parent_pinhole: image.parent_pinhole,
+                draw_order: image.draw_order,
             })
+            .or_default()
+            .push(image);
     }
-    .collect_vec();
 
-    // Then, for each planar group do resorting and change transparency.
-    for mut grouped_images in images_grouped_by_plane {
+    // Then, for each group do resorting and change transparency.
+    for (_, mut images) in image_groups {
+        // Since we change transparency depending on order and re_renderer doesn't handle transparency
+        // ordering either, we need to ensure that sorting is stable at the very least.
+        // Sorting is done by depth offset, not by draw order which is the same for the entire group.
+        images.sort_by_key(|image| image.textured_rect.options.depth_offset);
+
         // Class id images should generally come last within the same layer 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);
+        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() {
+        let total_num_images = images.len();
+        for (idx, image) in images.iter_mut().enumerate() {
             // make top images transparent
             let opacity = if idx == 0 {
                 1.0
@@ -160,7 +137,7 @@ fn handle_image_layering(scene: &mut SceneSpatial) {
                 .multiply(opacity);
         }
 
-        scene.primitives.images.extend(grouped_images);
+        scene.primitives.images.extend(images);
     }
 }
 
@@ -253,6 +230,7 @@ impl ImagesPart {
                     ent_path: ent_path.clone(),
                     tensor,
                     textured_rect,
+                    parent_pinhole: transforms.parent_pinhole(ent_path),
                     draw_order: draw_order.unwrap_or(DrawOrder::DEFAULT_IMAGE),
                 });
             }