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

Massive performance issue in 2D viewport when zooming out on huge TileMap #85871

Open
SlyRebula opened this issue Dec 6, 2023 · 15 comments
Open

Comments

@SlyRebula
Copy link

Tested versions

Tested on 4.2 stable mono
Also tested with custom build #84963 (compiled my self)

System information

Godot v4.2.stable.mono - Ubuntu 23.10 23.10 - Wayland - Vulkan (Forward+) - dedicated Intel(R) Arc(tm) A770 Graphics (DG2) () - AMD Ryzen 7 2700X Eight-Core Processor (16 Threads)

Issue description

I am getting horrible performance on "Forward+" compared to "Compatibility" with large tilemap,
at first, I thought this was a driver issue so I tried intel mesa drivers and the latest Ubuntu repa mesa drivers but none of these solved the problem then i tried with Custom Build(#84963 ) but that didn't solve the problem.

That being said it is slow with Compatibility but not horrible(on forward it can escalate into a crash..). The most noticeable slowness appears with Zooming out when zoomed in the slowness disappears.

Selecting the Tilemap node will create the same lag.

Screenshot from 2023-12-07 01-17-42

Steps to reproduce

Create a large tilemap and zoom out this will create the issue.

Minimal reproduction project (MRP)

TestTileMapLag.zip

@clayjohn
Copy link
Member

clayjohn commented Dec 6, 2023

What do you mean by "horrible performance"? Can you provide a bit more info about what you are using to measure performance and what numbers you are getting?

@SlyRebula
Copy link
Author

SlyRebula commented Dec 6, 2023

What do you mean by "horrible performance"? Can you provide a bit more info about what you are using to measure performance and what numbers you are getting?

There is no tool used, when I move the map with middle mouse button it's laggy.
If there is a tool I can use to measure and get outputs, I will provide you with more info

@akien-mga
Copy link
Member

akien-mga commented Dec 7, 2023

I can confirm the issue, which is caused by a pathological case of a humongous tilemap with 1873319 16x16 tiles.

The more tiles are in view, the lower the FPS when moving the viewport.

With GL Compatibility, I get around 7 FPS when panning the view on a Radeon RX Vega M on Linux (Mesa 23.1). When I click the TileMap node and the "grid" gizmo is shown on a portion of the map, the FPS drops further to 1-2 FPS.

With Forward+, I get 1-2 FPS (or likely less) in both scenarios.

image

Increasing the rendering quadrant size from 16 to 128 doesn't seem to help.

@akien-mga akien-mga changed the title Horrible performance on Forward+ compared to compatibility (GPU Arc A770) Massive performance issue in 2D viewport when zooming out on huge TileMap Dec 7, 2023
@groud
Copy link
Member

groud commented Dec 7, 2023

Well, at some point there's kind of no way for to ensure good performance with an infinite amount of tile. Displaying 2 millions tiles at once should already be taxing on any hardware. I am not sure what should be the limit here, we can't chase performance improvement forever.

Here, I suspect the most taxing part is probably the grid drawing. We could probably disable it after a given zoom level or something, but users might complain their grid does not draw anymore.

@akien-mga
Copy link
Member

The grid should definitely be greatly simplified when zooming out, it makes no sense to have such a performance heavy fine mesh as far away zoom levels. It can be replaced by a simple rect probably to still show where the mouse focus is, if that's useful. And transitioned back to a grid when reaching a zoom level where it makes sense.

But even without grid I have 7 FPS. I wonder if some batching or picking low res mipmaps might help.

Alternatively, I was joking about it on chat, but at such zoom levels, it could make sense to replace the dynamically rendered tilemap by a single static texture... If it's going to take 1/7th of a frame to render it again, we can as well render it once to a ViewportTexture and apply it as a plain background texture. Of course this wouldn't allow editing the tilemap, but I don't think this should be supported at such zoom level. Zooming out is useful to navigate / get an overview of the whole map, it shouldn't be a problem if it's read-only.

@groud
Copy link
Member

groud commented Dec 7, 2023

Alternatively, I was joking about it on chat, but at such zoom levels, it could make sense to replace the dynamically rendered tilemap by a single static texture... If it's going to take 1/7th of a frame to render it again, we can as well render it once to a ViewportTexture and apply it as a plain background texture. Of course this wouldn't allow editing the tilemap, but I don't think this should be supported at such zoom level. Zooming out is useful to navigate / get an overview of the whole map, it shouldn't be a problem if it's read-only.

That is an interesting solution, but I fear that people might be annoyed if they cannot, like, copy-paste huge parts of the map. TBH, I feel like the editor is already a complex enough beast. If we can avoid additional checks about being at a given zoom level, that's better IMO.

So I think disabling the grid, or simply replace it by a localized grid around the cursor should probably be good enough for now. Then for further optimizations, I'd prefer we figure out a way to optimize the underlying layers (likely the rendering servers parts), rather than making more complex the Tilemap node and/or the TileMap editor.

But in general, I think we kind of have to set ourselves what we consider an "acceptable" level of performance anyway. Because with that kind of test projects, it's kind of easy to find a point where things would break (you simply have to add tiles until performance becomes bad 🤷 ). You can't really expect to have hundreds of millions of tiles and still expect things to work, whatever the game engine I mean.

Eventually, I'd like us to have some sort of a chunk system, for dynamic loading of big maps, but for now, users should replace that by splitting their TileMaps into smaller ones.

@SlyRebula
Copy link
Author

SlyRebula commented Dec 7, 2023

I think this is two different issues for me,
One "Massive performance issue in 2D viewport when zooming out on huge TileMap".
Second Compatibility renderer works better than Forward+ and I think that should be the opposite.

I tried to check and get more info on this using Mangohud to check fps, but it does not work with "Forward+".
It does work with "Compatibility" with --dlsym flag.

@akien-mga
Copy link
Member

Second Compatibility renderer works better than Forward+ and I think that should be the opposite.

Well this doesn't seem to be a bug. The Compatibility renderer is more lightweight, so it's faster. Advanced features found in the Forward+ renderer come at a performance cost.

@gokiburikin
Copy link

For what it's worth I poked around a bit and though I don't expect 2 million tiles to render very quickly, the performance is also very bad when even when zoomed in if the tilemap was selected.

The grid in this case is not heavy at all (tested by disabling the grid in the editor settings) but this block of code is extremely slow. Building a version of godot with it commented out makes zoomed in performance even with this large tilemap acceptable. Assuming the MRP above doesn't have invalid IDs I'm not even sure this block of code actually does anything apart from cause slowdown.

// Draw tiles with invalid IDs in the grid.
if (tile_map_layer >= 0) {
ERR_FAIL_COND(tile_map_layer >= tile_map->get_layers_count());
TypedArray<Vector2i> used_cells = tile_map->get_used_cells(tile_map_layer);
for (int i = 0; i < used_cells.size(); i++) {
Vector2i coords = used_cells[i];
int tile_source_id = tile_map->get_cell_source_id(tile_map_layer, coords);
if (tile_source_id >= 0) {
Vector2i tile_atlas_coords = tile_map->get_cell_atlas_coords(tile_map_layer, coords);
int tile_alternative_tile = tile_map->get_cell_alternative_tile(tile_map_layer, coords);
TileSetSource *source = nullptr;
if (tile_set->has_source(tile_source_id)) {
source = *tile_set->get_source(tile_source_id);
}
if (!source || !source->has_tile(tile_atlas_coords) || !source->has_alternative_tile(tile_atlas_coords, tile_alternative_tile)) {
// Generate a random color from the hashed values of the tiles.
Array a = tile_set->map_tile_proxy(tile_source_id, tile_atlas_coords, tile_alternative_tile);
if (int(a[0]) == tile_source_id && Vector2i(a[1]) == tile_atlas_coords && int(a[2]) == tile_alternative_tile) {
// Only display the pattern if we have no proxy tile.
Array to_hash;
to_hash.push_back(tile_source_id);
to_hash.push_back(tile_atlas_coords);
to_hash.push_back(tile_alternative_tile);
uint32_t hash = RandomPCG(to_hash.hash()).rand();
Color color;
color = color.from_hsv(
(float)((hash >> 24) & 0xFF) / 256.0,
Math::lerp(0.5, 1.0, (float)((hash >> 16) & 0xFF) / 256.0),
Math::lerp(0.5, 1.0, (float)((hash >> 8) & 0xFF) / 256.0),
0.8);
// Draw the scaled tile.
Transform2D tile_xform;
tile_xform.set_origin(tile_map->map_to_local(coords));
tile_xform.set_scale(tile_shape_size);
tile_set->draw_tile_shape(p_overlay, xform * tile_xform, color, true, warning_pattern_texture);
}
// Draw the warning icon.
Vector2::Axis min_axis = missing_tile_texture->get_size().min_axis_index();
Vector2 icon_size;
icon_size[min_axis] = tile_set->get_tile_size()[min_axis] / 3;
icon_size[(min_axis + 1) % 2] = (icon_size[min_axis] * missing_tile_texture->get_size()[(min_axis + 1) % 2] / missing_tile_texture->get_size()[min_axis]);
Rect2 rect = Rect2(xform.xform(tile_map->map_to_local(coords)) - (icon_size * xform.get_scale() / 2), icon_size * xform.get_scale());
p_overlay->draw_texture_rect(missing_tile_texture, rect);
}
}
}
}

sleepy_2023-12-08_04-10-39

@groud
Copy link
Member

groud commented Dec 8, 2023

The grid in this case is not heavy at all (tested by disabling the grid in the editor settings) but this block of code is extremely slow. Building a version of godot with it commented out makes zoomed in performance even with this large tilemap acceptable. Assuming the MRP above doesn't have invalid IDs I'm not even sure this block of code actually does anything apart from cause slowdown.

That's quite interesting. The problem may be the get_used_cells() call. It's doing a huge allocation, as it's available to the GScript API. There's no simple way there to expose an "Iterable".

I can have a look.

@groud
Copy link
Member

groud commented Dec 8, 2023

I investigated the issue a bit, and I know what is happening. What makes panning/zooming slow is the fact the TileMapEditor drawing has to fetch data from the TileSet when drawing things. Basically, it will have to check for invalid tiles every time you move the view, which is a bit time consuming as in needs to access the TileSet data, which is stored into an HashMap.

There are two issues:

  • fetching the data from the TileMap is relatively slow (using get_cell_source_id, get_cell_atlas_coords and get_cell_alternative_tile are HashMap accesses)
  • fetching the corresponding data in the TileSet is slow too (several HashMap accesses too).

I think that, to solve this issue, we should probably cache the data about the tile being valid or not somewhere. This is however hard to do without modifying the TileMap itself, which I'd rather avoid instrumenting for the editor uses only.
Another solution would be to draw the invalid tiles and grid on another CanvasItem/Control, that would be updated only when needed (basically only when the TileMap changes). But I am not sure it is possible, the drawing of editor things should be zoom-independent, so we would have to re-draw anyway I believe.

I think the best solution for now might be to find a way to simply cache whether or not invalid tiles exists in the TileMap, as a boolean. That could be done at the TileMap level without being too invasive I believe.

@atomar94
Copy link

atomar94 commented Dec 9, 2023

Edit: this is a separate issue #84591, thank you @akien-mga


I would like to add a quick data point. In 4.2 I am getting 1-2fps but in 4.1.3 I have >60fps.

Hardware: Macbook Air M1
Tilemap size: ~2000px x 2000px
Tiles on tilemap: 12
Tileset size: 1 sheet of 125.8 kbps for about 280 tiles

@akien-mga
Copy link
Member

@atomar94 That's a separate issue #84591. It should be fixed in 4.2.1 RC 1 already.

@SlyRebula
Copy link
Author

I have found a temporary solution to this problem it may not solve all the possible problems but it solves mine:
first, convert the 16x16 tile to a 160x160 tile(you can do this on aseperite by copying and pasting the 16x16 tiles next to each other) then create a 160x160 tile set and use this to create a very large map size if you want to use small tiles for decoration new type of grass, etc create a second 16x16 tile map and just place the tile you want as usual this way you can have a very large map and no lag.

@stuartcarnie
Copy link
Contributor

stuartcarnie commented May 13, 2024

The new ARG separates out the workloads to make it easier to visualise where the performance issues are. As I suspected, a massive amount of overhead is draw calls:

CleanShot 2024-05-14 at 07 38 29@2x

Note

On my MacBook Pro M1 Max, when zoomed out, I get about 4-5 FPS with Metal, and with MoltenVK I get < 1 FPS, which is a really great indicator of the lower overhead we will gain accessing Metal directly.

This makes me want to explore batching in 2D 😊

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

8 participants