Browse Source

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.
Rémi Verschelde 4 years ago
parent
commit
e268a8e523
1 changed files with 20 additions and 11 deletions
  1. 20 11
      modules/gltf/gltf_document.cpp

+ 20 - 11
modules/gltf/gltf_document.cpp

@@ -2918,21 +2918,30 @@ Error GLTFDocument::_parse_images(Ref<GLTFState> 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<Texture2D> 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<Texture2D>()); // Placeholder to keep count.
+						continue;
+					}
 					data_ptr = data.ptr();
 					data_size = data.size();
 				} else {
-					// Good old ResourceLoader will rely on file extension.
-					Ref<Texture2D> 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<Texture2D>()); // Placeholder to keep count.
 					continue;
 				}
 			}