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

Better tooltip size management #4471

Closed
emilk opened this issue May 7, 2024 · 3 comments
Closed

Better tooltip size management #4471

emilk opened this issue May 7, 2024 · 3 comments
Assignees
Labels
egui rerun Desired for Rerun.io

Comments

@emilk
Copy link
Owner

emilk commented May 7, 2024

Similar issues:

Tooltips sometimes flicker the first time they show up.
This is because we don't know their size before we show them, and their size affects their position.

I think we should hide the tooltips for the first frame, and use that only for sizing calculations.
Once calculated, we store the size for use the subsequent frames.
The tooltips size will be assumed to be constant during its lifetime.
Once a tooltip is hidden, we forget about its size.

This will also allow for better sizing within a tooltip: a user will be able to add "full width" widgets (such as Separator) that are only as wide as the widest element.

@abey79
Copy link
Collaborator

abey79 commented May 15, 2024

This PR is good test case because the tooltip has two sources of flicker:

  • needs to move above the cursor because there isn't enough room bellow
  • the component list takes 2 frames to be sized correctly

rerun-io/rerun#6309

tooltip-flicker.mp4

abey79 added a commit to rerun-io/rerun that referenced this issue May 22, 2024
### What

This PR adds support for `PropertyContent::exact_width(true)`, which
makes it possible to create lists which only use the needed width (as
opposed to using `ui.available_width()`. This was previous available for
`LabelContent` and used (in the future) in the streams view.

`LabelContent` exploits the fact that the content is mainly a label, the
width of which can be computed with `egui`. For `PropertyContent` it's
tricker because we delegate the second column to a closure. Instead, we
track the max width in each `list_item_context` on frame n, and request
that width in frame n+1.

The major drawback of this approach is the potential flicker on the
first frame. Since the plan is to use it for tooltip, it will greatly
help to have emilk/egui#4471

### 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/6325?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/6325?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/6325)
- [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`.
@emilk emilk self-assigned this May 27, 2024
@emilk emilk closed this as completed in 0039614 May 29, 2024
@emilk emilk reopened this May 29, 2024
@emilk
Copy link
Owner Author

emilk commented May 29, 2024

Probably closed by #4579 but should be tested with rerun-io/rerun#6309 first

abey79 added a commit to rerun-io/rerun that referenced this issue Jun 4, 2024
)

### What

- Fixes #4161
- Follow up to #6297
- Follow up to #6325

This PR uses `list_item` for the component list in the entity path
selection panel and tooltip.

~~Blocked by emilk/egui#4471 to resolve the
tooltip "first frame flicker"~~

TODO:
- [x] ⚠️ full_span_scope panic on hover on spatial space view:
#6246


#### selection panel

before:

<img width="519" alt="image"
src="https://github.com/rerun-io/rerun/assets/49431240/afa51449-d7bf-4c0c-9d57-0ac25fedcf9f">


after:

<img width="345" alt="image"
src="https://github.com/rerun-io/rerun/assets/49431240/1b524065-f5c0-4834-83ab-bc6b5f83ec37">


*Note*: the hover behaviour is the biggest change here: now full span,
previously just on the left-column button

#### tooltip

before:

<img width="511" alt="image"
src="https://github.com/rerun-io/rerun/assets/49431240/cecb2337-4124-43ed-8b24-b895e71c3b26">

after:
<img width="510" alt="image"
src="https://github.com/rerun-io/rerun/assets/49431240/4adb5113-898d-4585-be56-876d83b7694a">


### 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/6309?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/6309?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/6309)
- [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`.
@emilk
Copy link
Owner Author

emilk commented Jun 5, 2024

Yup, tooltips feel great now!

@emilk emilk closed this as completed Jun 5, 2024
hacknus pushed a commit to hacknus/egui that referenced this issue Oct 30, 2024
This simplifies and improves the tooltip positioning

* Closes emilk#4568

### For a follow-up PR
* [ ] Test if it closes emilk#4471
* [ ] Add an API to close emilk#890
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
egui rerun Desired for Rerun.io
Projects
None yet
Development

No branches or pull requests

2 participants