godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
91.15k stars 21.19k forks source link

GLTF import crash with "Embed as Basis Universal" and missing texture #85577

Open Malcolmnixon opened 11 months ago

Malcolmnixon commented 11 months ago

Godot version

4.2-stable

System information

Windows 11, gl_compatibility, NVidia RTX 3070 TI

Issue description

Setting GLTF "Embedded Image Handling" to "Embed as Basis Universal" with a missing image results in a null-dereference crash.

Steps to reproduce

Add the basemodule_C.bin and basemodule_c.gltf files (in the attached zip) basemodule_C.zip (originally from https://godotengine.org/asset-library/asset/2124)

The gltf is expecting a "spacebits_texture.png" but we omit it as we're planning on eventually supplying an external exture-atlas material.

Click on the gltf file, set the "Embedded Image Handling" to "Embed as Basis Universal" and click on [Reimport].

The crash occurs in GLTFDocument::_get_texture with the following call stack:

godot.windows.editor.dev.x86_64.exe!GLTFDocument::_get_texture(Ref<GLTFState> p_state, const int p_texture, int p_texture_types) Line 3500  C++
godot.windows.editor.dev.x86_64.exe!GLTFDocument::_parse_materials(Ref<GLTFState> p_state) Line 3997    C++
godot.windows.editor.dev.x86_64.exe!GLTFDocument::_parse_gltf_state(Ref<GLTFState> p_state, const String & p_search_path) Line 7600 C++
godot.windows.editor.dev.x86_64.exe!GLTFDocument::_parse(Ref<GLTFState> p_state, String p_path, Ref<FileAccess> p_file) Line 7151   C++
godot.windows.editor.dev.x86_64.exe!GLTFDocument::append_from_file(String p_path, Ref<GLTFState> p_state, unsigned int p_flags, String p_base_path) Line 7656   C++
godot.windows.editor.dev.x86_64.exe!EditorSceneFormatImporterGLTF::import_scene(const String & p_path, unsigned int p_flags, const HashMap<StringName,Variant,HashMapHasherDefault,HashMapComparatorDefault<StringName>,DefaultTypedAllocator<HashMapElement<StringName,Variant>>> & p_options, List<String,DefaultAllocator> * r_missing_deps, Error * r_err) Line 62  C++

Specifically on line 3500:

        portable_texture->set_keep_compressed_buffer(true);
        Ref<Image> new_img = p_state->source_images[image]->duplicate();
        ERR_FAIL_COND_V(new_img.is_null(), Ref<Texture2D>());

It's trying to duplicate source_images[0], however this is value is null.

The reason source_images[0] is null is from gltf_document.cpp line 3379:

                    WARN_PRINT(vformat("glTF: Image index '%d' couldn't be loaded as a buffer of MIME type '%s' from URI: %s because there was no data to load. Skipping it.", i, mime_type, uri));
                    p_state->images.push_back(Ref<Texture2D>()); // Placeholder to keep count.
                    p_state->source_images.push_back(Ref<Image>());
                    continue;

Minimal reproduction project

N/A - instructions included in reproduction steps.

BastiaanOlij commented 11 months ago

cc @fire

fire commented 11 months ago

@lyuma We worked on this. Let me know if you remember any details.

Alby7 commented 4 months ago

Any news on this?

fire commented 1 week ago

Looking for a hero.