Fix crash when reimporting AtlasTextures #60888
Conversation
EspeuteClement
changed the title
EspeuteClement
changed the title
|
So after further testing, it seems that adding for (int i = 0; i < reimport_files.size(); i++) {
+ if (groups_to_reimport.has(reimport_files[i].path)) {
+ continue;
+ }
if (use_threads && reimport_files[i].threaded) {to the |
|
After checking the code, files that are in groups are reimported at the end of the function, so there is no point in reimporting them when importing all the files. I've added the proposed changes in a new commit. |
|
I have added another fix for the importing of group files that avoid them being re-imported at each tick of the editor because they didn't have valid uids |
editor/editor_file_system.cpp
Outdated
| @@ -2121,6 +2144,11 @@ void EditorFileSystem::reimport_files(const Vector<String> &p_files) { | |||
|
|
|||
| int from = 0; | |||
| for (int i = 0; i < reimport_files.size(); i++) { | |||
| if (groups_to_reimport.has(reimport_files[i].path)) { | |||
| //skip reimporting files that are in groups, as they will be reimported anyways | |||
There was a problem hiding this comment.
Suggested change:
// Skip reimporting files that are in groups, as they will be reimported anyways.
There was a problem hiding this comment.
I based this comment style to match the other ones in editor_file_system.cpp, but yes they don't match the comment style guide at https://docs.godotengine.org/en/stable/community/contributing/code_style_guidelines.html#comment-style-guide
editor/editor_file_system.cpp
Outdated
| uid = ResourceUID::get_singleton()->create_id(); | ||
| } | ||
|
|
||
| f->store_line("uid=\"" + ResourceUID::get_singleton()->id_to_text(uid) + "\""); //store in readable format |
There was a problem hiding this comment.
Suggested change:
f->store_line("uid=\"" + ResourceUID::get_singleton()->id_to_text(uid) + "\""); // Store in readable format.
There was a problem hiding this comment.
I copy/pasted this line from somewhere else in the file that had the same comment, but I can update this one to match the comment style guide yes. I'll do that later today
The crash was caused by the reimport_files function trying to reimport the atlas file multiple times using multithreading, which caused deadlocks and hang the editor. This fix prevents files from being added to the reimport_files array if they are already present inside of it. Fixes godotengine#54817
Files that are groups of other files will be reimported anyways at the end of the function, so there is no point in re-importing them before they are processed
This fix the concerned files being reimported at each tick of the editor
|
I updated the pr to the latest master commits, and added the suggested changes to comment styles |
|
Closing as superseded by #68324 |
The crash was caused by the reimport_files function trying to reimport the same atlas file multiple times using multithreading, which caused deadlocks and hung the editor.
This fix prevents files from being added to the
reimport_filesarray if they are already present inside of it.Fixes #54817
Note : I have only tested this fix on a sample project. This could potentially break more complex projects, but I'm not enough familiar with the godot import system to tell if that is the case. Someone knowledgable with that system should review this before merging.
Additionaly, I don't know if files that are both as key in
groups_to_reimportand in thereimport_filesarray should be reimported when handling thereimport_filesarray. I left it as is, but maybe these files end up being reimported twice.