From 9d5f650a69e6a81d3f2f2ef46621b26250c0249c Mon Sep 17 00:00:00 2001 From: Clement Rey Date: Thu, 6 Feb 2025 19:02:40 +0100 Subject: [PATCH 1/2] Automated `check_transform_clamping` (#8961) Title. --- .../re_view_spatial/tests/draw_order.rs | 1 + .../snapshots/transform_clamping_boxes.png | 3 + .../snapshots/transform_clamping_spheres.png | 3 + .../tests/transform_clamping.rs | 273 ++++++++++++++++++ .../check_component_and_transform_clamping.py | 88 ------ 5 files changed, 280 insertions(+), 88 deletions(-) create mode 100644 crates/viewer/re_view_spatial/tests/snapshots/transform_clamping_boxes.png create mode 100644 crates/viewer/re_view_spatial/tests/snapshots/transform_clamping_spheres.png create mode 100644 crates/viewer/re_view_spatial/tests/transform_clamping.rs delete mode 100644 tests/python/release_checklist/check_component_and_transform_clamping.py diff --git a/crates/viewer/re_view_spatial/tests/draw_order.rs b/crates/viewer/re_view_spatial/tests/draw_order.rs index 484af9919398..8c0dc6ff7ada 100644 --- a/crates/viewer/re_view_spatial/tests/draw_order.rs +++ b/crates/viewer/re_view_spatial/tests/draw_order.rs @@ -217,6 +217,7 @@ fn run_view_ui_and_save_snapshot( harness.run(); + // TODO(#8924): To account for platform-specific AA. let broken_percent_threshold = 0.003; let num_pixels = (size.x * size.y).ceil() as u64; 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 new file mode 100644 index 000000000000..1f019f173f8c --- /dev/null +++ b/crates/viewer/re_view_spatial/tests/snapshots/transform_clamping_boxes.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:b69285886b72b07ec7bd39a7dfcb2cbe5c9598603ad93d53ee0db4171ffbc857 +size 1014277 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 new file mode 100644 index 000000000000..cef74b7d78df --- /dev/null +++ b/crates/viewer/re_view_spatial/tests/snapshots/transform_clamping_spheres.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:db909b39b31a0b1aa2dfd79838920185700007e61ab09072bb5111278d7a06b2 +size 1021534 diff --git a/crates/viewer/re_view_spatial/tests/transform_clamping.rs b/crates/viewer/re_view_spatial/tests/transform_clamping.rs new file mode 100644 index 000000000000..eb3501c7c45f --- /dev/null +++ b/crates/viewer/re_view_spatial/tests/transform_clamping.rs @@ -0,0 +1,273 @@ +use re_chunk_store::RowId; +use re_log_types::TimePoint; +use re_view_spatial::SpatialView3D; +use re_viewer_context::test_context::TestContext; +use re_viewer_context::{RecommendedView, ViewClass, ViewId}; +use re_viewport_blueprint::test_context_ext::TestContextExt; +use re_viewport_blueprint::ViewBlueprint; + +#[test] +pub fn test_transform_clamping() { + let mut test_context = get_test_context(); + + { + test_context.log_entity("boxes/clamped_colors".into(), |builder| { + builder.with_archetype( + RowId::new(), + TimePoint::default(), + &re_types::archetypes::Boxes3D::from_half_sizes([(1.0, 1.0, 1.0), (2.0, 2.0, 2.0)]) + .with_colors([0xFF0000FF]), + ) + }); + + test_context.log_entity("boxes/ignored_colors".into(), |builder| { + builder.with_archetype( + RowId::new(), + TimePoint::default(), + &re_types::archetypes::Boxes3D::from_centers_and_half_sizes( + [(5.0, 0.0, 0.0)], + [(1.0, 1.0, 1.0)], + ) + .with_colors([0x00FF00FF, 0xFF00FFFF]), + ) + }); + + test_context.log_entity("boxes/more_transforms_than_sizes".into(), |builder| { + builder + .with_archetype( + RowId::new(), + TimePoint::default(), + &re_types::archetypes::Boxes3D::from_centers_and_half_sizes( + [(0.0, 5.0, 0.0)], + [(1.0, 1.0, 1.0)], + ) + .with_colors([0x0000FFFF]), + ) + .with_archetype( + RowId::new(), + TimePoint::default(), + &re_types::archetypes::InstancePoses3D::new() + .with_scales([(1.0, 1.0, 1.0), (2.0, 2.0, 2.0)]), + ) + }); + + test_context.log_entity("boxes/no_primaries".into(), |builder| { + builder + .with_archetype( + RowId::new(), + TimePoint::default(), + &re_types::archetypes::Boxes3D::update_fields() + .with_centers([(5.0, 0.0, 0.0)]) + .with_colors([0xFF00FFFF]), + ) + .with_archetype( + RowId::new(), + TimePoint::default(), + &re_types::archetypes::InstancePoses3D::new() + .with_scales([(1.0, 1.0, 1.0), (2.0, 2.0, 2.0)]), + ) + }); + } + + { + test_context.log_entity("spheres/clamped_colors".into(), |builder| { + builder.with_archetype( + RowId::new(), + TimePoint::default(), + &re_types::archetypes::Ellipsoids3D::from_half_sizes([ + (1.0, 1.0, 1.0), + (2.0, 2.0, 2.0), + ]) + .with_colors([0xFF0000FF]), + ) + }); + + test_context.log_entity("spheres/ignored_colors".into(), |builder| { + builder.with_archetype( + RowId::new(), + TimePoint::default(), + &re_types::archetypes::Ellipsoids3D::from_centers_and_half_sizes( + [(5.0, 0.0, 0.0)], + [(1.0, 1.0, 1.0)], + ) + .with_colors([0x00FF00FF, 0xFF00FFFF]), + ) + }); + + test_context.log_entity("spheres/more_transforms_than_sizes".into(), |builder| { + builder + .with_archetype( + RowId::new(), + TimePoint::default(), + &re_types::archetypes::Ellipsoids3D::from_centers_and_half_sizes( + [(0.0, 5.0, 0.0)], + [(1.0, 1.0, 1.0)], + ) + .with_colors([0x0000FFFF]), + ) + .with_archetype( + RowId::new(), + TimePoint::default(), + &re_types::archetypes::InstancePoses3D::new() + .with_scales([(1.0, 1.0, 1.0), (2.0, 2.0, 2.0)]), + ) + }); + + test_context.log_entity("spheres/no_primaries".into(), |builder| { + builder + .with_archetype( + RowId::new(), + TimePoint::default(), + &re_types::archetypes::Ellipsoids3D::update_fields() + .with_centers([(5.0, 0.0, 0.0)]) + .with_colors([0xFF00FFFF]), + ) + .with_archetype( + RowId::new(), + TimePoint::default(), + &re_types::archetypes::InstancePoses3D::new() + .with_scales([(1.0, 1.0, 1.0), (2.0, 2.0, 2.0)]), + ) + }); + } + + let view_ids = setup_blueprint(&mut test_context); + run_view_ui_and_save_snapshot( + &mut test_context, + view_ids, + "transform_clamping", + // TODO(#8924): A lot of pixels won't pass the diff because of anti-aliasing. + // By bumping the resolution, we lessen the weight of edges. + egui::vec2(300.0, 300.0) * 3.0, + ); +} + +fn get_test_context() -> TestContext { + let mut test_context = TestContext::default(); + + // It's important to first register the view class before adding any entities, + // otherwise the `VisualizerEntitySubscriber` for our visualizers doesn't exist yet, + // and thus will not find anything applicable to the visualizer. + test_context.register_view_class::(); + + // Make sure we can draw stuff in the hover tables. + test_context.component_ui_registry = re_component_ui::create_component_ui_registry(); + // Also register the legacy UIs. + re_data_ui::register_component_uis(&mut test_context.component_ui_registry); + + test_context +} + +#[allow(clippy::unwrap_used)] +fn setup_blueprint(test_context: &mut TestContext) -> (ViewId, ViewId) { + test_context.setup_viewport_blueprint(|_ctx, blueprint| { + let view_blueprint_boxes = ViewBlueprint::new( + re_view_spatial::SpatialView3D::identifier(), + RecommendedView { + origin: "/boxes".into(), + query_filter: "+ $origin/**".try_into().unwrap(), + }, + ); + + let view_blueprint_spheres = ViewBlueprint::new( + re_view_spatial::SpatialView3D::identifier(), + RecommendedView { + origin: "/spheres".into(), + query_filter: "+ $origin/**".try_into().unwrap(), + }, + ); + + let view_id_boxes = view_blueprint_boxes.id; + let view_id_spheres = view_blueprint_spheres.id; + + blueprint.add_views( + [view_blueprint_boxes, view_blueprint_spheres].into_iter(), + None, + None, + ); + + (view_id_boxes, view_id_spheres) + }) +} + +fn run_view_ui_and_save_snapshot( + test_context: &mut TestContext, + (view_id_boxes, view_id_spheres): (ViewId, ViewId), + name: &str, + size: egui::Vec2, +) { + for (target, view_id) in [("boxes", view_id_boxes), ("spheres", view_id_spheres)] { + let mut harness = test_context + .setup_kittest_for_rendering() + .with_size(size) + .build(|ctx| { + re_ui::apply_style_and_install_loaders(ctx); + + egui::CentralPanel::default().show(ctx, |ui| { + test_context.run(ctx, |ctx| { + let view_class = ctx + .view_class_registry + .get_class_or_log_error(SpatialView3D::identifier()); + + let view_blueprint = ViewBlueprint::try_from_db( + view_id, + ctx.store_context.blueprint, + ctx.blueprint_query, + ) + .expect("we just created that view"); + + let mut view_states = test_context.view_states.lock(); + + let view_state = view_states.get_mut_or_create(view_id, view_class); + let (view_query, system_execution_output) = + re_viewport::execute_systems_for_view( + ctx, + &view_blueprint, + ctx.current_query().at(), // TODO(andreas): why is this even needed to be passed in? + view_state, + ); + view_class + .ui(ctx, ui, view_state, &view_query, system_execution_output) + .expect("failed to run view ui"); + }); + + test_context.handle_system_commands(); + }); + }); + + { + // This test checks the clamping behavior of components & instance poses on boxes & spheres. + // + // One view shows spheres, the other boxes. + // + // For both you should see: + // * 2x red (one bigger than the other) + // * 1x green + // * 2x blue (one bigger than the other) + // * NO other boxes/spheres, in particular no magenta ones! + + let name = format!("{name}_{target}"); + let raw_input = harness.input_mut(); + raw_input + .events + .push(egui::Event::PointerMoved((150.0, 150.0).into())); + raw_input.events.push(egui::Event::MouseWheel { + unit: egui::MouseWheelUnit::Line, + delta: egui::Vec2::UP * 2.0, + modifiers: egui::Modifiers::default(), + }); + harness.run_steps(10); + + // TODO(#8924): To account for platform-specific AA. + let broken_percent_threshold = 0.003; + let num_pixels = (size.x * size.y).ceil() as u64; + + use re_viewer_context::test_context::HarnessExt as _; + harness.snapshot_with_broken_pixels_threshold( + &name, + num_pixels, + broken_percent_threshold, + ); + } + } +} diff --git a/tests/python/release_checklist/check_component_and_transform_clamping.py b/tests/python/release_checklist/check_component_and_transform_clamping.py deleted file mode 100644 index eeef6fd6607d..000000000000 --- a/tests/python/release_checklist/check_component_and_transform_clamping.py +++ /dev/null @@ -1,88 +0,0 @@ -from __future__ import annotations - -import os -from argparse import Namespace -from uuid import uuid4 - -import rerun as rr -import rerun.blueprint as rrb - -README = """\ -# Component & transform clamping behavior - -This test checks the clamping behavior of components & instance poses on boxes & spheres. - -One view shows spheres, the other boxes. - -For both you should see: -* 2x red (one bigger than the other) -* 1x green -* 2x blue (one bigger than the other) -* NO other boxes/spheres, in particular no magenta ones! -""" - -rerun_obj_path = f"{os.path.dirname(os.path.realpath(__file__))}/../../assets/rerun.obj" - - -def log_readme() -> None: - rr.log("readme", rr.TextDocument(README, media_type=rr.MediaType.MARKDOWN), static=True) - - -def blueprint() -> rrb.BlueprintLike: - return rrb.Blueprint( - rrb.Horizontal( - rrb.TextDocumentView(origin="readme"), - rrb.Spatial3DView(origin="spheres"), - rrb.Spatial3DView(origin="boxes"), - ) - ) - - -def log_data() -> None: - rr.log("/boxes/clamped_colors", rr.Boxes3D(half_sizes=[[1, 1, 1], [2, 2, 2]], colors=[[255, 0, 0]])) - rr.log( - "/boxes/ignored_colors", - rr.Boxes3D(half_sizes=[[1, 1, 1]], centers=[[5, 0, 0]], colors=[[0, 255, 0], [255, 0, 255]]), - ) - rr.log( - "/boxes/more_transforms_than_sizes", - rr.Boxes3D(half_sizes=[[1, 1, 1]], centers=[[0, 5, 0]], colors=[[0, 0, 255]]), - rr.InstancePoses3D(scales=[[1, 1, 1], [2, 2, 2]]), - ) - rr.log( - "/boxes/no_primaries", - rr.Boxes3D(half_sizes=[], centers=[[5, 0, 0]], colors=[[255, 0, 255]]), - rr.InstancePoses3D(scales=[[1, 1, 1], [2, 2, 2]]), - ) - # Same again but with spheres. - rr.log("/spheres/clamped_colors", rr.Ellipsoids3D(half_sizes=[[1, 1, 1], [2, 2, 2]], colors=[[255, 0, 0]])) - rr.log( - "/spheres/ignored_colors", - rr.Ellipsoids3D(half_sizes=[[1, 1, 1]], centers=[[5, 0, 0]], colors=[[0, 255, 0], [255, 0, 255]]), - ) - rr.log( - "/spheres/more_transforms_than_sizes", - rr.Ellipsoids3D(half_sizes=[[1, 1, 1]], centers=[[0, 5, 0]], colors=[[0, 0, 255]]), - rr.InstancePoses3D(scales=[[1, 1, 1], [2, 2, 2]]), - ) - rr.log( - "/spheres/no_primaries", - rr.Ellipsoids3D(half_sizes=[], centers=[[5, 0, 0]], colors=[[255, 0, 255]]), - rr.InstancePoses3D(scales=[[1, 1, 1], [2, 2, 2]]), - ) - - -def run(args: Namespace) -> None: - rr.script_setup(args, f"{os.path.basename(__file__)}", recording_id=uuid4()) - rr.send_blueprint(blueprint(), make_active=True, make_default=True) - log_readme() - log_data() - - -if __name__ == "__main__": - import argparse - - parser = argparse.ArgumentParser(description="Interactive release checklist") - rr.script_add_args(parser) - args = parser.parse_args() - run(args) From e1b19fa151f94c79863aa887abfcb8344336445c Mon Sep 17 00:00:00 2001 From: Antoine Beyeler <49431240+abey79@users.noreply.github.com> Date: Thu, 6 Feb 2025 19:37:35 +0100 Subject: [PATCH 2/2] Stop forgetting about updating the migration guide link (#8960) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ☝🏻 --- .github/workflows/reusable_checks.yml | 3 ++ docs/content/reference/migration.md | 2 +- scripts/ci/check_migration_guide_redirect.py | 32 ++++++++++++++++++++ 3 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 scripts/ci/check_migration_guide_redirect.py diff --git a/.github/workflows/reusable_checks.yml b/.github/workflows/reusable_checks.yml index 23a1dca03a2c..3e78d915a03d 100644 --- a/.github/workflows/reusable_checks.yml +++ b/.github/workflows/reusable_checks.yml @@ -108,6 +108,9 @@ jobs: - name: Check example manifest coverage run: pixi run ./scripts/check_example_manifest_coverage.py + - name: Check the migration guide redirect + run: pixi run python scripts/ci/check_migration_guide_redirect.py + lint-md: name: Lint markdown runs-on: ubuntu-latest diff --git a/docs/content/reference/migration.md b/docs/content/reference/migration.md index e172dd5c8243..9d054a2f0e07 100644 --- a/docs/content/reference/migration.md +++ b/docs/content/reference/migration.md @@ -1,5 +1,5 @@ --- title: Migration Guides order: 1000 -redirect: reference/migration/migration-0-19 +redirect: reference/migration/migration-0-22 --- diff --git a/scripts/ci/check_migration_guide_redirect.py b/scripts/ci/check_migration_guide_redirect.py new file mode 100644 index 000000000000..0b72ec46eb4d --- /dev/null +++ b/scripts/ci/check_migration_guide_redirect.py @@ -0,0 +1,32 @@ +from __future__ import annotations + +from pathlib import Path + +MIGRATION_DIR = Path(__file__).parent.parent.parent / "docs" / "content" / "reference" / "migration" +MIGRATION_TOC = Path(__file__).parent.parent.parent / "docs" / "content" / "reference" / "migration.md" + + +def extract_version(path: Path) -> tuple[int, ...]: + version = path.name.removesuffix(".md").removeprefix("migration-") + return tuple(map(int, version.split("-"))) + + +def extract_current_redirect_version() -> tuple[int, ...] | None: + for line in MIGRATION_TOC.read_text().splitlines(): + if line.startswith("redirect:"): + return extract_version(Path(line.removeprefix("redirect: "))) + + return None + + +def main() -> None: + assert MIGRATION_TOC.exists(), "Could not find the `migration.md` file in the docs" + assert MIGRATION_DIR.exists() and MIGRATION_DIR.is_dir(), "Could not find the `migration` directory in the docs" + + max_version = max(extract_version(guide) for guide in MIGRATION_DIR.glob("*.md")) + + assert max_version == extract_current_redirect_version(), "The current `migration.md` redirect is not up to date" + + +if __name__ == "__main__": + main()