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.

Sign up for GitHub

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

Jump to bottom

Make whether labels are shown a configurable property #3465

Closed
jleibs opened this issue Sep 25, 2023 · 4 comments · Fixed by #7266
Closed

Make whether labels are shown a configurable property #3465

jleibs opened this issue Sep 25, 2023 · 4 comments · Fixed by #7266
Assignees
@kpreid
Labels
enhancement New feature or request 📺 re_viewer affects re_viewer itself user-request This is a pressing issue for one of our users

Comments

@jleibs
Copy link
Member

jleibs commented Sep 25, 2023 •
edited by Wumpf
Loading

We currently have hard-coded threshold of 30 labels for whether to show labels.

These values should just govern a threshold, but whether we show labels our not should be configurable via the UI.
@jleibs jleibs added enhancement New feature or request 📺 re_viewer affects re_viewer itself labels Sep 25, 2023
@Wumpf Wumpf mentioned this issue Jul 3, 2024
Merged
5 tasks
@Wumpf
Copy link
Member

Wumpf commented Jul 3, 2024

Related to

Wumpf added a commit that referenced this issue Jul 4, 2024
@Wumpf
Multiple instances of points/arrows/boxes with single label display l…
9137e49 
…abel now at the center (#6741)

### What

Previously, single labels would splat out, quickly hitting the "can't
display more labels" limit (described in #3465).
Now, there's a special meaning for a single labels: Instead of splatting
it across all instances, we place the single label at the center.

This is particularly useful with in-viewer overrides, as it allows to
set labels on objects with many instances.

![image](https://github.com/rerun-io/rerun/assets/1220815/a4e2462f-3fee-4cca-a4bd-d05cff263621)

A drawback of bounding box based placement is that the label moves with
the box. But I reckon it's a lot better than the alternative.

https://github.com/rerun-io/rerun/assets/1220815/866058a4-8edf-4ce7-9cd5-63f8b00e34a3

* Part of #6595

### 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/6741?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/6741?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/6741)
- [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`.
@Wumpf Wumpf added the user-request This is a pressing issue for one of our users label Jul 23, 2024
@Wumpf Wumpf mentioned this issue Jul 23, 2024
Closed
@Wumpf
Copy link
Member

Wumpf commented Jul 23, 2024 •
edited
Loading

Adding a bit of context since this comes up more and more often now:
The limit is in place for usecases where labels should show only on hover, most typically this is an issue with large point clouds where it wouldn't make sense to show all labels at once.

I propose to introduce a ShowLabels boolean component that internally has a visualizer-dependent fallback that accounts for this very issue. This may not be the final solution we want to arrive here (e.g. independent archetype is still on the table #4266), but it would be fairly easy to implement and solves the problem at hand that various users run into!

@jleibs
Copy link
Member Author

jleibs commented Jul 24, 2024

This sounds great -- let's do it.

@emilk
Copy link
Member

emilk commented Aug 5, 2024

Maybe something for @kpreid

@kpreid kpreid self-assigned this Aug 15, 2024
@kpreid kpreid mentioned this issue Aug 16, 2024
Merged
6 tasks
Wumpf pushed a commit that referenced this issue Aug 16, 2024
@kpreid
Deduplicate space view label processing code (#7208)
61dce72 
### What

* Combine code of `process_labels_3d` and `process_labels_2d` into one
core function, `process_labels`.
* Move all copies of the code deciding how many labels to show into
`process_labels`.

This is a (hopefully) pure refactoring which removes duplicate code and
should also make #3465 easier to implement.

### 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)
* [ ] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/7208?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/7208?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/7208)
- [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`.
@kpreid kpreid mentioned this issue Aug 21, 2024
Merged
6 tasks
kpreid added a commit that referenced this issue Aug 24, 2024
@kpreid @emilk
Add ShowLabels component, which controls whether instances’ labels …
dc4e3f3 
…are shown. (#7249)

* Implements #3465

### What
* Each archetype which has labels gets a `ShowLabels` non-repeated
component. (I figured having it set per-instance would be hardly ever
useful.)
* Visualizers pass this component to `process_labels()`.
* `process_labels()` uses it to override the existing “show labels if
less than 30” policy.
* Refactoring: `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
* [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)
* [ ] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/7249?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/7249?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/7249)
- [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`.

---------

Co-authored-by: Emil Ernerfeldt <[email protected]>
@kpreid kpreid mentioned this issue Aug 24, 2024
Merged
6 tasks
@jleibs jleibs closed this as completed in dc462a8 Aug 28, 2024
jleibs pushed a commit that referenced this issue Aug 28, 2024
@kpreid @jleibs
Pass ShowLabels default through component fallback provider system. (…
cbfad29 
…#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
Assignees

@kpreid kpreid

Labels
enhancement New feature or request 📺 re_viewer affects re_viewer itself user-request This is a pressing issue for one of our users
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Pass ShowLabels default through component fallback provider system. kpreid/rerun
4 participants
@kpreid @emilk @Wumpf @jleibs