From e268a8e5239a1da24a629af983cf7045ef3c81f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Verschelde?= Date: Tue, 5 Jan 2021 16:12:34 +0100 Subject: [PATCH] glTF: Fix loading external images as buffer We should first attempt loading as external files, thus creating a dependency. Loading as a buffer should only be used as fallback to support manually loading as PNG or JPEG depending on the defined mimeType. Fixes #44309, was a regression from #42504. --- modules/gltf/gltf_document.cpp | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/modules/gltf/gltf_document.cpp b/modules/gltf/gltf_document.cpp index 2b6211095ae..580bc006f5f 100644 --- a/modules/gltf/gltf_document.cpp +++ b/modules/gltf/gltf_document.cpp @@ -2918,21 +2918,30 @@ Error GLTFDocument::_parse_images(Ref state, const String &p_base_pat } } else { // Relative path to an external image file. uri = p_base_path.plus_file(uri).replace("\\", "/"); // Fix for Windows. - // The spec says that if mimeType is defined, we should enforce it. - // So we should only rely on ResourceLoader::load if mimeType is not defined, - // otherwise we should use the same logic as for buffers. - if (mimetype == "image/png" || mimetype == "image/jpeg") { - // Load data buffer and rely on PNG and JPEG-specific logic below to load the image. - // This makes it possible to load a file with a wrong extension but correct MIME type, - // e.g. "foo.jpg" containing PNG data and with MIME type "image/png". ResourceLoader would fail. + // ResourceLoader will rely on the file extension to use the relevant loader. + // The spec says that if mimeType is defined, it should take precedence (e.g. + // there could be a `.png` image which is actually JPEG), but there's no easy + // API for that in Godot, so we'd have to load as a buffer (i.e. embedded in + // the material), so we do this only as fallback. + Ref texture = ResourceLoader::load(uri); + if (texture.is_valid()) { + state->images.push_back(texture); + continue; + } else if (mimetype == "image/png" || mimetype == "image/jpeg") { + // Fallback to loading as byte array. + // This enables us to support the spec's requirement that we honor mimetype + // regardless of file URI. data = FileAccess::get_file_as_array(uri); - ERR_FAIL_COND_V_MSG(data.size() == 0, ERR_PARSE_ERROR, "glTF: Couldn't load image file as an array: " + uri); + if (data.size() == 0) { + WARN_PRINT(vformat("glTF: Image index '%d' couldn't be loaded as a buffer of MIME type '%s' from URI: %s. Skipping it.", i, mimetype, uri)); + state->images.push_back(Ref()); // Placeholder to keep count. + continue; + } data_ptr = data.ptr(); data_size = data.size(); } else { - // Good old ResourceLoader will rely on file extension. - Ref texture = ResourceLoader::load(uri); - state->images.push_back(texture); + WARN_PRINT(vformat("glTF: Image index '%d' couldn't be loaded from URI: %s. Skipping it.", i, uri)); + state->images.push_back(Ref()); // Placeholder to keep count. continue; } }