Chunkified, deserialization-free Point Cloud visualizers #7011

teh-cmc · 2024-07-29T15:34:35Z

Title!

Because this is the first spatial view we've ever chunkified, this includes a new entity_iterator etc.
The old ones will be removed once everything has been ported.

rerun/crates/viewer/re_space_view_spatial/src/visualizers/points3d.rs

Lines 202 to 271 in 35b5990

    
           super::entity_iterator::process_archetype2::<Self, Points3D, _>( 
        
               ctx, 
        
               view_query, 
        
               context_systems, 
        
               |ctx, spatial_ctx, results| { 
        
                   use re_space_view::RangeResultsExt2 as _; 
        
                   let Some(all_position_chunks) = results.get_required_chunks(&Position3D::name()) 
        
                   else { 
        
                       return Ok(()); 
        
                   }; 
        
                   let num_positions = all_position_chunks 
        
                       .iter() 
        
                       .flat_map(|chunk| chunk.iter_primitive_array::<3, f32>(&Position3D::name())) 
        
                       .map(|points| points.len()) 
        
                       .sum(); 
        
                   if num_positions == 0 { 
        
                       return Ok(()); 
        
                   } 
        
                   point_builder.reserve(num_positions)?; 
        
                   let timeline = ctx.query.timeline(); 
        
                   let all_positions_indexed = all_position_chunks.iter().flat_map(|chunk| { 
        
                       itertools::izip!( 
        
                           chunk.iter_component_indices(&timeline, &Position3D::name()), 
        
                           chunk.iter_primitive_array::<3, f32>(&Position3D::name()) 
        
                       ) 
        
                   }); 
        
                   let all_colors = results.iter_as(timeline, Color::name()); 
        
                   let all_radii = results.iter_as(timeline, Radius::name()); 
        
                   let all_labels = results.iter_as(timeline, Text::name()); 
        
                   let all_class_ids = results.iter_as(timeline, ClassId::name()); 
        
                   let all_keypoint_ids = results.iter_as(timeline, KeypointId::name()); 
        
                   let data = re_query2::range_zip_1x5( 
        
                       all_positions_indexed, 
        
                       all_colors.primitive::<u32>(), 
        
                       all_radii.primitive::<f32>(), 
        
                       all_labels.string(), 
        
                       all_class_ids.primitive::<u16>(), 
        
                       all_keypoint_ids.primitive::<u16>(), 
        
                   ) 
        
                   .map( 
        
                       |(_index, positions, colors, radii, labels, class_ids, keypoint_ids)| { 
        
                           Points3DComponentData { 
        
                               positions: bytemuck::cast_slice(positions), 
        
                               colors: colors.map_or(&[], |colors| bytemuck::cast_slice(colors)), 
        
                               radii: radii.map_or(&[], |radii| bytemuck::cast_slice(radii)), 
        
                               labels: labels.unwrap_or_default(), 
        
                               class_ids: class_ids 
        
                                   .map_or(&[], |class_ids| bytemuck::cast_slice(class_ids)), 
        
                               keypoint_ids: keypoint_ids 
        
                                   .map_or(&[], |keypoint_ids| bytemuck::cast_slice(keypoint_ids)), 
        
                           } 
        
                       }, 
        
                   ); 
        
                   self.process_data( 
        
                       ctx, 
        
                       &mut point_builder, 
        
                       &mut line_builder, 
        
                       view_query, 
        
                       spatial_ctx, 
        
                       data, 
        
                   ) 
        
               }, 
        
           )?;

Checklist

I have read and agree to Contributor Guide and the Code of Conduct
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
- Using full set of examples from nightly build: rerun.io/viewer
The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG
If applicable, add a new check to the release checklist!
If have noted any breaking changes to the log API in CHANGELOG.md and the migration guide

To run all checks from main, comment on the PR with @rerun-bot full-check.

Wumpf

some concerns on usability/how easy it is to mess up. Otherwise looking great.

iter_primitive_arrays <3. Ages ago I whined "why aren't we just reinterpret casting our float arrays T_T" and now we finally have it. What a time to be alive!

Wumpf · 2024-07-30T12:50:19Z

crates/store/re_chunk/src/iter.rs

+        Either::Right(
+            self.iter_component_offsets(component_name)
+                .map(move |(idx, len)| {
+                    bytemuck::cast_slice(&values[idx * size..idx * size + len * size])


oh, the doc wasn't joking, we're talking ptr cast fast here :O

should we be nervous though about alignment issues?
I figure right now we don't have any special alignment requirements on any of our types and I'm not sure its possible to have that and still implement arrow2::types::NativeType 🤔

should we be nervous though about alignment issues?

I wouldn't be: bytemuck::cast_slice panics on misalignment, which can only happen if someone has just modified this view's code and/or the arrow representation of points, at which point I sure hope said someone will try the view to see if it still works (and would insta panic).

Wumpf · 2024-07-30T12:57:14Z

crates/viewer/re_space_view_spatial/src/visualizers/utilities/entity_iterator.rs

+        let world_from_entity =
+            if view_ctx.space_view_class_identifier() == SpatialSpaceView3D::identifier() {
+                transforms.reference_from_entity(&data_result.entity_path)
+            } else {
+                transforms.reference_from_entity_ignoring_pinhole(
+                    &data_result.entity_path,
+                    ctx.recording(),
+                    &latest_at,
+                )
+            };


(fyi I'm break this on another PR, merge should be trivial thougyh)

Wumpf · 2024-07-30T13:12:09Z

crates/viewer/re_space_view_spatial/src/visualizers/points2d.rs

-                    .sum::<usize>();
+                let num_positions = all_position_chunks
+                    .iter()
+                    .flat_map(|chunk| chunk.iter_primitive_array::<2, f32>(&Position2D::name()))


it's a bit unfortunate how easy it is to get this wrong here: We first specify Position2D's name, then have to know this is two floats and then cast it to Position2D. If any of these steps are wrong, we get into a weird state.

Same remark as above: you don't really get into a weird state most of the time, you just insta panic. And again you have to be actively modifying this code etc.

Wumpf · 2024-07-30T13:14:12Z

crates/viewer/re_space_view_spatial/src/visualizers/points2d.rs

+                let all_positions_indexed = all_position_chunks.iter().flat_map(|chunk| {
+                    itertools::izip!(
+                        chunk.iter_component_indices(&timeline, &Position2D::name()),
+                        chunk.iter_primitive_array::<2, f32>(&Position2D::name())
+                    )
+                });


it would be nice to have a similar utility like iter_as here to make this shorter

😭 I used to have them, but it seemed really over the top to maintain yet another set of helpers just for this part.

I'll try it once again and see how it feels.

Brought them back! 🙇

Title! This introduces new neat zero-deser zero-copy helper for generic arrow buffers. - DNM: requires #7011 --- #### Mesh3D https://github.com/rerun-io/rerun/blob/c070efbee3713bf6459190aa5ace203a5a8b3678/crates/viewer/re_space_view_spatial/src/visualizers/meshes.rs#L179-L277 ![image](https://github.com/user-attachments/assets/12b42364-1681-47b0-9804-db215e099aee) #### Asset3D https://github.com/rerun-io/rerun/blob/c070efbee3713bf6459190aa5ace203a5a8b3678/crates/viewer/re_space_view_spatial/src/visualizers/assets3d.rs#L149-L206 ![image](https://github.com/user-attachments/assets/801ddc93-c114-46f0-a94f-f73ca6ca51c6)

teh-cmc added the do-not-merge Do not merge this PR label Jul 29, 2024

teh-cmc force-pushed the cmc/chunkified_queries_9_store_configs_in_my_stash branch from af25ad5 to ee0abec Compare July 29, 2024 16:10

Base automatically changed from cmc/chunkified_queries_9_store_configs_in_my_stash to main July 29, 2024 16:10

teh-cmc force-pushed the cmc/chunkified_queries_10_spatial branch 3 times, most recently from 97bf8e9 to 35b5990 Compare July 30, 2024 09:07

teh-cmc added 📺 re_viewer affects re_viewer itself 🔍 re_query affects re_query itself 🍏 primitives Relating to Rerun primitives include in changelog 🚀 performance Optimization, memory use, etc and removed do-not-merge Do not merge this PR labels Jul 30, 2024

teh-cmc marked this pull request as ready for review July 30, 2024 09:13

teh-cmc mentioned this pull request Jul 30, 2024

Chunkified, (almost)deserialization-free Mesh/Asset visualizers #7016

Merged

6 tasks

Wumpf self-requested a review July 30, 2024 12:47

teh-cmc force-pushed the cmc/chunkified_queries_10_spatial branch from a52cea0 to afdb95c Compare July 30, 2024 13:03

Wumpf approved these changes Jul 30, 2024

View reviewed changes

teh-cmc added 8 commits July 30, 2024 16:38

introduce new Chunk zero-copy zero-deser iterators

d78b744

introduce new HybridResults zero-deser helpers

0852b4b

introduce new chunk-based entity_iterator

a8404bb

zero-deser point2d visualizer

a5004e1

zero-deser point3d visualizer

d1ebc4a

lints

76244c7

unrelated fix to chunkified override APIs

0cced74

review

0e29c96

teh-cmc force-pushed the cmc/chunkified_queries_10_spatial branch from afdb95c to 0e29c96 Compare July 30, 2024 14:49

teh-cmc merged commit 9056ec9 into main Jul 30, 2024
27 of 28 checks passed

teh-cmc deleted the cmc/chunkified_queries_10_spatial branch July 30, 2024 14:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Chunkified, deserialization-free Point Cloud visualizers #7011

Chunkified, deserialization-free Point Cloud visualizers #7011

teh-cmc commented Jul 29, 2024 •

edited by github-actions bot

Loading

Wumpf left a comment

Wumpf Jul 30, 2024

Wumpf Jul 30, 2024

teh-cmc Jul 30, 2024

Wumpf Jul 30, 2024

Wumpf Jul 30, 2024

teh-cmc Jul 30, 2024

Wumpf Jul 30, 2024

teh-cmc Jul 30, 2024

teh-cmc Jul 30, 2024

	super::entity_iterator::process_archetype2::<Self, Points3D, _>(
	ctx,
	view_query,
	context_systems,
	\|ctx, spatial_ctx, results\| {
	use re_space_view::RangeResultsExt2 as _;

	let Some(all_position_chunks) = results.get_required_chunks(&Position3D::name())
	else {
	return Ok(());
	};

	let num_positions = all_position_chunks
	.iter()
	.flat_map(\|chunk\| chunk.iter_primitive_array::<3, f32>(&Position3D::name()))
	.map(\|points\| points.len())
	.sum();

	if num_positions == 0 {
	return Ok(());
	}

	point_builder.reserve(num_positions)?;

	let timeline = ctx.query.timeline();
	let all_positions_indexed = all_position_chunks.iter().flat_map(\|chunk\| {
	itertools::izip!(
	chunk.iter_component_indices(&timeline, &Position3D::name()),
	chunk.iter_primitive_array::<3, f32>(&Position3D::name())
	)
	});
	let all_colors = results.iter_as(timeline, Color::name());
	let all_radii = results.iter_as(timeline, Radius::name());
	let all_labels = results.iter_as(timeline, Text::name());
	let all_class_ids = results.iter_as(timeline, ClassId::name());
	let all_keypoint_ids = results.iter_as(timeline, KeypointId::name());

	let data = re_query2::range_zip_1x5(
	all_positions_indexed,
	all_colors.primitive::<u32>(),
	all_radii.primitive::<f32>(),
	all_labels.string(),
	all_class_ids.primitive::<u16>(),
	all_keypoint_ids.primitive::<u16>(),
	)
	.map(
	\|(_index, positions, colors, radii, labels, class_ids, keypoint_ids)\| {
	Points3DComponentData {
	positions: bytemuck::cast_slice(positions),
	colors: colors.map_or(&[], \|colors\| bytemuck::cast_slice(colors)),
	radii: radii.map_or(&[], \|radii\| bytemuck::cast_slice(radii)),
	labels: labels.unwrap_or_default(),
	class_ids: class_ids
	.map_or(&[], \|class_ids\| bytemuck::cast_slice(class_ids)),
	keypoint_ids: keypoint_ids
	.map_or(&[], \|keypoint_ids\| bytemuck::cast_slice(keypoint_ids)),
	}
	},
	);

	self.process_data(
	ctx,
	&mut point_builder,
	&mut line_builder,
	view_query,
	spatial_ctx,
	data,
	)
	},
	)?;

Chunkified, deserialization-free Point Cloud visualizers #7011

Chunkified, deserialization-free Point Cloud visualizers #7011

Conversation

teh-cmc commented Jul 29, 2024 • edited by github-actions bot Loading

Checklist

Wumpf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

teh-cmc commented Jul 29, 2024 •

edited by github-actions bot

Loading