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

Remove Axes3D archetype and add axis_length to Transform3D #6676

Merged
merged 8 commits into from
Jun 28, 2024

Conversation

jleibs
Copy link
Member

@jleibs jleibs commented Jun 27, 2024

What

It turns out having a visualizer with no required entities causes all kinds of problems. This moves us back to only allowing axes if you have a transform at an entity.

The heuristics are still a bit fancy for backwards compatibility. We don't use Transform3D as an indicator since that would cause every transform to get axes by default. Instead, we only create the Transform3D visualizer if:

  • If we have no other visualizers, but otherwise meet the criteria for Transform3DArrows.
  • If someone set an axis_length explicitly, so AxisLengthDetector is applicable.
  • If we already have the CamerasVisualizer active.

Shortcomings

  • The one big shortcoming still present here is that there's still no way to use a default to force the Transform3D axes to be visible.

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.

@jleibs jleibs requested a review from Wumpf June 27, 2024 23:58
@jleibs jleibs added 🍏 primitives Relating to Rerun primitives 🪵 Log & send APIs Affects the user-facing API for all languages include in changelog labels Jun 27, 2024
@jleibs jleibs marked this pull request as ready for review June 27, 2024 23:58
Copy link

github-actions bot commented Jun 27, 2024

Deployed docs

Commit Link
1ea7320 https://landing-647srzau8-rerun.vercel.app/docs

Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

good work! Still messy and we're surely not done here, but a good step forward

another thing for the shortcoming list: override of axislength will not make the arrows show up either

@Wumpf Wumpf merged commit bdc1fd1 into main Jun 28, 2024
39 of 41 checks passed
@Wumpf Wumpf deleted the jleibs/axes_on_transforms branch June 28, 2024 09:28
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 🍏 primitives Relating to Rerun primitives
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Transform3DArrows causes non-spatial archetypes to have applicable visualizers in 3d spaces
2 participants