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

Re-visit "child-of-root" space view spawning heuristics #4926

Closed
Wumpf opened this issue Jan 28, 2024 · 0 comments · Fixed by #5188
Closed

Re-visit "child-of-root" space view spawning heuristics #4926

Wumpf opened this issue Jan 28, 2024 · 0 comments · Fixed by #5188
Labels
😤 annoying Something in the UI / SDK is annoying to use 💬 discussion

Comments

@Wumpf
Copy link
Member

Wumpf commented Jan 28, 2024

Largely a follow-up of:

There's a bunch of cases where we treat child entities of the root special for the purpose of spawning space views:

  • 3D space views
  • time series space views
    • Spawns space views iff there's several child of root which don't have timeseries indicators themselves but have children that do
    • See plots demo why this is needed: We expect separate plots for each child of the root node. Each plot in turn contains several entities

This is problematic since it's a fairly complicated heuristic both for a user to understand and for our implementation to maintain!
On the flip side it allows to easily crate many space views without user ui interaction & logging anything extra!

Mitigation / strategies for removal:

  • Spawn a 3D space view wherever there's ViewCoordinate, don't spawn in the "remaining space" if this wouldn't add any more entities
  • Plots seems harder: We could just accept a single plot or a plot for each entity (this breaks the plots demo as-is)
  • ... TODO: would the above be enough?
@Wumpf Wumpf added 💬 discussion 😤 annoying Something in the UI / SDK is annoying to use labels Jan 28, 2024
@Wumpf Wumpf mentioned this issue Feb 7, 2024
5 tasks
Wumpf added a commit that referenced this issue Feb 7, 2024
### What

* partially solves #4926
* fixes new issues where we get two spaces views in our 2d code examples
for lines

Removes "child of root" heuristics for 2D space views and improves image
grouping behavior a bit.
Had to do slight adjustments to object tracking demo so that it still
looks good:


![image](https://github.com/rerun-io/rerun/assets/1220815/5b0987f7-2c3a-4f7e-b954-087e85a1a548)

But imho the adjusted hierarchy also makes a lot of sense. Soon user can
configure this from blueprint, we can then think about removing
non-explicit bucketing. See
* #3985


### 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/5087/index.html)
* Using examples from latest `main` build:
[app.rerun.io](https://app.rerun.io/pr/5087/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/5087/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](tests/python/release_checklist)!

- [PR Build Summary](https://build.rerun.io/pr/5087)
- [Docs
preview](https://rerun.io/preview/4040710e64651d60a9b1f1afacfa899be19e11cc/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/4040710e64651d60a9b1f1afacfa899be19e11cc/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
Wumpf added a commit that referenced this issue Feb 16, 2024
…d space view generation heuristics (#5188)

### What

* Fixes #4926
* it does not address child-of-root heuristics for time series, but they
weren't as much of a concern
* Fixes heuristic issues observed in
rerun-io/cpp-example-opencv-eigen#20

Two changes in here:
* we no longer determine the dimensionality of a topological subspace,
instead we only determine whether it can't display 2d/3d
   * 2d: we now state we can _always_ display 2d objects
* 3d: we can display it unless the parent of the active subspace is a
pinhole in which case we only make the entity _at_ the origin as
visualizable
* make `ViewCoordinates` add "heuristic hints" to spatial topology
* this is all that's needed to get rid of child-of-root heuristics in 3d
without regressing existing examples
   
OpenCV example that didn't work before:

![image](https://github.com/rerun-io/rerun/assets/1220815/571ad089-98cd-4295-ba5e-3b81c39c93c2)

New release check:

![image](https://github.com/rerun-io/rerun/assets/1220815/d4d577e1-7fc1-41e5-90b4-6764c9db2f0d)



### 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/5188/index.html)
* Using examples from latest `main` build:
[app.rerun.io](https://app.rerun.io/pr/5188/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/5188/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/5188)
- [Docs
preview](https://rerun.io/preview/cc3735c02cd092df5843f7fe97bfa998db2e459d/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/cc3735c02cd092df5843f7fe97bfa998db2e459d/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
Wumpf added a commit that referenced this issue Feb 21, 2024
…d space view generation heuristics (#5188)

### What

* Fixes #4926
* it does not address child-of-root heuristics for time series, but they
weren't as much of a concern
* Fixes heuristic issues observed in
rerun-io/cpp-example-opencv-eigen#20

Two changes in here:
* we no longer determine the dimensionality of a topological subspace,
instead we only determine whether it can't display 2d/3d
   * 2d: we now state we can _always_ display 2d objects
* 3d: we can display it unless the parent of the active subspace is a
pinhole in which case we only make the entity _at_ the origin as
visualizable
* make `ViewCoordinates` add "heuristic hints" to spatial topology
* this is all that's needed to get rid of child-of-root heuristics in 3d
without regressing existing examples
   
OpenCV example that didn't work before:

![image](https://github.com/rerun-io/rerun/assets/1220815/571ad089-98cd-4295-ba5e-3b81c39c93c2)

New release check:

![image](https://github.com/rerun-io/rerun/assets/1220815/d4d577e1-7fc1-41e5-90b4-6764c9db2f0d)



### 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/5188/index.html)
* Using examples from latest `main` build:
[app.rerun.io](https://app.rerun.io/pr/5188/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/5188/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/5188)
- [Docs
preview](https://rerun.io/preview/cc3735c02cd092df5843f7fe97bfa998db2e459d/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/cc3735c02cd092df5843f7fe97bfa998db2e459d/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
😤 annoying Something in the UI / SDK is annoying to use 💬 discussion
Projects
None yet
1 participant