From 5e144022e70975a246a14f0343215cde92893b7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20J=2E=20Est=C3=A9banez?= Date: Thu, 22 Feb 2024 12:53:19 +0100 Subject: [PATCH] Enhance cache modes in resource loading - Unify documentation, hoping to clear misconcepctions about about propagation of the cache mode across dependant loads. - Clarify in docs that `CACHE_MODE_REPLACE` now also works on the main resource (from #87008). - Add two recursive modes, counterparts of `CACHE_MODE_REPLACE` and `CACHE_MODE_IGNORE`, since it seems some need them (see #59669, #82830). - Let resources, even loaded with one of the ignore-cache modes, get a path, which is useful for tools. --- core/core_bind.cpp | 2 ++ core/core_bind.h | 8 +++--- core/io/resource.h | 2 +- core/io/resource_format_binary.cpp | 31 +++++++++++++++++------ core/io/resource_format_binary.h | 1 + core/io/resource_loader.cpp | 14 +++++++---- core/io/resource_loader.h | 8 +++--- doc/classes/ResourceFormatLoader.xml | 9 +++++++ doc/classes/ResourceLoader.xml | 12 ++++++--- modules/gdscript/gdscript.cpp | 3 ++- modules/mono/csharp_script.cpp | 2 ++ scene/resources/packed_scene.cpp | 5 ++++ scene/resources/packed_scene.h | 2 ++ scene/resources/resource_format_text.cpp | 32 ++++++++++++++++++++---- scene/resources/resource_format_text.h | 1 + 15 files changed, 104 insertions(+), 28 deletions(-) diff --git a/core/core_bind.cpp b/core/core_bind.cpp index f5c69b9b980..8ccf7d1f510 100644 --- a/core/core_bind.cpp +++ b/core/core_bind.cpp @@ -145,6 +145,8 @@ void ResourceLoader::_bind_methods() { BIND_ENUM_CONSTANT(CACHE_MODE_IGNORE); BIND_ENUM_CONSTANT(CACHE_MODE_REUSE); BIND_ENUM_CONSTANT(CACHE_MODE_REPLACE); + BIND_ENUM_CONSTANT(CACHE_MODE_IGNORE_DEEP); + BIND_ENUM_CONSTANT(CACHE_MODE_REPLACE_DEEP); } ////// ResourceSaver ////// diff --git a/core/core_bind.h b/core/core_bind.h index f884426881e..34405311241 100644 --- a/core/core_bind.h +++ b/core/core_bind.h @@ -63,9 +63,11 @@ public: }; enum CacheMode { - CACHE_MODE_IGNORE, // Resource and subresources do not use path cache, no path is set into resource. - CACHE_MODE_REUSE, // Resource and subresources use patch cache, reuse existing loaded resources instead of loading from disk when available. - CACHE_MODE_REPLACE, // Resource and subresource use path cache, but replace existing loaded resources when available with information from disk. + CACHE_MODE_IGNORE, + CACHE_MODE_REUSE, + CACHE_MODE_REPLACE, + CACHE_MODE_IGNORE_DEEP, + CACHE_MODE_REPLACE_DEEP, }; static ResourceLoader *get_singleton() { return singleton; } diff --git a/core/io/resource.h b/core/io/resource.h index b885b773acd..f0f686af57f 100644 --- a/core/io/resource.h +++ b/core/io/resource.h @@ -106,7 +106,7 @@ public: virtual void set_path(const String &p_path, bool p_take_over = false); String get_path() const; - void set_path_cache(const String &p_path); // Set raw path without involving resource cache. + virtual void set_path_cache(const String &p_path); // Set raw path without involving resource cache. _FORCE_INLINE_ bool is_built_in() const { return path_cache.is_empty() || path_cache.contains("::") || path_cache.begins_with("local://"); } static String generate_scene_unique_id(); diff --git a/core/io/resource_format_binary.cpp b/core/io/resource_format_binary.cpp index 20c494516be..17cffb878e0 100644 --- a/core/io/resource_format_binary.cpp +++ b/core/io/resource_format_binary.cpp @@ -430,7 +430,7 @@ Error ResourceLoaderBinary::parse_variant(Variant &r_v) { path = remaps[path]; } - Ref res = ResourceLoader::load(path, exttype); + Ref res = ResourceLoader::load(path, exttype, cache_mode_for_external); if (res.is_null()) { WARN_PRINT(String("Couldn't load resource: " + path).utf8().get_data()); @@ -683,7 +683,7 @@ Error ResourceLoaderBinary::load() { } external_resources.write[i].path = path; //remap happens here, not on load because on load it can actually be used for filesystem dock resource remap - external_resources.write[i].load_token = ResourceLoader::_load_start(path, external_resources[i].type, use_sub_threads ? ResourceLoader::LOAD_THREAD_DISTRIBUTE : ResourceLoader::LOAD_THREAD_FROM_CURRENT, ResourceFormatLoader::CACHE_MODE_REUSE); + external_resources.write[i].load_token = ResourceLoader::_load_start(path, external_resources[i].type, use_sub_threads ? ResourceLoader::LOAD_THREAD_DISTRIBUTE : ResourceLoader::LOAD_THREAD_FROM_CURRENT, cache_mode_for_external); if (!external_resources[i].load_token.is_valid()) { if (!ResourceLoader::get_abort_on_missing_resources()) { ResourceLoader::notify_dependency_error(local_path, path, external_resources[i].type); @@ -772,10 +772,12 @@ Error ResourceLoaderBinary::load() { } res = Ref(r); - if (!path.is_empty() && cache_mode != ResourceFormatLoader::CACHE_MODE_IGNORE) { - r->set_path(path, cache_mode == ResourceFormatLoader::CACHE_MODE_REPLACE); //if got here because the resource with same path has different type, replace it - } else if (!path.is_resource_file()) { - r->set_path_cache(path); + if (!path.is_empty()) { + if (cache_mode != ResourceFormatLoader::CACHE_MODE_IGNORE) { + r->set_path(path, cache_mode == ResourceFormatLoader::CACHE_MODE_REPLACE); // If got here because the resource with same path has different type, replace it. + } else { + r->set_path_cache(path); + } } r->set_scene_unique_id(id); } @@ -1187,7 +1189,22 @@ Ref ResourceFormatLoaderBinary::load(const String &p_path, const Strin ERR_FAIL_COND_V_MSG(err != OK, Ref(), "Cannot open file '" + p_path + "'."); ResourceLoaderBinary loader; - loader.cache_mode = p_cache_mode; + switch (p_cache_mode) { + case CACHE_MODE_IGNORE: + case CACHE_MODE_REUSE: + case CACHE_MODE_REPLACE: + loader.cache_mode = p_cache_mode; + loader.cache_mode_for_external = CACHE_MODE_REUSE; + break; + case CACHE_MODE_IGNORE_DEEP: + loader.cache_mode = CACHE_MODE_IGNORE; + loader.cache_mode_for_external = p_cache_mode; + break; + case CACHE_MODE_REPLACE_DEEP: + loader.cache_mode = CACHE_MODE_REPLACE; + loader.cache_mode_for_external = p_cache_mode; + break; + } loader.use_sub_threads = p_use_sub_threads; loader.progress = r_progress; String path = !p_original_path.is_empty() ? p_original_path : p_path; diff --git a/core/io/resource_format_binary.h b/core/io/resource_format_binary.h index e64485d4043..e01c5fa4670 100644 --- a/core/io/resource_format_binary.h +++ b/core/io/resource_format_binary.h @@ -85,6 +85,7 @@ class ResourceLoaderBinary { Error error = OK; ResourceFormatLoader::CacheMode cache_mode = ResourceFormatLoader::CACHE_MODE_REUSE; + ResourceFormatLoader::CacheMode cache_mode_for_external = ResourceFormatLoader::CACHE_MODE_REUSE; friend class ResourceFormatLoaderBinary; diff --git a/core/io/resource_loader.cpp b/core/io/resource_loader.cpp index a3fc7bc370e..ff563a35b2d 100644 --- a/core/io/resource_loader.cpp +++ b/core/io/resource_loader.cpp @@ -188,6 +188,8 @@ void ResourceFormatLoader::_bind_methods() { BIND_ENUM_CONSTANT(CACHE_MODE_IGNORE); BIND_ENUM_CONSTANT(CACHE_MODE_REUSE); BIND_ENUM_CONSTANT(CACHE_MODE_REPLACE); + BIND_ENUM_CONSTANT(CACHE_MODE_IGNORE_DEEP); + BIND_ENUM_CONSTANT(CACHE_MODE_REPLACE_DEEP); GDVIRTUAL_BIND(_get_recognized_extensions); GDVIRTUAL_BIND(_recognize_path, "path", "type"); @@ -339,9 +341,11 @@ void ResourceLoader::_thread_load_function(void *p_userdata) { load_task.cond_var = nullptr; } + bool ignoring = load_task.cache_mode == ResourceFormatLoader::CACHE_MODE_IGNORE || load_task.cache_mode == ResourceFormatLoader::CACHE_MODE_IGNORE_DEEP; + bool replacing = load_task.cache_mode == ResourceFormatLoader::CACHE_MODE_REPLACE || load_task.cache_mode == ResourceFormatLoader::CACHE_MODE_REPLACE_DEEP; if (load_task.resource.is_valid()) { - if (load_task.cache_mode != ResourceFormatLoader::CACHE_MODE_IGNORE) { - if (load_task.cache_mode == ResourceFormatLoader::CACHE_MODE_REPLACE) { + if (!ignoring) { + if (replacing) { Ref old_res = ResourceCache::get_ref(load_task.local_path); if (old_res.is_valid() && old_res != load_task.resource) { // If resource is already loaded, only replace its data, to avoid existing invalidating instances. @@ -349,8 +353,8 @@ void ResourceLoader::_thread_load_function(void *p_userdata) { load_task.resource = old_res; } } - load_task.resource->set_path(load_task.local_path, load_task.cache_mode == ResourceFormatLoader::CACHE_MODE_REPLACE); - } else if (!load_task.local_path.is_resource_file()) { + load_task.resource->set_path(load_task.local_path, replacing); + } else { load_task.resource->set_path_cache(load_task.local_path); } @@ -370,7 +374,7 @@ void ResourceLoader::_thread_load_function(void *p_userdata) { if (_loaded_callback) { _loaded_callback(load_task.resource, load_task.local_path); } - } else if (load_task.cache_mode != ResourceFormatLoader::CACHE_MODE_IGNORE) { + } else if (!ignoring) { Ref existing = ResourceCache::get_ref(load_task.local_path); if (existing.is_valid()) { load_task.resource = existing; diff --git a/core/io/resource_loader.h b/core/io/resource_loader.h index 1f79b83f112..5caf699d321 100644 --- a/core/io/resource_loader.h +++ b/core/io/resource_loader.h @@ -47,9 +47,11 @@ class ResourceFormatLoader : public RefCounted { public: enum CacheMode { - CACHE_MODE_IGNORE, // Resource and subresources do not use path cache, no path is set into resource. - CACHE_MODE_REUSE, // Resource and subresources use patch cache, reuse existing loaded resources instead of loading from disk when available. - CACHE_MODE_REPLACE, // Resource and subresource use path cache, but replace existing loaded resources when available with information from disk. + CACHE_MODE_IGNORE, + CACHE_MODE_REUSE, + CACHE_MODE_REPLACE, + CACHE_MODE_IGNORE_DEEP, + CACHE_MODE_REPLACE_DEEP, }; protected: diff --git a/doc/classes/ResourceFormatLoader.xml b/doc/classes/ResourceFormatLoader.xml index 61eafb4527b..4e4adc86c41 100644 --- a/doc/classes/ResourceFormatLoader.xml +++ b/doc/classes/ResourceFormatLoader.xml @@ -99,10 +99,19 @@ + Neither the main resource (the one requested to be loaded) nor any of its subresources are retrieved from cache nor stored into it. Dependencies (external resources) are loaded with [constant CACHE_MODE_REUSE]. + The main resource (the one requested to be loaded), its subresources, and its dependencies (external resources) are retrieved from cache if present, instead of loaded. Those not cached are loaded and then stored into the cache. The same rules are propagated recursively down the tree of dependencies (external resources). + Like [constant CACHE_MODE_REUSE], but the cache is checked for the main resource (the one requested to be loaded) as well as for each of its subresources. Those already in the cache, as long as the loaded and cached types match, have their data refreshed from storage into the already existing instances. Otherwise, they are recreated as completely new objects. + + + Like [constant CACHE_MODE_IGNORE], but propagated recursively down the tree of dependencies (external resources). + + + Like [constant CACHE_MODE_REPLACE], but propagated recursively down the tree of dependencies (external resources). diff --git a/doc/classes/ResourceLoader.xml b/doc/classes/ResourceLoader.xml index 54e95d8c307..95fbee4a245 100644 --- a/doc/classes/ResourceLoader.xml +++ b/doc/classes/ResourceLoader.xml @@ -138,13 +138,19 @@ The resource was loaded successfully and can be accessed via [method load_threaded_get]. - The resource is always loaded from disk, even if a cache entry exists for its path, and the newly loaded copy will not be cached. Instances loaded with this mode will exist independently. + Neither the main resource (the one requested to be loaded) nor any of its subresources are retrieved from cache nor stored into it. Dependencies (external resources) are loaded with [constant CACHE_MODE_REUSE]. - If a resource is cached, returns the cached reference. Otherwise it's loaded from disk. + The main resource (the one requested to be loaded), its subresources, and its dependencies (external resources) are retrieved from cache if present, instead of loaded. Those not cached are loaded and then stored into the cache. The same rules are propagated recursively down the tree of dependencies (external resources). - The resource is always loaded from disk, even if a cache entry exists for its path. The cached entry will be replaced by the newly loaded copy. + Like [constant CACHE_MODE_REUSE], but the cache is checked for the main resource (the one requested to be loaded) as well as for each of its subresources. Those already in the cache, as long as the loaded and cached types match, have their data refreshed from storage into the already existing instances. Otherwise, they are recreated as completely new objects. + + + Like [constant CACHE_MODE_IGNORE], but propagated recursively down the tree of dependencies (external resources). + + + Like [constant CACHE_MODE_REPLACE], but propagated recursively down the tree of dependencies (external resources). diff --git a/modules/gdscript/gdscript.cpp b/modules/gdscript/gdscript.cpp index 2d6c0c1d113..70f4411dec3 100644 --- a/modules/gdscript/gdscript.cpp +++ b/modules/gdscript/gdscript.cpp @@ -2817,7 +2817,8 @@ Ref GDScriptLanguage::get_script_by_fully_qualified_name(const String Ref ResourceFormatLoaderGDScript::load(const String &p_path, const String &p_original_path, Error *r_error, bool p_use_sub_threads, float *r_progress, CacheMode p_cache_mode) { Error err; - Ref scr = GDScriptCache::get_full_script(p_path, err, "", p_cache_mode == CACHE_MODE_IGNORE); + bool ignoring = p_cache_mode == CACHE_MODE_IGNORE || p_cache_mode == CACHE_MODE_IGNORE_DEEP; + Ref scr = GDScriptCache::get_full_script(p_path, err, "", ignoring); if (err && scr.is_valid()) { // If !scr.is_valid(), the error was likely from scr->load_source_code(), which already generates an error. diff --git a/modules/mono/csharp_script.cpp b/modules/mono/csharp_script.cpp index 88fe82c6b83..9ccaa27e84e 100644 --- a/modules/mono/csharp_script.cpp +++ b/modules/mono/csharp_script.cpp @@ -2854,6 +2854,7 @@ Ref ResourceFormatLoaderCSharpScript::load(const String &p_path, const Ref existing = ResourceCache::get_ref(p_path); switch (p_cache_mode) { case ResourceFormatLoader::CACHE_MODE_IGNORE: + case ResourceFormatLoader::CACHE_MODE_IGNORE_DEEP: break; case ResourceFormatLoader::CACHE_MODE_REUSE: if (existing.is_null()) { @@ -2863,6 +2864,7 @@ Ref ResourceFormatLoaderCSharpScript::load(const String &p_path, const } break; case ResourceFormatLoader::CACHE_MODE_REPLACE: + case ResourceFormatLoader::CACHE_MODE_REPLACE_DEEP: scr->set_path(p_original_path, true); break; } diff --git a/scene/resources/packed_scene.cpp b/scene/resources/packed_scene.cpp index a59ac9b56dc..89501aaa5a6 100644 --- a/scene/resources/packed_scene.cpp +++ b/scene/resources/packed_scene.cpp @@ -2045,6 +2045,11 @@ void PackedScene::set_path(const String &p_path, bool p_take_over) { Resource::set_path(p_path, p_take_over); } +void PackedScene::set_path_cache(const String &p_path) { + state->set_path(p_path); + Resource::set_path_cache(p_path); +} + void PackedScene::reset_state() { clear(); } diff --git a/scene/resources/packed_scene.h b/scene/resources/packed_scene.h index e6cbc3e16ba..2dc85404997 100644 --- a/scene/resources/packed_scene.h +++ b/scene/resources/packed_scene.h @@ -258,6 +258,8 @@ public: virtual void reload_from_file() override; virtual void set_path(const String &p_path, bool p_take_over = false) override; + virtual void set_path_cache(const String &p_path) override; + #ifdef TOOLS_ENABLED virtual void set_last_modified_time(uint64_t p_time) override { Resource::set_last_modified_time(p_time); diff --git a/scene/resources/resource_format_text.cpp b/scene/resources/resource_format_text.cpp index d08919a8c65..6d0796f1b96 100644 --- a/scene/resources/resource_format_text.cpp +++ b/scene/resources/resource_format_text.cpp @@ -471,7 +471,7 @@ Error ResourceLoaderText::load() { ext_resources[id].path = path; ext_resources[id].type = type; - ext_resources[id].load_token = ResourceLoader::_load_start(path, type, use_sub_threads ? ResourceLoader::LOAD_THREAD_DISTRIBUTE : ResourceLoader::LOAD_THREAD_FROM_CURRENT, ResourceFormatLoader::CACHE_MODE_REUSE); + ext_resources[id].load_token = ResourceLoader::_load_start(path, type, use_sub_threads ? ResourceLoader::LOAD_THREAD_DISTRIBUTE : ResourceLoader::LOAD_THREAD_FROM_CURRENT, cache_mode_for_external); if (!ext_resources[id].load_token.is_valid()) { if (ResourceLoader::get_abort_on_missing_resources()) { error = ERR_FILE_CORRUPT; @@ -584,7 +584,7 @@ Error ResourceLoaderText::load() { if (do_assign) { if (cache_mode != ResourceFormatLoader::CACHE_MODE_IGNORE) { res->set_path(path, cache_mode == ResourceFormatLoader::CACHE_MODE_REPLACE); - } else if (!path.is_resource_file()) { + } else { res->set_path_cache(path); } res->set_scene_unique_id(id); @@ -721,6 +721,8 @@ Error ResourceLoaderText::load() { resource->set_path(res_path); } resource->set_as_translation_remapped(translation_remapped); + } else { + resource->set_path_cache(res_path); } } return error; @@ -805,8 +807,13 @@ Error ResourceLoaderText::load() { error = OK; //get it here resource = packed_scene; - if (cache_mode != ResourceFormatLoader::CACHE_MODE_IGNORE && !ResourceCache::has(res_path)) { - packed_scene->set_path(res_path); + if (cache_mode != ResourceFormatLoader::CACHE_MODE_IGNORE) { + if (!ResourceCache::has(res_path)) { + packed_scene->set_path(res_path); + } + } else { + packed_scene->get_state()->set_path(res_path); + packed_scene->set_path_cache(res_path); } resource_current++; @@ -1650,7 +1657,22 @@ Ref ResourceFormatLoaderText::load(const String &p_path, const String ResourceLoaderText loader; String path = !p_original_path.is_empty() ? p_original_path : p_path; - loader.cache_mode = p_cache_mode; + switch (p_cache_mode) { + case CACHE_MODE_IGNORE: + case CACHE_MODE_REUSE: + case CACHE_MODE_REPLACE: + loader.cache_mode = p_cache_mode; + loader.cache_mode_for_external = CACHE_MODE_REUSE; + break; + case CACHE_MODE_IGNORE_DEEP: + loader.cache_mode = ResourceFormatLoader::CACHE_MODE_IGNORE; + loader.cache_mode_for_external = p_cache_mode; + break; + case CACHE_MODE_REPLACE_DEEP: + loader.cache_mode = ResourceFormatLoader::CACHE_MODE_REPLACE; + loader.cache_mode_for_external = p_cache_mode; + break; + } loader.use_sub_threads = p_use_sub_threads; loader.local_path = ProjectSettings::get_singleton()->localize_path(path); loader.progress = r_progress; diff --git a/scene/resources/resource_format_text.h b/scene/resources/resource_format_text.h index 1734ccc98b4..02898ce9842 100644 --- a/scene/resources/resource_format_text.h +++ b/scene/resources/resource_format_text.h @@ -69,6 +69,7 @@ class ResourceLoaderText { VariantParser::Tag next_tag; ResourceFormatLoader::CacheMode cache_mode = ResourceFormatLoader::CACHE_MODE_REUSE; + ResourceFormatLoader::CacheMode cache_mode_for_external = ResourceFormatLoader::CACHE_MODE_REUSE; bool use_sub_threads = false; float *progress = nullptr;