GLTF: Don't give up loading a texture if importing it fails

[aaronfranke]

This fix is needed for #82455, but this PR does not completely solve that issue.

In the current master, if in the editor and the import settings are set to extract textures (default), the code in GLTFDocument will fail if it cannot extract and import a texture, resulting in that texture being missing from the model.

This PR is a fairly straightforward change: If we can't import, just load it uncompressed from the Image resource already loaded in memory, instead of giving up and inserting a blank texture.

More details: For each case where the image cannot be imported from res://, instead of setting a blank texture, use the loaded image. The return in this function now only happens after successful cases, so as a fallback, it will drop down to the bottom of the function, the uncompressed code. This is the same as if performing a runtime import. When writing this code I was curious if it would work if the project is exported, and actually yes, it does work.

As a bonus, I also added some text to two err_fail_conds in EditorFileSystem to make them distinct from each other.

[lyuma]

In my opinion p_state->base_path.is_empty() case should assign a name (such as a number) and extract it, not load it uncompressed.

this should be the same as a glb file with embedded unnamed textures, which also should get extracted.

[aaronfranke]

@lyuma I'm assuming you mean the p_image->get_name().is_empty() condition. If the base path is empty, we can't extract, because we have nowhere to save it. I've updated the PR. Old PR code:

if (p_state->base_path.is_empty()) {
	WARN_PRINT("glTF: Couldn't extract image because the base path is empty. It will be loaded directly instead, uncompressed.");
} else if (p_image->get_name().is_empty()) {
	WARN_PRINT(vformat("glTF: Image index '%d' couldn't be named. This cannot be saved to a file. It will be loaded directly instead, uncompressed.", p_index));
} else {
	bool must_import = true;
	// and so on

Code in this PR with your suggestion:

if (p_state->base_path.is_empty()) {
	WARN_PRINT("glTF: Couldn't extract image because the base path is empty. It will be loaded directly instead, uncompressed.");
} else {
	if (p_image->get_name().is_empty()) {
		WARN_PRINT(vformat("glTF: Image index '%d' did not have a name. It will be automatically given a name based on its index.", p_index));
		p_image->set_name(itos(p_index));
	}
	bool must_import = true;
	// and so on

Note that this is technically a separate issue, but it's the same part of the code, so I'm personally good with fixing 2 issues with one PR as long as that's good with you.

[aaronfranke]

Updated the PR to have a second commit which adds extract_path and extract_prefix settings, used by the Blender importer. This allows textures extracted from the glTF file to be placed next to the .blend file, as suggested by @lyuma in Discord messages.

[lyuma]

Awesome! A good combination of additional error checking and bugfixes.

[Repiteo]

Some nitpicks, but otherwise looks good

[Repiteo]

Thanks!

[demolke]

This has a surprising (at least to me) side effect - if you're not using embedded textures in blend files your textures will get duplicated.

model.blend pointing to on-disk texture.png will also create model_texture.png

Is that working as intended?

It feels like there are three options

• gltf with embedded images -> should be saved to disk
• gltf pointing to textures within res:// directory -> should be skipped
• gltf pointing to textures outside res:// directory -> should be saved to disk

[aaronfranke]

@demolke True. If needed, we can revert the second commit in this PR, which will instead load the image directly instead of importing it in this case. However, I think really this is highlighting an existing problem - wouldn't this mean that a non-embedded texture from a Blend file isn't being used? How can we detect and use these textures?

[demolke]

wouldn't this mean that a non-embedded texture from a Blend file isn't being used? How can we detect and use these textures?

I'm not sure what you mean - I only went through the code superficially so don't see how it detects if the original texture.png is already imported/used. The part with md5 only checks for the prefixed model_texture.png.import so that will not help with de-duplication.

This project shows that after import you will get model_texture.png even though texture.png has already been imported
nonembed.zip

[demolke]

I spent some time poking at this and I think this #98909 addresses most of the use cases