Browse Source

Fix Command Queue Crash

* No longer allow sending an object (texture) to the server as material parameter
* Keep a parameter cache locally in ShaderMaterial
reduz 4 years ago
parent
commit
d41e3f9aeb

+ 6 - 1
scene/3d/visual_instance_3d.cpp

@@ -293,7 +293,12 @@ void GeometryInstance3D::set_shader_instance_uniform(const StringName &p_uniform
 		instance_uniforms.erase(p_value);
 	} else {
 		instance_uniforms[p_uniform] = p_value;
-		RS::get_singleton()->instance_geometry_set_shader_parameter(get_instance(), p_uniform, p_value);
+		if (p_value.get_type() == Variant::OBJECT) {
+			RID tex_id = p_value;
+			RS::get_singleton()->instance_geometry_set_shader_parameter(get_instance(), p_uniform, tex_id);
+		} else {
+			RS::get_singleton()->instance_geometry_set_shader_parameter(get_instance(), p_uniform, p_value);
+		}
 	}
 }
 

+ 12 - 2
scene/main/shader_globals_override.cpp

@@ -63,7 +63,12 @@ bool ShaderGlobalsOverride::_set(const StringName &p_name, const Variant &p_valu
 		if (o) {
 			o->override = p_value;
 			if (active) {
-				RS::get_singleton()->global_variable_set_override(*r, p_value);
+				if (o->override.get_type() == Variant::OBJECT) {
+					RID tex_rid = p_value;
+					RS::get_singleton()->global_variable_set_override(*r, tex_rid);
+				} else {
+					RS::get_singleton()->global_variable_set_override(*r, p_value);
+				}
 			}
 			o->in_use = p_value.get_type() != Variant::NIL;
 			return true;
@@ -228,7 +233,12 @@ void ShaderGlobalsOverride::_activate() {
 		while ((K = overrides.next(K))) {
 			Override *o = overrides.getptr(*K);
 			if (o->in_use && o->override.get_type() != Variant::NIL) {
-				RS::get_singleton()->global_variable_set_override(*K, o->override);
+				if (o->override.get_type() == Variant::OBJECT) {
+					RID tex_rid = o->override;
+					RS::get_singleton()->global_variable_set_override(*K, tex_rid);
+				} else {
+					RS::get_singleton()->global_variable_set_override(*K, o->override);
+				}
 			}
 		}
 

+ 29 - 4
scene/resources/material.cpp

@@ -130,7 +130,7 @@ bool ShaderMaterial::_set(const StringName &p_name, const Variant &p_value) {
 			}
 		}
 		if (pr) {
-			RenderingServer::get_singleton()->material_set_param(_get_material(), pr, p_value);
+			set_shader_param(pr, p_value);
 			return true;
 		}
 	}
@@ -152,7 +152,12 @@ bool ShaderMaterial::_get(const StringName &p_name, Variant &r_ret) const {
 		}
 
 		if (pr) {
-			r_ret = RenderingServer::get_singleton()->material_get_param(_get_material(), pr);
+			const Map<StringName, Variant>::Element *E = param_cache.find(pr);
+			if (E) {
+				r_ret = E->get();
+			} else {
+				r_ret = Variant();
+			}
 			return true;
 		}
 	}
@@ -219,11 +224,31 @@ Ref<Shader> ShaderMaterial::get_shader() const {
 }
 
 void ShaderMaterial::set_shader_param(const StringName &p_param, const Variant &p_value) {
-	RS::get_singleton()->material_set_param(_get_material(), p_param, p_value);
+	if (p_value.get_type() == Variant::NIL) {
+		param_cache.erase(p_param);
+		RS::get_singleton()->material_set_param(_get_material(), p_param, Variant());
+	} else {
+		param_cache[p_param] = p_value;
+		if (p_value.get_type() == Variant::OBJECT) {
+			RID tex_rid = p_value;
+			if (tex_rid == RID()) {
+				param_cache.erase(p_param);
+				RS::get_singleton()->material_set_param(_get_material(), p_param, Variant());
+			} else {
+				RS::get_singleton()->material_set_param(_get_material(), p_param, tex_rid);
+			}
+		} else {
+			RS::get_singleton()->material_set_param(_get_material(), p_param, p_value);
+		}
+	}
 }
 
 Variant ShaderMaterial::get_shader_param(const StringName &p_param) const {
-	return RS::get_singleton()->material_get_param(_get_material(), p_param);
+	if (param_cache.has(p_param)) {
+		return param_cache[p_param];
+	} else {
+		return Variant();
+	}
 }
 
 void ShaderMaterial::_shader_changed() {

+ 2 - 0
scene/resources/material.h

@@ -79,6 +79,8 @@ class ShaderMaterial : public Material {
 	GDCLASS(ShaderMaterial, Material);
 	Ref<Shader> shader;
 
+	Map<StringName, Variant> param_cache;
+
 protected:
 	bool _set(const StringName &p_name, const Variant &p_value);
 	bool _get(const StringName &p_name, Variant &r_ret) const;

+ 21 - 15
scene/resources/particles_material.cpp

@@ -852,52 +852,54 @@ void ParticlesMaterial::set_param_texture(Parameter p_param, const Ref<Texture2D
 
 	tex_parameters[p_param] = p_texture;
 
+	RID tex_rid = p_texture.is_valid() ? p_texture->get_rid() : RID();
+
 	switch (p_param) {
 		case PARAM_INITIAL_LINEAR_VELOCITY: {
 			//do none for this one
 		} break;
 		case PARAM_ANGULAR_VELOCITY: {
-			RenderingServer::get_singleton()->material_set_param(_get_material(), shader_names->angular_velocity_texture, p_texture);
+			RenderingServer::get_singleton()->material_set_param(_get_material(), shader_names->angular_velocity_texture, tex_rid);
 			_adjust_curve_range(p_texture, -360, 360);
 		} break;
 		case PARAM_ORBIT_VELOCITY: {
-			RenderingServer::get_singleton()->material_set_param(_get_material(), shader_names->orbit_velocity_texture, p_texture);
+			RenderingServer::get_singleton()->material_set_param(_get_material(), shader_names->orbit_velocity_texture, tex_rid);
 			_adjust_curve_range(p_texture, -500, 500);
 		} break;
 		case PARAM_LINEAR_ACCEL: {
-			RenderingServer::get_singleton()->material_set_param(_get_material(), shader_names->linear_accel_texture, p_texture);
+			RenderingServer::get_singleton()->material_set_param(_get_material(), shader_names->linear_accel_texture, tex_rid);
 			_adjust_curve_range(p_texture, -200, 200);
 		} break;
 		case PARAM_RADIAL_ACCEL: {
-			RenderingServer::get_singleton()->material_set_param(_get_material(), shader_names->radial_accel_texture, p_texture);
+			RenderingServer::get_singleton()->material_set_param(_get_material(), shader_names->radial_accel_texture, tex_rid);
 			_adjust_curve_range(p_texture, -200, 200);
 		} break;
 		case PARAM_TANGENTIAL_ACCEL: {
-			RenderingServer::get_singleton()->material_set_param(_get_material(), shader_names->tangent_accel_texture, p_texture);
+			RenderingServer::get_singleton()->material_set_param(_get_material(), shader_names->tangent_accel_texture, tex_rid);
 			_adjust_curve_range(p_texture, -200, 200);
 		} break;
 		case PARAM_DAMPING: {
-			RenderingServer::get_singleton()->material_set_param(_get_material(), shader_names->damping_texture, p_texture);
+			RenderingServer::get_singleton()->material_set_param(_get_material(), shader_names->damping_texture, tex_rid);
 			_adjust_curve_range(p_texture, 0, 100);
 		} break;
 		case PARAM_ANGLE: {
-			RenderingServer::get_singleton()->material_set_param(_get_material(), shader_names->angle_texture, p_texture);
+			RenderingServer::get_singleton()->material_set_param(_get_material(), shader_names->angle_texture, tex_rid);
 			_adjust_curve_range(p_texture, -360, 360);
 		} break;
 		case PARAM_SCALE: {
-			RenderingServer::get_singleton()->material_set_param(_get_material(), shader_names->scale_texture, p_texture);
+			RenderingServer::get_singleton()->material_set_param(_get_material(), shader_names->scale_texture, tex_rid);
 			_adjust_curve_range(p_texture, 0, 1);
 		} break;
 		case PARAM_HUE_VARIATION: {
-			RenderingServer::get_singleton()->material_set_param(_get_material(), shader_names->hue_variation_texture, p_texture);
+			RenderingServer::get_singleton()->material_set_param(_get_material(), shader_names->hue_variation_texture, tex_rid);
 			_adjust_curve_range(p_texture, -1, 1);
 		} break;
 		case PARAM_ANIM_SPEED: {
-			RenderingServer::get_singleton()->material_set_param(_get_material(), shader_names->anim_speed_texture, p_texture);
+			RenderingServer::get_singleton()->material_set_param(_get_material(), shader_names->anim_speed_texture, tex_rid);
 			_adjust_curve_range(p_texture, 0, 200);
 		} break;
 		case PARAM_ANIM_OFFSET: {
-			RenderingServer::get_singleton()->material_set_param(_get_material(), shader_names->anim_offset_texture, p_texture);
+			RenderingServer::get_singleton()->material_set_param(_get_material(), shader_names->anim_offset_texture, tex_rid);
 		} break;
 		case PARAM_MAX:
 			break; // Can't happen, but silences warning
@@ -923,7 +925,8 @@ Color ParticlesMaterial::get_color() const {
 
 void ParticlesMaterial::set_color_ramp(const Ref<Texture2D> &p_texture) {
 	color_ramp = p_texture;
-	RenderingServer::get_singleton()->material_set_param(_get_material(), shader_names->color_ramp, p_texture);
+	RID tex_rid = p_texture.is_valid() ? p_texture->get_rid() : RID();
+	RenderingServer::get_singleton()->material_set_param(_get_material(), shader_names->color_ramp, tex_rid);
 	_queue_shader_change();
 	notify_property_list_changed();
 }
@@ -965,17 +968,20 @@ void ParticlesMaterial::set_emission_box_extents(Vector3 p_extents) {
 
 void ParticlesMaterial::set_emission_point_texture(const Ref<Texture2D> &p_points) {
 	emission_point_texture = p_points;
-	RenderingServer::get_singleton()->material_set_param(_get_material(), shader_names->emission_texture_points, p_points);
+	RID tex_rid = p_points.is_valid() ? p_points->get_rid() : RID();
+	RenderingServer::get_singleton()->material_set_param(_get_material(), shader_names->emission_texture_points, tex_rid);
 }
 
 void ParticlesMaterial::set_emission_normal_texture(const Ref<Texture2D> &p_normals) {
 	emission_normal_texture = p_normals;
-	RenderingServer::get_singleton()->material_set_param(_get_material(), shader_names->emission_texture_normal, p_normals);
+	RID tex_rid = p_normals.is_valid() ? p_normals->get_rid() : RID();
+	RenderingServer::get_singleton()->material_set_param(_get_material(), shader_names->emission_texture_normal, tex_rid);
 }
 
 void ParticlesMaterial::set_emission_color_texture(const Ref<Texture2D> &p_colors) {
 	emission_color_texture = p_colors;
-	RenderingServer::get_singleton()->material_set_param(_get_material(), shader_names->emission_texture_color, p_colors);
+	RID tex_rid = p_colors.is_valid() ? p_colors->get_rid() : RID();
+	RenderingServer::get_singleton()->material_set_param(_get_material(), shader_names->emission_texture_color, tex_rid);
 	_queue_shader_change();
 }
 

+ 4 - 2
scene/resources/sky_material.cpp

@@ -270,7 +270,8 @@ ProceduralSkyMaterial::~ProceduralSkyMaterial() {
 
 void PanoramaSkyMaterial::set_panorama(const Ref<Texture2D> &p_panorama) {
 	panorama = p_panorama;
-	RS::get_singleton()->material_set_param(_get_material(), "source_panorama", panorama);
+	RID tex_rid = p_panorama.is_valid() ? p_panorama->get_rid() : RID();
+	RS::get_singleton()->material_set_param(_get_material(), "source_panorama", tex_rid);
 }
 
 Ref<Texture2D> PanoramaSkyMaterial::get_panorama() const {
@@ -411,7 +412,8 @@ float PhysicalSkyMaterial::get_dither_strength() const {
 
 void PhysicalSkyMaterial::set_night_sky(const Ref<Texture2D> &p_night_sky) {
 	night_sky = p_night_sky;
-	RS::get_singleton()->material_set_param(_get_material(), "night_sky", night_sky);
+	RID tex_rid = p_night_sky.is_valid() ? p_night_sky->get_rid() : RID();
+	RS::get_singleton()->material_set_param(_get_material(), "night_sky", tex_rid);
 }
 
 Ref<Texture2D> PhysicalSkyMaterial::get_night_sky() const {

+ 4 - 0
servers/rendering/renderer_rd/renderer_storage_rd.cpp

@@ -1601,6 +1601,7 @@ void RendererStorageRD::material_set_param(RID p_material, const StringName &p_p
 	if (p_value.get_type() == Variant::NIL) {
 		material->params.erase(p_param);
 	} else {
+		ERR_FAIL_COND(p_value.get_type() == Variant::OBJECT); //object not allowed
 		material->params[p_param] = p_value;
 	}
 
@@ -8322,6 +8323,9 @@ void RendererStorageRD::global_variable_set_override(const StringName &p_name, c
 	if (!global_variables.variables.has(p_name)) {
 		return; //variable may not exist
 	}
+
+	ERR_FAIL_COND(p_value.get_type() == Variant::OBJECT);
+
 	GlobalVariables::Variable &gv = global_variables.variables[p_name];
 
 	gv.override = p_value;

+ 2 - 0
servers/rendering/renderer_scene_cull.cpp

@@ -1315,6 +1315,8 @@ void RendererSceneCull::instance_geometry_set_shader_parameter(RID p_instance, c
 	Instance *instance = instance_owner.getornull(p_instance);
 	ERR_FAIL_COND(!instance);
 
+	ERR_FAIL_COND(p_value.get_type() == Variant::OBJECT);
+
 	Map<StringName, Instance::InstanceShaderParameter>::Element *E = instance->instance_shader_parameters.find(p_parameter);
 
 	if (!E) {