Skip to content

Commit

Permalink
Add ProcMeshVisBuffer, deduplicating visualizer code. (#7507)
Browse files Browse the repository at this point in the history
### What

`ProcMeshVisBuffer` extracts common elements from `Boxes3DVisualizer`
and `Ellipsoids3DVisualizer`. This will be useful to minimize duplicated
code when further proc-mesh shapes are added (#1361).

Additionally, `ComponentFallbackProvider` is no longer a supertrait of
`VisualizerSystem`. Instead, the fallback provider is always accessed
through `as_fallback_provider()` (which may return `&self` if it likes).
The advantage of this is that a visualizer may access its desired
fallback logic even while parts of itself are exclusively borrowed, by
putting the `TypedComponentFallbackProvider` implementations on a
separate type. (Strictly speaking, the supertrait change is not needed,
if we instead let a `ComponentFallbackProvider` implementation be made
from `TypedComponentFallbackProvider`s on a different type, but that
seems like a worse design to me.)

### 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)
* [ ] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/7507?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/7507?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)!
* [X] If have noted any breaking changes to the log API in
`CHANGELOG.md` and the migration guide

- [PR Build Summary](https://build.rerun.io/pr/7507)
- [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
kpreid authored Sep 26, 2024
1 parent 0034e6d commit 6976742
Show file tree
Hide file tree
Showing 32 changed files with 441 additions and 430 deletions.
8 changes: 6 additions & 2 deletions crates/viewer/re_selection_panel/src/defaults_ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ fn active_default_ui(
component_name,
row_id,
Some(&*component_array),
visualizer.as_fallback_provider(),
visualizer.fallback_provider(),
);
};

Expand Down Expand Up @@ -294,7 +294,11 @@ fn add_popup_ui(
.visualizer_collection
.get_by_identifier(viz)
.ok()
.and_then(|sys| sys.fallback_for(&query_context, component_name).ok())
.and_then(|sys| {
sys.fallback_provider()
.fallback_for(&query_context, component_name)
.ok()
})
else {
re_log::warn!(
"Could not identify an initial value for: {}",
Expand Down
5 changes: 4 additions & 1 deletion crates/viewer/re_selection_panel/src/visualizer_ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,10 @@ fn visualizer_components(
let result_default = query_result.defaults.get(&component_name);
let raw_default = non_empty_component_batch_raw(result_default, &component_name);

let raw_fallback = match visualizer.fallback_for(&query_ctx, component_name) {
let raw_fallback = match visualizer
.fallback_provider()
.fallback_for(&query_ctx, component_name)
{
Ok(fallback) => fallback,
Err(err) => {
re_log::warn_once!("Failed to get fallback for component {component_name}: {err}");
Expand Down
2 changes: 1 addition & 1 deletion crates/viewer/re_space_view/src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ impl DataResultQuery for DataResult {
};

if vis.visualizer_query_info().queried.contains(&component) {
return Some(vis.as_fallback_provider());
return Some(vis.fallback_provider());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ impl VisualizerSystem for BarChartVisualizerSystem {
self
}

fn as_fallback_provider(&self) -> &dyn re_viewer_context::ComponentFallbackProvider {
fn fallback_provider(&self) -> &dyn re_viewer_context::ComponentFallbackProvider {
self
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ impl VisualizerSystem for EmptySystem {
self
}

fn as_fallback_provider(&self) -> &dyn re_viewer_context::ComponentFallbackProvider {
fn fallback_provider(&self) -> &dyn re_viewer_context::ComponentFallbackProvider {
self
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ impl VisualizerSystem for Arrows2DVisualizer {
self
}

fn as_fallback_provider(&self) -> &dyn re_viewer_context::ComponentFallbackProvider {
fn fallback_provider(&self) -> &dyn re_viewer_context::ComponentFallbackProvider {
self
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ impl VisualizerSystem for Arrows3DVisualizer {
self
}

fn as_fallback_provider(&self) -> &dyn re_viewer_context::ComponentFallbackProvider {
fn fallback_provider(&self) -> &dyn re_viewer_context::ComponentFallbackProvider {
self
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ impl VisualizerSystem for Asset3DVisualizer {
self
}

fn as_fallback_provider(&self) -> &dyn re_viewer_context::ComponentFallbackProvider {
fn fallback_provider(&self) -> &dyn re_viewer_context::ComponentFallbackProvider {
self
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ impl VisualizerSystem for Boxes2DVisualizer {
self
}

fn as_fallback_provider(&self) -> &dyn re_viewer_context::ComponentFallbackProvider {
fn fallback_provider(&self) -> &dyn re_viewer_context::ComponentFallbackProvider {
self
}
}
Expand Down
226 changes: 46 additions & 180 deletions crates/viewer/re_space_view_spatial/src/visualizers/boxes3d.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
use re_entity_db::InstancePathHash;
use re_log_types::Instance;
use re_renderer::{
renderer::MeshInstance, LineDrawableBuilder, PickingLayerInstanceId, RenderContext,
};
use std::iter;

use re_types::{
archetypes::Boxes3D,
components::{ClassId, Color, FillMode, HalfSize3D, KeypointId, Radius, ShowLabels, Text},
Expand All @@ -15,17 +12,12 @@ use re_viewer_context::{
VisualizerQueryInfo, VisualizerSystem,
};

use crate::{
contexts::SpatialSceneEntityContext,
instance_hash_conversions::picking_layer_id_from_instance_path_hash, proc_mesh,
view_kind::SpatialSpaceViewKind,
};
use crate::{contexts::SpatialSceneEntityContext, proc_mesh, view_kind::SpatialSpaceViewKind};

use super::{
entity_iterator::clamped_or_nothing, filter_visualizable_3d_entities,
process_annotation_and_keypoint_slices, process_color_slice, process_labels_3d,
process_radius_slice, utilities::LabeledBatch, SpatialViewVisualizerData,
SIZE_BOOST_IN_POINTS_FOR_LINE_OUTLINES,
filter_visualizable_3d_entities,
utilities::{ProcMeshBatch, ProcMeshDrawableBuilder},
SpatialViewVisualizerData,
};

// ---
Expand All @@ -43,133 +35,37 @@ impl Default for Boxes3DVisualizer {
// NOTE: Do not put profile scopes in these methods. They are called for all entities and all
// timestamps within a time range -- it's _a lot_.
impl Boxes3DVisualizer {
#[allow(clippy::too_many_arguments)]
fn process_data<'a>(
&mut self,
ctx: &QueryContext<'_>,
line_builder: &mut LineDrawableBuilder<'_>,
mesh_instances: &mut Vec<MeshInstance>,
query: &ViewQuery<'_>,
builder: &mut ProcMeshDrawableBuilder<'_, Fallback>,
query_context: &QueryContext<'_>,
ent_context: &SpatialSceneEntityContext<'_>,
data: impl Iterator<Item = Boxes3DComponentData<'a>>,
render_ctx: &RenderContext,
batches: impl Iterator<Item = Boxes3DComponentData<'a>>,
) -> Result<(), SpaceViewSystemExecutionError> {
let entity_path = ctx.target_entity_path;

for data in data {
if data.half_sizes.is_empty() {
continue;
}

// Draw as many boxes as we have max(instances, boxes), all components get repeated over that number.
// TODO(#7026): We should formalize this kind of hybrid joining better.
let num_instances = data
.half_sizes
.len()
.max(ent_context.transform_info.reference_from_instances.len());
let half_sizes = clamped_or_nothing(data.half_sizes, num_instances);

let (annotation_infos, _) = process_annotation_and_keypoint_slices(
query.latest_at,
num_instances,
std::iter::repeat(glam::Vec3::ZERO).take(num_instances),
data.keypoint_ids,
data.class_ids,
&ent_context.annotations,
);

// Has not custom fallback for radius, so we use the default.
// TODO(andreas): It would be nice to have this handle this fallback as part of the query.
let radii =
process_radius_slice(entity_path, num_instances, data.radii, Radius::default());
let colors =
process_color_slice(ctx, self, num_instances, &annotation_infos, data.colors);

let mut line_batch = line_builder
.batch("boxes3d")
.depth_offset(ent_context.depth_offset)
.outline_mask_ids(ent_context.highlight.overall)
.picking_object_id(re_renderer::PickingLayerObjectId(entity_path.hash64()));

let mut world_space_bounding_box = re_math::BoundingBox::NOTHING;

let world_from_instances = ent_context
.transform_info
.clamped_reference_from_instances();

for (instance_index, (half_size, world_from_instance, radius, &color)) in
itertools::izip!(half_sizes, world_from_instances, radii, &colors).enumerate()
{
let instance = Instance::from(instance_index as u64);
let proc_mesh_key = proc_mesh::ProcMeshKey::Cube;

let world_from_instance = world_from_instance
* glam::Affine3A::from_scale(glam::Vec3::from(*half_size) * 2.0);
world_space_bounding_box = world_space_bounding_box.union(
proc_mesh_key
.simple_bounding_box()
.transform_affine3(&world_from_instance),
);

match data.fill_mode {
FillMode::DenseWireframe | FillMode::MajorWireframe => {
let box3d = line_batch
.add_box_outline_from_transform(world_from_instance)
.color(color)
.radius(radius)
.picking_instance_id(PickingLayerInstanceId(instance_index as _));

if let Some(outline_mask_ids) =
ent_context.highlight.instances.get(&instance)
{
box3d.outline_mask_ids(*outline_mask_ids);
}
}
FillMode::Solid => {
let Some(solid_mesh) =
ctx.viewer_ctx.cache.entry(|c: &mut proc_mesh::SolidCache| {
c.entry(proc_mesh_key, render_ctx)
})
else {
return Err(SpaceViewSystemExecutionError::DrawDataCreationError(
"Failed to allocate solid mesh".into(),
));
};

mesh_instances.push(MeshInstance {
gpu_mesh: solid_mesh.gpu_mesh,
mesh: None,
world_from_mesh: world_from_instance,
outline_mask_ids: ent_context.highlight.index_outline_mask(instance),
picking_layer_id: picking_layer_id_from_instance_path_hash(
InstancePathHash::instance(entity_path, instance),
),
additive_tint: color,
});
}
}
}

self.0
.bounding_boxes
.push((entity_path.hash(), world_space_bounding_box));

self.0.ui_labels.extend(process_labels_3d(
LabeledBatch {
entity_path,
num_instances,
overall_position: world_space_bounding_box.center(),
instance_positions: ent_context
.transform_info
.clamped_reference_from_instances()
.map(|t| t.translation.into()),
labels: &data.labels,
colors: &colors,
show_labels: data.show_labels.unwrap_or_else(|| self.fallback_for(ctx)),
annotation_infos: &annotation_infos,
let proc_mesh_key = proc_mesh::ProcMeshKey::Cube;

// `ProcMeshKey::Cube` is scaled to side length of 1, i.e. a half-size of 0.5.
// Therefore, to make scaling by half_size work out to the correct result,
// apply a factor of 2.
// TODO(kpreid): Is there any non-historical reason to keep that.
let constant_instance_transform = glam::Affine3A::from_scale(glam::Vec3::splat(2.0));

for batch in batches {
builder.add_batch(
query_context,
ent_context,
constant_instance_transform,
ProcMeshBatch {
half_sizes: batch.half_sizes,
meshes: iter::repeat(proc_mesh_key),
fill_modes: iter::repeat(batch.fill_mode),
line_radii: batch.radii,
colors: batch.colors,
labels: &batch.labels,
show_labels: batch.show_labels,
keypoint_ids: batch.keypoint_ids,
class_ids: batch.class_ids,
},
glam::Affine3A::IDENTITY,
));
)?;
}

Ok(())
Expand Down Expand Up @@ -224,15 +120,8 @@ impl VisualizerSystem for Boxes3DVisualizer {
return Err(SpaceViewSystemExecutionError::NoRenderContextError);
};

let mut line_builder = LineDrawableBuilder::new(render_ctx);
line_builder.radius_boost_in_ui_points_for_outlines(SIZE_BOOST_IN_POINTS_FOR_LINE_OUTLINES);

// Collects solid (that is, triangles rather than wireframe) instances to be drawn.
//
// Why do we draw solid surfaces using instanced meshes and wireframes using instances?
// No good reason; only, those are the types of renderers that have been implemented so far.
// This code should be revisited with an eye on performance.
let mut solid_instances: Vec<MeshInstance> = Vec::new();
let mut builder =
ProcMeshDrawableBuilder::new(&mut self.0, render_ctx, view_query, "boxes3d", &Fallback);

use super::entity_iterator::{iter_primitive_array, process_archetype};
process_archetype::<Self, Boxes3D, _>(
Expand Down Expand Up @@ -281,8 +170,8 @@ impl VisualizerSystem for Boxes3DVisualizer {
match fill_mode {
FillMode::DenseWireframe | FillMode::MajorWireframe => {
// Each box consists of 12 independent lines with 2 vertices each.
line_builder.reserve_strips(num_boxes * 12)?;
line_builder.reserve_vertices(num_boxes * 12 * 2)?;
builder.line_builder.reserve_strips(num_boxes * 12)?;
builder.line_builder.reserve_vertices(num_boxes * 12 * 2)?;
}
FillMode::Solid => {
// No lines.
Expand Down Expand Up @@ -325,38 +214,13 @@ impl VisualizerSystem for Boxes3DVisualizer {
},
);

self.process_data(
ctx,
&mut line_builder,
&mut solid_instances,
view_query,
spatial_ctx,
data,
render_ctx,
)?;
Self::process_data(&mut builder, ctx, spatial_ctx, data)?;

Ok(())
},
)?;

let wireframe_draw_data: re_renderer::QueueableDrawData =
line_builder.into_draw_data()?.into();

let solid_draw_data: Option<re_renderer::QueueableDrawData> =
match re_renderer::renderer::MeshDrawData::new(render_ctx, &solid_instances) {
Ok(draw_data) => Some(draw_data.into()),
Err(err) => {
re_log::error_once!(
"Failed to create mesh draw data from mesh instances: {err}"
);
None
}
};

Ok([solid_draw_data, Some(wireframe_draw_data)]
.into_iter()
.flatten()
.collect())
builder.into_draw_data()
}

fn data(&self) -> Option<&dyn std::any::Any> {
Expand All @@ -367,21 +231,23 @@ impl VisualizerSystem for Boxes3DVisualizer {
self
}

fn as_fallback_provider(&self) -> &dyn re_viewer_context::ComponentFallbackProvider {
self
fn fallback_provider(&self) -> &dyn re_viewer_context::ComponentFallbackProvider {
&Fallback
}
}

impl TypedComponentFallbackProvider<Color> for Boxes3DVisualizer {
struct Fallback;

impl TypedComponentFallbackProvider<Color> for Fallback {
fn fallback_for(&self, ctx: &QueryContext<'_>) -> Color {
auto_color_for_entity_path(ctx.target_entity_path)
}
}

impl TypedComponentFallbackProvider<ShowLabels> for Boxes3DVisualizer {
impl TypedComponentFallbackProvider<ShowLabels> for Fallback {
fn fallback_for(&self, ctx: &QueryContext<'_>) -> ShowLabels {
super::utilities::show_labels_fallback::<HalfSize3D>(ctx)
}
}

re_viewer_context::impl_component_fallback_provider!(Boxes3DVisualizer => [Color, ShowLabels]);
re_viewer_context::impl_component_fallback_provider!(Fallback => [Color, ShowLabels]);
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ impl VisualizerSystem for CamerasVisualizer {
self
}

fn as_fallback_provider(&self) -> &dyn re_viewer_context::ComponentFallbackProvider {
fn fallback_provider(&self) -> &dyn re_viewer_context::ComponentFallbackProvider {
self
}
}
Expand Down
Loading

0 comments on commit 6976742

Please sign in to comment.