Skip to content

Commit

Permalink
glTF: Fix loading external images as buffer
Browse files Browse the repository at this point in the history
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.

(cherry picked from commit e268a8e)
  • Loading branch information
akien-mga committed Jan 5, 2021
1 parent 0fc433d commit 9d916b8
Showing 1 changed file with 20 additions and 11 deletions.
31 changes: 20 additions & 11 deletions editor/import/editor_scene_importer_gltf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1331,21 +1331,30 @@ Error EditorSceneImporterGLTF::_parse_images(GLTFState &state, const String &p_b
}
} 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> 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<Texture>()); // Placeholder to keep count.
continue;
}
data_ptr = data.ptr();
data_size = data.size();
} else {
// Good old ResourceLoader will rely on file extension.
Ref<Texture> 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<Texture>()); // Placeholder to keep count.
continue;
}
}
Expand Down

0 comments on commit 9d916b8

Please sign in to comment.