Add a TileMap set_cell_region method to set a rectangular area of tiles faster

[dariomnz]

Describe the project you are working on:
2d game

Describe the problem or limitation you are having in your project:

When setting a large amount of the same tile in a tilemap inside a doble for, it takes a lot of time.
For example if you want to fill a tilemap, and then you are going to remove the tiles for creating the rooms.

Describe the feature / enhancement and how it helps to overcome the problem or limitation:

The new feature is to create a new method in the tilemap class (source code) like set_cell_region.

Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams:
Code in c++

void TileMap::set_cell_region(const Vector2 &p_start, const Vector2 &p_end , int p_tile, bool p_flip_x, bool p_flip_y, bool p_transpose, Vector2 p_autotile_coord) {
    for (int x = p_start.x ; x <= p_end.x ; x++) {
        for (int y = p_start.y ; y <= p_end.y ; y++) {
            set_cell(x, y, p_tile, p_flip_x, p_flip_y, p_transpose, p_autotile_coord);
        }
    }
}

If this enhancement will not be used often, can it be worked around with a few lines of script?:

It can be do in 3 lines of code in gdscript but take a lot of time the execution.

Is there a reason why this should be core and not an add-on in the asset library?:

I already compile the engine with this new feature and I can test it.
The results are this for area of 1000x1000 tiles:

Time consumed in set_cell in a double for: 4080 msec
Time consumed in set_cell_region: 2345 msec
set_cell_region: 42.52451% less than set_cell in a double for.

I think that there is a good new feature. I already have the source code. But because I new i don't know if I need to create a pull request or this is later.

[Calinou]

When setting a large amount of the same tile in a tilemap inside a doble for, it takes a lot of time.

You can use a single for loop for this as long as you know the region's width (or height). It should give you a noticeable speedup already:

func _ready():
    var width = 6
    var height = 4

	# Instead of:
	for i in width:
		for j in height:
			printt(i, j)

	print()

	# Use:
	for i in width * height:
		printt(i / height, i % height)

[dariomnz]

@Calinou
I already tested that, and the results are the same. I think the bottle neck is a kind of form that gdscript calls the functions, because the number of calls to set_cell is the same. The code I use for the tests:

var init = Vector2(0,0)
var fin = Vector2(1000,1000)

func _ready():
	
	var start_time = OS.get_ticks_msec()
	with_set_cell_double()
	var time_set_cell_double = OS.get_ticks_msec()-start_time
	print("Time consumed in set_cell in a double for: "+str(time_set_cell_double))
	
	start_time = OS.get_ticks_msec()
	with_set_cell()
	var time_set_cell = OS.get_ticks_msec()-start_time
	print("Time consumed in set_cell in a for: "+str(time_set_cell))
	
	start_time = OS.get_ticks_msec()
	with_set_cell_region()
	var time_set_cell_region = OS.get_ticks_msec()-start_time
	print("Time consumed in set_cell_region: "+str(time_set_cell_region))
	
	print("set_cell_region: "+str((1-time_set_cell_region*1.0/time_set_cell)*100) +"% less than set_cell in a for.")

func with_set_cell():
	var tilemap = $TileMap2
	var width = int(fin.x-init.x)
	var height = int(fin.y-init.y)
	for i in width*height:
		tilemap.set_cell(i/height,i%height,0)

func with_set_cell_double():
	var tilemap = $TileMap3
	for x in range(init.x,fin.x-1):
		for y in range(init.y,fin.y-1):
			tilemap.set_cell(x,y,0)

func with_set_cell_region():
	$TileMap.set_cell_region(init,fin,0)

This code is in a Node2D and have 3 tilemaps as childs with the same characteristics.

The results are:

Time consumed in set_cell in a double for: 3096
Time consumed in set_cell in a for: 3150
Time consumed in set_cell_region: 2327
set_cell_region: 26.126984% less than set_cell in a for.

[Calinou]

@dariomnz Feel free to open a pull request with your changes then 🙂

Make sure to open the pull request against the master branch, then we can cherry-pick it to the 3.2 branch later on.

[zzz-assault]

Just to not lose the already done discussion on this topic, here a reference to an previous issue (in the main repository), with same background = set_cell() performance -> #31020

[Xkonti]

I would like to also mention that similar functionality but not-regionalized would be very helpful. Something like set_cells that would take a list of cells to update that can be referring to random cells in the whole tile map.

I believe there's a common use case for this. Let's consider an example: a colony managing game that uses the tile map to display a top-down view. Every frame a vegetation growth system decides what plants have grown and updates the displayed tiles appropriately. This might involve many tile throughout the whole map needing to update.

Another use case would be where due to the TileMap being just mostly a visual representation the actual map and it's data is being tracked as a completely separate data structure. All game systems could operate on that specific data and then at the end consolidating all changed tiles into a single list. That list then can be sent to TileMap to update in a single call.

[ShayBox]

Any progress on this? the biggest time consumer is set_cell for me, and I can't chunk my tilemap up because base.set_cell_ex isn't thread safe Send (gdext) anyway, this is inherently a single threaded task.

[Calinou]

See this comment: godotengine/godot#39833 (comment)

We need benchmarks to see if this really makes a significant performance difference.