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

Fix FileSystem dock won't show any file folders (reverted) #92650

Conversation

Hilderin
Copy link
Contributor

@Hilderin Hilderin commented Jun 1, 2024

Fixes #92335, Fixes #47053 and Fixes #85082

This PR improves:

  • FileSystem Dock not showing progress bar on first file scanning
  • Better progression (%) for scanning large projects with a lot of folders and files
  • Performance optimization on scanning project files for large projects
  • Add progression popup for _update_scan_actions step (instead of Editor freeze)
  • Add progression popup for _update_script_classes step (instead of Editor freeze)
  • Add progression popup for loading editor layout and reopening scenes (instead of Editor freeze)

I tested these modifications with a project having all the files from https://github.com/PMDCollab/SpriteCollab/tree/master/sprite (more than 125000 files and 385Mb).

Before:
FileSystem was empty without any feedback for a couple of minutes and after that the editor was just frozen:

image

After modifications:
image
image

Performance optimization on project scanning before asset importation
For a 30000 files project, it took around 2m50s on startup to scan the project.
After a couple of optimizations, it now takes 1m30s for the same project.

Before:
image

After:
image

Causes:

  • The FileAccess:exists was calling the open method which has a couple of overhead over file_exists.
  • The _scan_new_dir was double sorting the folders.
  • The reimport_files was recalculating the import_groupe_file even if _update_scan_actions did that just before.

** Edit: Add progression popup for loading editor layout and reopening scenes.

@Hilderin
Copy link
Contributor Author

Hilderin commented Jun 1, 2024

I need your advise... if you look at the issue #85082, one of the problem was that an empty scene was present during the loading.
With this PR, this empty scene is present but a progress bar is displayed in the FileSystem Dock. My concern is that the user can modify this empty scene and do a lot of things in the editor while the project file scan is in progress.

I can let this that way, at lease now, the user as a feedback from the editor.

An easy fix for that would be to run the first scan on the matin thread, blocking the user from interacting with the editor. Anyway, if there are files to reimport, the editor does it on the main thread. I could display the progression for the scan in a popup instead of the FileSystem, but the empty scene would still be visible.

I guess the perfect solution would be to not show the editor while loading, only progress popups, but I think was would need a lot more work?

@KoBeWi
Copy link
Member

KoBeWi commented Jun 8, 2024

I tested it with my project. I got FileSystemScan, then some new Scan Actions popup, then scene group scan popup and then an awkward 3 or 4 seconds freeze. Though on subsequent launches only the scan was performed, so I think other actions came from version difference.

With this PR, this empty scene is present but a progress bar is displayed in the FileSystem Dock. My concern is that the user can modify this empty scene and do a lot of things in the editor while the project file scan is in progress.

That's not really a problem, as the changes are not lost. If we really wanted to prevent that, the simplest solution is displaying a fully transparent Control over the whole editor that will block input.

core/io/resource_importer.h Outdated Show resolved Hide resolved
core/io/resource_loader.cpp Outdated Show resolved Hide resolved
doc/classes/EditorFileSystem.xml Outdated Show resolved Hide resolved
doc/classes/EditorFileSystem.xml Outdated Show resolved Hide resolved
editor/editor_file_system.h Outdated Show resolved Hide resolved
editor/editor_file_system.cpp Outdated Show resolved Hide resolved
editor/editor_file_system.h Outdated Show resolved Hide resolved
editor/editor_file_system.h Outdated Show resolved Hide resolved
editor/editor_file_system.cpp Outdated Show resolved Hide resolved
editor/editor_file_system.cpp Outdated Show resolved Hide resolved
editor/editor_file_system.cpp Outdated Show resolved Hide resolved
@Hilderin Hilderin force-pushed the fix-fileSystem-dock-wont-show-any-file-folders branch from 7a4c9bc to 94a862f Compare June 8, 2024 22:46
@Hilderin
Copy link
Contributor Author

Hilderin commented Jun 8, 2024

Thanks for the review. I definitively still need time to adjust to Godot standard. I'll try to be more careful next time.
All requested changes should be effective now.

@Hilderin
Copy link
Contributor Author

Hilderin commented Jun 8, 2024

@KoBeWi thanks for testing this PR.

I tested it with my project. I got FileSystemScan, then some new Scan Actions popup, then scene group scan popup and then an awkward 3 or 4 seconds freeze.

I'm intrigued with the 3 or 4 seconds freeze, it's sounds like what @Zireael07 or @smix8 was explaining on the first load in #47053. Do you have the freeze after a reboot or the next day? I guess I can't have access to your project? Maybe you can explain to me briefly the amount and type of resources you have in your project so I can try to reproduce. Are you on Windows, Linux or Mac?

@KoBeWi
Copy link
Member

KoBeWi commented Jun 8, 2024

It might be related to plugins, autoloads and global scripts. Especially plugins, as I have many of them. Though it was a one-time thing, I'll see if it repeats tomorrow and report back.

@KoBeWi
Copy link
Member

KoBeWi commented Jun 9, 2024

I tested again and it still happened, but only once. Though it's not the same as reported by Zireael and smix, because it's much shorter. I think the original issue with unresponsive editor is covered by the new scan progresses, the remaining time can also come from restoring scenes.

I can send you the project if you want to test it, but it's quite big. DM me on the chat.

@Hilderin Hilderin force-pushed the fix-fileSystem-dock-wont-show-any-file-folders branch from 94a862f to 6e27a78 Compare June 9, 2024 20:33
@Hilderin
Copy link
Contributor Author

Hilderin commented Jun 9, 2024

After testing with the project from KoBeWi, I found 2 problems:

First: Update scene groups was triggered during reimportation because the ResourceImporterCSVTranslation is calling ResourceLoader.save() which trigger update_files. I added a condition to prevent updating script and scene groups during importation. It is done at the end of first scan.
That was causing a double progress:
image
image

Second: As KoBeWi suspected, the froze editor was caused by the reopening of the scenes. On my PC, it takes 8 secs processing the first sources_changed caused by EditorNode::_load_editor_layout. I added another progress popup to prevent the frozen effect.
image
image

That is what is looks now:

Loading.process.mp4

@Hilderin Hilderin force-pushed the fix-fileSystem-dock-wont-show-any-file-folders branch from 6e27a78 to e4a9303 Compare June 10, 2024 22:18
@Hilderin Hilderin force-pushed the fix-fileSystem-dock-wont-show-any-file-folders branch from e4a9303 to 72856d6 Compare June 10, 2024 23:57
@Hilderin
Copy link
Contributor Author

Just rebased following conflicts.

@akien-mga akien-mga merged commit 0a9f2d2 into godotengine:master Jun 11, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@akien-mga
Copy link
Member

Apologies, I jumped the gun on this PR which touches very core parts of the engine, prone to regressions and subtle bugs. I trusted existing reviews and forgot to assess how risky the changes are myself.

I think this should be kept for 4.4 as there will be likely be a number of follow-up issues that will be best handled in the 4.4 dev cycle than now as we try to stabilize 4.3.

So I'll revert the merge for 4.3, and would suggest reopening a PR with the same changes, that will target 4.4 and require more testing before merge.

@akien-mga akien-mga changed the title Fix FileSystem dock won't show any file folders Fix FileSystem dock won't show any file folders (reverted) Jun 11, 2024
@Hilderin
Copy link
Contributor Author

No problem @akien-mga , I'll reopen later!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment