From 90e1be23408b0bf70a82a2842d08b9d96bfc317f Mon Sep 17 00:00:00 2001 From: jprochazk Date: Wed, 23 Apr 2025 15:23:18 +0200 Subject: [PATCH 01/20] image opacity heuristic --- crates/viewer/re_view_spatial/src/ui.rs | 84 +++++++++++++++---- .../src/visualizers/encoded_image.rs | 19 ++++- .../re_view_spatial/src/visualizers/images.rs | 18 +++- .../src/visualizers/segmentation_images.rs | 9 +- 4 files changed, 104 insertions(+), 26 deletions(-) diff --git a/crates/viewer/re_view_spatial/src/ui.rs b/crates/viewer/re_view_spatial/src/ui.rs index 5a3bc3e81964..1549420c3429 100644 --- a/crates/viewer/re_view_spatial/src/ui.rs +++ b/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::{ eye::EyeMode, @@ -37,13 +37,27 @@ impl From for WidgetText { } } +/// Number of images per image kind. +#[derive(Clone, Copy, Default)] +pub struct ImageCounts { + pub segmentation: usize, + pub color: usize, + pub depth: usize, +} + +impl ImageCounts { + pub fn total(&self) -> usize { + self.segmentation + self.color + self.depth + } +} + /// 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, @@ -80,19 +94,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::() - .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::() { + for pickable_rect in data.pickable_rects.iter() { + 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 +167,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 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.segmentation > 1) || (counts.color + counts.depth > 0) { + // Segmentation images should always be opaque if there was more than one image in the view, + // including other segmentation images + 0.5 + } else { + 1.0 + } + } + ImageKind::Color => { + if (counts.color > 1) || (counts.color > 0) { + 0.5 + } else { + 1.0 + } + } + // NOTE: Depth images do not support opacity + ImageKind::Depth => 1.0, + } + } } pub fn create_labels( diff --git a/crates/viewer/re_view_spatial/src/visualizers/encoded_image.rs b/crates/viewer/re_view_spatial/src/visualizers/encoded_image.rs index 3e592bca70cd..ea3b4dd1af61 100644 --- a/crates/viewer/re_view_spatial/src/visualizers/encoded_image.rs +++ b/crates/viewer/re_view_spatial/src/visualizers/encoded_image.rs @@ -2,6 +2,7 @@ use re_log_types::hash::Hash64; use re_types::{ archetypes::EncodedImage, components::{Blob, DrawOrder, MediaType, Opacity}, + image::ImageKind, Component as _, }; use re_view::{diff_component_filter, HybridResults}; @@ -14,6 +15,7 @@ use re_viewer_context::{ use crate::{ contexts::SpatialSceneEntityContext, + ui::SpatialViewState, view_kind::SpatialViewKind, visualizers::{filter_visualizable_2d_entities, textured_rect_from_image}, PickableRectSourceData, PickableTexturedRect, @@ -205,8 +207,21 @@ impl EncodedImageVisualizer { } impl TypedComponentFallbackProvider 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::() 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() } } diff --git a/crates/viewer/re_view_spatial/src/visualizers/images.rs b/crates/viewer/re_view_spatial/src/visualizers/images.rs index d107ef590ef8..1f354c83fa8f 100644 --- a/crates/viewer/re_view_spatial/src/visualizers/images.rs +++ b/crates/viewer/re_view_spatial/src/visualizers/images.rs @@ -15,6 +15,7 @@ use re_viewer_context::{ use crate::{ contexts::SpatialSceneEntityContext, + ui::SpatialViewState, view_kind::SpatialViewKind, visualizers::{filter_visualizable_2d_entities, textured_rect_from_image}, PickableRectSourceData, PickableTexturedRect, @@ -183,8 +184,21 @@ impl ImageVisualizer { } impl TypedComponentFallbackProvider 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 { + // Segmentation 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::() else { + return 1.0.into(); + }; + + // Known cosmetic issues with this approach: + // * The first frame we have more than one image, the segmentation 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() } } diff --git a/crates/viewer/re_view_spatial/src/visualizers/segmentation_images.rs b/crates/viewer/re_view_spatial/src/visualizers/segmentation_images.rs index c1b9694a7ddc..4e1010b1a903 100644 --- a/crates/viewer/re_view_spatial/src/visualizers/segmentation_images.rs +++ b/crates/viewer/re_view_spatial/src/visualizers/segmentation_images.rs @@ -190,12 +190,9 @@ impl TypedComponentFallbackProvider 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() } } From 5e2eed97275e458f90e9595754ee80a51a902a24 Mon Sep 17 00:00:00 2001 From: jprochazk Date: Wed, 23 Apr 2025 15:30:19 +0200 Subject: [PATCH 02/20] there there --- crates/viewer/re_view_spatial/src/ui.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/viewer/re_view_spatial/src/ui.rs b/crates/viewer/re_view_spatial/src/ui.rs index 1549420c3429..a7f12c0567ba 100644 --- a/crates/viewer/re_view_spatial/src/ui.rs +++ b/crates/viewer/re_view_spatial/src/ui.rs @@ -170,7 +170,7 @@ 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 there is at least one image of the same kind with equal or lower draw order. + // 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. // From 151f1f1551025341b2e5a2f0b3bbbf402538d500 Mon Sep 17 00:00:00 2001 From: jprochazk Date: Wed, 23 Apr 2025 15:36:31 +0200 Subject: [PATCH 03/20] wake up gh From 3ff58d34cf55fbc4a66f62c4f4cd55f67ddb3a1b Mon Sep 17 00:00:00 2001 From: jprochazk Date: Wed, 23 Apr 2025 15:40:02 +0200 Subject: [PATCH 04/20] better naming --- crates/viewer/re_view_spatial/src/ui.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/viewer/re_view_spatial/src/ui.rs b/crates/viewer/re_view_spatial/src/ui.rs index a7f12c0567ba..95d5929cccdb 100644 --- a/crates/viewer/re_view_spatial/src/ui.rs +++ b/crates/viewer/re_view_spatial/src/ui.rs @@ -181,7 +181,9 @@ impl SpatialViewState { let counts = self.image_counts_last_frame; match kind { ImageKind::Segmentation => { - if (counts.segmentation > 1) || (counts.color + counts.depth > 0) { + let multiple_segmentation_images = counts.segmentation > 1; + let any_behind_segmentation_images = (counts.color + counts.depth) > 0; + if multiple_segmentation_images || any_behind_segmentation_images { // Segmentation images should always be opaque if there was more than one image in the view, // including other segmentation images 0.5 @@ -190,7 +192,9 @@ impl SpatialViewState { } } ImageKind::Color => { - if (counts.color > 1) || (counts.color > 0) { + let multiple_color_images = counts.color > 1; + let any_behind_color_images = counts.depth > 0; + if multiple_color_images || any_behind_color_images { 0.5 } else { 1.0 From 5ba499447fce417993eedad14ad2d6cf78699b89 Mon Sep 17 00:00:00 2001 From: jprochazk Date: Wed, 23 Apr 2025 16:05:32 +0200 Subject: [PATCH 05/20] do not use opacity for multiple segmentation images also update snapshots --- crates/viewer/re_view_spatial/src/ui.rs | 16 +++------------- .../tests/snapshots/draw_order.png | 4 ++-- .../tests/snapshots/transform_clamping_boxes.png | 4 ++-- .../snapshots/transform_clamping_spheres.png | 4 ++-- .../snapshots/transform_hierarchy_step_0.png | 4 ++-- .../snapshots/transform_hierarchy_step_1.png | 4 ++-- .../snapshots/transform_hierarchy_step_2.png | 4 ++-- .../snapshots/transform_hierarchy_step_3.png | 4 ++-- .../snapshots/transform_hierarchy_step_4.png | 4 ++-- .../snapshots/transform_hierarchy_step_5.png | 4 ++-- .../snapshots/transform_hierarchy_step_6.png | 4 ++-- .../snapshots/transform_hierarchy_step_7.png | 4 ++-- 12 files changed, 25 insertions(+), 35 deletions(-) diff --git a/crates/viewer/re_view_spatial/src/ui.rs b/crates/viewer/re_view_spatial/src/ui.rs index 95d5929cccdb..cc3928b52fc8 100644 --- a/crates/viewer/re_view_spatial/src/ui.rs +++ b/crates/viewer/re_view_spatial/src/ui.rs @@ -45,12 +45,6 @@ pub struct ImageCounts { pub depth: usize, } -impl ImageCounts { - pub fn total(&self) -> usize { - self.segmentation + self.color + self.depth - } -} - /// TODO(andreas): Should turn this "inside out" - [`SpatialViewState`] should be used by [`View3DState`], not the other way round. #[derive(Clone, Default)] pub struct SpatialViewState { @@ -181,20 +175,16 @@ impl SpatialViewState { let counts = self.image_counts_last_frame; match kind { ImageKind::Segmentation => { - let multiple_segmentation_images = counts.segmentation > 1; - let any_behind_segmentation_images = (counts.color + counts.depth) > 0; - if multiple_segmentation_images || any_behind_segmentation_images { + if counts.color + counts.depth > 0 { // Segmentation images should always be opaque if there was more than one image in the view, - // including other segmentation images + // excluding other segmentation images. 0.5 } else { 1.0 } } ImageKind::Color => { - let multiple_color_images = counts.color > 1; - let any_behind_color_images = counts.depth > 0; - if multiple_color_images || any_behind_color_images { + if counts.depth > 0 { 0.5 } else { 1.0 diff --git a/crates/viewer/re_view_spatial/tests/snapshots/draw_order.png b/crates/viewer/re_view_spatial/tests/snapshots/draw_order.png index 4a40c83ef5d2..09b8e3331d6c 100644 --- a/crates/viewer/re_view_spatial/tests/snapshots/draw_order.png +++ b/crates/viewer/re_view_spatial/tests/snapshots/draw_order.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:800712b4ac7fd2969ec4b96e9496b7a29fe75c51c539d15fccb7bc0ae14bf92a -size 25609 +oid sha256:11dea3a03f00d62e9b2ae34f80d851d678400d5ab1aac1cc7f7aef31104d3b53 +size 25437 diff --git a/crates/viewer/re_view_spatial/tests/snapshots/transform_clamping_boxes.png b/crates/viewer/re_view_spatial/tests/snapshots/transform_clamping_boxes.png index d6085b0392ef..95361adaa0a7 100644 --- a/crates/viewer/re_view_spatial/tests/snapshots/transform_clamping_boxes.png +++ b/crates/viewer/re_view_spatial/tests/snapshots/transform_clamping_boxes.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:af57df577fa656bf4c008858eccfda9c276425fc0797a8cabedea9c2f0c6d928 -size 109341 +oid sha256:f2806ec16cd3da88fadeacd7fadefcef09da72619cd2873d4892ce2b4ae37bd2 +size 108877 diff --git a/crates/viewer/re_view_spatial/tests/snapshots/transform_clamping_spheres.png b/crates/viewer/re_view_spatial/tests/snapshots/transform_clamping_spheres.png index 7a659e156a3e..ef885a9ba052 100644 --- a/crates/viewer/re_view_spatial/tests/snapshots/transform_clamping_spheres.png +++ b/crates/viewer/re_view_spatial/tests/snapshots/transform_clamping_spheres.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:8ee2108d9a877c391d6c97fdc3769d136ef8390add1852add46f45c1f4aacfdf -size 112072 +oid sha256:87a35ed7802e57178cb32ec56e6998de2207f661fe50bc928e76fe1d9e7becc7 +size 111550 diff --git a/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_0.png b/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_0.png index 6c9714c6ab0f..6d564a922b5c 100644 --- a/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_0.png +++ b/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_0.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:8443fcb30ff0a6a308c8cebcbb543909c6af93948d0a08ff7889fefdd7e82f07 -size 55885 +oid sha256:fc37d33ad204ded8276906d8d404ee03e8cfc5d1434f6e551e13074c880957f8 +size 56183 diff --git a/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_1.png b/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_1.png index 456aa8723a21..49b379b4ae0a 100644 --- a/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_1.png +++ b/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_1.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:1d9b7fd8d97771fb1ef84836db96f49af0a7a410b7fe52fdac7e788106a1262e -size 55980 +oid sha256:4b3dbde3cb9d3a733c3b14ef7bd2a506e9af452181225f78154bdb5b256f4bcb +size 56202 diff --git a/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_2.png b/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_2.png index 6c9714c6ab0f..6d564a922b5c 100644 --- a/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_2.png +++ b/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_2.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:8443fcb30ff0a6a308c8cebcbb543909c6af93948d0a08ff7889fefdd7e82f07 -size 55885 +oid sha256:fc37d33ad204ded8276906d8d404ee03e8cfc5d1434f6e551e13074c880957f8 +size 56183 diff --git a/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_3.png b/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_3.png index 9a1d39773647..d0aec39630e4 100644 --- a/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_3.png +++ b/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_3.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:d21b4827f3fdc0aad78117fc9694685c7ab71fe3385d9f937c80d208dba9d299 -size 55172 +oid sha256:4fb60f8ff4076eac4187acb87c04e9b3c3e32088b5af1d70b169135a6b9f168e +size 55443 diff --git a/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_4.png b/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_4.png index 6c9714c6ab0f..6d564a922b5c 100644 --- a/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_4.png +++ b/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_4.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:8443fcb30ff0a6a308c8cebcbb543909c6af93948d0a08ff7889fefdd7e82f07 -size 55885 +oid sha256:fc37d33ad204ded8276906d8d404ee03e8cfc5d1434f6e551e13074c880957f8 +size 56183 diff --git a/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_5.png b/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_5.png index a7bb1723b8f9..9fc761810785 100644 --- a/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_5.png +++ b/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_5.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:6eb90b8f79eabc815576309050f44a001d04e2bab7bcef55ec96a4e872f06b28 -size 54803 +oid sha256:295cfa77a120220ce846746977d7c287e92af0c3815de4b787df3b6b8d2dd09b +size 55058 diff --git a/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_6.png b/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_6.png index f5642aed3e2d..4c2d031e5590 100644 --- a/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_6.png +++ b/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_6.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:c1f15b2dcefa420c7a2bd1195b0697ced1c1c158b2de26688be5a08180a2547c -size 54860 +oid sha256:91a66da02d1d7f104449ce3a1af73d4e9715efc56bf84fdc946a37cbc24bebd9 +size 55144 diff --git a/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_7.png b/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_7.png index a7bb1723b8f9..9fc761810785 100644 --- a/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_7.png +++ b/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_7.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:6eb90b8f79eabc815576309050f44a001d04e2bab7bcef55ec96a4e872f06b28 -size 54803 +oid sha256:295cfa77a120220ce846746977d7c287e92af0c3815de4b787df3b6b8d2dd09b +size 55058 From facaec15bd562eefac012cf2902746a21d9db8b8 Mon Sep 17 00:00:00 2001 From: jprochazk Date: Tue, 29 Apr 2025 14:13:14 +0200 Subject: [PATCH 06/20] comments --- crates/viewer/re_view_spatial/src/visualizers/images.rs | 4 ++-- crates/viewer/re_view_spatial/src/visualizers/videos.rs | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/viewer/re_view_spatial/src/visualizers/images.rs b/crates/viewer/re_view_spatial/src/visualizers/images.rs index 1f354c83fa8f..e0255cb9713b 100644 --- a/crates/viewer/re_view_spatial/src/visualizers/images.rs +++ b/crates/viewer/re_view_spatial/src/visualizers/images.rs @@ -185,14 +185,14 @@ impl ImageVisualizer { impl TypedComponentFallbackProvider for ImageVisualizer { fn fallback_for(&self, ctx: &re_viewer_context::QueryContext<'_>) -> Opacity { - // Segmentation images should be transparent whenever they're on top of other images, + // 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::() else { return 1.0.into(); }; // Known cosmetic issues with this approach: - // * The first frame we have more than one image, the segmentation image will be opaque. + // * 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. diff --git a/crates/viewer/re_view_spatial/src/visualizers/videos.rs b/crates/viewer/re_view_spatial/src/visualizers/videos.rs index 11a2df336fe9..9340c9d8a98b 100644 --- a/crates/viewer/re_view_spatial/src/visualizers/videos.rs +++ b/crates/viewer/re_view_spatial/src/visualizers/videos.rs @@ -34,6 +34,7 @@ use super::{ UiLabelTarget, }; +// TODO(#9832): Support opacity for videos pub struct VideoFrameReferenceVisualizer { pub data: SpatialViewVisualizerData, } From b222641dcfbbe8a9f5afcb567ccce9c372550ffa Mon Sep 17 00:00:00 2001 From: jprochazk Date: Tue, 29 Apr 2025 14:15:15 +0200 Subject: [PATCH 07/20] more comment --- crates/viewer/re_view_spatial/src/visualizers/videos.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/viewer/re_view_spatial/src/visualizers/videos.rs b/crates/viewer/re_view_spatial/src/visualizers/videos.rs index 9340c9d8a98b..66bf1285bd42 100644 --- a/crates/viewer/re_view_spatial/src/visualizers/videos.rs +++ b/crates/viewer/re_view_spatial/src/visualizers/videos.rs @@ -35,6 +35,7 @@ use super::{ }; // 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, } From f4568749ad1fa68acf8f4508fdb7156cab5f5d04 Mon Sep 17 00:00:00 2001 From: jprochazk Date: Tue, 29 Apr 2025 15:44:05 +0200 Subject: [PATCH 08/20] update --- .../tests/snapshots/transform_clamping_boxes.png | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/viewer/re_view_spatial/tests/snapshots/transform_clamping_boxes.png b/crates/viewer/re_view_spatial/tests/snapshots/transform_clamping_boxes.png index 95361adaa0a7..57d88a8a7e02 100644 --- a/crates/viewer/re_view_spatial/tests/snapshots/transform_clamping_boxes.png +++ b/crates/viewer/re_view_spatial/tests/snapshots/transform_clamping_boxes.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:f2806ec16cd3da88fadeacd7fadefcef09da72619cd2873d4892ce2b4ae37bd2 -size 108877 +oid sha256:ee938cdacb9f114a1db34c0a6f5ecd0f7dda6950a909d1aeeb4270ad7ea5dd62 +size 109387 From d7ddac8ca99804dcadf2979f1b48476141db48d8 Mon Sep 17 00:00:00 2001 From: jprochazk Date: Tue, 29 Apr 2025 15:57:26 +0200 Subject: [PATCH 09/20] update --- crates/viewer/re_view_spatial/tests/snapshots/draw_order.png | 4 ++-- .../tests/snapshots/transform_clamping_spheres.png | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/viewer/re_view_spatial/tests/snapshots/draw_order.png b/crates/viewer/re_view_spatial/tests/snapshots/draw_order.png index 09b8e3331d6c..5120fd467a47 100644 --- a/crates/viewer/re_view_spatial/tests/snapshots/draw_order.png +++ b/crates/viewer/re_view_spatial/tests/snapshots/draw_order.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:11dea3a03f00d62e9b2ae34f80d851d678400d5ab1aac1cc7f7aef31104d3b53 -size 25437 +oid sha256:a7b0015e11ac089e5791b9541459c02ff626c2a626cbe92d783b1813d6cedb26 +size 25617 diff --git a/crates/viewer/re_view_spatial/tests/snapshots/transform_clamping_spheres.png b/crates/viewer/re_view_spatial/tests/snapshots/transform_clamping_spheres.png index ef885a9ba052..52dfaff7fc15 100644 --- a/crates/viewer/re_view_spatial/tests/snapshots/transform_clamping_spheres.png +++ b/crates/viewer/re_view_spatial/tests/snapshots/transform_clamping_spheres.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:87a35ed7802e57178cb32ec56e6998de2207f661fe50bc928e76fe1d9e7becc7 -size 111550 +oid sha256:d5667827f4726d374bccaa978619990253f9e02ccff2d1937bb2c416a26d0aa5 +size 112103 From 62e0759c6579775128ec4eb67393be74a19c9abe Mon Sep 17 00:00:00 2001 From: jprochazk Date: Tue, 29 Apr 2025 16:32:06 +0200 Subject: [PATCH 10/20] update --- .../tests/snapshots/transform_hierarchy_step_0.png | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_0.png b/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_0.png index 6d564a922b5c..1f15711ea929 100644 --- a/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_0.png +++ b/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_0.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:fc37d33ad204ded8276906d8d404ee03e8cfc5d1434f6e551e13074c880957f8 -size 56183 +oid sha256:dfa703de770475483fcd45664043fff6d259ec1434648d5253a189e9c7ca0193 +size 56271 From 317e435958ec6ac676ff9e3a97d1cc2566103e73 Mon Sep 17 00:00:00 2001 From: jprochazk Date: Tue, 29 Apr 2025 16:47:52 +0200 Subject: [PATCH 11/20] report all steps at once --- .../tests/transform_hierarchy.rs | 4 +- .../re_viewer_context/src/test_context.rs | 39 +++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/crates/viewer/re_view_spatial/tests/transform_hierarchy.rs b/crates/viewer/re_view_spatial/tests/transform_hierarchy.rs index 18058d3b3c42..65437a7b4411 100644 --- a/crates/viewer/re_view_spatial/tests/transform_hierarchy.rs +++ b/crates/viewer/re_view_spatial/tests/transform_hierarchy.rs @@ -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"); } } diff --git a/crates/viewer/re_viewer_context/src/test_context.rs b/crates/viewer/re_viewer_context/src/test_context.rs index 3c29cd9cbe87..3e7268e78c0c 100644 --- a/crates/viewer/re_viewer_context/src/test_context.rs +++ b/crates/viewer/re_viewer_context/src/test_context.rs @@ -29,6 +29,15 @@ pub trait HarnessExt { num_pixels: u64, broken_percent_threshold: f64, ); + + fn try_snapshot_with_broken_pixels_threshold( + &mut self, + name: &str, + num_pixels: u64, + broken_percent_threshold: f64, + ) -> bool { + todo!() + } } impl HarnessExt for egui_kittest::Harness<'_> { @@ -59,6 +68,36 @@ impl HarnessExt for egui_kittest::Harness<'_> { }, } } + + fn try_snapshot_with_broken_pixels_threshold( + &mut self, + name: &str, + num_pixels: u64, + broken_percent_threshold: f64, + ) -> bool { + match self.try_snapshot(name) { + Ok(_) => return true, + + Err(err) => match err { + egui_kittest::SnapshotError::Diff { + name, + diff: num_broken_pixels, + diff_path, + } => { + let broken_percent = num_broken_pixels as f64 / num_pixels as f64; + re_log::debug!(num_pixels, num_broken_pixels, broken_percent); + if broken_percent <= broken_percent_threshold { + eprintln!( + "{name} failed because {broken_percent} > {broken_percent_threshold}\n{diff_path:?}"); + return false; + } + return true; + } + + _ => panic!("{name} failed: {err}"), + }, + } + } } /// Harness to execute code that rely on [`crate::ViewerContext`]. From 1a598a3576b91fe65050d4975d2f51e1953ab3d2 Mon Sep 17 00:00:00 2001 From: jprochazk Date: Tue, 29 Apr 2025 16:57:19 +0200 Subject: [PATCH 12/20] run tests in parallel --- .github/workflows/reusable_checks_rust.yml | 57 ++++++++++++++++------ 1 file changed, 42 insertions(+), 15 deletions(-) diff --git a/.github/workflows/reusable_checks_rust.yml b/.github/workflows/reusable_checks_rust.yml index 313a21ec890c..b39afb5c06ad 100644 --- a/.github/workflows/reusable_checks_rust.yml +++ b/.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/install-vulkan-sdk-action@v1.1.0 - 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,45 @@ 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 + 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/setup-pixi@v0.8.1 + 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/install-vulkan-sdk-action@v1.1.0 + 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() @@ -114,6 +140,7 @@ jobs: name: test-results-ubuntu path: "**/tests/snapshots" + # Run some basics tests on Mac and Windows mac-windows-tests: name: Test on ${{ matrix.name }} From b9145869509bc9e7b3696952df9236be8ee6fd2e Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Tue, 29 Apr 2025 17:19:43 +0200 Subject: [PATCH 13/20] update snapshot screenshots --- .../tests/snapshots/transform_clamping_boxes.png | 4 ++-- .../tests/snapshots/transform_clamping_spheres.png | 4 ++-- .../tests/snapshots/transform_hierarchy_step_0.png | 4 ++-- .../tests/snapshots/transform_hierarchy_step_1.png | 4 ++-- .../tests/snapshots/transform_hierarchy_step_2.png | 4 ++-- .../tests/snapshots/transform_hierarchy_step_3.png | 4 ++-- .../tests/snapshots/transform_hierarchy_step_4.png | 4 ++-- .../tests/snapshots/transform_hierarchy_step_5.png | 4 ++-- .../tests/snapshots/transform_hierarchy_step_6.png | 4 ++-- .../tests/snapshots/transform_hierarchy_step_7.png | 4 ++-- 10 files changed, 20 insertions(+), 20 deletions(-) diff --git a/crates/viewer/re_view_spatial/tests/snapshots/transform_clamping_boxes.png b/crates/viewer/re_view_spatial/tests/snapshots/transform_clamping_boxes.png index 57d88a8a7e02..94d97caf9469 100644 --- a/crates/viewer/re_view_spatial/tests/snapshots/transform_clamping_boxes.png +++ b/crates/viewer/re_view_spatial/tests/snapshots/transform_clamping_boxes.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:ee938cdacb9f114a1db34c0a6f5ecd0f7dda6950a909d1aeeb4270ad7ea5dd62 -size 109387 +oid sha256:714ac0022905fbf6e70304e4e4935685e0551702c5b67e95addbd9f4779d68df +size 109166 diff --git a/crates/viewer/re_view_spatial/tests/snapshots/transform_clamping_spheres.png b/crates/viewer/re_view_spatial/tests/snapshots/transform_clamping_spheres.png index 52dfaff7fc15..6f0ad4b21e74 100644 --- a/crates/viewer/re_view_spatial/tests/snapshots/transform_clamping_spheres.png +++ b/crates/viewer/re_view_spatial/tests/snapshots/transform_clamping_spheres.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:d5667827f4726d374bccaa978619990253f9e02ccff2d1937bb2c416a26d0aa5 -size 112103 +oid sha256:442b326dece86994fd2b6a3bd19f19e27e42eebefe1b432ddc75f08fa6011de4 +size 111841 diff --git a/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_0.png b/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_0.png index 1f15711ea929..e391eecdde82 100644 --- a/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_0.png +++ b/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_0.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:dfa703de770475483fcd45664043fff6d259ec1434648d5253a189e9c7ca0193 -size 56271 +oid sha256:2454f25d0774b5d08771208ef306c75330d1f19c9a2bffb4a7a2a127b7d69163 +size 56133 diff --git a/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_1.png b/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_1.png index 49b379b4ae0a..4afa6ffe0e0e 100644 --- a/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_1.png +++ b/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_1.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:4b3dbde3cb9d3a733c3b14ef7bd2a506e9af452181225f78154bdb5b256f4bcb -size 56202 +oid sha256:d335848268d860107c5962ba48cb2fa1bca97470878ff308235501f29e7fd1e2 +size 56136 diff --git a/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_2.png b/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_2.png index 6d564a922b5c..e391eecdde82 100644 --- a/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_2.png +++ b/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_2.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:fc37d33ad204ded8276906d8d404ee03e8cfc5d1434f6e551e13074c880957f8 -size 56183 +oid sha256:2454f25d0774b5d08771208ef306c75330d1f19c9a2bffb4a7a2a127b7d69163 +size 56133 diff --git a/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_3.png b/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_3.png index d0aec39630e4..0b1edab690b8 100644 --- a/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_3.png +++ b/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_3.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:4fb60f8ff4076eac4187acb87c04e9b3c3e32088b5af1d70b169135a6b9f168e -size 55443 +oid sha256:6b2d4db521d05da6bdf81e28fd4775027fc058b46e0835f062918c506e5c9c39 +size 55395 diff --git a/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_4.png b/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_4.png index 6d564a922b5c..e391eecdde82 100644 --- a/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_4.png +++ b/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_4.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:fc37d33ad204ded8276906d8d404ee03e8cfc5d1434f6e551e13074c880957f8 -size 56183 +oid sha256:2454f25d0774b5d08771208ef306c75330d1f19c9a2bffb4a7a2a127b7d69163 +size 56133 diff --git a/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_5.png b/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_5.png index 9fc761810785..2f2b875e64f6 100644 --- a/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_5.png +++ b/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_5.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:295cfa77a120220ce846746977d7c287e92af0c3815de4b787df3b6b8d2dd09b -size 55058 +oid sha256:7e995b2dbc5376ad925d4128303b49db76bae4419ca9251677bf85fc35fc5b1e +size 55024 diff --git a/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_6.png b/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_6.png index 4c2d031e5590..e63537fc7989 100644 --- a/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_6.png +++ b/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_6.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:91a66da02d1d7f104449ce3a1af73d4e9715efc56bf84fdc946a37cbc24bebd9 -size 55144 +oid sha256:fb0353e6f4963c9cf9ac748b5dfa271fc21c0c59ce7ff834874d34d7cc1dc786 +size 55076 diff --git a/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_7.png b/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_7.png index 9fc761810785..2f2b875e64f6 100644 --- a/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_7.png +++ b/crates/viewer/re_view_spatial/tests/snapshots/transform_hierarchy_step_7.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:295cfa77a120220ce846746977d7c287e92af0c3815de4b787df3b6b8d2dd09b -size 55058 +oid sha256:7e995b2dbc5376ad925d4128303b49db76bae4419ca9251677bf85fc35fc5b1e +size 55024 From d85738fc422cb12db8e67dcc19f0a81de431b4f4 Mon Sep 17 00:00:00 2001 From: jprochazk Date: Tue, 29 Apr 2025 17:27:59 +0200 Subject: [PATCH 14/20] do not deny in test --- .github/workflows/reusable_checks_rust.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/reusable_checks_rust.yml b/.github/workflows/reusable_checks_rust.yml index b39afb5c06ad..b33872f56701 100644 --- a/.github/workflows/reusable_checks_rust.yml +++ b/.github/workflows/reusable_checks_rust.yml @@ -98,6 +98,9 @@ jobs: 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: From 416a11f21ea03a935b9cbdbb4f1770dcc4322655 Mon Sep 17 00:00:00 2001 From: jprochazk Date: Tue, 29 Apr 2025 17:47:45 +0200 Subject: [PATCH 15/20] dont deny warnings for tests --- scripts/ci/rust_checks.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/scripts/ci/rust_checks.py b/scripts/ci/rust_checks.py index 1e091157ccf3..bb3f4e06116b 100755 --- a/scripts/ci/rust_checks.py +++ b/scripts/ci/rust_checks.py @@ -40,7 +40,7 @@ def __init__(self, command: str, success: bool, duration: float) -> None: self.duration = duration -def run_cargo(cargo_cmd: str, cargo_args: str, clippy_conf: str | None = None) -> Result: +def run_cargo(cargo_cmd: str, cargo_args: str, clippy_conf: str | None = None, deny_warnings: bool = True) -> Result: args = ["cargo", cargo_cmd] if cargo_cmd not in ["deny", "fmt", "format", "nextest"]: args.append("--quiet") @@ -57,8 +57,8 @@ def run_cargo(cargo_cmd: str, cargo_args: str, clippy_conf: str | None = None) - additional_env_vars = {} # Compilation will fail we don't manually set `--cfg=web_sys_unstable_apis`, # because env vars are not propagated from CI. - additional_env_vars["RUSTFLAGS"] = "--cfg=web_sys_unstable_apis --deny warnings" - additional_env_vars["RUSTDOCFLAGS"] = "--cfg=web_sys_unstable_apis --deny warnings" + additional_env_vars["RUSTFLAGS"] = f"--cfg=web_sys_unstable_apis {'--deny warnings' if deny_warnings else ''}" + additional_env_vars["RUSTDOCFLAGS"] = f"--cfg=web_sys_unstable_apis {'--deny warnings' if deny_warnings else ''}" if clippy_conf is not None: additional_env_vars["CLIPPY_CONF_DIR"] = ( # Clippy has issues finding this directory on CI when we're not using an absolute path here. @@ -287,20 +287,20 @@ def docs_slow(results: list[Result]) -> None: def tests(results: list[Result]) -> None: # We first use `--no-run` to measure the time of compiling vs actually running - results.append(run_cargo("test", "--all-targets --all-features --no-run")) - results.append(run_cargo("nextest", "run --all-targets --all-features")) + results.append(run_cargo("test", "--all-targets --all-features --no-run", deny_warnings=False)) + results.append(run_cargo("nextest", "run --all-targets --all-features", deny_warnings=False)) if not results[-1].success: print(test_failure_message) # Cargo nextest doesn't support doc tests yet, run those separately. - results.append(run_cargo("test", "--all-features --doc")) + results.append(run_cargo("test", "--all-features --doc", deny_warnings=False)) def tests_without_all_features(results: list[Result]) -> None: # We first use `--no-run` to measure the time of compiling vs actually running - results.append(run_cargo("test", "--all-targets --no-run")) - results.append(run_cargo("nextest", "run --all-targets")) + results.append(run_cargo("test", "--all-targets --no-run", deny_warnings=False)) + results.append(run_cargo("nextest", "run --all-targets", deny_warnings=False)) if not results[-1].success: print(test_failure_message) From 5119e843d8ff26bfaf295b26602c13091948ee74 Mon Sep 17 00:00:00 2001 From: jprochazk Date: Tue, 29 Apr 2025 17:56:54 +0200 Subject: [PATCH 16/20] fix lints --- crates/viewer/re_viewer_context/src/test_context.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/viewer/re_viewer_context/src/test_context.rs b/crates/viewer/re_viewer_context/src/test_context.rs index 3e7268e78c0c..d6da12678857 100644 --- a/crates/viewer/re_viewer_context/src/test_context.rs +++ b/crates/viewer/re_viewer_context/src/test_context.rs @@ -35,9 +35,7 @@ pub trait HarnessExt { name: &str, num_pixels: u64, broken_percent_threshold: f64, - ) -> bool { - todo!() - } + ) -> bool; } impl HarnessExt for egui_kittest::Harness<'_> { From 1989935532ccf164afbef02cb92d71a602bdbb90 Mon Sep 17 00:00:00 2001 From: jprochazk Date: Tue, 29 Apr 2025 17:58:25 +0200 Subject: [PATCH 17/20] fix fmt --- .github/workflows/reusable_checks_rust.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/reusable_checks_rust.yml b/.github/workflows/reusable_checks_rust.yml index b33872f56701..fce91c315aae 100644 --- a/.github/workflows/reusable_checks_rust.yml +++ b/.github/workflows/reusable_checks_rust.yml @@ -143,7 +143,6 @@ jobs: name: test-results-ubuntu path: "**/tests/snapshots" - # Run some basics tests on Mac and Windows mac-windows-tests: name: Test on ${{ matrix.name }} From d7e70345f4ace52c08e6ed6d8f476e062cd85102 Mon Sep 17 00:00:00 2001 From: jprochazk Date: Tue, 29 Apr 2025 18:02:03 +0200 Subject: [PATCH 18/20] fix lint --- crates/viewer/re_view_spatial/src/ui.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/viewer/re_view_spatial/src/ui.rs b/crates/viewer/re_view_spatial/src/ui.rs index cc3928b52fc8..7f5e1bec9fc4 100644 --- a/crates/viewer/re_view_spatial/src/ui.rs +++ b/crates/viewer/re_view_spatial/src/ui.rs @@ -90,7 +90,7 @@ impl SpatialViewState { let view_systems = &system_output.view_systems; for data in view_systems.iter_visualizer_data::() { - for pickable_rect in data.pickable_rects.iter() { + for pickable_rect in &data.pickable_rects { let PickableRectSourceData::Image { image: ImageInfo { kind, .. }, .. From b9073926ed6c117b50ed2a8e30609e3e1cc6dc00 Mon Sep 17 00:00:00 2001 From: jprochazk Date: Tue, 29 Apr 2025 18:21:20 +0200 Subject: [PATCH 19/20] oops --- crates/viewer/re_viewer_context/src/test_context.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/viewer/re_viewer_context/src/test_context.rs b/crates/viewer/re_viewer_context/src/test_context.rs index d6da12678857..eda1223eee73 100644 --- a/crates/viewer/re_viewer_context/src/test_context.rs +++ b/crates/viewer/re_viewer_context/src/test_context.rs @@ -84,7 +84,7 @@ impl HarnessExt for egui_kittest::Harness<'_> { } => { let broken_percent = num_broken_pixels as f64 / num_pixels as f64; re_log::debug!(num_pixels, num_broken_pixels, broken_percent); - if broken_percent <= broken_percent_threshold { + if broken_percent >= broken_percent_threshold { eprintln!( "{name} failed because {broken_percent} > {broken_percent_threshold}\n{diff_path:?}"); return false; From 22c819db1837b4e9d4eb2f90be40eddbb9f9b911 Mon Sep 17 00:00:00 2001 From: jprochazk Date: Tue, 29 Apr 2025 18:32:32 +0200 Subject: [PATCH 20/20] fix lint --- crates/viewer/re_viewer_context/src/test_context.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/viewer/re_viewer_context/src/test_context.rs b/crates/viewer/re_viewer_context/src/test_context.rs index eda1223eee73..f976201b27fc 100644 --- a/crates/viewer/re_viewer_context/src/test_context.rs +++ b/crates/viewer/re_viewer_context/src/test_context.rs @@ -74,7 +74,7 @@ impl HarnessExt for egui_kittest::Harness<'_> { broken_percent_threshold: f64, ) -> bool { match self.try_snapshot(name) { - Ok(_) => return true, + Ok(_) => true, Err(err) => match err { egui_kittest::SnapshotError::Diff { @@ -85,11 +85,11 @@ impl HarnessExt for egui_kittest::Harness<'_> { let broken_percent = num_broken_pixels as f64 / num_pixels as f64; re_log::debug!(num_pixels, num_broken_pixels, broken_percent); if broken_percent >= broken_percent_threshold { - eprintln!( - "{name} failed because {broken_percent} > {broken_percent_threshold}\n{diff_path:?}"); + re_log::error!("{name} failed because {broken_percent} > {broken_percent_threshold}\n{diff_path:?}"); return false; } - return true; + + true } _ => panic!("{name} failed: {err}"),