Skip to content

Commit c2593b9

Browse files
jleibsteh-cmc
authored andcommitted
Resolve unexpected view-partitioning by only bucket images when creating a new 2d view (#4361)
### What - Resolves: #4284 Fixes two issues: - Image bucketing only makes sense for 2d spatial views - The bucket logic would previously trigger even when the aggregate view should have lost out according to the scoring, so we move this into the `should_spawn_new` code-path. ### 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 [app.rerun.io](https://app.rerun.io/pr/4361) (if applicable) * [x] The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG - [PR Build Summary](https://build.rerun.io/pr/4361) - [Docs preview](https://rerun.io/preview/c996f7d7a26f19ce3c281e25663d39ed8f8f4016/docs) <!--DOCS-PREVIEW--> - [Examples preview](https://rerun.io/preview/c996f7d7a26f19ce3c281e25663d39ed8f8f4016/examples) <!--EXAMPLES-PREVIEW--> - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
1 parent 6b3f61a commit c2593b9

File tree

1 file changed

+75
-70
lines changed

1 file changed

+75
-70
lines changed

crates/re_viewport/src/space_view_heuristics.rs

+75-70
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ fn is_spatial_class(class: &SpaceViewClassName) -> bool {
2727
class.as_str() == "3D" || class.as_str() == "2D"
2828
}
2929

30+
fn is_spatial_2d_class(class: &SpaceViewClassName) -> bool {
31+
class.as_str() == "2D"
32+
}
33+
3034
fn spawn_one_space_view_per_entity(class: &SpaceViewClassName) -> bool {
3135
// For tensors create one space view for each tensor (even though we're able to stack them in one view)
3236
// TODO(emilk): query the actual [`ViewPartSystem`] instead.
@@ -264,76 +268,6 @@ pub fn default_created_space_views(
264268
continue;
265269
}
266270

267-
// Spatial views with images get extra treatment as well.
268-
if is_spatial_class(candidate.class_name()) {
269-
#[derive(Hash, PartialEq, Eq)]
270-
enum ImageBucketing {
271-
BySize((u64, u64)),
272-
ExplicitDrawOrder,
273-
}
274-
275-
let mut images_by_bucket: HashMap<ImageBucketing, Vec<EntityPath>> = HashMap::default();
276-
277-
// For this we're only interested in the direct children.
278-
for entity_path in &candidate.contents.root_group().entities {
279-
if let Some(tensor) =
280-
store.query_latest_component::<TensorData>(entity_path, &query)
281-
{
282-
if let Some([height, width, _]) = tensor.image_height_width_channels() {
283-
if store
284-
.query_latest_component::<re_types::components::DrawOrder>(
285-
entity_path,
286-
&query,
287-
)
288-
.is_some()
289-
{
290-
// Put everything in the same bucket if it has a draw order.
291-
images_by_bucket
292-
.entry(ImageBucketing::ExplicitDrawOrder)
293-
.or_default()
294-
.push(entity_path.clone());
295-
} else {
296-
// Otherwise, distinguish buckets by image size.
297-
images_by_bucket
298-
.entry(ImageBucketing::BySize((height, width)))
299-
.or_default()
300-
.push(entity_path.clone());
301-
}
302-
}
303-
}
304-
}
305-
306-
if images_by_bucket.len() > 1 {
307-
// If all images end up in the same bucket, proceed as normal. Otherwise stack images as instructed.
308-
for bucket in images_by_bucket.keys() {
309-
// Ignore every image from another bucket. Keep all other entities.
310-
let images_of_different_size = images_by_bucket
311-
.iter()
312-
.filter_map(|(other_bucket, images)| {
313-
(bucket != other_bucket).then_some(images)
314-
})
315-
.flatten()
316-
.cloned()
317-
.collect::<IntSet<_>>();
318-
let entities = candidate
319-
.contents
320-
.entity_paths()
321-
.filter(|path| !images_of_different_size.contains(path))
322-
.cloned()
323-
.collect_vec();
324-
325-
let mut space_view = SpaceViewBlueprint::new(
326-
*candidate.class_name(),
327-
&candidate.space_origin,
328-
entities.iter(),
329-
);
330-
space_view.entities_determined_by_user = true; // Suppress auto adding of entities.
331-
space_views.push((space_view, AutoSpawnHeuristic::AlwaysSpawn));
332-
}
333-
continue;
334-
}
335-
}
336-
337271
// TODO(andreas): Interaction of [`AutoSpawnHeuristic`] with above hardcoded heuristics is a bit wonky.
338272

339273
// `AutoSpawnHeuristic::SpawnClassWithHighestScoreForRoot` means we're competing with other candidates for the same root.
@@ -367,6 +301,77 @@ pub fn default_created_space_views(
367301
}
368302

369303
if should_spawn_new {
304+
// 2D views with images get extra treatment as well.
305+
if is_spatial_2d_class(candidate.class_name()) {
306+
#[derive(Hash, PartialEq, Eq)]
307+
enum ImageBucketing {
308+
BySize((u64, u64)),
309+
ExplicitDrawOrder,
310+
}
311+
312+
let mut images_by_bucket: HashMap<ImageBucketing, Vec<EntityPath>> =
313+
HashMap::default();
314+
315+
// For this we're only interested in the direct children.
316+
for entity_path in &candidate.contents.root_group().entities {
317+
if let Some(tensor) =
318+
store.query_latest_component::<TensorData>(entity_path, &query)
319+
{
320+
if let Some([height, width, _]) = tensor.image_height_width_channels() {
321+
if store
322+
.query_latest_component::<re_types::components::DrawOrder>(
323+
entity_path,
324+
&query,
325+
)
326+
.is_some()
327+
{
328+
// Put everything in the same bucket if it has a draw order.
329+
images_by_bucket
330+
.entry(ImageBucketing::ExplicitDrawOrder)
331+
.or_default()
332+
.push(entity_path.clone());
333+
} else {
334+
// Otherwise, distinguish buckets by image size.
335+
images_by_bucket
336+
.entry(ImageBucketing::BySize((height, width)))
337+
.or_default()
338+
.push(entity_path.clone());
339+
}
340+
}
341+
}
342+
}
343+
344+
if images_by_bucket.len() > 1 {
345+
// If all images end up in the same bucket, proceed as normal. Otherwise stack images as instructed.
346+
for bucket in images_by_bucket.keys() {
347+
// Ignore every image from another bucket. Keep all other entities.
348+
let images_of_different_size = images_by_bucket
349+
.iter()
350+
.filter_map(|(other_bucket, images)| {
351+
(bucket != other_bucket).then_some(images)
352+
})
353+
.flatten()
354+
.cloned()
355+
.collect::<IntSet<_>>();
356+
let entities = candidate
357+
.contents
358+
.entity_paths()
359+
.filter(|path| !images_of_different_size.contains(path))
360+
.cloned()
361+
.collect_vec();
362+
363+
let mut space_view = SpaceViewBlueprint::new(
364+
*candidate.class_name(),
365+
&candidate.space_origin,
366+
entities.iter(),
367+
);
368+
space_view.entities_determined_by_user = true; // Suppress auto adding of entities.
369+
space_views.push((space_view, AutoSpawnHeuristic::AlwaysSpawn));
370+
}
371+
continue;
372+
}
373+
}
374+
370375
space_views.push((candidate, spawn_heuristic));
371376
}
372377
} else {

0 commit comments

Comments
 (0)