Skip to content

Commit

Permalink
Remove Axes3D archetype and add axis_length to Transform3D (rerun-io#…
Browse files Browse the repository at this point in the history
…6676)

### What
- Resolves: rerun-io#6578

It turns out having a visualizer with no required entities causes all
kinds of problems. This moves us back to only allowing axes if you have
a transform at an entity.

The heuristics are still a bit fancy for backwards compatibility. We
don't use Transform3D as an indicator since that would cause every
transform to get axes by default. Instead, we only create the
Transform3D visualizer if:
- If we have no other visualizers, but otherwise meet the criteria for
Transform3DArrows.
- If someone set an axis_length explicitly, so AxisLengthDetector is
applicable.
  - If we already have the CamerasVisualizer active.

### Shortcomings
- The one big shortcoming still present here is that there's still no
way to use a `default` to force the Transform3D axes to be visible.

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6676?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6676?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!

- [PR Build Summary](https://build.rerun.io/pr/6676)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.
  • Loading branch information
jleibs authored Jun 28, 2024
1 parent d278f92 commit bdc1fd1
Show file tree
Hide file tree
Showing 36 changed files with 211 additions and 609 deletions.
96 changes: 52 additions & 44 deletions crates/re_space_view_spatial/src/view_3d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use re_viewer_context::{
VisualizableEntities, VisualizableFilterContext,
};

use crate::visualizers::{CamerasVisualizer, Transform3DArrowsVisualizer, Transform3DDetector};
use crate::visualizers::{AxisLengthDetector, CamerasVisualizer, Transform3DArrowsVisualizer};
use crate::{
contexts::register_spatial_contexts,
heuristics::default_visualized_entities_for_visualizer_kind,
Expand Down Expand Up @@ -194,59 +194,67 @@ impl SpaceViewClass for SpatialSpaceView3D {
visualizable_entities_per_visualizer: &PerVisualizer<VisualizableEntities>,
indicated_entities_per_visualizer: &PerVisualizer<IndicatedEntities>,
) -> SmallVisualizerSet {
let applicable_visualizers: HashSet<&ViewSystemIdentifier> =
applicable_entities_per_visualizer
.iter()
.filter_map(|(visualizer, ents)| {
if ents.contains(entity_path) {
Some(visualizer)
} else {
None
}
})
.collect();

let available_visualizers: HashSet<&ViewSystemIdentifier> =
visualizable_entities_per_visualizer
.iter()
.filter_map(|(visualizer, ents)| {
if ents.contains(entity_path) {
Some(visualizer)
} else {
None
}
})
.collect();
let arrows_viz = Transform3DArrowsVisualizer::identifier();
let axis_detector = AxisLengthDetector::identifier();
let camera_viz = CamerasVisualizer::identifier();

let mut visualizers: SmallVisualizerSet = available_visualizers
let applicable: HashSet<&ViewSystemIdentifier> = applicable_entities_per_visualizer
.iter()
.filter_map(|visualizer| {
if indicated_entities_per_visualizer
.get(*visualizer)
.map_or(false, |matching_list| matching_list.contains(entity_path))
{
Some(**visualizer)
.filter_map(|(visualizer, ents)| {
if ents.contains(entity_path) {
Some(visualizer)
} else {
None
}
})
.collect();

if available_visualizers.contains(&Transform3DArrowsVisualizer::identifier()) {
// There are two cases where we want to activate the [`Transform3DArrowVisualizer`]:
// - If we have no visualizers, but [`Transform3DDetector`] indicates there is a transform here.
// - If we have the [`CamerasVisualizer`] active.
if (visualizers.is_empty()
&& applicable_visualizers.contains(&Transform3DDetector::identifier()))
|| visualizers.contains(&CamerasVisualizer::identifier())
{
visualizers.push(Transform3DArrowsVisualizer::identifier());
}
}
let visualizable: HashSet<&ViewSystemIdentifier> = visualizable_entities_per_visualizer
.iter()
.filter_map(|(visualizer, ents)| {
if ents.contains(entity_path) {
Some(visualizer)
} else {
None
}
})
.collect();

// We never want to consider `Transform3DArrows` as directly indicated since it uses the
// the Transform3D archetype. This is often used to transform other 3D primitives, where
// it might be annoying to always have the arrows show up.
let indicated: HashSet<&ViewSystemIdentifier> = indicated_entities_per_visualizer
.iter()
.filter_map(|(visualizer, ents)| {
if visualizer != &arrows_viz && ents.contains(entity_path) {
Some(visualizer)
} else {
None
}
})
.collect();

// If there were no other visualizers, or this is a camera, then we will include axes.
// Start with all the entities which are both indicated and visualizable.
let mut chosen: SmallVisualizerSet = indicated
.intersection(&visualizable)
.copied()
.copied()
.collect();

// There are three cases where we want to activate the [`Transform3DArrowVisualizer`]:
// - If we have no visualizers, but otherwise meet the criteria for Transform3DArrows.
// - If someone set an axis_length explicitly, so [`AxisLengthDetector`] is applicable.
// - If we already have the [`CamerasVisualizer`] active.
if !chosen.contains(&arrows_viz)
&& visualizable.contains(&arrows_viz)
&& ((chosen.is_empty() && visualizable.contains(&arrows_viz))
|| applicable.contains(&axis_detector)
|| chosen.contains(&camera_viz))
{
chosen.push(arrows_viz);
}

visualizers
chosen
}

fn spawn_heuristics(
Expand Down
6 changes: 3 additions & 3 deletions crates/re_space_view_spatial/src/visualizers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ mod transform3d_arrows;
pub use cameras::CamerasVisualizer;
pub use images::{ImageVisualizer, ViewerImage};
pub use spatial_view_visualizer::SpatialViewVisualizerData;
pub use transform3d_arrows::{add_axis_arrows, Transform3DArrowsVisualizer, Transform3DDetector};
pub use transform3d_arrows::{add_axis_arrows, AxisLengthDetector, Transform3DArrowsVisualizer};

// ---

Expand Down Expand Up @@ -69,8 +69,8 @@ pub fn register_2d_spatial_visualizers(
system_registry.register_visualizer::<meshes::Mesh3DVisualizer>()?;
system_registry.register_visualizer::<points2d::Points2DVisualizer>()?;
system_registry.register_visualizer::<points3d::Points3DVisualizer>()?;
system_registry.register_visualizer::<transform3d_arrows::AxisLengthDetector>()?;
system_registry.register_visualizer::<transform3d_arrows::Transform3DArrowsVisualizer>()?;
system_registry.register_visualizer::<transform3d_arrows::Transform3DDetector>()?;
Ok(())
}

Expand All @@ -89,8 +89,8 @@ pub fn register_3d_spatial_visualizers(
system_registry.register_visualizer::<meshes::Mesh3DVisualizer>()?;
system_registry.register_visualizer::<points2d::Points2DVisualizer>()?;
system_registry.register_visualizer::<points3d::Points3DVisualizer>()?;
system_registry.register_visualizer::<transform3d_arrows::AxisLengthDetector>()?;
system_registry.register_visualizer::<transform3d_arrows::Transform3DArrowsVisualizer>()?;
system_registry.register_visualizer::<transform3d_arrows::Transform3DDetector>()?;
Ok(())
}

Expand Down
30 changes: 18 additions & 12 deletions crates/re_space_view_spatial/src/visualizers/transform3d_arrows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ use egui::Color32;
use re_log_types::{EntityPath, Instance};
use re_space_view::DataResultQuery;
use re_types::{
archetypes::{Axes3D, Pinhole, Transform3D},
archetypes::{Pinhole, Transform3D},
components::{AxisLength, ImagePlaneDistance},
Loggable,
};
use re_viewer_context::{
ApplicableEntities, IdentifiedViewSystem, QueryContext, SpaceViewStateExt,
Expand Down Expand Up @@ -37,7 +38,7 @@ impl IdentifiedViewSystem for Transform3DArrowsVisualizer {

impl VisualizerSystem for Transform3DArrowsVisualizer {
fn visualizer_query_info(&self) -> VisualizerQueryInfo {
VisualizerQueryInfo::from_archetype::<Axes3D>()
VisualizerQueryInfo::from_archetype::<Transform3D>()
}

fn filter_visualizable_entities(
Expand Down Expand Up @@ -84,8 +85,8 @@ impl VisualizerSystem for Transform3DArrowsVisualizer {
world_from_obj,
);

let results =
data_result.latest_at_with_blueprint_resolved_data::<Axes3D>(ctx, &latest_at_query);
let results = data_result
.latest_at_with_blueprint_resolved_data::<Transform3D>(ctx, &latest_at_query);
let axis_length = results.get_mono_with_fallback::<AxisLength>().into();

add_axis_arrows(
Expand Down Expand Up @@ -213,22 +214,27 @@ impl TypedComponentFallbackProvider<AxisLength> for Transform3DArrowsVisualizer

re_viewer_context::impl_component_fallback_provider!(Transform3DArrowsVisualizer => [AxisLength]);

/// The `Transform3DDetector` doesn't actually visualize anything, but it allows us to detect
/// when a transform should otherwise be visualized.
/// The `AxisLengthDetector` doesn't actually visualize anything, but it allows us to detect
/// when a transform has set the [`AxisLength`] component.
///
/// See the logic in [`crate::SpatialSpaceView3D`]`::choose_default_visualizers`.
#[derive(Default)]
pub struct Transform3DDetector();
pub struct AxisLengthDetector();

impl IdentifiedViewSystem for Transform3DDetector {
impl IdentifiedViewSystem for AxisLengthDetector {
fn identifier() -> re_viewer_context::ViewSystemIdentifier {
"Transform3DDetector".into()
"AxisLengthDetector".into()
}
}

impl VisualizerSystem for Transform3DDetector {
impl VisualizerSystem for AxisLengthDetector {
fn visualizer_query_info(&self) -> VisualizerQueryInfo {
VisualizerQueryInfo::from_archetype::<Transform3D>()
let mut query_info = VisualizerQueryInfo::from_archetype::<Transform3D>();

query_info.required.insert(AxisLength::name());
query_info.indicators = Default::default();

query_info
}

fn execute(
Expand Down Expand Up @@ -259,4 +265,4 @@ impl VisualizerSystem for Transform3DDetector {
}
}

re_viewer_context::impl_component_fallback_provider!(Transform3DDetector => []);
re_viewer_context::impl_component_fallback_provider!(AxisLengthDetector => []);
1 change: 0 additions & 1 deletion crates/re_types/definitions/rerun/archetypes.fbs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ include "./archetypes/annotation_context.fbs";
include "./archetypes/arrows2d.fbs";
include "./archetypes/arrows3d.fbs";
include "./archetypes/asset3d.fbs";
include "./archetypes/axes3d.fbs";
include "./archetypes/bar_chart.fbs";
include "./archetypes/boxes2d.fbs";
include "./archetypes/boxes3d.fbs";
Expand Down
30 changes: 0 additions & 30 deletions crates/re_types/definitions/rerun/archetypes/axes3d.fbs

This file was deleted.

8 changes: 8 additions & 0 deletions crates/re_types/definitions/rerun/archetypes/transform3d.fbs
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,12 @@ table Transform3D (
) {
/// The transform
transform: rerun.components.Transform3D ("attr.rerun.component_required", order: 1000);

// --- Optional ---

/// Visual length of the 3 axes.
///
/// The length is interpreted in the local coordinate system of the transform.
/// If the transform is scaled, the axes will be scaled accordingly.
axis_length: rerun.components.AxisLength ("attr.rerun.component_optional", nullable, order: 2000);
}
1 change: 0 additions & 1 deletion crates/re_types/src/archetypes/.gitattributes

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit bdc1fd1

Please sign in to comment.