Improve 2D heuristic fallback values for image opacity (#9787)

Co-authored-by: Andreas Reich <[email protected]>

Author: jprochazk

.github/workflows/reusable_checks_rust.yml

@@ -55,7 +55,7 @@ jobs:
   # ---------------------------------------------------------------------------
 
   rs-lints:
-    name: Rust lints (fmt, check, clippy, tests, doc)
+    name: Rust lints (fmt, check, clippy, doc)
     # TODO(andreas): setup-vulkan doesn't work on 24.4 right now due to missing .so
     runs-on: ubuntu-22.04-large
     steps:
@@ -76,22 +76,9 @@ jobs:
         with:
           pixi-version: v0.41.4
 
-      # Install the Vulkan SDK, so we can use the software rasterizer.
-      # TODO(andreas): It would be nice if `setup_software_rasterizer.py` could do that for us as well (note though that this action here is very fast when cached!)
-      - name: Install Vulkan SDK
-        uses: rerun-io/[email protected]
-        with:
-          vulkan_version: ${{ env.VULKAN_SDK_VERSION }}
-          install_runtime: true
-          cache: true
-          stripdown: true
-
-      - name: Setup software rasterizer
-        run: pixi run python ./scripts/ci/setup_software_rasterizer.py
-
       - name: Rust checks (PR subset)
         if: ${{ inputs.CHANNEL == 'pr' }}
-        run: pixi run rs-check --only base_checks sdk_variations cargo_deny wasm docs tests
+        run: pixi run rs-check --only base_checks sdk_variations cargo_deny wasm docs
 
       - name: Rust most checks & tests
         if: ${{ inputs.CHANNEL == 'main' }}
@@ -107,6 +94,48 @@ jobs:
         # See tests/assets/rrd/README.md for more
         run: pixi run check-backwards-compatibility
 
+  rs-tests:
+    name: Rust lints (tests)
+    # TODO(andreas): setup-vulkan doesn't work on 24.4 right now due to missing .so
+    runs-on: ubuntu-22.04-large
+    env:
+      RUSTFLAGS: --cfg=web_sys_unstable_apis
+      RUSTDOCFLAGS: ""
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.ref || '' }}
+          lfs: true
+
+      - name: Set up Rust
+        uses: ./.github/actions/setup-rust
+        with:
+          cache_key: "build-linux"
+          save_cache: true
+          workload_identity_provider: ${{ secrets.GOOGLE_WORKLOAD_IDENTITY_PROVIDER }}
+          service_account: ${{ secrets.GOOGLE_SERVICE_ACCOUNT }}
+
+      - uses: prefix-dev/[email protected]
+        with:
+          pixi-version: v0.41.4
+
+      # Install the Vulkan SDK, so we can use the software rasterizer.
+      # TODO(andreas): It would be nice if `setup_software_rasterizer.py` could do that for us as well (note though that this action here is very fast when cached!)
+      - name: Install Vulkan SDK
+        uses: rerun-io/[email protected]
+        with:
+          vulkan_version: ${{ env.VULKAN_SDK_VERSION }}
+          install_runtime: true
+          cache: true
+          stripdown: true
+
+      - name: Setup software rasterizer
+        run: pixi run python ./scripts/ci/setup_software_rasterizer.py
+
+      - name: Rust checks (PR subset)
+        if: ${{ inputs.CHANNEL == 'pr' }}
+        run: pixi run rs-check --only tests
+
       - name: Upload test results
         uses: actions/upload-artifact@v4
         if: always()

crates/viewer/re_view_spatial/src/ui.rs

@@ -6,7 +6,7 @@ use re_types::{
     blueprint::components::VisualBounds2D, components::ViewCoordinates, image::ImageKind,
 };
 use re_ui::UiExt as _;
-use re_viewer_context::{HoverHighlight, SelectionHighlight, ViewHighlights, ViewState};
+use re_viewer_context::{HoverHighlight, ImageInfo, SelectionHighlight, ViewHighlights, ViewState};
 
 use crate::{
     Pinhole,
@@ -37,13 +37,21 @@ impl From<AutoSizeUnit> for WidgetText {
     }
 }
 
+/// Number of images per image kind.
+#[derive(Clone, Copy, Default)]
+pub struct ImageCounts {
+    pub segmentation: usize,
+    pub color: usize,
+    pub depth: usize,
+}
+
 /// TODO(andreas): Should turn this "inside out" - [`SpatialViewState`] should be used by [`View3DState`], not the other way round.
 #[derive(Clone, Default)]
 pub struct SpatialViewState {
     pub bounding_boxes: SceneBoundingBoxes,
 
-    /// Number of images & depth images processed last frame.
-    pub num_non_segmentation_images_last_frame: usize,
+    /// Number of images per image kind processed last frame.
+    pub image_counts_last_frame: ImageCounts,
 
     /// Last frame's picking result.
     pub previous_picking_result: Option<PickingResult>,
@@ -80,19 +88,24 @@ impl SpatialViewState {
             .update(ui, &system_output.view_systems, space_kind);
 
         let view_systems = &system_output.view_systems;
-        self.num_non_segmentation_images_last_frame = view_systems
-            .iter_visualizer_data::<SpatialViewVisualizerData>()
-            .flat_map(|data| {
-                data.pickable_rects.iter().map(|pickable_rect| {
-                    if let PickableRectSourceData::Image { image, .. } = &pickable_rect.source_data
-                    {
-                        (image.kind != ImageKind::Segmentation) as usize
-                    } else {
-                        0
-                    }
-                })
-            })
-            .sum();
+
+        for data in view_systems.iter_visualizer_data::<SpatialViewVisualizerData>() {
+            for pickable_rect in &data.pickable_rects {
+                let PickableRectSourceData::Image {
+                    image: ImageInfo { kind, .. },
+                    ..
+                } = &pickable_rect.source_data
+                else {
+                    continue;
+                };
+
+                match kind {
+                    ImageKind::Segmentation => self.image_counts_last_frame.segmentation += 1,
+                    ImageKind::Color => self.image_counts_last_frame.color += 1,
+                    ImageKind::Depth => self.image_counts_last_frame.depth += 1,
+                }
+            }
+        }
     }
 
     pub fn bounding_box_ui(&self, ui: &mut egui::Ui, spatial_kind: SpatialViewKind) {
@@ -148,6 +161,39 @@ impl SpatialViewState {
             });
         }
     }
+
+    pub fn fallback_opacity_for_image_kind(&self, kind: ImageKind) -> f32 {
+        // If we have multiple images in the same view, they should not be fully opaque
+        // if there is at least one image of the same kind with equal or lower draw order.
+        //
+        // Here we also assume that if the opacity is unchanged, neither is the draw order.
+        //
+        // By default, the draw order is (front to back):
+        // * segmentation image
+        // * color image
+        // * depth image
+        let counts = self.image_counts_last_frame;
+        match kind {
+            ImageKind::Segmentation => {
+                if counts.color + counts.depth > 0 {
+                    // Segmentation images should always be opaque if there was more than one image in the view,
+                    // excluding other segmentation images.
+                    0.5
+                } else {
+                    1.0
+                }
+            }
+            ImageKind::Color => {
+                if counts.depth > 0 {
+                    0.5
+                } else {
+                    1.0
+                }
+            }
+            // NOTE: Depth images do not support opacity
+            ImageKind::Depth => 1.0,
+        }
+    }
 }
 
 pub fn create_labels(

crates/viewer/re_view_spatial/src/visualizers/encoded_image.rs

@@ -1,6 +1,7 @@
 use re_types::{
     archetypes::EncodedImage,
     components::{DrawOrder, MediaType, Opacity},
+    image::ImageKind,
 };
 use re_view::{HybridResults, diff_component_filter};
 use re_viewer_context::{
@@ -13,6 +14,7 @@ use re_viewer_context::{
 use crate::{
     PickableRectSourceData, PickableTexturedRect,
     contexts::SpatialSceneEntityContext,
+    ui::SpatialViewState,
     view_kind::SpatialViewKind,
     visualizers::{filter_visualizable_2d_entities, textured_rect_from_image},
 };
@@ -209,8 +211,21 @@ impl EncodedImageVisualizer {
 }
 
 impl TypedComponentFallbackProvider<Opacity> for EncodedImageVisualizer {
-    fn fallback_for(&self, _ctx: &re_viewer_context::QueryContext<'_>) -> Opacity {
-        1.0.into()
+    fn fallback_for(&self, ctx: &re_viewer_context::QueryContext<'_>) -> Opacity {
+        // Color images should be transparent whenever they're on top of other images,
+        // But fully opaque if there are no other images in the scene.
+        let Some(view_state) = ctx.view_state.as_any().downcast_ref::<SpatialViewState>() else {
+            return 1.0.into();
+        };
+
+        // Known cosmetic issues with this approach:
+        // * The first frame we have more than one image, the image will be opaque.
+        //      It's too complex to do a full view query just for this here.
+        //      However, we should be able to analyze the `DataQueryResults` instead to check how many entities are fed to the Image/DepthImage visualizers.
+        // * In 3D scenes, images that are on a completely different plane will cause this to become transparent.
+        view_state
+            .fallback_opacity_for_image_kind(ImageKind::Color)
+            .into()
     }
 }
 

crates/viewer/re_view_spatial/src/visualizers/images.rs

@@ -14,6 +14,7 @@ use re_viewer_context::{
 use crate::{
     PickableRectSourceData, PickableTexturedRect,
     contexts::SpatialSceneEntityContext,
+    ui::SpatialViewState,
     view_kind::SpatialViewKind,
     visualizers::{filter_visualizable_2d_entities, textured_rect_from_image},
 };
@@ -182,8 +183,21 @@ impl ImageVisualizer {
 }
 
 impl TypedComponentFallbackProvider<Opacity> for ImageVisualizer {
-    fn fallback_for(&self, _ctx: &re_viewer_context::QueryContext<'_>) -> Opacity {
-        1.0.into()
+    fn fallback_for(&self, ctx: &re_viewer_context::QueryContext<'_>) -> Opacity {
+        // Color images should be transparent whenever they're on top of other images,
+        // But fully opaque if there are no other images in the scene.
+        let Some(view_state) = ctx.view_state.as_any().downcast_ref::<SpatialViewState>() else {
+            return 1.0.into();
+        };
+
+        // Known cosmetic issues with this approach:
+        // * The first frame we have more than one image, the color image will be opaque.
+        //      It's too complex to do a full view query just for this here.
+        //      However, we should be able to analyze the `DataQueryResults` instead to check how many entities are fed to the Image/DepthImage visualizers.
+        // * In 3D scenes, images that are on a completely different plane will cause this to become transparent.
+        view_state
+            .fallback_opacity_for_image_kind(ImageKind::Color)
+            .into()
     }
 }
 

crates/viewer/re_view_spatial/src/visualizers/segmentation_images.rs

@@ -188,12 +188,9 @@ impl TypedComponentFallbackProvider<Opacity> for SegmentationImageVisualizer {
         //      It's too complex to do a full view query just for this here.
         //      However, we should be able to analyze the `DataQueryResults` instead to check how many entities are fed to the Image/DepthImage visualizers.
         // * In 3D scenes, images that are on a completely different plane will cause this to become transparent.
-        if view_state.num_non_segmentation_images_last_frame == 0 {
-            1.0
-        } else {
-            0.5
-        }
-        .into()
+        view_state
+            .fallback_opacity_for_image_kind(ImageKind::Segmentation)
+            .into()
     }
 }
 

crates/viewer/re_view_spatial/src/visualizers/videos.rs

@@ -34,6 +34,8 @@ use super::{
     entity_iterator::process_archetype,
 };
 
+// TODO(#9832): Support opacity for videos
+// TODO(jan): Fallback opacity in the same way as color/depth/segmentation images
 pub struct VideoFrameReferenceVisualizer {
     pub data: SpatialViewVisualizerData,
 }

crates/viewer/re_view_spatial/tests/snapshots/draw_order.png

@@ -237,6 +237,7 @@ fn run_view_ui_and_save_snapshot(
         });
         harness.run_steps(8);
 
+        let mut success = true;
         for time in 0..=7 {
             let name = format!("{name}_{}_{time}", timeline.name());
 
@@ -251,11 +252,12 @@ fn run_view_ui_and_save_snapshot(
             let num_pixels = (size.x * size.y).ceil() as u64;
 
             use re_viewer_context::test_context::HarnessExt as _;
-            harness.snapshot_with_broken_pixels_threshold(
+            success = harness.try_snapshot_with_broken_pixels_threshold(
                 &name,
                 num_pixels,
                 broken_percent_threshold,
             );
         }
+        assert!(success, "one or more snapshots failed");
     }
 }