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

Delay in text-update vs layout-update, creating wiggling text #15197

Closed
tjlaxs opened this issue Sep 14, 2024 · 4 comments · Fixed by #16097
Closed

Delay in text-update vs layout-update, creating wiggling text #15197

tjlaxs opened this issue Sep 14, 2024 · 4 comments · Fixed by #16097
Labels
A-Text Rendering and layout for characters A-UI Graphical user interfaces, styles, layouts, and widgets C-Bug An unexpected or incorrect behavior S-Needs-Investigation This issue requires detective work to figure out what's going wrong

Comments

@tjlaxs
Copy link
Contributor

tjlaxs commented Sep 14, 2024 •
edited
Loading

Bevy version

Using main to dev (1fd478277)

Relevant system information

  • cargo 1.80.1 (376290515 2024-07-16)
  • Linux terska-2k24 6.8.0-44-generic 44-Ubuntu SMP PREEMPT_DYNAMIC Tue Aug 13 13:35:26 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux

What you did

Started the text_debug example.

What went wrong

Texts that are on right wiggle one pixel back and forth horizontally.

Additional information

Not sure if it is related to the texts at the bottom that move around a lot because of the FPS and frame time numbers. You would think the text in the top right would stay still but it still wiggles.

Note: You can not see the fps/frame time text until #15190 has been merged because they are pushed out of frame unless you make the window larger.
@tjlaxs tjlaxs added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Sep 14, 2024
@kfjustis
Copy link

Confirmed this happens to me as well.

My Info

Bevy: main, b36443b6

SystemInfo { os: "Linux 22.04 Pop!_OS", kernel: "6.9.3-76060903-generic", cpu: "Intel(R) Core(TM) i7-7700K CPU @ 4.20GHz", core_count: "4", memory: "31.3 GiB" }

Cargo: cargo 1.81.0 (2dbb1af80 2024-08-20)

Description

Text on the right side wiggled just like the description when I first ran the example. Then I tried resizing the window and different nodes started wiggling for me. When I made the window really narrow, the wiggle stopped, but the wiggle came back when I made the window larger again.

@tjlaxs
Copy link
Contributor Author

tjlaxs commented Sep 15, 2024

Seems like the wiggling is removed when the widest TextSection with changing numbers does not have numbers that change. So my guess is that this is related to the fact that children don't act fast enough to the width changes. Or children of children or something like that.
image

@tjlaxs
Copy link
Contributor Author

tjlaxs commented Sep 15, 2024 •
edited
Loading

Looking at the debug outlines:
image
You would think the children of right_column would stay inside the right column, but it looks like not in the wiggle state.

@alice-i-cecile alice-i-cecile added A-Text Rendering and layout for characters and removed S-Needs-Triage This issue needs to be labelled labels Sep 16, 2024
@alice-i-cecile
Copy link
Member

I bet that the order of our systems is wrong, and we're getting a one-frame delay.

@alice-i-cecile alice-i-cecile added the S-Needs-Investigation This issue requires detective work to figure out what's going wrong label Sep 16, 2024
@alice-i-cecile alice-i-cecile changed the title Text wiggle in text_debug example Delay in text-update vs layout-update, creating wiggling text Sep 16, 2024
@alice-i-cecile alice-i-cecile added the A-UI Graphical user interfaces, styles, layouts, and widgets label Sep 16, 2024
@ickshonpe ickshonpe mentioned this issue Oct 25, 2024
Merged
github-merge-queue bot pushed a commit that referenced this issue Oct 28, 2024
@ickshonpe @rparrett
Remove custom rounding (#16097)
05d9068 
# Objective

Taffy added layout rounding a while ago but it had a couple of bugs and
caused some problems with the fussy `ab_glyph` text implementation. So I
disabled Taffy's builtin rounding and added some hacks ad hoc that fixed
(some) of those issues. Since then though Taffy's rounding algorithm has
improved while we've changed layout a lot and migrated to `cosmic-text`
so those hacks don't help any more and in some cases cause significant
problems.

Also our rounding implementation only rounds to the nearest logical
pixel, whereas Taffy rounds to the nearest physical pixel meaning it's
much more accurate with high dpi displays.

fixes #15197

## Some examples of layout rounding errors visible in the UI examples

These errors are much more obvious at high scale factor, you might not
see any problems at a scale factor of 1.

`cargo run --example text_wrap_debug`

<img width="1000" alt="text_debug_gaps"
src="https://github.com/user-attachments/assets/5a584016-b8e2-487b-8842-f0f359077391">

The narrow horizontal and vertical lines are gaps in the layout caused
by errors in the coordinate rounding.

`cargo run --example text_debug`

<img width="1000" alt="text_debug"
src="https://github.com/user-attachments/assets/a4b37c02-a2fd-441c-a7bd-cd7a1a72e7dd">

The two text blocks here are aligned right to the same boundary but in
this screen shot you can see that the lower block is one pixel off to
the left. Because the size of this text node changes between frames with
the reported framerate the rounding errors cause it to jump left and
right.

## Solution

Remove all our custom rounding hacks and reenable Taffy's layout
rounding.

The gaps in the `text_wrap_debug` example are gone:
<img width="1000" alt="text_wrap_debug_fix"
src="https://github.com/user-attachments/assets/92d2dd97-30c6-4ac8-99f1-6d65358995a7">

This doesn't fix some of the gaps that occur between borders and content
but they seem appear to be a rendering problem as they disappear with
`UiAntiAlias::Off` set.

## Testing

Run the examples as described above in the `Objective` section. With
this PR the problems mentioned shouldn't appear.

Also added an example in a separate PR #16096 `layout_rounding_debug`
for identifying these issues.

## Migration Guide

`UiSurface::get_layout` now also returns the final sizes before
rounding. Call `.0` on the `Ok` result to get the previously returned
`taffy::Layout` value.

---------

Co-authored-by: Rob Parrett <[email protected]>
mockersf pushed a commit that referenced this issue Nov 5, 2024
@ickshonpe @rparrett @mockersf
Remove custom rounding (#16097)
ebce9db 
# Objective

Taffy added layout rounding a while ago but it had a couple of bugs and
caused some problems with the fussy `ab_glyph` text implementation. So I
disabled Taffy's builtin rounding and added some hacks ad hoc that fixed
(some) of those issues. Since then though Taffy's rounding algorithm has
improved while we've changed layout a lot and migrated to `cosmic-text`
so those hacks don't help any more and in some cases cause significant
problems.

Also our rounding implementation only rounds to the nearest logical
pixel, whereas Taffy rounds to the nearest physical pixel meaning it's
much more accurate with high dpi displays.

fixes #15197

## Some examples of layout rounding errors visible in the UI examples

These errors are much more obvious at high scale factor, you might not
see any problems at a scale factor of 1.

`cargo run --example text_wrap_debug`

<img width="1000" alt="text_debug_gaps"
src="https://github.com/user-attachments/assets/5a584016-b8e2-487b-8842-f0f359077391">

The narrow horizontal and vertical lines are gaps in the layout caused
by errors in the coordinate rounding.

`cargo run --example text_debug`

<img width="1000" alt="text_debug"
src="https://github.com/user-attachments/assets/a4b37c02-a2fd-441c-a7bd-cd7a1a72e7dd">

The two text blocks here are aligned right to the same boundary but in
this screen shot you can see that the lower block is one pixel off to
the left. Because the size of this text node changes between frames with
the reported framerate the rounding errors cause it to jump left and
right.

## Solution

Remove all our custom rounding hacks and reenable Taffy's layout
rounding.

The gaps in the `text_wrap_debug` example are gone:
<img width="1000" alt="text_wrap_debug_fix"
src="https://github.com/user-attachments/assets/92d2dd97-30c6-4ac8-99f1-6d65358995a7">

This doesn't fix some of the gaps that occur between borders and content
but they seem appear to be a rendering problem as they disappear with
`UiAntiAlias::Off` set.

## Testing

Run the examples as described above in the `Objective` section. With
this PR the problems mentioned shouldn't appear.

Also added an example in a separate PR #16096 `layout_rounding_debug`
for identifying these issues.

## Migration Guide

`UiSurface::get_layout` now also returns the final sizes before
rounding. Call `.0` on the `Ok` result to get the previously returned
`taffy::Layout` value.

---------

Co-authored-by: Rob Parrett <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-Text Rendering and layout for characters A-UI Graphical user interfaces, styles, layouts, and widgets C-Bug An unexpected or incorrect behavior S-Needs-Investigation This issue requires detective work to figure out what's going wrong
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove custom rounding ickshonpe/bevy
3 participants
@alice-i-cecile @tjlaxs @kfjustis