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

RichTextLabel flickers when changing text while threading is enabled #80613

Open
gokiburikin opened this issue Aug 14, 2023 · 4 comments
Open

Comments

@gokiburikin
Copy link

gokiburikin commented Aug 14, 2023

Godot version

v4.1.stable.official [9704596]

System information

Windows 10 - Vulkan (Forward+) - GTX 970 - i7-10700K

Issue description

Enabling threading on RichTextLabel causes it to flicker when you update its text property.

At first I thought this was only when BBCode was enabled but when making the MRP it turned out to be any text update. If you change text once a frame the text functionally disappears.

Also happens in v4.2.dev3.official [013e8e3].

Steps to reproduce

Enable threading on RichTextLabel and change its text. MRP attached.

A visual example (flashing lights warning):

x264.Godot_v4.2-dev3_win64_2023-08-14_07-41-45.mp4

Minimal reproduction project

rtl-threading-mrp.zip

@bruvzg
Copy link
Member

bruvzg commented Aug 15, 2023

I would say it's expected behavior, threaded mode is intended for processing large amounts of text and displays the current state, so if the text is replaced faster than it's processed it will show nothing or partial text. There's no reason to enable it for something like this demo, it's only useful if you have a lot of static text or constantly adding new text (e.g., a log window and help pages in the editor).

@Calinou
Copy link
Member

Calinou commented Aug 15, 2023

I would say it's expected behavior, threaded mode is intended for processing large amounts of text and displays the current state, so if the text is replaced faster than it's processed it will show nothing or partial text.

Could we replace the displayed text while rendering only if it's longer than the previous fully rendered text? (Of course, if the final text is shorter, it should be displayed in place of the previously longer text.)

@gokiburikin
Copy link
Author

gokiburikin commented Aug 15, 2023

I would say it's expected behavior, threaded mode is intended for processing large amounts of text...There's no reason to enable it for something like this demo

That's fair. I only considered using it because the performance of setting RTL text seems to have gone down a decent amount in Godot 4.1 compared to Godot 3.5. About 45% from my simple timing tests. This turned out to not be true. Setting text is not much slower, uncached text rendering is, likely due to a bug that has been addressed.

Unnecessary Change

Took some digging but I found out that the root cause of this issue may have already been addressed by this pr. #77280

I'm leaving the rest of the post in place below the fold, but it's very likely the issue that caused me to look into threading in the first place has been addressed, so changes likely wouldn't be necessary.

Potentially Irrelevant Testing Information

The context of this issue being that I was writing coloured text to the screen every frame (wastefully; it wasn't changing much) and setting the RTL's text property was causing noticeable performance degradation even with a not very large amount of text and the documentation suggests turning threading on to prevent stuttering.

Timing comparison for setting text property (270 usec vs 510 usec)

image

image

Picture of the amount of text being parsed every frame

image

It feels like it shouldn't be slow enough to cause stuttering but it's noticeably there when setting only so much text every frame so my options were listen to the documentation and turn on threading, lower the update rate, separate RTLs that have text that change at different rates, or use the push and pop methods.

I should probably just use the push and pop functions but then I have to pass the RTL itself around instead of building a string which is somewhat less convenient, so I've just lowered the update rate.

More Testing

Out of curiosity I wrote a little bbcode builder that pushes text and color and only has support for those two things to see what performance could be like but it's actually not as big of an improvement as I expected, but the reason turned out to be that rendering text is just much, much slower than I realized.

Clearing out an RTL over and over again has massive performance implications that I wasn't aware of because they aren't picked up in the timings. That explains why even though the timings are small the stuttering happens. This is mitigated when the text doesn't change because of some really snappy caching I assume, but the difference between 3.5 and 4.1 is ridiculously large.

x264.4.1_text_rendering.mp4
x264.3.5_text_rendering.mp4

Without updating the text every frame (if the text isn't changing) the rendering is totally fine. It seems like this just isn't a viable thing to do in 4.1, at least not for a screen full of text.

@TheSofox
Copy link
Contributor

I'm encountering this bug while trying to fix #84718 . Essentially, I'm trying to put a line limit on the Output Log (which uses RichTextLabel).
Normally, you can flood messages to the Output log and it'll display them fine. However, I added a line limit that would trigger "remove_paragraph" to remove lines from the start of the Output log when it went past the limit. The moment this feature is activated, the log flickers/goes blank so badly you can't read it until you stop flooding the log with messages.
Obviously for a log, you want to be able to keep up with quickly delivered messages, even if just a glimpse. You definitely don't want to get the idea that there aren't any messages when in fact there are a lot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants