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

Prevent text shrinking in tooltips; round wrap-width to integer #5161

Merged
merged 2 commits into from
Sep 25, 2024

Conversation

emilk
Copy link
Owner

@emilk emilk commented Sep 25, 2024

Protect against rounding errors in egui layout code.

Say the user asks to wrap at width 200.0.
The text layout wraps, and reports that the final width was 196.0 points.
This than trickles up the Ui chain and gets stored as the width for a tooltip (say).
On the next frame, this is then set as the max width for the tooltip,
and we end up calling the text layout code again, this time with a wrap width of 196.0.
Except, somewhere in the Ui chain with added margins etc, a rounding error was introduced,
so that we actually set a wrap-width of 195.9997 instead.
Now the text that fit perfectly at 196.0 needs to wrap one word earlier,
and so the text re-wraps and reports a new width of 185.0 points.
And then the cycle continues.

So this PR limits the text wrap-width to be an integer.

Related issues:


Pleas test this @rustbasic

@emilk emilk added bug Something is broken egui epaint labels Sep 25, 2024
@emilk emilk changed the title Round text wrap-width to an integer Prevent text shrinking in tooltips; round wrap-width to integer Sep 25, 2024
Copy link

Preview available at https://egui-pr-preview.github.io/pr/5161-emilk/round-max-width
Note that it might take a couple seconds for the update to show up after the preview_build workflow has completed.

@emilk emilk merged commit f97f850 into master Sep 25, 2024
40 checks passed
@emilk emilk deleted the emilk/round-max-width branch September 25, 2024 09:31
Wumpf added a commit to rerun-io/rerun that referenced this pull request Sep 25, 2024
* Had this fix in it: emilk/egui#5161
* #6801 is NOT fixed by this PR
:(

@Wumpf you had a pretty solid repro of this yesterday - please test 🙏 

### 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/7509?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/7509?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/7509)
- [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: Andreas Reich <andreas@rerun.io>
@rustbasic
Copy link
Contributor

Dear emilk.

After a brief test, it works well.
Even with multiple popups, the size doesn't change, so I think we can close #5079.
I will let you know if I find any other issues.

Thank you, emilk.

hacknus pushed a commit to hacknus/egui that referenced this pull request Oct 30, 2024
…k#5161)

* Closes emilk#5106
* Closes emilk#5084


Protect against rounding errors in egui layout code.

Say the user asks to wrap at width 200.0.
The text layout wraps, and reports that the final width was 196.0
points.
This than trickles up the `Ui` chain and gets stored as the width for a
tooltip (say).
On the next frame, this is then set as the max width for the tooltip,
and we end up calling the text layout code again, this time with a wrap
width of 196.0.
Except, somewhere in the `Ui` chain with added margins etc, a rounding
error was introduced,
so that we actually set a wrap-width of 195.9997 instead.
Now the text that fit perfectly at 196.0 needs to wrap one word earlier,
and so the text re-wraps and reports a new width of 185.0 points.
And then the cycle continues.

So this PR limits the text wrap-width to be an integer.

Related issues:
* emilk#4927
* emilk#4928
* emilk#5163

--- 

Pleas test this @rustbasic
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is broken egui epaint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

#5077 After update, more and more Wrap are added every time the mouse is moved.
2 participants