Avoid sync issues in materials with scheduled shader updates

core/io/resource_loader.h

@@ -199,6 +199,8 @@ public:
 	static ThreadLoadStatus load_threaded_get_status(const String &p_path, float *r_progress = nullptr);
 	static Ref<Resource> load_threaded_get(const String &p_path, Error *r_error = nullptr);
 
+	static bool is_within_load() { return load_nesting > 0; };
+
 	static Ref<Resource> load(const String &p_path, const String &p_type_hint = "", ResourceFormatLoader::CacheMode p_cache_mode = ResourceFormatLoader::CACHE_MODE_REUSE, Error *r_error = nullptr);
 	static bool exists(const String &p_path, const String &p_type_hint = "");
 

scene/resources/canvas_item_material.cpp

@@ -161,7 +161,7 @@ void CanvasItemMaterial::flush_changes() {
 void CanvasItemMaterial::_queue_shader_change() {
 	MutexLock lock(material_mutex);
 
-	if (is_initialized && !element.in_list()) {
+	if (_is_initialized() && !element.in_list()) {
 		dirty_materials->add(&element);
 	}
 }
@@ -287,8 +287,8 @@ CanvasItemMaterial::CanvasItemMaterial() :
 	set_particles_anim_loop(false);
 
 	current_key.invalid_key = 1;
-	is_initialized = true;
+
-	_queue_shader_change();
+	_mark_initialized(callable_mp(this, &CanvasItemMaterial::_queue_shader_change));
 }
 
 CanvasItemMaterial::~CanvasItemMaterial() {

scene/resources/canvas_item_material.h

@@ -105,7 +105,6 @@ private:
 	_FORCE_INLINE_ void _queue_shader_change();
 	_FORCE_INLINE_ bool _is_shader_dirty() const;
 
-	bool is_initialized = false;
 	BlendMode blend_mode = BLEND_MODE_MIX;
 	LightMode light_mode = LIGHT_MODE_NORMAL;
 	bool particles_animation = false;

scene/resources/material.cpp

@@ -82,6 +82,23 @@ void Material::_validate_property(PropertyInfo &p_property) const {
 	}
 }
 
+void Material::_mark_initialized(const Callable &p_queue_shader_change_callable) {
+	// If this is happening as part of resource loading, it is not safe to queue the update
+	// as an addition to the dirty list, unless the load is happening on the main thread.
+	if (ResourceLoader::is_within_load() && Thread::get_caller_id() != Thread::get_main_id()) {
+		DEV_ASSERT(init_state != INIT_STATE_READY);
+		if (init_state == INIT_STATE_UNINITIALIZED) { // Prevent queueing twice.
+			// Queue an individual update of this material (the ResourceLoader knows how to handle deferred calls safely).
+			p_queue_shader_change_callable.call_deferred();
+			init_state = INIT_STATE_INITIALIZING;
+		}
+	} else {
+		// Straightforward conditions.
+		init_state = INIT_STATE_READY;
+		p_queue_shader_change_callable.callv(Array());
+	}
+}
+
 void Material::inspect_native_shader_code() {
 	SceneTree *st = Object::cast_to<SceneTree>(OS::get_singleton()->get_main_loop());
 	RID shader = get_shader_rid();
@@ -1485,7 +1502,7 @@ void BaseMaterial3D::flush_changes() {
 void BaseMaterial3D::_queue_shader_change() {
 	MutexLock lock(material_mutex);
 
-	if (is_initialized && !element.in_list()) {
+	if (_is_initialized() && !element.in_list()) {
 		dirty_materials->add(&element);
 	}
 }
@@ -3028,8 +3045,7 @@ BaseMaterial3D::BaseMaterial3D(bool p_orm) :
 	flags[FLAG_ALBEDO_TEXTURE_MSDF] = false;
 	flags[FLAG_USE_TEXTURE_REPEAT] = true;
 
-	is_initialized = true;
+	_mark_initialized(callable_mp(this, &BaseMaterial3D::_queue_shader_change));
-	_queue_shader_change();
 }
 
 BaseMaterial3D::~BaseMaterial3D() {

scene/resources/material.h

@@ -46,6 +46,12 @@ class Material : public Resource {
 	Ref<Material> next_pass;
 	int render_priority;
 
+	enum {
+		INIT_STATE_UNINITIALIZED,
+		INIT_STATE_INITIALIZING,
+		INIT_STATE_READY,
+	} init_state = INIT_STATE_UNINITIALIZED;
+
 	void inspect_native_shader_code();
 
 protected:
@@ -56,6 +62,9 @@ protected:
 
 	void _validate_property(PropertyInfo &p_property) const;
 
+	void _mark_initialized(const Callable &p_queue_shader_change_callable);
+	bool _is_initialized() { return init_state == INIT_STATE_READY; }
+
 	GDVIRTUAL0RC(RID, _get_shader_rid)
 	GDVIRTUAL0RC(Shader::Mode, _get_shader_mode)
 	GDVIRTUAL0RC(bool, _can_do_next_pass)
@@ -452,7 +461,6 @@ private:
 	_FORCE_INLINE_ void _queue_shader_change();
 	_FORCE_INLINE_ bool _is_shader_dirty() const;
 
-	bool is_initialized = false;
 	bool orm;
 
 	Color albedo;

scene/resources/particle_process_material.cpp

@@ -915,7 +915,7 @@ void ParticleProcessMaterial::flush_changes() {
 void ParticleProcessMaterial::_queue_shader_change() {
 	MutexLock lock(material_mutex);
 
-	if (is_initialized && !element.in_list()) {
+	if (_is_initialized() && !element.in_list()) {
 		dirty_materials->add(&element);
 	}
 }
@@ -1889,8 +1889,7 @@ ParticleProcessMaterial::ParticleProcessMaterial() :
 
 	current_key.invalid_key = 1;
 
-	is_initialized = true;
+	_mark_initialized(callable_mp(this, &ParticleProcessMaterial::_queue_shader_change));
-	_queue_shader_change();
 }
 
 ParticleProcessMaterial::~ParticleProcessMaterial() {

scene/resources/particle_process_material.h

@@ -261,7 +261,6 @@ private:
 	_FORCE_INLINE_ void _queue_shader_change();
 	_FORCE_INLINE_ bool _is_shader_dirty() const;
 
-	bool is_initialized = false;
 	Vector3 direction;
 	float spread = 0.0f;
 	float flatness = 0.0f;