From 0b7354dfa93dfe5cccfb5e5b75347057bf4c74d1 Mon Sep 17 00:00:00 2001 From: Aaron Franke Date: Tue, 10 Sep 2024 00:15:31 -0700 Subject: [PATCH 1/2] GLTF: Don't give up loading image if import fails --- editor/editor_file_system.cpp | 4 ++-- modules/gltf/gltf_document.cpp | 41 +++++++++++++++++----------------- 2 files changed, 22 insertions(+), 23 deletions(-) diff --git a/editor/editor_file_system.cpp b/editor/editor_file_system.cpp index cd02482bc7..558eed98c6 100644 --- a/editor/editor_file_system.cpp +++ b/editor/editor_file_system.cpp @@ -2581,7 +2581,7 @@ Error EditorFileSystem::_reimport_group(const String &p_group_file, const Vector EditorFileSystemDirectory *fs = nullptr; int cpos = -1; bool found = _find_file(file, &fs, cpos); - ERR_FAIL_COND_V_MSG(!found, ERR_UNCONFIGURED, "Can't find file '" + file + "'."); + ERR_FAIL_COND_V_MSG(!found, ERR_UNCONFIGURED, vformat("Can't find file '%s' during group reimport.", file)); //update modified times, to avoid reimport fs->files[cpos]->modified_time = FileAccess::get_modified_time(file); @@ -2631,7 +2631,7 @@ Error EditorFileSystem::_reimport_file(const String &p_file, const HashMap p_state, const Vector< #ifdef TOOLS_ENABLED if (Engine::get_singleton()->is_editor_hint() && handling == GLTFState::GLTFHandleBinary::HANDLE_BINARY_EXTRACT_TEXTURES) { if (p_state->base_path.is_empty()) { - p_state->images.push_back(Ref()); - p_state->source_images.push_back(Ref()); - } else if (p_image->get_name().is_empty()) { - WARN_PRINT(vformat("glTF: Image index '%d' couldn't be named. Skipping it.", p_index)); - p_state->images.push_back(Ref()); - p_state->source_images.push_back(Ref()); + 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; Vector img_data = p_image->get_data(); Dictionary generator_parameters; @@ -3559,14 +3558,11 @@ void GLTFDocument::_parse_image_save_image(Ref p_state, const Vector< if (saved_image.is_valid()) { p_state->images.push_back(saved_image); p_state->source_images.push_back(saved_image->get_image()); + return; } else { - WARN_PRINT(vformat("glTF: Image index '%d' couldn't be loaded with the name: %s. Skipping it.", p_index, p_image->get_name())); - // Placeholder to keep count. - p_state->images.push_back(Ref()); - p_state->source_images.push_back(Ref()); + WARN_PRINT(vformat("glTF: Image index '%d' with the name '%s' couldn't be imported. It will be loaded directly instead, uncompressed.", p_index, p_image->get_name())); } } - return; } #endif // TOOLS_ENABLED if (handling == GLTFState::GLTFHandleBinary::HANDLE_BINARY_EMBED_AS_BASISU) { @@ -3649,16 +3645,19 @@ Error GLTFDocument::_parse_images(Ref p_state, const String &p_base_p ERR_FAIL_COND_V(p_base_path.is_empty(), ERR_INVALID_PARAMETER); uri = uri.uri_decode(); uri = p_base_path.path_join(uri).replace("\\", "/"); // Fix for Windows. - // 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 only do that only as fallback. - Ref texture = ResourceLoader::load(uri); - if (texture.is_valid()) { - p_state->images.push_back(texture); - p_state->source_images.push_back(texture->get_image()); - continue; + // If the image is in the .godot/imported directory, we can't use ResourceLoader. + if (!p_base_path.begins_with("res://.godot/imported")) { + // 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 only do that only as fallback. + Ref texture = ResourceLoader::load(uri, "Texture2D"); + if (texture.is_valid()) { + p_state->images.push_back(texture); + p_state->source_images.push_back(texture->get_image()); + continue; + } } // mimeType is optional, but if we have it in the file extension, let's use it. // If the mimeType does not match with the file extension, either it should be From 02d8c6eefe1a53d0ea45e1fab1e96a0dff30f8fd Mon Sep 17 00:00:00 2001 From: Aaron Franke Date: Wed, 30 Oct 2024 02:36:47 -0700 Subject: [PATCH 2/2] GLTF: Add extract_path and extract_prefix settings Only used by the Blender importer --- .../editor/editor_scene_importer_blend.cpp | 2 ++ modules/gltf/gltf_document.cpp | 16 +++++++------ modules/gltf/gltf_state.cpp | 24 ++++++++++++++++++- modules/gltf/gltf_state.h | 10 +++++++- 4 files changed, 43 insertions(+), 9 deletions(-) diff --git a/modules/gltf/editor/editor_scene_importer_blend.cpp b/modules/gltf/editor/editor_scene_importer_blend.cpp index 542e00e560..2db46adef4 100644 --- a/modules/gltf/editor/editor_scene_importer_blend.cpp +++ b/modules/gltf/editor/editor_scene_importer_blend.cpp @@ -319,6 +319,8 @@ Node *EditorSceneFormatImporterBlend::import_scene(const String &p_path, uint32_ state->set_import_as_skeleton_bones(true); } state->set_scene_name(blend_basename); + state->set_extract_path(p_path.get_base_dir()); + state->set_extract_prefix(blend_basename); err = gltf->append_from_file(sink.get_basename() + ".gltf", state, p_flags, base_dir); if (err != OK) { if (r_err) { diff --git a/modules/gltf/gltf_document.cpp b/modules/gltf/gltf_document.cpp index 9b835c456c..360b4ec07f 100644 --- a/modules/gltf/gltf_document.cpp +++ b/modules/gltf/gltf_document.cpp @@ -3501,8 +3501,10 @@ void GLTFDocument::_parse_image_save_image(Ref p_state, const Vector< } #ifdef TOOLS_ENABLED if (Engine::get_singleton()->is_editor_hint() && handling == GLTFState::GLTFHandleBinary::HANDLE_BINARY_EXTRACT_TEXTURES) { - 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."); + if (p_state->extract_path.is_empty()) { + WARN_PRINT("glTF: Couldn't extract image because the base and extract paths are empty. It will be loaded directly instead, uncompressed."); + } else if (p_state->extract_path.begins_with("res://.godot/imported")) { + WARN_PRINT(vformat("glTF: Extract path is in the imported directory. Image index '%d' will be loaded directly, uncompressed.", p_index)); } 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)); @@ -3511,7 +3513,7 @@ void GLTFDocument::_parse_image_save_image(Ref p_state, const Vector< bool must_import = true; Vector img_data = p_image->get_data(); Dictionary generator_parameters; - String file_path = p_state->get_base_path().path_join(p_state->filename.get_basename() + "_" + p_image->get_name()); + String file_path = p_state->get_extract_path().path_join(p_state->get_extract_prefix() + "_" + p_image->get_name()); file_path += p_file_extension.is_empty() ? ".png" : p_file_extension; if (FileAccess::exists(file_path + ".import")) { Ref config; @@ -7349,7 +7351,7 @@ PackedByteArray GLTFDocument::generate_buffer(Ref p_state) { Error GLTFDocument::write_to_filesystem(Ref p_state, const String &p_path) { Ref state = p_state; ERR_FAIL_COND_V(state.is_null(), ERR_INVALID_PARAMETER); - state->base_path = p_path.get_base_dir(); + state->set_base_path(p_path.get_base_dir()); state->filename = p_path.get_file(); Error err = _serialize(state); if (err != OK) { @@ -7462,7 +7464,7 @@ Error GLTFDocument::append_from_buffer(PackedByteArray p_bytes, String p_base_pa Ref file_access; file_access.instantiate(); file_access->open_custom(p_bytes.ptr(), p_bytes.size()); - state->base_path = p_base_path.get_base_dir(); + state->set_base_path(p_base_path.get_base_dir()); err = _parse(p_state, state->base_path, file_access); ERR_FAIL_COND_V(err != OK, err); for (Ref ext : document_extensions) { @@ -7479,7 +7481,7 @@ Error GLTFDocument::append_from_file(String p_path, Ref p_state, uint if (state == Ref()) { state.instantiate(); } - state->filename = p_path.get_file().get_basename(); + state->set_filename(p_path.get_file().get_basename()); state->use_named_skin_binds = p_flags & GLTF_IMPORT_USE_NAMED_SKIN_BINDS; state->discard_meshes_and_materials = p_flags & GLTF_IMPORT_DISCARD_MESHES_AND_MATERIALS; state->force_generate_tangents = p_flags & GLTF_IMPORT_GENERATE_TANGENT_ARRAYS; @@ -7493,7 +7495,7 @@ Error GLTFDocument::append_from_file(String p_path, Ref p_state, uint if (base_path.is_empty()) { base_path = p_path.get_base_dir(); } - state->base_path = base_path; + state->set_base_path(base_path); err = _parse(p_state, base_path, file); ERR_FAIL_COND_V(err != OK, err); for (Ref ext : document_extensions) { diff --git a/modules/gltf/gltf_state.cpp b/modules/gltf/gltf_state.cpp index 7763874d02..2488e73d08 100644 --- a/modules/gltf/gltf_state.cpp +++ b/modules/gltf/gltf_state.cpp @@ -397,8 +397,27 @@ String GLTFState::get_base_path() { return base_path; } -void GLTFState::set_base_path(String p_base_path) { +void GLTFState::set_base_path(const String &p_base_path) { base_path = p_base_path; + if (extract_path.is_empty()) { + extract_path = p_base_path; + } +} + +String GLTFState::get_extract_path() { + return extract_path; +} + +void GLTFState::set_extract_path(const String &p_extract_path) { + extract_path = p_extract_path; +} + +String GLTFState::get_extract_prefix() { + return extract_prefix; +} + +void GLTFState::set_extract_prefix(const String &p_extract_prefix) { + extract_prefix = p_extract_prefix; } String GLTFState::get_filename() const { @@ -407,6 +426,9 @@ String GLTFState::get_filename() const { void GLTFState::set_filename(const String &p_filename) { filename = p_filename; + if (extract_prefix.is_empty()) { + extract_prefix = p_filename.get_basename(); + } } Variant GLTFState::get_additional_data(const StringName &p_extension_name) { diff --git a/modules/gltf/gltf_state.h b/modules/gltf/gltf_state.h index 7954049192..042384f546 100644 --- a/modules/gltf/gltf_state.h +++ b/modules/gltf/gltf_state.h @@ -51,6 +51,8 @@ class GLTFState : public Resource { protected: String base_path; + String extract_path; + String extract_prefix; String filename; Dictionary json; int major_version = 0; @@ -186,7 +188,13 @@ public: void set_scene_name(String p_scene_name); String get_base_path(); - void set_base_path(String p_base_path); + void set_base_path(const String &p_base_path); + + String get_extract_path(); + void set_extract_path(const String &p_extract_path); + + String get_extract_prefix(); + void set_extract_prefix(const String &p_extract_prefix); String get_filename() const; void set_filename(const String &p_filename);