Skip to content

Commit

Permalink
Improved tracking of which space views were generated by a heuristic (#…
Browse files Browse the repository at this point in the history
…5419)

### What

* Fixes #5404

We now track haches of all "recommended" space views that we spawned so
far. Seems to work much more reliably so far than anything else we had
so far.
Natrually, I kept the "is this is a redundant space view" logic since
it's still useful to have around both to figure duplicates within a set
of recommendation and to figure out if user created space views cover
what the heuristic suggests.


### 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 newly built examples:
[app.rerun.io](https://app.rerun.io/pr/5419/index.html)
* Using examples from latest `main` build:
[app.rerun.io](https://app.rerun.io/pr/5419/index.html?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[app.rerun.io](https://app.rerun.io/pr/5419/index.html?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/5419)
- [Docs
preview](https://rerun.io/preview/bd21f004abd144d1e57e7f12a7dd9f0bbfc57bf5/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/bd21f004abd144d1e57e7f12a7dd9f0bbfc57bf5/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
  • Loading branch information
Wumpf authored Mar 8, 2024
1 parent 4804b66 commit e66438b
Show file tree
Hide file tree
Showing 54 changed files with 1,059 additions and 551 deletions.
12 changes: 12 additions & 0 deletions crates/re_log_types/src/path/entity_path_filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,18 @@ pub struct EntityPathFilter {
rules: BTreeMap<EntityPathRule, RuleEffect>,
}

impl std::hash::Hash for EntityPathFilter {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
self.formatted().hash(state);
}
}

impl std::fmt::Debug for EntityPathFilter {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "EntityPathFilter({:?})", self.formatted())
}
}

#[derive(Clone, Debug, PartialEq, Eq)]
pub struct EntityPathRule {
pub path: EntityPath,
Expand Down
25 changes: 3 additions & 22 deletions crates/re_space_view/src/space_view_contents.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,7 @@ use re_entity_db::{
};
use re_log_types::{path::RuleEffect, EntityPath, EntityPathFilter, EntityPathRule, StoreKind};
use re_types::{
blueprint::{
archetypes as blueprint_archetypes,
components::{EntitiesDeterminedByUser, QueryExpression},
},
blueprint::{archetypes as blueprint_archetypes, components::QueryExpression},
Archetype as _,
};
use re_types_core::{components::VisualizerOverrides, ComponentName};
Expand Down Expand Up @@ -42,9 +39,6 @@ pub struct SpaceViewContents {

pub space_view_class_identifier: SpaceViewClassIdentifier,
pub entity_path_filter: EntityPathFilter,

/// True if the user is expected to add entities themselves. False otherwise.
pub entities_determined_by_user: bool,
}

impl SpaceViewContents {
Expand Down Expand Up @@ -94,7 +88,6 @@ impl SpaceViewContents {
blueprint_entity_path,
space_view_class_identifier,
entity_path_filter,
entities_determined_by_user: false,
}
}

Expand All @@ -109,10 +102,7 @@ impl SpaceViewContents {
blueprint_archetypes::SpaceViewContents,
>(id, blueprint_db, query);

let blueprint_archetypes::SpaceViewContents {
query,
entities_determined_by_user,
} = contents.unwrap_or_else(|err| {
let blueprint_archetypes::SpaceViewContents { query } = contents.unwrap_or_else(|err| {
re_log::warn_once!(
"Failed to load SpaceViewContents for {:?} from blueprint store at {:?}: {}",
id,
Expand All @@ -128,7 +118,6 @@ impl SpaceViewContents {
blueprint_entity_path,
space_view_class_identifier,
entity_path_filter,
entities_determined_by_user: entities_determined_by_user.map_or(false, |b| b.0),
}
}

Expand All @@ -141,8 +130,7 @@ impl SpaceViewContents {
pub fn save_to_blueprint_store(&self, ctx: &ViewerContext<'_>) {
ctx.save_blueprint_archetype(
self.blueprint_entity_path.clone(),
&blueprint_archetypes::SpaceViewContents::new(self.entity_path_filter.formatted())
.with_entities_determined_by_user(self.entities_determined_by_user),
&blueprint_archetypes::SpaceViewContents::new(self.entity_path_filter.formatted()),
);
}

Expand All @@ -159,13 +147,6 @@ impl SpaceViewContents {
&self.blueprint_entity_path,
&QueryExpression(new_entity_path_filter.formatted().into()),
);

if !self.entities_determined_by_user {
ctx.save_blueprint_component(
&self.blueprint_entity_path,
&EntitiesDeterminedByUser(true),
);
}
}

pub fn build_resolver<'a>(
Expand Down
6 changes: 3 additions & 3 deletions crates/re_types/definitions/rerun/blueprint.fbs
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,21 @@ include "./blueprint/components/auto_layout.fbs";
include "./blueprint/components/auto_space_views.fbs";
include "./blueprint/components/column_share.fbs";
include "./blueprint/components/container_kind.fbs";
include "./blueprint/components/entities_determined_by_user.fbs";
include "./blueprint/components/corner_2d.fbs";
include "./blueprint/components/entity_properties_component.fbs";
include "./blueprint/components/grid_columns.fbs";
include "./blueprint/components/included_content.fbs";
include "./blueprint/components/included_space_view.fbs";
include "./blueprint/components/lock_range_during_zoom.fbs";
include "./blueprint/components/panel_view.fbs";
include "./blueprint/components/query_expression.fbs";
include "./blueprint/components/root_container.fbs";
include "./blueprint/components/row_share.fbs";
include "./blueprint/components/space_view_class.fbs";
include "./blueprint/components/space_view_maximized.fbs";
include "./blueprint/components/space_view_origin.fbs";
include "./blueprint/components/viewer_recommendation_hash.fbs";
include "./blueprint/components/visible.fbs";
include "./blueprint/components/corner_2d.fbs";
include "./blueprint/components/lock_range_during_zoom.fbs";

include "./blueprint/archetypes/container_blueprint.fbs";
include "./blueprint/archetypes/space_view_blueprint.fbs";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,4 @@ table SpaceViewContents (
///
/// They determine which entities are part of the spaceview.
query: rerun.blueprint.components.QueryExpression ("attr.rerun.component_required", order: 1000);

// --- Optional ---

/// True if the user is has added entities themselves. False otherwise.
///
/// This is used by the viewer to determine whether it should regard this space view as created by the heuristic or not.
entities_determined_by_user: rerun.blueprint.components.EntitiesDeterminedByUser ("attr.rerun.component_optional", nullable, order: 2000);
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,27 @@ table ViewportBlueprint (
/// Show one tab as maximized?
maximized: rerun.blueprint.components.SpaceViewMaximized ("attr.rerun.component_optional", nullable, order: 3000);

// TODO(andreas): This is to be removed in the future, all new space views without an explicit container
// TODO(andreas): This is to be removed in the future, all new Space Views without an explicit container
// should always insert themselves using a heuristic.
/// Whether the viewport layout is determined automatically.
///
/// If `true`, the container layout will be reset whenever a new space view is added or removed.
/// This defaults to `false` and is automatically set to `false` when there is user determined layout.
auto_layout: rerun.blueprint.components.AutoLayout ("attr.rerun.component_optional", nullable, order: 4000);

// TODO(jleibs): This should come with an optional container id that specifies where to insert new space views.
/// Whether or not space views should be created automatically.
// TODO(jleibs): This should come with an optional container id that specifies where to insert new Space Views.
/// Whether or not Space Views should be created automatically.
///
/// If `true`, the viewer will only add Space Views that it hasn't considered previously (as identified by `past_viewer_recommendations`)
/// and which aren't deemed redundant to existing Space Views.
/// This defaults to `false` and is automatically set to `false` when the user adds Space Views manually in the viewer.
auto_space_views: rerun.blueprint.components.AutoSpaceViews ("attr.rerun.component_optional", nullable, order: 5000);

/// Hashes of all recommended Space Views the viewer has already added and that should not be added again.
///
/// This is an internal field and should not be set usually.
/// If you want the viewer from stopping to add Space Views, you should set `auto_space_views` to `false`.
///
/// The viewer uses this to determine whether it should keep adding Space Views.
past_viewer_recommendations: [rerun.blueprint.components.ViewerRecommendationHash] ("attr.rerun.component_optional", nullable, order: 6000);
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
include "arrow/attributes.fbs";
include "python/attributes.fbs";
include "rust/attributes.fbs";

include "rerun/datatypes.fbs";
include "rerun/attributes.fbs";

namespace rerun.blueprint.components;

// ---

/// Hash of a viewer recommendation.
///
/// The formation of this hash is considered an internal implementation detail of the viewer.
table ViewerRecommendationHash (
"attr.arrow.transparent",
"attr.rerun.scope": "blueprint",
"attr.python.aliases": "str",
"attr.rust.derive": "PartialEq, Eq, PartialOrd, Ord",
"attr.rust.repr": "transparent"
) {
value: rerun.datatypes.UInt64 (order: 100);
}
1 change: 1 addition & 0 deletions crates/re_types/definitions/rerun/datatypes.fbs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ include "./datatypes/transform3d.fbs";
include "./datatypes/translation_and_mat3x3.fbs";
include "./datatypes/translation_rotation_scale3d.fbs";
include "./datatypes/uint32.fbs";
include "./datatypes/uint64.fbs";
include "./datatypes/utf8.fbs";
include "./datatypes/uuid.fbs";
include "./datatypes/uvec2d.fbs";
Expand Down
2 changes: 2 additions & 0 deletions crates/re_types/definitions/rerun/datatypes/uint32.fbs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ namespace rerun.datatypes;
/// A 32bit unsigned integer.
struct UInt32 (
"attr.arrow.transparent",
"attr.python.aliases": "int",
"attr.python.array_aliases": "int, npt.NDArray[np.uint32]",
"attr.rust.derive": "Copy, PartialEq, Eq, PartialOrd, Ord",
"attr.rust.override_crate": "re_types_core",
"attr.rust.tuple_struct"
Expand Down
20 changes: 20 additions & 0 deletions crates/re_types/definitions/rerun/datatypes/uint64.fbs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
include "arrow/attributes.fbs";
include "python/attributes.fbs";
include "fbs/attributes.fbs";
include "rust/attributes.fbs";
include "docs/attributes.fbs";

namespace rerun.datatypes;

/// A 64bit unsigned integer.
struct UInt64 (
"attr.arrow.transparent",
"attr.python.aliases": "int",
"attr.python.array_aliases": "int, npt.NDArray[np.uint64]",
"attr.rust.derive": "Copy, PartialEq, Eq, PartialOrd, Ord",
"attr.rust.override_crate": "re_types_core",
"attr.rust.tuple_struct",
"attr.docs.unreleased"
) {
value: uint64 (order: 100);
}
53 changes: 6 additions & 47 deletions crates/re_types/src/blueprint/archetypes/space_view_contents.rs

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

2 changes: 1 addition & 1 deletion crates/re_types/src/blueprint/components/.gitattributes

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

4 changes: 2 additions & 2 deletions crates/re_types/src/blueprint/components/mod.rs

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

Loading

0 comments on commit e66438b

Please sign in to comment.