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 new archetype for Axes3D #6510

Merged
merged 29 commits into from
Jun 6, 2024
Merged

Introduce new archetype for Axes3D #6510

merged 29 commits into from
Jun 6, 2024

Conversation

jleibs
Copy link
Member

@jleibs jleibs commented Jun 6, 2024

What

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

TODO:

  • [] Still need to implement the example in rust / cpp.

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 added 📺 re_viewer affects re_viewer itself 🟦 blueprint The data that defines our UI include in changelog 🪵 Log & send APIs Affects the user-facing API for all languages do-not-merge Do not merge this PR labels Jun 6, 2024
Copy link

github-actions bot commented Jun 6, 2024

Deployed docs

Commit Link
62c354a https://landing-aid4jaaji-rerun.vercel.app/docs

@jleibs jleibs marked this pull request as ready for review June 6, 2024 01:19
@abey79
Copy link
Member

abey79 commented Jun 6, 2024

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.

Again, lots of this is way over my head, but lets not block that on Andreas being back to office :)

Curious about what you think is needed to make the switch to enabling override_ui for this space view (instead of using the feature flag) and remove the legacy UI entirely?

On that note, II wasn't able to add a AxisLength component override in the UI, other than by manipulating the legacy UI. Is that expected?

Base automatically changed from jleibs/image_plane_component to main June 6, 2024 17:58
@jleibs jleibs removed the do-not-merge Do not merge this PR label Jun 6, 2024
@jleibs jleibs merged commit 5a10357 into main Jun 6, 2024
40 checks passed
@jleibs jleibs deleted the jleibs/transform_axis branch June 6, 2024 21:00
@emilk
Copy link
Member

emilk commented Jun 10, 2024

This PR seems to have broken CI: https://github.com/rerun-io/rerun/actions/runs/9436507239/job/25991550784

In particular, pixi run -e wheel-test RUST_LOG=debug python tests/roundtrips.py --release --target aarch64-unknown-linux-gnu --no-py-build fails with make: *** No rule to make target 'roundtrip_axes3d'.

It seems we don't run the roundtrip tests on each PR (understandable), but that means we miss adding roundtrip tests when we need them. We should have a fast CI job that only checks that we have roundtrip tests, without running them.

Fix coming in #6524

emilk added a commit that referenced this pull request Jun 10, 2024
`main` CI is failing:
https://github.com/rerun-io/rerun/actions/runs/9436507239/job/25991550784

#6510 added a new archetype
(`Axes3D`) without adding roundtrip tests for it. This was easily missed
because the PR CI doesn't run the roundtrip tests.

This PR adds a fast PR CI check to make sure we have roundtrip tests for
all archetype, but without paying the cost of actually running them. The
failure looks like this:


![image](https://github.com/rerun-io/rerun/assets/1148717/61bf60b6-4a88-4f13-be65-917741d7aff8)

This PR also adds an exception for the `Axes3D` archetype. I've created
a separate issue for that, adding it to this cycle:
* #6525

---

* [x] I love checkboxes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🟦 blueprint The data that defines our UI 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.

3 participants