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

Introduce ImagePlaneDistance Component #6505

Merged
merged 20 commits into from
Jun 6, 2024
Merged

Conversation

jleibs
Copy link
Member

@jleibs jleibs commented Jun 5, 2024

What

This introduces a new ViewContext that holds onto the state so that we don't need to continuously pass it around everywhere. Additionally, introduces some new helpers to make querying with overrides and fallbacks easier.

We are accumulating more information in the HybridDataResults so that we can now find a fallback provider and resolve fallbacks.

Specifically this makes it possible to now use the DataResult to directly produce a HybridLatestAtResults, which in turn allows you to make calls like get_mono_with_fallback().

For better or for worse, this will eat all manner of promise/deser errors along the way and just get you a data value you can use.

    let results = data_result.latest_at_with_overrides::<re_types::archetypes::Pinhole>(ctx, query);

    let image_plane_distance = Some(results.get_mono_with_fallback());

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!

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

Copy link

github-actions bot commented Jun 5, 2024

Deployed docs

Commit Link
1f6c977 https://landing-ql9k5n9xc-rerun.vercel.app/docs

@Wumpf Wumpf force-pushed the andreas/remove-to-archetype branch from 733cff7 to cc8fc7d Compare June 5, 2024 13:45
@jleibs jleibs force-pushed the jleibs/image_plane_component branch from 585eebf to c1578dc Compare June 5, 2024 14:00
@Wumpf Wumpf force-pushed the andreas/remove-to-archetype branch from cc8fc7d to 7c938a6 Compare June 5, 2024 14:24
Base automatically changed from andreas/remove-to-archetype to main June 5, 2024 14:35
@jleibs jleibs changed the title Jleibs/image plane component Introduce ImagePlaneDistance Component Jun 5, 2024
@jleibs jleibs added 🐍 Python API Python logging API 📺 re_viewer affects re_viewer itself 🪵 Log & send APIs Affects the user-facing API for all languages include in changelog and removed 🐍 Python API Python logging API labels Jun 5, 2024
@jleibs jleibs force-pushed the jleibs/image_plane_component branch from c1578dc to 39cb672 Compare June 5, 2024 20:56
@jleibs jleibs marked this pull request as ready for review June 5, 2024 20:56
@jleibs jleibs mentioned this pull request Jun 6, 2024
5 tasks
Copy link
Member

@abey79 abey79 left a comment

Choose a reason for hiding this comment

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

Looks good to me, with the caveat that lots of details are still beyond my grasp.

Comment on lines +232 to +247
let visualizer_collection = ctx
.space_view_class_registry
.new_visualizer_collection(space_view.class_identifier());

let Some(view_state) = view_states
.get(*space_view_id)
.map(|states| states.view_state.as_ref())
else {
return;
};

let view_ctx = ViewContext {
viewer_ctx: ctx,
view_state,
visualizer_collection: &visualizer_collection,
};
Copy link
Member

Choose a reason for hiding this comment

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

A helper would be nice.

Suggested change
let visualizer_collection = ctx
.space_view_class_registry
.new_visualizer_collection(space_view.class_identifier());
let Some(view_state) = view_states
.get(*space_view_id)
.map(|states| states.view_state.as_ref())
else {
return;
};
let view_ctx = ViewContext {
viewer_ctx: ctx,
view_state,
visualizer_collection: &visualizer_collection,
};
if let Some(view_ctx) = ctx.as_view_context() else {
return;
};

edit: I saw elsewhere that you are constructing it slightly differently, so this may not make much sense

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm going to come back and do this one on top of the next PR in this stack once I know the different variants of how it needs to get built.

Copy link
Member

Choose a reason for hiding this comment

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

makes sense 👍🏻

@jleibs jleibs merged commit 0346905 into main Jun 6, 2024
19 checks passed
@jleibs jleibs deleted the jleibs/image_plane_component branch June 6, 2024 17:58
jleibs added a commit that referenced this pull request Jun 6, 2024
### What
- Builds on top of: #6505 (merge
that first and rebase)

This replaces the axes-related entity properties with a new Axes3D
archetype.

The archetype allows overriding the axis-length from code.
Controlling whether the axes are visible now happens via enabling of the
visualizer.

We use similar heuristics to before to enable axes3d visualizers on
transforms in certain situations.

The legacy UI has been remapped to the new indicator/heuristics
implementation.


![image](https://github.com/rerun-io/rerun/assets/3312232/e24bbe52-5470-41e1-ada6-79d0d8059cef)

TODO:
- [] Still need to implement the example in rust / cpp.

### 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 examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6510?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6510?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/6510)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
include in changelog 🪵 Log & send APIs Affects the user-facing API for all languages 📺 re_viewer affects re_viewer itself
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow specifying camera image plane distance as part of Pinhole archetype
2 participants