CodeEdit: improve render time by 2x

[rune-scape]

with gdscript syntax highlighting: before: 30.0385588566243ms avg draw time (1089 samples) after: 13.0933055755083ms avg draw time (1089 samples)

without syntax highlighting: before: 17.9521260292186ms avg draw time (1089 samples) after: 12.9270402524754ms avg draw time (1089 samples)

many improvements to do with:

avoiding redraw:

• use set_process_internal(true) instead of queue_redraw to avoid drawing 2-3 times in the same frame when gui_input() is called
  • tests needed to be updated bc of this
• set_code_hint and set_code_hint_draw_below avoid redrawing when the values dont change

improving render speed:

• cache line syntax highlighting
• line numbers:
  • caching line number shaped text
  • simple enough to use TextServer RIDs directy instead of TextLine
  • only render line numbers on screen
• _main_gutter_draw_callback: use get_hovered_gutter() instead of multiple calls to the more expensive get_local_mouse_pos()
• cache total_visible_line_count and Line::line_count
• delay calculating text max width and height in case its not needed
• GutterInfo, which contains a custom callable in both of godots script editor gutters, was getting copied once per gutter per line per frame,

some less impactful (more code style) things ..

• avoid creating HashMaps every draw frame, just use vectors
• const-ify copies of Vectors to avoid possible copy-on-write when theres no write

its obvious that most of the problem was the line syntax highlighting being called every frame but whats not obvious is that searching through the dictionary and converting to and from variants was a very significant part of it

the performance improvements are especially noticeable on debug builds, as there arent any 'double draws' making the editor lag when scrolling

benchmark: CodeEditText.zip tested with godot compiled using: production=yes optimize=size tests=yes deprecated=no scu_build=yes bc it uses the GDScriptSyntaxHighlighter, to benchmark you have open the project in the editor

[KoBeWi]

use set_process_internal(true) instead of queue_redraw to avoid drawing 2-3 times in the same frame

Drawing is already limited to once per frame. It's called queue_redraw(), because it queues one redraw no matter how many times you call it. A different result would be a bug.

[rune-scape]

it very much does draw multiple times per process loop (maybe different meaning than frame) bc queue_redraw adds a message to the MessageQueue which gets flushed multiple times per iteration

[KoBeWi]

How https://github.com/godotengine/godot/blob/e96ad5af98547df71b50c4c4695ac348638113e0/scene/main/canvas_item.cpp#L427-L433

[AThousandShips]

How does queue_redraw get called multiple times per frame though? The queue gets flushed a few times but it's in different phases of the frame, and unless some code calls queue_redraw in different stages it won't be drawn multiple times?

[rune-scape]

it does get called in multiple stages, specifically in gui_input and in NOTIFICATION_INTERNAL_PHYSICS_PROCESS and somehow in another place , sometimes, (beware i tested this on 4.2) i tested how many times per iteration it was drawing, and it was between 2 and 3 every frame when scrolling vigarously

i can recreate the test if you want

[KoBeWi]

Ah yeah, the queue is flushed multiple times per iteration. So it could happen e.g. if you queue redraw in both process frame and physics frame. Weird that it happens tho.

[MewPurPur]

CodeEdit should not be using physics process, it's a temporary ugly solution to get smooth scrolling that causes bugs, but no one has managed to solve this yet.

[rune-scape]

i see.. i.. assumed it was simply copied over from VScroll and never changed, im curious what the bug is, i havent noticed it yet, ive been using this PR for a bit now

[MewPurPur]

What I'm saying about the scrollbar is on the side - no need to address it here, but in general, we really need a GUI wizard to implement smooth scroll properly without using physics process. Our editor shouldn't be using physics process.

[rune-scape]

sorry for not being clear about how it draws multiple times every frame but i didn't expect it to be unbelievable i could split out out the redraw part of this PR, it seems controversial, but heres a test to show im not making this up: CodeEditMultipleDrawTest.zip

i tested it just now on master on a debug linux build and it seems to draw 8! times per process iteration when scrolling vigarously

and @MewPurPur, i asked about the smooth scrolling bug bc in this PR ive changed the TextEdit to use NOTIFICATION_INTERNAL_PROCESS instead of physics process (it seemed like you were saying that causes a bug), and i have some people using this, i'd like to know how serious the bug is.

[MewPurPur]

Here's an issue about this bug: https://github.com/godotengine/godot/issues/28385

An old PR by Calinou that suggests to outright disable smooth scrolling by default due to bugs: https://github.com/godotengine/godot/pull/42704

[lawnjelly]

it very much does draw multiple times per process loop (maybe different meaning than frame) bc queue_redraw adds a message to the MessageQueue which gets flushed multiple times per iteration

Some options off top of my head:

• Frame specific MessageQueue.
• Bespoke list that is flushed once per frame (i.e. CanvasItem redraw queue).

I've used the latter multiple times, notably for the physics interpolation. An advantage here is you'd have tighter control of the timing of the calls.

Frame specific MessageQueue I suspect would inevitably end up getting flushed multiple times and you'd end up with the same problem.

Another potential snag in either case is if the update caused side effects, like queuing the update of another CanvasItem, or adding stuff to the MessageQueue, or even setting up recursive loops, so you might need to limit what could be done in these updates, or deal with this possibility explicitly:

e.g.

while (!MessageQueue.is_empty())
{
flush_canvas_item_updates();
flush_message_queue();
}

Depending on what is done in these updates, UI code is typically full of recursive stuff for layouts.

[MewPurPur]

I'm finding a lot of instances where _draw() is called twice in 4.3 when it was only called once in 4.2.2, could that be related? I haven't set on to make a MRP yet, but someone else encountered the same:

_draw runs for me twice too for some reason but only after nesting the script with the _draw in another control for me it was panel

[rune-scape]

reverted back to using queue redraw and NOTIFICATION_INTERNAL_PHYSICS_PROCESS

edit: although its reverted, some of the changes prevent calling queue_redraw() when unnecessary, so alot of the duplicate redraws are still fixed

Also, when there are many lines in a script (over 1000) the line numbers jitter left and right when adding/removing a line.

i also removed the change that caused this and tested it no longer happens, i would appreciate u confirming also @kitbdev

When trying your example (https://github.com/godotengine/godot/pull/92865#issuecomment-2155763912) I could only rarely get it to draw 2-3 times per frame.

it was probably bc i was using the debug build of the editor, and it was hitting all 8 max physics frames per process frame, it makes the debug version of the editor alot harder to use

edit: i also don't see that its fixing https://github.com/godotengine/godot/issues/93036, but that issue also isn't so descriptive .. so idk

[rune-scape]

However, I get a reproducible slowdown in terms of editor FPS when opening the Platformer 2D's player.gd script in the editor

this may point out at some scalability issues in the new code (e.g. methods that have O(N²) scaling has replaced methods that were previously O(N)).

@Calinou im quite sure theres only one place this happened (replacing a hashmap lookup with Vector.has), and it only applies to having multiple cursors,

when i tested this i didnt get any meaningful slowdown, (probly bc i dont have a 4k monitor, only a 1440p) but i have a slight suspicion that changing it to use NOTIFICATION_INTERNAL_PROCESS caused this, and now that its changed back to use NOTIFICATION_INTERNAL_PHYSICS_PROCESS it may not have that slowdown

[rune-scape]

@kitbdev thank you for reviewing and finding these bugs!!! everything u mentioned should be fixed now,

seems the max line height/width being invalidated by setting it to -1 was not a good idea,, lol also rebased

[akien-mga]

Needs rebase.

[akien-mga]

Thanks!

[arkology]

I have tested speed improvement by changing zoom inside script editor using mouse wheel and difference is really noticeable🚀