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

Improve Tilemap Performance -> allow set_cell via Array #31020

Closed
zzz-assault opened this issue Aug 1, 2019 · 25 comments
Closed

Improve Tilemap Performance -> allow set_cell via Array #31020

zzz-assault opened this issue Aug 1, 2019 · 25 comments

Comments

@zzz-assault
Copy link
Contributor

zzz-assault commented Aug 1, 2019

Godot version:

  • 3.1 (2D)

OS/device including version:

  • Windows 10
  • RTX 2080
  • I7 8700

Issue description:
For huge procedural Tilemaps the most critical bottle neck is "set_cells", as you need to go via code with a double loop through each cell and call "set_cell" for them.

Unfortunately also multi threading via "Thread" does not work, as when the Tilemap is active in the scene tree it leads to an immediately crash (active scene tree is NOT thread safe). The only possitiblity is using a seperate Thread for filling the Tilemap while it's instanced but not active in the scene tree (add_child not executed). But it's not possible to have multiple threads accessing the Tilemap in parallel (Thread Safety) -> will also lead to crash.

I'm aware, that a lot of devs solve this issue, by loading junks of the Tilemap (i.e. while the Player moves), but for RTS and eco simulations you need the full map available at the beginning.

Solution:
Create a Tilemap method which allows to set the whole Tilemap cells via loading an Array. If possible for flexibility reasons I would like to get the following options.

  • set_cells (Array2D)
    -> should set cell ID accordingly to the Array2D for the full Tilemap
  • set_cells(y, Array)
    -> should set cell ID accordingly to the Array for the specified line (allows line by line update)

Both solutions would in addition to batch update of Tilemap cells also allow multi threading, as Arrays in GDScript can be accessed via multiple threads.

If there is any other existing solution to create a Tilemap of i. e. 10kx10k in one step with not minutes of loading required, I would be happy to know. I myself wasn't able to come up with an better idea as described above (besides switching to native C++ -> but also C++ would benefit from multi thread access to Tilemap )

Minimal reproduction project:
To reproduce the performance issue just create a new scene, add a Tilemap with a Tileset (only containing one item) and then perform in code with a double loop the loading of i.e. 10.000x10.000 Tiles.

@Skaruts
Copy link

Skaruts commented Aug 2, 2019

Could this issue be extended to draw functions like draw_rect, draw_texture_rect_region, etc, as well as to some form of Sprite batching, where one could set the properties (and shader params) of many sprites at once?

@dioptryk
Copy link
Contributor

dioptryk commented Aug 2, 2019

This may be a bit ugly, but can't you generate a text scene file in memory, filling it with data you want (even with multiple threads, since the tilemap is just a big int array)? Then you save it raw to "usr://" and load(). Should be orders of magnitude faster than working on a Scene object.
Of course, this means you become dependent on the serialized TileMap format not changing.

@Zylann
Copy link
Contributor

Zylann commented Aug 2, 2019

You could theoretically assign the tile_data property using set("tile_data", array), where the array is an interleaved list of tile IDs and rotations, something like this:

PoolIntArray( 131077, 0, 0, 131078, 0, 0, 131080, 0, 0, 131081, 0, 0, 131082, 0, 0, 131083, 0, 0, 196613, 0, 0, 196614, 0, 0, 196615, 0, 0, 196616, 0, 0, 262149, 0, 0, 262152, 0, 0, 327685, 0, 0, 327686, 0, 0, 327687, 0, 0, 327688, 0, 0 )

That's how tilemaps load from a file, except you would be providing that data by script instead. It's not documented, that use case isn't common.

Another way is to not use Godot's tilemap. It's just one implementation after all, nothing stops you from drawing the tiles yourself :p
For immensely huge tilemaps I'm sure you can get decent perf drawing only what you need and using C# to run heavy logic if GDScript is too slow for you (and then might eliminate the need for multiple threads in the first place).

@zzz-assault
Copy link
Contributor Author

zzz-assault commented Aug 2, 2019

@Skaruts: I don't see the connection, but be welcome if this might be combinable ... :)

@dioptryk: Thats a strange but interesting workaround ... should work as long as there is no format difference between the hardcoded and Tilemap text. But as it's anyhow an Array my feature request should be rather simple to implement ... I still have time to wait for a realized feature, as I work on Flame of ZaZuaZ at the moment and this feature would be needed for the next game.

@zzz-assault
Copy link
Contributor Author

zzz-assault commented Aug 2, 2019

@Zylann: That sounds like an approach I could use ... I like the Tilemap implementation and want to use it in my game ... and eventhough the usecase might not be common, the implementation of such a method or documenting your idea (if it's working) should be an easy way to expand the Tilemap usability.

Don't get me wrong, but the engine is promoting GD-Script and also has Thread features, so simply suggesting a language switch to solve performance issues seems to be an odd approach or? (If I would take your statement to the extreme I would question why Thread in GD-Script exists at all) .. 😉

Edit:
Your suggestion is working, but I have no idea how to correctly interleave to get the correct tile positions. I also checked the engine source code, but my C++ knowledge is not enough to even find the calculation ... could you or someone else share the algorithm?

@Zylann
Copy link
Contributor

Zylann commented Aug 2, 2019

Switching language is sometimes the thing to do. GDScript can work in a very wide range of cases, sometimes the GPU can even be used, sometimes the use case is relevant enough to justify a PR. But if something is really slow, implementing it in C# or even C++ can also outweight the cons. But I agree it's not always convenient to do that, I was just suggesting it as an option.

@dioptryk
Copy link
Contributor

dioptryk commented Aug 2, 2019

@zzz-assault Check out the tile_map.cpp, _set_tile_data method. If I see correctly, each tile is represented by 3 unsigned integers in the array. First int represents the coordinates (ushort x and ushort y bitshifted into int). The first 29 bits of the second integer represent the tile id in the tileset, the other 3 bits mean flip and transposition. Dunno about the third integer. Since the coordinates don't depend on the position in the array but are decoded, you write the cells in whatever order.

@zzz-assault
Copy link
Contributor Author

@Zylann @dioptryk Thank you both for your feedback so far, at least my tests showed that using set("tile_data", array) would work for multi threading, but unfortunately the algorithm of getting the correct values (interleave with bitshift) is above my knowledge and current understanding of programming … so I'm not able to transfer it into GD-Script.

Anyhow for myself and probably also others trying to utilize Tilemap with the need of loading a huge amount of Tiles at once, I hope that someone might be willing to add the mentioned methods to the Godot Engine.

@Skaruts
Copy link

Skaruts commented Aug 3, 2019

@zzz-assault

I don't see the connection, but be welcome if this might be combinable ... :)

Well, it's the three places where batching would be useful: the TileMap (for most games), the draw functions (for something like Game Of Life or whatever else) and Sprites (for text based roguelikes, for example). In the cases where the TileMap isn't the way to go you'll still be faced with the same problem of having to iterate and draw each shape individually or set all sprite properties individually.

I made a Game Of Life in Godot using draw functions, and it performs abysmally bad...


@Zylann

Another way is to not use Godot's tilemap. It's just one implementation after all, nothing stops you from drawing the tiles yourself :p

The raw performance is hundreds of frames lower if you do it in gdscript with Sprites, though. In some cases you really have to do that, but if you can avoid it, all the better. And also, for big maps, too many Sprites will inflate map loading times. In my tests with roguelike maps like 100x100 (10k Sprites) or bigger, they were taking quite a bit to load (which made me switch to emulating a terminal with a fixed amount of sprites, 80x50, which loads practically instantly, and in a fixed time).


As for all the other solutions, they are potentially nice workarounds for the meanwhile, but is there any reason why Godot shouldn't support batching? To my experience batching is a common feature in engines and frameworks.

@zzz-assault
Copy link
Contributor Author

zzz-assault commented Aug 3, 2019

@Skaruts understood … but I assume the solution / implementation would be very different, at least for the Tilemap issue (which is anyhow an Array) I assume loading via an Array could be small change (if I understood the source code correctly).

As said, I have no problem with combining the enhancement requests, if there is any synergy possible … ;)

@Zylann
Copy link
Contributor

Zylann commented Aug 3, 2019

@Skaruts

The raw performance is hundreds of frames lower if you do it in gdscript with Sprites, though

I never said to use sprites :p could be done in _draw(), with draw_texture or draw_primitives with a 2D mesh, and you could use C# if GDScript is the bottleneck.

@Ranoller
Copy link
Contributor

Ranoller commented Aug 3, 2019

I like this idea, it doesn't have to be very difficult to implement. It would only be to move the for loop from gdscript to tile_map.cpp and make the corresponding setters and getters. And the difference in performance would be amazing. Making in the past a random map generator I also concluded that the bottleneck was to call set_cell thousands of times, not in gdscript.
I will look at it (Not promises). Good idea!

@Skaruts
Copy link

Skaruts commented Aug 4, 2019

@Zylann fair enough. But you still lose some performance. The TileMap really is quite a boost on tile rendering over the alternatives.

Though C# is only an option given certain conditions though. If you already know some C# and are already using the mono version, or if you're interested in C# enough to go through the trouble of learning it and fetching the mono version. I'm personally not interested in C# at all.

But even in C# you'd benefit from batching.


@zzz-assault

@Skaruts understood … but I assume the solution / implementation would be very different, at least for the Tilemap issue (which is anyhow an Array) I assume loading via an Array could be small change (if I understood the source code correctly).

Yes a Sprite batching would require more work and somewhere deep in the engine. I presume the draw functions wouldn't; it's deeper in the engine too, but the code changes shouldn't be very different from the TileMap's -- just add functions that accept arrays and which do the same job as the others (or call the others in a loop).

For Sprites, my best guesses are:

  • they could do the same as the above in the VisualServer, and/or...
  • ...create some kind of sprite grouping that could be accessed and manipulated through code (my vision on this is kind of fuzzy)
  • ...create a MultiSprite node, much like the MultiMesh node but for 2D Sprites. (The TileMap already is essentially a MultiSprite, the problem is it's too limited for some use cases.)

If it requires opening another issue I wouldn't mind doing so.

@zzz-assault
Copy link
Contributor Author

zzz-assault commented Aug 4, 2019

@Ranoller Sounds great, perhaps it would also be possible to just load directly the internal Tilemap Array with the input Array instead of executing the loop via set_cells inside the .cpp, but I'm not sure if there would be any dependencies.

In any case … big THX for having a look into the possible implementation.

@Skaruts I'm not sure how the Godot Engine GitHub issue tracking or organization is done, but as the implementation would be totally different, I would recommend to open a new issue with your request (perhaps even splitting your two requests, if they also need different implementations?).

@zzz-assault
Copy link
Contributor Author

zzz-assault commented Sep 4, 2019

@MrDudeIII came up with a correct "interleave with bitshift" and he agreed to share his solution ... so if anyone needs a workaround I'll post it here … :)

example

Just an Addition: By using directly the "set" of the Tilemap paramter "tile_data" it seems that the memory usage is a lot higher then by using set_cell for the same amount of cells. Probably some Tilemap internals are not casted by direct access and so messing up the internal optimizations.
I also had Problems with activated y-sort, which caused some weird crashes. In addition the Speed bonus is not really high, as I need to convert beforehand in GD-Script.

Really looking forward to an integrated method to load a Array or 2dArray.

@Sungray
Copy link

Sungray commented Dec 23, 2019

This is huge, it makes it possible to load a tilemap in the background using threads and local arrays, stitching the arrays back together into one huge array and dump it into the tilemap instantly with not a single stutter from the main thread. It's a bit of a hack but works perfectly. Should be incorporated into TileMap's methods. In addition to allowing background load, it also cuts the time necessary to load big tilemaps hundredfolds by not having to call set_cell on each tile, which is huge for map streaming. Thanks.

Side note, I don't know why, but on some tilemaps I had to create another convert_cell method which does NOT include orientation for it to work. So the returned value is still a PoolIntArray but each tile only has a coord and a tile_id.

@zzz-assault
Copy link
Contributor Author

@Ranoller Any update? :)

@Ranoller
Copy link
Contributor

Not working on that, sorry

@MrDudeIII
Copy link

I feel like should mention that even writing directly to the tile_data wasn't fast enough for my use case (I was trying to update 1000x1000 tiles every frame), so I ended up writing another shader draw each pixel of a 1000x1000 png as a corresponding tile in a 16000x16000 png based on color. This was fast enough (no lag with per-frame updates), but really limited as far as what the tiles can do.

@Skaruts
Copy link

Skaruts commented Mar 19, 2020

@zzz-assault
There's some improvements being made by @lawnjelly at #35957 (and also #35696) that I think you might find interesting.

@byfron
Copy link
Contributor

byfron commented Mar 30, 2020

In case it helps anybody. The interleaving format for each cell is:

[tile coordinates (16 bits each), tile_id (32 bits encoding id(24), flip_x(1), flip_y(1), transpose(1)), and 32 bits with the atlas sub-tile coordinate (16 bits each)]

The details can be checked here: https://github.com/godotengine/godot/blob/master/scene/2d/tile_map.cpp#L1210

I've been trying to also copy a "bulk" of tilemap, but having a tileset in ATLAS mode would not work due to the subtile coords missing.

@zzz-assault
Copy link
Contributor Author

zzz-assault commented Apr 19, 2020

@Skaruts Thx … but unfortunately it's only valid for GLES2 and it would only solve draw performance (haven't seen a big bottleneck there), biggest problem is still to use a double loop for cell wise loading :-(

@Calinou
Copy link
Member

Calinou commented Sep 19, 2020

Closing in favor of godotengine/godot-proposals#1538, as feature proposals are now tracked in the Godot proposals repository.

@sxdxs
Copy link

sxdxs commented Oct 14, 2020

So....is it possible to do like oblique tiles? Pulling my hair out trying on all the engines

@MrDudeIII
Copy link

@sxdxs I'm not sure what you mean by opaque. Also, you might want to try asking that question on the official Godot engine discord. I'm not sure you'll get a timely response here, and your question seems pretty off topic from this issue.

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