Merge pull request #216 from jss2a98aj/backport-assorted-fixes

[backport] Assorted fixes

core/string/ustring.cpp

@@ -3827,14 +3827,12 @@ int String::_count(const String &p_string, int p_from, int p_to, bool p_case_ins
 		return 0;
 	}
 	int c = 0;
-	int idx = -1;
-	do {
-		idx = p_case_insensitive ? str.findn(p_string) : str.find(p_string);
-		if (idx != -1) {
-			str = str.substr(idx + slen, str.length() - slen);
-			++c;
-		}
-	} while (idx != -1);
+	int idx = 0;
+	while ((idx = p_case_insensitive ? str.findn(p_string, idx) : str.find(p_string, idx)) != -1) {
+		// Skip the occurrence itself.
+		idx += slen;
+		++c;
+	}
 	return c;
 }
 
@@ -3866,14 +3864,12 @@ int String::_count(const char *p_string, int p_from, int p_to, bool p_case_insen
 		return 0;
 	}
 	int c = 0;
-	int idx = -1;
-	do {
-		idx = p_case_insensitive ? str.findn(p_string) : str.find(p_string);
-		if (idx != -1) {
-			str = str.substr(idx + substring_length, str.length() - substring_length);
-			++c;
-		}
-	} while (idx != -1);
+	int idx = 0;
+	while ((idx = p_case_insensitive ? str.findn(p_string, idx) : str.find(p_string, idx)) != -1) {
+		// Skip the occurrence itself.
+		idx += substring_length;
+		++c;
+	}
 	return c;
 }
 

doc/classes/AABB.xml

@@ -345,14 +345,14 @@
 	</methods>
 	<members>
 		<member name="end" type="Vector3" setter="" getter="" default="Vector3(0, 0, 0)">
-			The ending point. This is usually the corner on the top-right and forward of the bounding box, and is equivalent to [code]position + size[/code]. Setting this point affects the [member size].
+			The ending point. This is usually the corner on the top-right and back of the bounding box, and is equivalent to [code]position + size[/code]. Setting this point affects the [member size].
 		</member>
 		<member name="position" type="Vector3" setter="" getter="" default="Vector3(0, 0, 0)">
-			The origin point. This is usually the corner on the bottom-left and back of the bounding box.
+			The origin point. This is usually the corner on the bottom-left and forward of the bounding box.
 		</member>
 		<member name="size" type="Vector3" setter="" getter="" default="Vector3(0, 0, 0)">
 			The bounding box's width, height, and depth starting from [member position]. Setting this value also affects the [member end] point.
-			[b]Note:[/b] It's recommended setting the width, height, and depth to non-negative values. This is because most methods in Godot assume that the [member position] is the bottom-left-back corner, and the [member end] is the top-right-forward corner. To get an equivalent bounding box with non-negative size, use [method abs].
+			[b]Note:[/b] It's recommended setting the width, height, and depth to non-negative values. This is because most methods in Godot assume that the [member position] is the bottom-left-forward corner, and the [member end] is the top-right-back corner. To get an equivalent bounding box with non-negative size, use [method abs].
 		</member>
 	</members>
 	<operators>

drivers/gles3/effects/cubemap_filter.cpp

@@ -41,7 +41,10 @@ CubemapFilter *CubemapFilter::singleton = nullptr;
 
 CubemapFilter::CubemapFilter() {
 	singleton = this;
-	ggx_samples = GLOBAL_GET("rendering/reflections/sky_reflections/ggx_samples");
+	// Use a factor 4 larger for the compatibility renderer to make up for the fact
+	// That we don't use an array texture. We will reduce samples on low roughness
+	// to compensate.
+	ggx_samples = 4 * uint32_t(GLOBAL_GET("rendering/reflections/sky_reflections/ggx_samples"));
 
 	{
 		String defines;
@@ -57,10 +60,10 @@ CubemapFilter::CubemapFilter() {
 		const float qv[6] = {
 			-1.0f,
 			-1.0f,
-			3.0f,
-			-1.0f,
 			-1.0f,
 			3.0f,
+			3.0f,
+			-1.0f,
 		};
 
 		glBufferData(GL_ARRAY_BUFFER, sizeof(float) * 6, qv, GL_STATIC_DRAW);
@@ -146,10 +149,11 @@ void CubemapFilter::filter_radiance(GLuint p_source_cubemap, GLuint p_dest_cubem
 	}
 
 	if (p_layer > 0) {
-		const uint32_t sample_counts[4] = { 1, ggx_samples / 4, ggx_samples / 2, ggx_samples };
-		uint32_t sample_count = sample_counts[MIN(3, p_layer)];
+		const uint32_t sample_counts[5] = { 1, ggx_samples / 16, ggx_samples / 8, ggx_samples / 4, ggx_samples };
+		uint32_t sample_count = sample_counts[MIN(4, p_layer)];
 
-		float roughness = float(p_layer) / (p_mipmap_count);
+		float roughness = float(p_layer) / (p_mipmap_count - 1);
+		roughness *= roughness; // Convert to non-perceptual roughness.
 		float roughness4 = roughness * roughness;
 		roughness4 *= roughness4;
 
@@ -165,7 +169,7 @@ void CubemapFilter::filter_radiance(GLuint p_source_cubemap, GLuint p_dest_cubem
 			Vector3 dir = importance_sample_GGX(xi, roughness4);
 			Vector3 light_vec = (2.0 * dir.z * dir - Vector3(0.0, 0.0, 1.0));
 
-			if (light_vec.z < 0.0) {
+			if (light_vec.z <= 0.0) {
 				continue;
 			}
 

drivers/gles3/storage/light_storage.cpp

@@ -1385,7 +1385,7 @@ bool LightStorage::shadow_atlas_update_light(RID p_atlas, RID p_light_instance,
 		old_shadow = old_key & SHADOW_INDEX_MASK;
 
 		// Only re-allocate if a better option is available, and enough time has passed.
-		should_realloc = shadow_atlas->quadrants[old_quadrant].subdivision != (uint32_t)best_subdiv && (shadow_atlas->quadrants[old_quadrant].shadows[old_shadow].alloc_tick - tick > shadow_atlas_realloc_tolerance_msec);
+		should_realloc = shadow_atlas->quadrants[old_quadrant].subdivision != (uint32_t)best_subdiv && (tick - shadow_atlas->quadrants[old_quadrant].shadows[old_shadow].alloc_tick > shadow_atlas_realloc_tolerance_msec);
 		should_redraw = shadow_atlas->quadrants[old_quadrant].shadows[old_shadow].version != p_light_version;
 
 		if (!should_realloc) {

drivers/gles3/storage/mesh_storage.cpp

@@ -844,6 +844,7 @@ void MeshStorage::mesh_clear(RID p_mesh) {
 	mesh->surface_count = 0;
 	mesh->material_cache.clear();
 	mesh->has_bone_weights = false;
+	mesh->aabb = AABB();
 	mesh->dependency.changed_notify(Dependency::DEPENDENCY_CHANGED_MESH);
 
 	for (Mesh *E : mesh->shadow_owners) {

editor/editor_node.cpp

@@ -3434,6 +3434,8 @@ void EditorNode::unload_editor_addons() {
 		remove_editor_plugin(E.value, false);
 		memdelete(E.value);
 	}
+
+	addon_name_to_plugin.clear();
 }
 
 void EditorNode::_discard_changes(const String &p_str) {

modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/ScriptManagerBridge.cs

@@ -505,16 +505,29 @@ internal static unsafe godot_bool AddScriptBridge(IntPtr scriptPtr, godot_string
 
         private static unsafe bool AddScriptBridgeCore(IntPtr scriptPtr, string scriptPath)
         {
-            lock (_scriptTypeBiMap.ReadWriteLock)
+            _scriptTypeBiMap.ReadWriteLock.EnterUpgradeableReadLock();
+            try
             {
                 if (!_scriptTypeBiMap.IsScriptRegistered(scriptPtr))
                 {
                     if (!_pathTypeBiMap.TryGetScriptType(scriptPath, out Type? scriptType))
                         return false;
 
-                    _scriptTypeBiMap.Add(scriptPtr, scriptType);
+                    _scriptTypeBiMap.ReadWriteLock.EnterWriteLock();
+                    try
+                    {
+                        _scriptTypeBiMap.Add(scriptPtr, scriptType);
+                    }
+                    finally
+                    {
+                        _scriptTypeBiMap.ReadWriteLock.ExitWriteLock();
+                    }
                 }
             }
+            finally
+            {
+                _scriptTypeBiMap.ReadWriteLock.ExitUpgradeableReadLock();
+            }
 
             return true;
         }
@@ -537,7 +550,8 @@ internal static unsafe void GetOrCreateScriptBridgeForPath(godot_string* scriptP
 
         private static unsafe void GetOrCreateScriptBridgeForType(Type scriptType, godot_ref* outScript)
         {
-            lock (_scriptTypeBiMap.ReadWriteLock)
+            _scriptTypeBiMap.ReadWriteLock.EnterUpgradeableReadLock();
+            try
             {
                 if (_scriptTypeBiMap.TryGetScriptPtr(scriptType, out IntPtr scriptPtr))
                 {
@@ -549,14 +563,19 @@ private static unsafe void GetOrCreateScriptBridgeForType(Type scriptType, godot
                 // This path is slower, but it's only executed for the first instantiation of the type
                 CreateScriptBridgeForType(scriptType, outScript);
             }
+            finally
+            {
+                _scriptTypeBiMap.ReadWriteLock.ExitUpgradeableReadLock();
+            }
         }
 
         internal static unsafe void GetOrLoadOrCreateScriptForType(Type scriptType, godot_ref* outScript)
         {
             static bool GetPathOtherwiseGetOrCreateScript(Type scriptType, godot_ref* outScript,
                 [MaybeNullWhen(false)] out string scriptPath)
             {
-                lock (_scriptTypeBiMap.ReadWriteLock)
+                _scriptTypeBiMap.ReadWriteLock.EnterUpgradeableReadLock();
+                try
                 {
                     if (_scriptTypeBiMap.TryGetScriptPtr(scriptType, out IntPtr scriptPtr))
                     {
@@ -584,6 +603,10 @@ static bool GetPathOtherwiseGetOrCreateScript(Type scriptType, godot_ref* outScr
                     scriptPath = null;
                     return false;
                 }
+                finally
+                {
+                    _scriptTypeBiMap.ReadWriteLock.ExitUpgradeableReadLock();
+                }
             }
 
             static string GetVirtualConstructedGenericTypeScriptPath(Type scriptType, string scriptPath)
@@ -613,7 +636,16 @@ static string GetVirtualConstructedGenericTypeScriptPath(Type scriptType, string
                     // IMPORTANT: The virtual path must be added to _pathTypeBiMap before the first
                     // load of the script, otherwise the loaded script won't be added to _scriptTypeBiMap.
                     scriptPath = GetVirtualConstructedGenericTypeScriptPath(scriptType, scriptPath);
-                    _pathTypeBiMap.Add(scriptPath, scriptType);
+
+                    _scriptTypeBiMap.ReadWriteLock.EnterWriteLock();
+                    try
+                    {
+                        _pathTypeBiMap.Add(scriptPath, scriptType);
+                    }
+                    finally
+                    {
+                        _scriptTypeBiMap.ReadWriteLock.ExitWriteLock();
+                    }
                 }
 
                 // This must be done outside the read-write lock, as the script resource loading can lock it
@@ -643,89 +675,108 @@ private static unsafe void CreateScriptBridgeForType(Type scriptType, godot_ref*
         {
             Debug.Assert(!scriptType.IsGenericTypeDefinition, $"Script type must be a constructed generic type or not generic at all. Type: {scriptType}.");
 
-            NativeFuncs.godotsharp_internal_new_csharp_script(outScript);
-            IntPtr scriptPtr = outScript->Reference;
+            _scriptTypeBiMap.ReadWriteLock.EnterWriteLock();
+            try
+            {
+                NativeFuncs.godotsharp_internal_new_csharp_script(outScript);
+                IntPtr scriptPtr = outScript->Reference;
 
-            // Caller takes care of locking
-            _scriptTypeBiMap.Add(scriptPtr, scriptType);
+                _scriptTypeBiMap.Add(scriptPtr, scriptType);
+            }
+            finally
+            {
+                _scriptTypeBiMap.ReadWriteLock.ExitWriteLock();
+            }
 
-            NativeFuncs.godotsharp_internal_reload_registered_script(scriptPtr);
+            NativeFuncs.godotsharp_internal_reload_registered_script(outScript->Reference);
         }
 
         [UnmanagedCallersOnly]
         internal static void RemoveScriptBridge(IntPtr scriptPtr)
         {
+            _scriptTypeBiMap.ReadWriteLock.EnterWriteLock();
             try
             {
-                lock (_scriptTypeBiMap.ReadWriteLock)
-                {
-                    _scriptTypeBiMap.Remove(scriptPtr);
-                }
+                _scriptTypeBiMap.Remove(scriptPtr);
             }
             catch (Exception e)
             {
                 ExceptionUtils.LogException(e);
             }
+            finally
+            {
+                _scriptTypeBiMap.ReadWriteLock.ExitWriteLock();
+            }
         }
 
         [UnmanagedCallersOnly]
         internal static godot_bool TryReloadRegisteredScriptWithClass(IntPtr scriptPtr)
         {
+            _scriptTypeBiMap.ReadWriteLock.EnterUpgradeableReadLock();
             try
             {
-                lock (_scriptTypeBiMap.ReadWriteLock)
+                if (_scriptTypeBiMap.TryGetScriptType(scriptPtr, out _))
                 {
-                    if (_scriptTypeBiMap.TryGetScriptType(scriptPtr, out _))
-                    {
-                        // NOTE:
-                        // Currently, we reload all scripts, not only the ones from the unloaded ALC.
-                        // As such, we need to handle this case instead of treating it as an error.
-                        NativeFuncs.godotsharp_internal_reload_registered_script(scriptPtr);
-                        return godot_bool.True;
-                    }
+                    // NOTE:
+                    // Currently, we reload all scripts, not only the ones from the unloaded ALC.
+                    // As such, we need to handle this case instead of treating it as an error.
+                    NativeFuncs.godotsharp_internal_reload_registered_script(scriptPtr);
+                    return godot_bool.True;
+                }
 
-                    if (!_scriptDataForReload.TryGetValue(scriptPtr, out var dataForReload))
-                    {
-                        GD.PushError("Missing class qualified name for reloading script");
-                        return godot_bool.False;
-                    }
+                if (!_scriptDataForReload.TryGetValue(scriptPtr, out var dataForReload))
+                {
+                    GD.PushError("Missing class qualified name for reloading script");
+                    return godot_bool.False;
+                }
 
-                    _ = _scriptDataForReload.TryRemove(scriptPtr, out _);
+                _ = _scriptDataForReload.TryRemove(scriptPtr, out _);
 
-                    if (dataForReload.assemblyName == null)
-                    {
-                        GD.PushError(
-                            $"Missing assembly name of class '{dataForReload.classFullName}' for reloading script");
-                        return godot_bool.False;
-                    }
+                if (dataForReload.assemblyName == null)
+                {
+                    GD.PushError(
+                        $"Missing assembly name of class '{dataForReload.classFullName}' for reloading script");
+                    return godot_bool.False;
+                }
 
-                    var scriptType = ReflectionUtils.FindTypeInLoadedAssemblies(dataForReload.assemblyName,
-                        dataForReload.classFullName);
+                var scriptType = ReflectionUtils.FindTypeInLoadedAssemblies(dataForReload.assemblyName,
+                    dataForReload.classFullName);
 
-                    if (scriptType == null)
-                    {
-                        // The class was removed, can't reload
-                        return godot_bool.False;
-                    }
+                if (scriptType == null)
+                {
+                    // The class was removed, can't reload
+                    return godot_bool.False;
+                }
 
-                    if (!typeof(GodotObject).IsAssignableFrom(scriptType))
-                    {
-                        // The class no longer inherits GodotObject, can't reload
-                        return godot_bool.False;
-                    }
+                if (!typeof(GodotObject).IsAssignableFrom(scriptType))
+                {
+                    // The class no longer inherits GodotObject, can't reload
+                    return godot_bool.False;
+                }
 
+                _scriptTypeBiMap.ReadWriteLock.EnterWriteLock();
+                try
+                {
                     _scriptTypeBiMap.Add(scriptPtr, scriptType);
+                }
+                finally
+                {
+                    _scriptTypeBiMap.ReadWriteLock.ExitWriteLock();
+                }
 
-                    NativeFuncs.godotsharp_internal_reload_registered_script(scriptPtr);
+                NativeFuncs.godotsharp_internal_reload_registered_script(scriptPtr);
 
-                    return godot_bool.True;
-                }
+                return godot_bool.True;
             }
             catch (Exception e)
             {
                 ExceptionUtils.LogException(e);
                 return godot_bool.False;
             }
+            finally
+            {
+                _scriptTypeBiMap.ReadWriteLock.ExitUpgradeableReadLock();
+            }
         }
 
         private static unsafe void GetScriptTypeInfo(Type scriptType, godot_csharp_type_info* outTypeInfo)

modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/ScriptManagerBridge.types.cs

@@ -4,6 +4,7 @@
 using System.Diagnostics.CodeAnalysis;
 using System.Linq;
 using System.Runtime.CompilerServices;
+using System.Threading;
 
 namespace Godot.Bridge;
 
@@ -13,7 +14,7 @@ public static partial class ScriptManagerBridge
 {
     private class ScriptTypeBiMap
     {
-        public readonly object ReadWriteLock = new();
+        public readonly ReaderWriterLockSlim ReadWriteLock = new(LockRecursionPolicy.SupportsRecursion);
         private System.Collections.Generic.Dictionary<IntPtr, Type> _scriptTypeMap = new();
         private System.Collections.Generic.Dictionary<Type, IntPtr> _typeScriptMap = new();