Skip to content

Commit bdc1fd1

Browse files
authored
Remove Axes3D archetype and add axis_length to Transform3D (#6676)
### What - Resolves: #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`.
1 parent d278f92 commit bdc1fd1

36 files changed

Lines changed: 211 additions & 609 deletions

crates/re_space_view_spatial/src/view_3d.rs

Lines changed: 52 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use re_viewer_context::{
1919
VisualizableEntities, VisualizableFilterContext,
2020
};
2121

22-
use crate::visualizers::{CamerasVisualizer, Transform3DArrowsVisualizer, Transform3DDetector};
22+
use crate::visualizers::{AxisLengthDetector, CamerasVisualizer, Transform3DArrowsVisualizer};
2323
use crate::{
2424
contexts::register_spatial_contexts,
2525
heuristics::default_visualized_entities_for_visualizer_kind,
@@ -194,59 +194,67 @@ impl SpaceViewClass for SpatialSpaceView3D {
194194
visualizable_entities_per_visualizer: &PerVisualizer<VisualizableEntities>,
195195
indicated_entities_per_visualizer: &PerVisualizer<IndicatedEntities>,
196196
) -> SmallVisualizerSet {
197-
let applicable_visualizers: HashSet<&ViewSystemIdentifier> =
198-
applicable_entities_per_visualizer
199-
.iter()
200-
.filter_map(|(visualizer, ents)| {
201-
if ents.contains(entity_path) {
202-
Some(visualizer)
203-
} else {
204-
None
205-
}
206-
})
207-
.collect();
208-
209-
let available_visualizers: HashSet<&ViewSystemIdentifier> =
210-
visualizable_entities_per_visualizer
211-
.iter()
212-
.filter_map(|(visualizer, ents)| {
213-
if ents.contains(entity_path) {
214-
Some(visualizer)
215-
} else {
216-
None
217-
}
218-
})
219-
.collect();
197+
let arrows_viz = Transform3DArrowsVisualizer::identifier();
198+
let axis_detector = AxisLengthDetector::identifier();
199+
let camera_viz = CamerasVisualizer::identifier();
220200

221-
let mut visualizers: SmallVisualizerSet = available_visualizers
201+
let applicable: HashSet<&ViewSystemIdentifier> = applicable_entities_per_visualizer
222202
.iter()
223-
.filter_map(|visualizer| {
224-
if indicated_entities_per_visualizer
225-
.get(*visualizer)
226-
.map_or(false, |matching_list| matching_list.contains(entity_path))
227-
{
228-
Some(**visualizer)
203+
.filter_map(|(visualizer, ents)| {
204+
if ents.contains(entity_path) {
205+
Some(visualizer)
229206
} else {
230207
None
231208
}
232209
})
233210
.collect();
234211

235-
if available_visualizers.contains(&Transform3DArrowsVisualizer::identifier()) {
236-
// There are two cases where we want to activate the [`Transform3DArrowVisualizer`]:
237-
// - If we have no visualizers, but [`Transform3DDetector`] indicates there is a transform here.
238-
// - If we have the [`CamerasVisualizer`] active.
239-
if (visualizers.is_empty()
240-
&& applicable_visualizers.contains(&Transform3DDetector::identifier()))
241-
|| visualizers.contains(&CamerasVisualizer::identifier())
242-
{
243-
visualizers.push(Transform3DArrowsVisualizer::identifier());
244-
}
245-
}
212+
let visualizable: HashSet<&ViewSystemIdentifier> = visualizable_entities_per_visualizer
213+
.iter()
214+
.filter_map(|(visualizer, ents)| {
215+
if ents.contains(entity_path) {
216+
Some(visualizer)
217+
} else {
218+
None
219+
}
220+
})
221+
.collect();
222+
223+
// We never want to consider `Transform3DArrows` as directly indicated since it uses the
224+
// the Transform3D archetype. This is often used to transform other 3D primitives, where
225+
// it might be annoying to always have the arrows show up.
226+
let indicated: HashSet<&ViewSystemIdentifier> = indicated_entities_per_visualizer
227+
.iter()
228+
.filter_map(|(visualizer, ents)| {
229+
if visualizer != &arrows_viz && ents.contains(entity_path) {
230+
Some(visualizer)
231+
} else {
232+
None
233+
}
234+
})
235+
.collect();
246236

247-
// If there were no other visualizers, or this is a camera, then we will include axes.
237+
// Start with all the entities which are both indicated and visualizable.
238+
let mut chosen: SmallVisualizerSet = indicated
239+
.intersection(&visualizable)
240+
.copied()
241+
.copied()
242+
.collect();
243+
244+
// There are three cases where we want to activate the [`Transform3DArrowVisualizer`]:
245+
// - If we have no visualizers, but otherwise meet the criteria for Transform3DArrows.
246+
// - If someone set an axis_length explicitly, so [`AxisLengthDetector`] is applicable.
247+
// - If we already have the [`CamerasVisualizer`] active.
248+
if !chosen.contains(&arrows_viz)
249+
&& visualizable.contains(&arrows_viz)
250+
&& ((chosen.is_empty() && visualizable.contains(&arrows_viz))
251+
|| applicable.contains(&axis_detector)
252+
|| chosen.contains(&camera_viz))
253+
{
254+
chosen.push(arrows_viz);
255+
}
248256

249-
visualizers
257+
chosen
250258
}
251259

252260
fn spawn_heuristics(

crates/re_space_view_spatial/src/visualizers/mod.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ mod transform3d_arrows;
1919
pub use cameras::CamerasVisualizer;
2020
pub use images::{ImageVisualizer, ViewerImage};
2121
pub use spatial_view_visualizer::SpatialViewVisualizerData;
22-
pub use transform3d_arrows::{add_axis_arrows, Transform3DArrowsVisualizer, Transform3DDetector};
22+
pub use transform3d_arrows::{add_axis_arrows, AxisLengthDetector, Transform3DArrowsVisualizer};
2323

2424
// ---
2525

@@ -69,8 +69,8 @@ pub fn register_2d_spatial_visualizers(
6969
system_registry.register_visualizer::<meshes::Mesh3DVisualizer>()?;
7070
system_registry.register_visualizer::<points2d::Points2DVisualizer>()?;
7171
system_registry.register_visualizer::<points3d::Points3DVisualizer>()?;
72+
system_registry.register_visualizer::<transform3d_arrows::AxisLengthDetector>()?;
7273
system_registry.register_visualizer::<transform3d_arrows::Transform3DArrowsVisualizer>()?;
73-
system_registry.register_visualizer::<transform3d_arrows::Transform3DDetector>()?;
7474
Ok(())
7575
}
7676

@@ -89,8 +89,8 @@ pub fn register_3d_spatial_visualizers(
8989
system_registry.register_visualizer::<meshes::Mesh3DVisualizer>()?;
9090
system_registry.register_visualizer::<points2d::Points2DVisualizer>()?;
9191
system_registry.register_visualizer::<points3d::Points3DVisualizer>()?;
92+
system_registry.register_visualizer::<transform3d_arrows::AxisLengthDetector>()?;
9293
system_registry.register_visualizer::<transform3d_arrows::Transform3DArrowsVisualizer>()?;
93-
system_registry.register_visualizer::<transform3d_arrows::Transform3DDetector>()?;
9494
Ok(())
9595
}
9696

crates/re_space_view_spatial/src/visualizers/transform3d_arrows.rs

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@ use egui::Color32;
22
use re_log_types::{EntityPath, Instance};
33
use re_space_view::DataResultQuery;
44
use re_types::{
5-
archetypes::{Axes3D, Pinhole, Transform3D},
5+
archetypes::{Pinhole, Transform3D},
66
components::{AxisLength, ImagePlaneDistance},
7+
Loggable,
78
};
89
use re_viewer_context::{
910
ApplicableEntities, IdentifiedViewSystem, QueryContext, SpaceViewStateExt,
@@ -37,7 +38,7 @@ impl IdentifiedViewSystem for Transform3DArrowsVisualizer {
3738

3839
impl VisualizerSystem for Transform3DArrowsVisualizer {
3940
fn visualizer_query_info(&self) -> VisualizerQueryInfo {
40-
VisualizerQueryInfo::from_archetype::<Axes3D>()
41+
VisualizerQueryInfo::from_archetype::<Transform3D>()
4142
}
4243

4344
fn filter_visualizable_entities(
@@ -84,8 +85,8 @@ impl VisualizerSystem for Transform3DArrowsVisualizer {
8485
world_from_obj,
8586
);
8687

87-
let results =
88-
data_result.latest_at_with_blueprint_resolved_data::<Axes3D>(ctx, &latest_at_query);
88+
let results = data_result
89+
.latest_at_with_blueprint_resolved_data::<Transform3D>(ctx, &latest_at_query);
8990
let axis_length = results.get_mono_with_fallback::<AxisLength>().into();
9091

9192
add_axis_arrows(
@@ -213,22 +214,27 @@ impl TypedComponentFallbackProvider<AxisLength> for Transform3DArrowsVisualizer
213214

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

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

223-
impl IdentifiedViewSystem for Transform3DDetector {
224+
impl IdentifiedViewSystem for AxisLengthDetector {
224225
fn identifier() -> re_viewer_context::ViewSystemIdentifier {
225-
"Transform3DDetector".into()
226+
"AxisLengthDetector".into()
226227
}
227228
}
228229

229-
impl VisualizerSystem for Transform3DDetector {
230+
impl VisualizerSystem for AxisLengthDetector {
230231
fn visualizer_query_info(&self) -> VisualizerQueryInfo {
231-
VisualizerQueryInfo::from_archetype::<Transform3D>()
232+
let mut query_info = VisualizerQueryInfo::from_archetype::<Transform3D>();
233+
234+
query_info.required.insert(AxisLength::name());
235+
query_info.indicators = Default::default();
236+
237+
query_info
232238
}
233239

234240
fn execute(
@@ -259,4 +265,4 @@ impl VisualizerSystem for Transform3DDetector {
259265
}
260266
}
261267

262-
re_viewer_context::impl_component_fallback_provider!(Transform3DDetector => []);
268+
re_viewer_context::impl_component_fallback_provider!(AxisLengthDetector => []);

crates/re_types/definitions/rerun/archetypes.fbs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ include "./archetypes/annotation_context.fbs";
22
include "./archetypes/arrows2d.fbs";
33
include "./archetypes/arrows3d.fbs";
44
include "./archetypes/asset3d.fbs";
5-
include "./archetypes/axes3d.fbs";
65
include "./archetypes/bar_chart.fbs";
76
include "./archetypes/boxes2d.fbs";
87
include "./archetypes/boxes3d.fbs";

crates/re_types/definitions/rerun/archetypes/axes3d.fbs

Lines changed: 0 additions & 30 deletions
This file was deleted.

crates/re_types/definitions/rerun/archetypes/transform3d.fbs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,4 +16,12 @@ table Transform3D (
1616
) {
1717
/// The transform
1818
transform: rerun.components.Transform3D ("attr.rerun.component_required", order: 1000);
19+
20+
// --- Optional ---
21+
22+
/// Visual length of the 3 axes.
23+
///
24+
/// The length is interpreted in the local coordinate system of the transform.
25+
/// If the transform is scaled, the axes will be scaled accordingly.
26+
axis_length: rerun.components.AxisLength ("attr.rerun.component_optional", nullable, order: 2000);
1927
}

crates/re_types/src/archetypes/.gitattributes

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)