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

Warning about upgrading mesh format to 4.2 is unclear about next steps, and there's no visual feedback when the conversion takes a long time #83287

Closed
noidexe opened this issue Oct 13, 2023 · 6 comments · Fixed by #83613

Comments

@noidexe
Copy link
Contributor

noidexe commented Oct 13, 2023

Godot version

v4.2.beta.custom_build [b137180]

System information

Godot v4.2.beta (b137180) - Windows 10.0.19045 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 2060 SUPER (NVIDIA; 31.0.15.3758) - AMD FX(tm)-8350 Eight-Core Processor (8 Threads)

Issue description

I'm opening a large 3d project and Godot was unresponsive for several minutes.

The console showed the following:
image

I do get the warnings in the debugger but only when the editor unfreezes itself.

I understand this could be considered an enhancement but to me it seems like a UX bug. Launching the editor without a console has been possible for a while and it's the default on platforms like Steam. For non-console users the experience is an unresponsive window with no idea of what's happening. If this were intended behavior there would be no output on the console either.

Steps to reproduce

  • Launch a project created in a version before 4.2 including a large amount of meshes

Expected: I get some sort of progress bar in the editor while the meshes are converted to the new surface format, as well as the warning about backwards compatibility

Actual: the editor hangs for minutes. No visual indication of what it's doing

Minimal reproduction project

N/A

@clayjohn
Copy link
Member

I'm not sure that there is much we can do here. The engine upgrades the mesh at load time. It doesn't iterate over the project and auto detect meshes to upgrade or anything. So it can't exactly provide a loading bar or anything.

But I understand the issue as updating the meshes can take some time.

Did you not get toasts popping up as well? The warning should display in a toast

@noidexe
Copy link
Contributor Author

noidexe commented Oct 13, 2023

Only after it's finished. The editor looks completely unresponsive during the process, at least on Windows
image

When it's finished I see toasts and I see all the warning spam in Output:
image

@Calinou
Copy link
Member

Calinou commented Oct 13, 2023

We can change the window title while meshes are being upgraded, but not much more can be done if the editor is unable to display toasts or a progress dialog.

@noidexe
Copy link
Contributor Author

noidexe commented Oct 13, 2023

Well if there is no other way around it I guess any indication of progress is better than none. At least you know the editor didn't hang up due to an error

@akien-mga
Copy link
Member

akien-mga commented Oct 18, 2023

We discussed this on the Godot contributors chat today (#general). I'll reclassify this as a bug as after discussion the UX flaws make it really difficult to deal with this upgrade, so we should solve this before 4.2.

Some takeaways about the problem from that discussion:

Problem statement by @akien-mga:

  • The warning isn't super clear about the fact that it's converting the format in memory, and that things need to be saved to commit it to disk.
  • 400 warnings for 400 unidentified meshes aren't super useful, and they're actually a performance drag (and no, hiding warnings or grouping them isn't a solution, it's just a workaround)
  • Even when understanding the issue, what to do is not trivial. Resaving all meshes is not a simple operation if you have some saved as .res.

And by @clayjohn:

  • Let's improve the warning message, that's an easy first step
  • The format conversion mostly happens inside the rendering server, so we have no idea what file it corresponds to (or even what mesh)
  • The warning may take time, but most likely the slow part is the actual format conversion which is happening every time the mesh is loaded (until the mesh is saved or re-imported)

Suggestion for a solution by @clayjohn:

  1. We add a callback into the function that does the surface upgrade
  2. The callback triggers a popup that tells the user an old format was detected in the project and asks the user if they would like to automatically upgrade (and save) all meshes, or if they would like to keep the meshes saved in the old format
  3. If the user selected upgrade, then a script like KoBeWi's is run over the project and all meshes are loaded and saved (and all scenes re-saved I guess). This would need to include imported meshes as well.
  4. If the user selects not to upgrade, then the popup closes and the user gets what currently happens now (i.e. a warning on every mesh, meshes are upgraded in memory, but not saved)

Editor script by @KoBeWi that can be used to resave all resources (might need tweaking to handle .res and .mesh) to get rid of this warning easily:

@tool
extends EditorScript

var files: Array[String]

func _run() -> void:
	files = []
	
	add_files("res://")
	
	for file in files:
		print(file)
		var res = load(file)
		ResourceSaver.save(res)

func add_files(dir: String):
	for file in DirAccess.get_files_at(dir):
		if file.get_extension() == "tscn" or file.get_extension() == "tres":
			files.append(dir.path_join(file))
	
	for dr in DirAccess.get_directories_at(dir):
		add_files(dir.path_join(dr))

Edit: I realize I partially hijacked this issue which was solely about the conversion taking a long time without visual feedback. We should aim at solving both problems at once, but if we don't we can split it further into two issues.

@akien-mga akien-mga added bug and removed enhancement labels Oct 18, 2023
@akien-mga akien-mga added this to the 4.2 milestone Oct 18, 2023
@akien-mga akien-mga changed the title No visual indication of surface format update Warning about upgrading mesh format to 4.2 is unclear about next steps, and there's no visual feedback when the conversion takes a long time Oct 18, 2023
@clayjohn
Copy link
Member

clayjohn commented Oct 18, 2023

A quick update from my first attempt at implementing the above:

  1. The callback was easy to add
  2. Resource loading is multithreaded. So while the popup is open, the rest of the meshes continue to load leading to the same freeze as before
    3. If the script loads and resaves all the files, the editor thinks the files have been modified externally and prompts the user to reload or resave (fixed)
    image
    4. I modified the script to look for tscn, tres, and res, but I dont think it considers imported meshes, I think more is needed in order to read and save imported meshes as well (as we can't just resave the gltf for example, we need to load and re-save the imported model) (fixed)

Overall, I'm a little wary about the loading side of things. Ideally we would be able to pause the loading of the project until the popup is dealt with, but I am concerned that we don't have any non-hacky way of doing that

Open questions that I don't know the answer to:

  1. Do we have a way to pause the loading process?
    2. Do we have a convenient way to iterate over imported assets? (we must have something for export time right?)
    3. How do we communicate to the editor that we were the ones to re-save a resource (or maybe we should just restart the editor after upgrading?)

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

Successfully merging a pull request may close this issue.

4 participants