godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.93k stars 21.15k forks source link

RichTextLabel flickers when changing text while threading is enabled #80613

Open gokiburikin opened 1 year ago

gokiburikin commented 1 year ago

Godot version

v4.1.stable.official [970459615]

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 [013e8e3af].

Steps to reproduce

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

A visual example (flashing lights warning):

https://github.com/godotengine/godot/assets/3820082/d81b2732-07b3-435e-a9dd-58d96484694c

Minimal reproduction project

rtl-threading-mrp.zip

bruvzg commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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. https://github.com/godotengine/godot/pull/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](https://docs.godotengine.org/en/stable/tutorials/ui/bbcode_in_richtextlabel.html#performance) and the documentation suggests turning threading on to prevent stuttering.
Timing comparison for setting text property (270 usec vs 510 usec) ![image](https://github.com/godotengine/godot/assets/3820082/16c317bb-8e25-4a3f-a3ce-b2db5bfa8d1c) ![image](https://github.com/godotengine/godot/assets/3820082/230ce23a-3662-452e-bc82-d0ffd74099d4)
Picture of the amount of text being parsed every frame ![image](https://github.com/godotengine/godot/assets/3820082/0bf450ab-17f4-431d-829a-f59f4635d5b5)
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. https://github.com/godotengine/godot/assets/3820082/ae23a08c-dd80-4cab-acc2-9a0fbcdfd073 https://github.com/godotengine/godot/assets/3820082/b54d90f4-c9cc-492f-ad14-624dda112827 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 commented 11 months ago

I'm encountering this bug while trying to fix https://github.com/godotengine/godot/issues/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.