Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved tracking of which space views were generated by a heuristic #5419

Merged
merged 11 commits into from
Mar 8, 2024

Conversation

Wumpf
Copy link
Member

@Wumpf Wumpf commented Mar 7, 2024

What

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

  • 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):
  • 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!

@Wumpf Wumpf added 📺 re_viewer affects re_viewer itself include in changelog labels Mar 7, 2024
@Wumpf Wumpf marked this pull request as draft March 7, 2024 13:54
@Wumpf
Copy link
Member Author

Wumpf commented Mar 7, 2024

drafting it: testing it again there seems to be something wrong

@Wumpf Wumpf marked this pull request as ready for review March 7, 2024 14:36
@Wumpf
Copy link
Member Author

Wumpf commented Mar 7, 2024

huh, that was a bit tough to get my head around what was going on, but figured it out!

Copy link
Member

@jleibs jleibs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome. Glad this worked out.

Comment on lines 45 to 51
/// 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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should split internal implementation details like this out into a separate unlisted archetype (maybe another sub-archetype?) -- there's no reason for a user to be aware of this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could add a hide attribute, but honestly I think it's kinda nice to be aware of this 🤔
Like now we have for the first time ever a way to document the viewer's behavior and in particular how it interacts with whatever you do

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For an advanced user looking at code, sure, they can look at the ViewportBlueprintInternalState archetype.

For a non-advanced user just trying to understand the blueprint API, this just makes things more confusing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not convinced, but I'll create an issue for it since I don't want to solve this on this PR

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Wumpf Wumpf merged commit e66438b into main Mar 8, 2024
39 checks passed
@Wumpf Wumpf deleted the andreas/improve-heuristic-tracking branch March 8, 2024 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Removing a data result sometimes duplicates the space view
2 participants