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

godot/scene/main/canvas_item.cpp

Lines 427 to 433 in e96ad5a

	if (pending_update) {
	return;
	}
	pending_update = true;
	callable_mp(this, &CanvasItem::_redraw_callback).call_deferred();

[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: #28385

An old PR by Calinou that suggests to outright disable smooth scrolling by default due to bugs: #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.

[Calinou]

Tested locally (rebased on top of master 5833f59), it works as expected. I've tested the script editor and didn't notice any regressions to common behavior (code folding, autocompletion, multiple carets).

Benchmark

On average, this PR is 2.16× faster than master with an optimized editor build (optimize=speed lto=full) on the MRP linked in the PR.

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

• Script is scrolled to its first line, with the caret on the first line and first character (no text is selected).
• Output panel is cleared and collapsed.
• Window is maximized on a 4K display at 100% scaling.
• Editor started with the --print-fps command line argument.
• V-Sync Mode: Disabled.
• Update Continuously checked.

Before this PR, I get 861 FPS (1.16 mspf) in a maximized editor window on average, while after this PR, I get 758 FPS (1.31 mspf) on average.

I've made sure I'm CPU-bottlenecked in this situation (GPU utilization does not exceed 40% in this scenario). This issue does not occur at lower resolutions (where less of the script is visible), such as a 1920x1080 window of the editor. In this case, I'm even more CPU-bottlenecked but this PR wins out over master.

Either way, I figure I'd point this out as 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)).

PC specifications

• CPU: Intel Core i9-13900K
• GPU: NVIDIA GeForce RTX 4090
• RAM: 64 GB (2×32 GB DDR5-5800 C30)
• SSD: Solidigm P44 Pro 2 TB
• OS: Linux (Fedora 39)

Before

10.0878296476422ms avg draw time (99 samples)
10.0065517907191ms avg draw time (198 samples)
9.94229878640737ms avg draw time (297 samples)
9.96660463737719ms avg draw time (396 samples)
9.95996937607274ms avg draw time (495 samples)
9.95242515396992ms avg draw time (594 samples)
9.94330463987408ms avg draw time (693 samples)
9.93462313305248ms avg draw time (792 samples)
9.93317573025037ms avg draw time (891 samples)
9.9287399137863ms avg draw time (990 samples)
9.92602815969588ms avg draw time (1089 samples)

After

4.75202184734922ms avg draw time (99 samples)
4.6268870132138ms avg draw time (198 samples)
4.60077375675291ms avg draw time (297 samples)
4.57970060483374ms avg draw time (396 samples)
4.56591904765428ms avg draw time (495 samples)
4.56037465169374ms avg draw time (594 samples)
4.5561594364447ms avg draw time (693 samples)
4.55680308919964ms avg draw time (792 samples)
4.55250055032681ms avg draw time (891 samples)
4.55153975823913ms avg draw time (990 samples)
4.55042635004273ms avg draw time (1089 samples)

[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

[kitbdev]

• fixes Toggling breakpoints break when line spacing is high #93036 (after its fixed)

It looks like queue_redraw has an issue that lets it be called multiple times in a frame, and it is not specifically related to TextEdit or CodeEdit. It should be fixed elsewhere and queue_redraw should continue to be used in these files instead of this set_process_internal workaround.
When trying your example (#92865 (comment)) I could only rarely get it to draw 2-3 times per frame.

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

Here I'm hitting backspace and enter, though the jittering doesn't look exactly the same in gif:

[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 (#92865 (comment)) 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 #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

[kitbdev]

The line number jitter when there are too many lines issues is fixed.

There is an issue when deleting the longest line, the width doesn't go to the second longest line:

Screen.Recording.2024-08-01.153242.mp4

Also there is an issue with the height when there is lots of wrapped text, the scrollbar doesn't show or is too small:

[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.

[kitbdev]

Previous issues fixed.

For #93036 it is fixed, See my comment here #93036 (comment).
Hovering below the last line in the breakpoint gutter will now show the hovered breakpoint on the last line. This now matches the click behavior, since clicking there will add a breakpoint to the last line. If it isn't wanted it can be fixed in another PR.

Just a couple of minor things:

[kitbdev]

Haven't tested the performance, but the functionality looks good to me.

[akien-mga]

Thanks!

[arkology]

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