Changing Hierarchy of or renaming Scene Nodes in a complex inherited scenes setup causes a 60+ second freeze

robcbryant commented 6 months ago

Tested versions

Reproducible in: v4.3.beta1.official [a4f2ea91a], 4.3-dev6

System information

Windows 10.0.19041 - Vulkan (Mobile) - dedicated AMD Radeon RX 5600 XT (Advanced Micro Devices, Inc.; 31.0.21029.6002) - AMD Ryzen 7 3700X 8-Core Processor (16 Threads)

Issue description

I use a lot of inherited scenes from a base scene that contains a lot of complex tilemap settings (so I don't have to redo the settings every new scene). With a loaded inherited scene, if I change the node hierarchy(e.g. reparent a node to a new node) or rename any node, the editor will freeze for roughly 60+ seconds and then be fine again. Not sure what's causing it.

A non-inherited scene doesn't seem to be affected by this 60 second hangup behavior and behaves as expected when renaming nodes or changing the node hierarchy.

Currently using v4.3.dev4.official [df78c0636] build because updating to any newer 4.3 dev or beta builds causes this functionality to break. Issue does not exist in dev4 or previous versions.

One possible contributing factor is that I have roughly 60 scenes all inheriting from a single base scene template. This number of inherited scenes may be causing some yet undefined slowdown behind the scenes with dev6 and beta1 engine builds.

Only tested with 2D Node scenes--not 3D, if that's helpful

Thanks!

Steps to reproduce

Make a scene, then make a new inherited scene, save it and then edit the hierarchy or rename child nodes.

Minimal reproduction project (MRP)

N/A

akien-mga commented 4 months ago

Can you still reproduce the issue in 4.3.rc1?

If so, could you make a minimal reproduction project that exhibits the issue?

robcbryant commented 4 months ago

Let me download and test it in a minute!

robcbryant commented 4 months ago

Can confirm it's still the same issue in 4.3.rc1

Non inherited scenes work fine, but changing the hierarchy of an inherited scene does not. I attempted to replicate the issue with a new project by making ~20 inherited scenes from a base scene, and they're all fine--no hangups or lag. One issue with making a new project though is that I can't simply recreate the complexity of my scenes for a test project. It's far too much work. Each inherited scene might have 100 nodes or so in different arrangements and the tilemap nodes in them will have a lot of painted data associated with them (1000s of tiles).

My only gut reaction to this is that because it's from an inherited scene and not a new scene, there's some upper limit to the engine's ability to parse all the scene files when trying to manipulate complex scene that inherits from a complex scene.

Some new tests I did:

Opening a new fresh inherited scene in my project before it's saved (when it says unsaved ) -- has the same issue. If I try and rename the top node in the hierarchy it will freeze for a minute or so.

Opening the original scene the others inherit from and changing the node structure or renaming a node also hangs up the editor for around a minute.

The one thing that can be done is to add a new node. Adding any new node doesn't effect the editor--only when that new node is moved or renamed.

akien-mga commented 4 months ago

Thanks for testing. Without a reproduction project it's harder for us to debug, but I fully understand that in this case it seems difficult to make one, as the slowdown seems tied to the complexity of the project. So we might need more work from you to identify what causes the slowdown, if you're willing.

Some things which could be useful:

Can you confirm that 4.3.dev5 is the first release that causes the slowdown? You mentioned staying on dev4 and having the issue in dev6 but didn't mention dev5.
If you have a setup for compiling C++ (Visual Studio or MinGW), you could try to compile Godot from source to bisect the regression between the commits of the last good (dev4, df78c0636) and first bad (dev5, c9c17d6ca) snapshot, to find the exact commit which caused the issue. The problem is likely a performance bottleneck which might be easy to identify if we can review the commit which caused it.
Alternatively, if you're familiar with a C++ debugger, you could compiling Godot from source (latest master) with debug symbols (scons debug_symbols=yes), and when the editor seems frozen, break into the process with the debugger to identify where it's spending all this time, which might also help narrow down which commit caused the issue and which section of the code is problematic.

CC @KoBeWi

robcbryant commented 4 months ago

I did not test dev 5--Just did now--interesting, so with 4.3 dev 5 it still stalls---but for only about 15-20 seconds rather than like 45+ seconds.

I don't have a setup for visual studio right now, but I can look into it. I haven't done c++ compiling in ages.

It might take me a few days though.

KoBeWi commented 4 months ago

Can't reproduce it. Some test project, with clear instructions on how to cause the bug, would be helpful.

robcbryant commented 4 months ago

capture.csv Very Sleepy Monitor save file AND callgrind format.zip

Here's the debug monitor from the latest Engine source I built with debugging on. I used Very Sleepy because I'm on windows. Not sure what format works best (I'm not great at reading these things) so I just included them all. Sleepy's native format and callgrind format are in the zip file.

The first few seconds are the last bit of it loading the project, then I wait a few seconds of idling, and then try renaming a node in an inherited scene and it immediately halts and freezes the editor. While monitoring it seems to double the time it takes for the editor to unfreeze.

I'm also currently building a series of different commits to try and figure out the specific commit culprit. Might take a few hours or more

EDIT: If it's helpful here are the profile statistics: Duration: 177.906 Date: Thu Aug 01 09:53:47 2024 Samples: 489744

robcbryant commented 4 months ago

My project is currently stuck on rc1.

I moved to a laptop with a fresh git clone of my project for dev4 and none of the source builds I tried making in the bisect searching of the bug will even open my project because it can't import any of the resources--it just gives an error to the resources not existing and therefore cannot load in the console. It will do this indefinitely and then eventually crash out.

Unfortunately I have no way of testing any of my builds because my project just won't open with them.

I looked for documentation on what may be the case for this but I couldn't find anything.

rc1 did something to my core project files that seems to completely ignore my git versioning. I'm having trouble reopening my project in dev4 now. On my laptop--the same project experiences the same issue in the just released rc2. I'm currently trying to get my project to work on dev 4 again.

Willing to hear any advice on why Godot seems to be ignoring any github reversions/resets of my project to commits made on dev4.

akien-mga commented 4 months ago

Make sure to delete the .godot folder after resetting to the desired Git commit.

robcbryant commented 4 months ago

I've tried that method multiple times--sometimes it works, and sometimes it absolutely will not work and just gives an assortment of errors. the .godot folder isn't part of my GitHub project repository so none of the commits I manually download to test will load/import reliably if at all.

EDIT 1:

I get a neverending stream of errors of missing files, sometimes saying it needs to be opened in the editor before it runs (which it is being opened in the editor) and then often it reaches a point where it's trying to import tilesets and gives the nonsense below:

`ERROR: The TileSetAtlasSource atlas has no tile at (29, 30). at: (scene/resources/2d/tile_set.cpp:5400) ERROR: TileSetAtlasSource has no tile at (29, 30). at: (scene/resources/2d/tile_set.cpp:5348) ERROR: Cannot create tile. The tile is outside the texture or tiles are already present in the space the tile would cover. at: (scene/resources/2d/tile_set.cpp:4963) ERROR: The TileSetAtlasSource atlas has no tile at (30, 30). at: (scene/resources/2d/tile_set.cpp:5400) ERROR: TileSetAtlasSource has no tile at (30, 30). at: (scene/resources/2d/tile_set.cpp:5348)

EDIT 2: If I remove all of my scenes with tilemaps then open it as a new project in the editor, it will go through the errors of loading resources for all the objects but then open. If I then add the scenes with tilemaps it will have no issues. Something about the UIDs seems broken on loading a project without a .godot folder.

akien-mga commented 3 months ago

Could you check 4.3.rc3 to see if things improved by any chance?

Given your problem seems linked to TileMaps, I'm not too hopeful as there haven't been recent changes there, but it's worth testing anyway.

robcbryant commented 3 months ago

Sorry--I've been super busy and distracted so haven't had time to continue investigating.

I'm thinking it might be linked to how massive and complex my tilemaps are--thousands of tiles spread across multiple layers--I use multiple ones as well: Tilemap for Collision Tilemap for static art layers 1-2 tilemaps for background parallax 1 tilemap for foreground parallax

They all share the same core tileset defined in the original parent inherited scene that has about 10 different imported sprite maps that are each 1024x1024 in size sliced into 16x16 pixel tiles (It's a LOT of art). Looking at the raw scene in notepad+ is staggering lol.

Last thing I checked on my laptop--I got my build to work finally on it using the same godot version (dev4) and it seemed to be experiencing the same issue that doesn't happen on my main desktop using dev4 so I'm at an absolute loss

Only thing I can think to do when I have time is to continue trying to understand the profiler on the debug build I made on y'alls recommendation.

akien-mga commented 3 months ago

I'm bumping this to 4.4 as the 4.3 release is imminent, and I don't think we'll have the opportunity to fix this in time, as this is difficult to reproduce. It's still a nasty performance regression we should investigate and, once fix, we can backport the fix for a 4.3.x release.

robcbryant commented 3 months ago

Totally Fair! Thanks for trying at least on 4.3. I'll see what I can do when I have time to work on it again using the debug profiler.

Praytic commented 3 months ago

issue-92612.zip I have prepared my MRP for this issue. It contains a single tileset which was broken after auto-migration from 4.1 to 4.3 release. When you run the project you should see the following errors:

...
ERROR: Cannot create tile. The tile is outside the texture or tiles are already present in the space the tile would cover.
   at: create_tile (scene/resources/2d/tile_set.cpp:4963)
2024-09-01 19:40:27.029062-0700 godot.macos.editor.arm64[42807:6147515] ERROR: The TileSetAtlasSource atlas has no tile at (2, 1).
at: has_alternative_tile (scene/resources/2d/tile_set.cpp:5415)
ERROR: The TileSetAtlasSource atlas has no tile at (2, 1).
   at: has_alternative_tile (scene/resources/2d/tile_set.cpp:5415)
2024-09-01 19:40:27.029082-0700 godot.macos.editor.arm64[42807:6147515] ERROR: TileSetAtlasSource has no tile at (2, 1).
at: create_alternative_tile (scene/resources/2d/tile_set.cpp:5363)
...

Zireael07 commented 3 months ago

@Praytic I think you mixed up the issues - this is about freezes while renaming, not about a tileset being broken

c6ryl commented 2 months ago

Hi there,

I have the same issue, everytime i rename, delete or change the position of node in the scene tree i have to wait between 30scd to a minute.

I also had huge load time when starting Godot or executing the game. Ater investigations it was caused by the tilesets i was using. Seems they were too big with too many elements.

First i renamed the file from .tscn to .scn as suggested in an other post, in order to improve performance. It did improve a little but not enough. Finally i reduced the sizes of my tilesets with aseprite and reimported only what i needed. So far it seems to have worked beause my loading time improved EXCEPT when moving nodes in the scenetree.

If anyone has suggestion or if @Praytic you managed to solve it i would be happy to know.

robcbryant commented 2 months ago

@c6ryl I still haven't had the time to solve this on my end (the person who originally brought up this issue)

I did do some quick tests with your suggestions and found that closing all the open scenes with tilemaps made running the game player significantly faster. I've been working on intro cinematic scenes as of late so don't need the tilemap scenes open. Leaving them open in the editor was causing like 60 second load times to run the game. Removing them and I'm back to running my other scenes in a few seconds.

There's something related to tilemaps in general I think that's causing these slowdowns if the tilemaps are large enough (and all of mine are quite large--probably like 5-6 1024x1024 images used for all the in-game world assets. There's no affect on in-game performance, just in-editor performance.

Still can't upgrade to 4.3 yet lol

robcbryant commented 4 weeks ago

So I had some time to make some additional headway.

I think the underlying issue just isn't something worth fixing but it opened up a new issue. Essentially using the old tilemap layers on inherited scenes that are complex in the new 4.3 system just slows the system down immensely. Fixing that actually doesn't make much sense because they are being deprecated anyhow.

When trying to convert (using the built-in editor tools) the old deprecated tilemap layers to the new separated tile layer nodes--all scenes that inherit from this parent node lose all previously painted tilemap layer work. They receive a new 4.3 tilemap layer series of nodes, but anything not in the original parent node they inherit from is lost / erased. Obviously not ideal. One temporary solution is to open each scene individually and manually convert their inherited tilemaps to the new layer system--but then this breaks the inheritance.

I'm not sure if I should submit this as a new issue or not--I found someone else experiencing this issue below and it was suggested to open an issue to the original author, but I'm not sure about the status on that. https://github.com/godotengine/godot-proposals/issues/10340#issuecomment-2320220062

So in conclusion--I think we can close this issue as a simple matter of don't use complex inherited scenes involving dense deprecated 4.2 tilemaps in Godot 4.3--but until we can successfully convert those deprecated tilemaps to the new layer node system that works with inherited scenes, there isn't a path forward beyond 4.2 for existing projects making heavy use of scene inheritance and tilemaps.

robcbryant commented 4 weeks ago

As a side note--anyone wanting to do this--it's very important to have backups, because once the base parent scene the children are inheriting from runs the tool to auto convert the tilemaps into distinct node layers--you can't CTRL - Z the downstream effects--and all that work will be absolutely lost. Making changes to the parent scene will obviously affect the data of the children for the project. To test this again I would have to re-clone my own repository again.

robcbryant commented 4 weeks ago

One solution that seems to work and I'm going to spend the day working on and post if it solves the hierarchy freezing bug:

Converting doesn't delete the original tilemap--so my solution will be tedious but seems possible. For all 60ish inherited scenes that I have, I will have to manually go into each one--run the converter on the still existing inherited 4.2 old tilemap node, select all/copy paste those painted tiles into the new inherited 4.3 layer. Once done for all the scenes, I'll delete all instances of the 4.2 tilemap nodes and see if that is precisely the slow down issue. Gonna take several hours though. Will post my progress later today.

robcbryant commented 4 weeks ago

UPDATE:

Unfortunately--despite painstakingly editing all ~60 scenes 4.2 version tilemap nodes and converting them into 4.3 tilemap layer nodes--I'm still experiencing the minute long slow hang ups when modifying the scene hierarchy or renaming any of the nodes. This only happens in scenes that are either inherited from a base scene--or the base scene the other ~60 scenes with tilemaps are inheriting from. Other scenes that have no inheritance don't suffer from this problem. To reiterate--I can create new nodes and save these inherited scenes as fast as one would expect--but as soon as I move a node in the hierarchy or rename a node, the entire editor freezes for a minute.

I was hoping the elimination of any older deprecated 4.2 tilemap stuff would work. But it does not.

I'll try a few new tests tomorrow but I'm running out of steam today. Although Godot didn't suffer this issue in 4.2 despite the sheer number of inherited scenes with high numbers of tilemap layers--it might be the culprit with 4.3 because something was recoded. E.g. -- my base scene the rest inherit from has about 28 tilemap layer nodes now and each tilemap node layer is tied to a tileset resource that has 7 1024x1024 spite atlases.

KoBeWi commented 4 weeks ago

You could try running C++ profiler if you can make a custom engine build. If not, any chance you can share your scene setup?

robcbryant commented 4 weeks ago

You could try running C++ profiler if you can make a custom engine build. If not, any chance you can share your scene setup?

I uploaded some profiles in a previous comment in the thread a couple months back after building a c++ profile debug build of 4.3rc1 . I'm not skilled enough to understand them to be honest lol but I'll try again tomorrow.

I'll try and put together a github project tonight/tomorrow that replaces all my art/sprite atlases with and other assets with redacted stuff.

I want to do a couple more tests like deleting 40 of the inherited scenes to see if that makes a difference as an additional way to hone in on the issue. I'm still not sure if it's the inheritance that's the issue or the complex tilemaps being IN the inherited scenes that are the issue.

robcbryant commented 4 weeks ago

In the mean time-- here's a screengrab of the base "level_basic" scene that the other 60 scenes inherit from. This scene acts as a master copy of the different tilemap layers / tileset atlas that the other scenes use as a template. I did it this way because if I want to edit the physics layer or other properties of an individual sprite, I need only edit it on the parent scene, and the 60+ scenes (levels) that inherit from it auto update the tileset as needed, but keep their tilemaps unique. There might be a more efficient way of doing this, but I only pointed out the issue because regardless of efficiency--it was working fine before 4.3.

More details as i can get some testing done.

robcbryant commented 4 weeks ago

AWESOME!

Success--I figured it out. It has nothing to do with inheritance--but rather with tilesets.

I took my most complicated scene and dis-inherited it from the master scene. I took my smallest scene and disinherited it from the master scene. Both were still having the issue until I deleted enough tilemap layers--for instance: This smaller scene would rename immediately or move nodes about the hierarchy when I deleted enough of the tilemap layers.

The larger scene which had ~25 tilemap layer nodes required several to be deleted before a noticeable effect--so yeah not a great solution obviously--but it led me to the actual problem with the 4.3 update. Before--in 4.2 all of the tilemap layers were using 2 tilesets because I created 2 separate ones--one tileset for physics collisions, and one without physics collisions. When converting the project to some version of 4.3--these tilemaps were losing their shared tileset resources, and each one was instancing a separate tileset. So basically--a scene would have 25 tilemap layer nodes--each with a large/complex unique instance of a tileset. When I manually saved 2 .tres resource files for the tilesets and quick loaded them into the 20+ tilemap layer nodes so all the nodes were once again sharing only 2 tilesets instead of 25 unique ones--things work perfect as before.

So in summary--something in a pre-stable 4.3 release around dev 4; when reading 4.2 tilemap layers is removing the shared instances of tilesets and making them all 'unique.' Because I didn't have them saved as actual .tres resource files in the file system, I would CTRL-D duplicate the 4.2 tilemap layer if I needed 2 or 3 separate ones (sometimes for parallax layers and other reasons) and they would automatically share the same instanced tileset because they were created through CTRL-D. I suspect had I made them .tres files BEFORE upgrading to 4.3, it would have kept the shared tileset reference (but it's possible it wouldn't).

So the solution was after a fresh clone of my earlier work on manually converting things to 4.3 tilemap layer nodes (which should probably be wrapped into a different GIT issue if it doesn't exist as one already) to go back and manually create 2 .tres tileset files and edit all of the tilemap layers of the parent of the inherited scenes and ensure they're using the same 2 .tres resource instances of my tilesets. I've looked at the raw text files of these before and they get quite large if you have several very large 1024x1024 atlases cut into 16 pixel sprites. Having 25-30 needlessly 'unique' large tilesets was bogging Godot down -- not the tilemap layers themselves and not the inheritance.

My 'solution' doesn't solve the underlying problem in that Godot won't properly automate this process in the conversion from 4.2 to 4.3, but I at least finally managed to identify exactly where the problem is occurring.

I've confirmed multiple times with a couple of cloned repositories now and doing this fixes it every time. They'll work as intended even if I make 50 tilemap layer nodes in a single scene--as long as they're all using the same 2 tileset resource files and not 25 unique ones. I would also point out this is probably unique to my project--and I think many smaller 2d projects wouldn't be as affected by this. One of my tilesets alone (because it's seven 1024x1024 16 pixel sprite sheets with physics/and custom data nodes for each sprite) is roughly 120K lines of text in the human readable .tres file. When I have 25 copies of this being made unique, Godot is suddenly having to update the scene file with 25 X 120k lines of code anytime a child of the parent or the parent is directly manipulated. If another developer is using much smaller and far less complex tilesets--I doubt they would have hit this brick wall that I did.

I'm just glad to have figured it out. I'm not sure if there is a solution for this other than to warn developers there does seem to be an upper limit to the number of unique tilesets you can have in a single scene within the editor. To reiterate--it seemed to have nothing to do with being an inherited scene or being a parent of an inherited scene--just the number of unique large tileset resources in a single scene.

This issue was also caused somewhere in the import/conversion process as outlined above, where Godot 4.3 will not properly maintain the shared tileset resources among tilemap layers making them all unique resources. (I did not test if it would successfully do this if the tilesets were pointing toward an existing .tres tileset file). Regardless it seems to be much better practice to keep the tilesets as a separate resource file that a scene can reference, rather than wrapping them INTO the actual scene file which bloats the scene files size and time needed to save / manipulate it.

Let me know if you have any other questions!

Zireael07 commented 4 weeks ago

TLDR: Having 25-30 needlessly 'unique' large tilesets was bogging Godot down -- not the tilemap layers themselves and not the inheritance.

In which case I am 99% sure it was already reported.

KoBeWi commented 4 weeks ago

I wonder if #53679 would help in this case. I assume your scenes were very big when it happened.

Also we should probably disallow built-in TileSets. They are super big and bloat the scenes, unless (unlikely) you only use very simple ones. It makes little sense to have a TileSet unique to one scene anyway.

robcbryant commented 4 weeks ago

I wonder if #53679 would help in this case. I assume your scenes were very big when it happened.

Also we should probably disallow built-in TileSets. They are super big and bloat the scenes, unless (unlikely) you only use very simple ones. It makes little sense to have a TileSet unique to one scene anyway.

The parent scene was very large yes, because it contained 2 complex tilesets, by default the scenes inheriting from it had to navigate that inherited scenes file when trying to save or manipulate themselves. When it ONLY contained 2 tilesets--in hindsight this was a bad idea--it still worked fine. When converting to 4.3 --something happens with trying to read a complex tileset from a .tcsn file that brings the system to a halt (and also something that in some cases seems to make duplicate tileset instances in memory--but I can't be 100% sure).

Regardless--I agree that the solution should be to disallow built-in TileSets for a scene file for 4.3 moving forward and force them to be separate resource files which solved all of my issues immediately. I think we can close this issue now with this solution in mind ?

Thanks for everyone's patience on this btw.

godotengine / godot