-
Notifications
You must be signed in to change notification settings - Fork 317
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
Add ShowLabels
component, which controls whether instances’ labels are shown.
#7249
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
emilk
approved these changes
Aug 23, 2024
Works like a charm! |
6 tasks
jleibs
pushed a commit
that referenced
this pull request
Aug 28, 2024
…#7266) ### What * Followup to #7249 * Fixes #3465 The default value for the `ShowLabels` component now passes through `ComponentFallbackProvider`, so the viewer UI will properly show the fallback value derived from the number of instances. Further possible work in this area would be to define “number of instances” as something that can be generically fetched based on an archetype, rather than requiring each visualizer to communicate how it counts that. ### 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/7266?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/7266?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)! * [X] If have noted any breaking changes to the log API in `CHANGELOG.md` and the migration guide - [PR Build Summary](https://build.rerun.io/pr/7266) - [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`.
jleibs
pushed a commit
that referenced
this pull request
Aug 28, 2024
…#7266) ### What * Followup to #7249 * Fixes #3465 The default value for the `ShowLabels` component now passes through `ComponentFallbackProvider`, so the viewer UI will properly show the fallback value derived from the number of instances. Further possible work in this area would be to define “number of instances” as something that can be generically fetched based on an archetype, rather than requiring each visualizer to communicate how it counts that. ### 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/7266?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/7266?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)! * [X] If have noted any breaking changes to the log API in `CHANGELOG.md` and the migration guide - [PR Build Summary](https://build.rerun.io/pr/7266) - [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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
ShowLabels
non-repeated component. (I figured having it set per-instance would be hardly ever useful.)process_labels()
.process_labels()
uses it to override the existing “show labels if less than 30” policy.process_labels()
and friends now take a struct instead of 8 separate parameters. This makes the callsites more legible and maintainable, and might eventually be useful for further refinement of the label rendering system (it's basically a "ComponentData" struct for labels).One element is currently missing: the automatic policy is not expressed as a component fallback. That means that the viewer UI does not know about it and won't display the real fallback value, but a bogus constant
true
. Other than that, it should be fully ready to go, so feel free to either merge or wait for that further work as you see fit.Checklist
main
build: rerun.io/viewernightly
build: rerun.io/viewerCHANGELOG.md
and the migration guideTo run all checks from
main
, comment on the PR with@rerun-bot full-check
.