Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix shader uniforms has null as default value #95626

Merged
merged 1 commit into from
Jan 31, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions drivers/gles3/storage/material_storage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,9 @@ Variant ShaderData::get_default_parameter(const StringName &p_parameter) const {
if (uniforms.has(p_parameter)) {
ShaderLanguage::ShaderNode::Uniform uniform = uniforms[p_parameter];
Vector<ShaderLanguage::Scalar> default_value = uniform.default_value;
if (default_value.is_empty()) {
return ShaderLanguage::get_default_datatype_value(uniform.type, uniform.array_size, uniform.hint);
}
return ShaderLanguage::constant_value_to_variant(default_value, uniform.type, uniform.array_size, uniform.hint);
}
return Variant();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -569,6 +569,9 @@ Variant MaterialStorage::ShaderData::get_default_parameter(const StringName &p_p
if (uniforms.has(p_parameter)) {
ShaderLanguage::ShaderNode::Uniform uniform = uniforms[p_parameter];
Vector<ShaderLanguage::Scalar> default_value = uniform.default_value;
if (default_value.is_empty()) {
return ShaderLanguage::get_default_datatype_value(uniform.type, uniform.array_size, uniform.hint);
}
return ShaderLanguage::constant_value_to_variant(default_value, uniform.type, uniform.array_size, uniform.hint);
}
return Variant();
Expand Down
291 changes: 291 additions & 0 deletions servers/rendering/shader_language.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4513,6 +4513,297 @@ Variant ShaderLanguage::constant_value_to_variant(const Vector<Scalar> &p_value,
return Variant();
}

Variant ShaderLanguage::get_default_datatype_value(DataType p_type, int p_array_size, ShaderLanguage::ShaderNode::Uniform::Hint p_hint) {
int array_size = p_array_size;

Variant value;
switch (p_type) {
case ShaderLanguage::TYPE_BOOL:
if (array_size > 0) {
PackedInt32Array array;
for (int i = 0; i < array_size; i++) {
array.push_back(false);
}
value = Variant(array);
} else {
VariantInitializer<bool>::init(&value);
VariantDefaultInitializer<bool>::init(&value);
}
break;
Comment on lines +4521 to +4532
Copy link
Member

@akien-mga akien-mga Jan 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TL;DR: I looked into how to improve this code myself but in the end I gave up.

I was still wondering if there isn't a simpler way to do this. There must exist a pre-existing method in the codebase to construct a Variant with a given type and default initialize it.

From a quick look, I found:

void Variant::construct(Variant::Type p_type, Variant &base, const Variant **p_args, int p_argcount, Callable::CallError &r_error);

Which I think you would use like this:

Callable::CallError ce;
Variant value;
Variant::construct(variant_type, value, nullptr, 0, ce);

This would allow to simplify the cases quite a bit, like (untested):

switch (p_type) {
	case ShaderLanguage::TYPE_BOOL:
		if (array_size > 0) {
			type = Variant::PACKED_INT32_ARRAY;
		} else {
			type = Variant::BOOL;
		}
		break;
	case ShaderLanguage::TYPE_BVEC2:
		if (array_size > 0) {
			type = Variant::PACKED_INT32_ARRAY;
			array_size *= 2;
		} else {
			type = Variant::INT;
		}
		break;
	...
	}
}

Callable::CallError ce;
Variant value;
Variant::construct(variant_type, value, nullptr, 0, ce);

if (array_size > 0) {
	// Figure out a way to resize the array in Variant and `zero()` it.
}

But I finally noticed that this is pretty much adapted from ShaderLanguage::constant_value_to_variant and so keeping a consistent format might be worth it. The type conversions are also less trivial than I thought with types not natively compatible with Variant like bvec2.

And so at this point I gave up on trying to make this better :P

case ShaderLanguage::TYPE_BVEC2:
array_size *= 2;

if (array_size > 0) {
PackedInt32Array array;
for (int i = 0; i < array_size; i++) {
array.push_back(false);
}
value = Variant(array);
} else {
VariantInitializer<int64_t>::init(&value);
VariantDefaultInitializer<int64_t>::init(&value);
}
break;
case ShaderLanguage::TYPE_BVEC3:
array_size *= 3;

if (array_size > 0) {
PackedInt32Array array;
for (int i = 0; i < array_size; i++) {
array.push_back(false);
}
value = Variant(array);
} else {
VariantInitializer<int64_t>::init(&value);
VariantDefaultInitializer<int64_t>::init(&value);
}
break;
case ShaderLanguage::TYPE_BVEC4:
array_size *= 4;

if (array_size > 0) {
PackedInt32Array array;
for (int i = 0; i < array_size; i++) {
array.push_back(false);
}
value = Variant(array);
} else {
VariantInitializer<int64_t>::init(&value);
VariantDefaultInitializer<int64_t>::init(&value);
}
break;
case ShaderLanguage::TYPE_INT:
if (array_size > 0) {
PackedInt32Array array;
for (int i = 0; i < array_size; i++) {
array.push_back(0);
}
value = Variant(array);
} else {
VariantInitializer<int64_t>::init(&value);
VariantDefaultInitializer<int64_t>::init(&value);
}
break;
case ShaderLanguage::TYPE_IVEC2:
if (array_size > 0) {
array_size *= 2;

PackedInt32Array array;
for (int i = 0; i < array_size; i++) {
array.push_back(0);
}
value = Variant(array);
} else {
VariantInitializer<Vector2i>::init(&value);
VariantDefaultInitializer<Vector2i>::init(&value);
}
break;
case ShaderLanguage::TYPE_IVEC3:
if (array_size > 0) {
array_size *= 3;

PackedInt32Array array;
for (int i = 0; i < array_size; i++) {
array.push_back(0);
}
value = Variant(array);
} else {
VariantInitializer<Vector3i>::init(&value);
VariantDefaultInitializer<Vector3i>::init(&value);
}
break;
case ShaderLanguage::TYPE_IVEC4:
if (array_size > 0) {
array_size *= 4;

PackedInt32Array array;
for (int i = 0; i < array_size; i++) {
array.push_back(0);
}
value = Variant(array);
} else {
VariantInitializer<Vector4i>::init(&value);
VariantDefaultInitializer<Vector4i>::init(&value);
}
break;
case ShaderLanguage::TYPE_UINT:
if (array_size > 0) {
PackedInt32Array array;
for (int i = 0; i < array_size; i++) {
array.push_back(0U);
}
value = Variant(array);
} else {
VariantInitializer<int64_t>::init(&value);
VariantDefaultInitializer<int64_t>::init(&value);
}
break;
case ShaderLanguage::TYPE_UVEC2:
if (array_size > 0) {
array_size *= 2;

PackedInt32Array array;
for (int i = 0; i < array_size; i++) {
array.push_back(0U);
}
value = Variant(array);
} else {
VariantInitializer<Vector2i>::init(&value);
VariantDefaultInitializer<Vector2i>::init(&value);
}
break;
case ShaderLanguage::TYPE_UVEC3:
if (array_size > 0) {
array_size *= 3;

PackedInt32Array array;
for (int i = 0; i < array_size; i++) {
array.push_back(0U);
}
value = Variant(array);
} else {
VariantInitializer<Vector3i>::init(&value);
VariantDefaultInitializer<Vector3i>::init(&value);
}
break;
case ShaderLanguage::TYPE_UVEC4:
if (array_size > 0) {
array_size *= 4;

PackedInt32Array array;
for (int i = 0; i < array_size; i++) {
array.push_back(0U);
}
value = Variant(array);
} else {
VariantInitializer<Vector4i>::init(&value);
VariantDefaultInitializer<Vector4i>::init(&value);
}
break;
case ShaderLanguage::TYPE_FLOAT:
if (array_size > 0) {
PackedFloat32Array array;
for (int i = 0; i < array_size; i++) {
array.push_back(0.0f);
}
value = Variant(array);
} else {
VariantInitializer<float>::init(&value);
VariantDefaultInitializer<float>::init(&value);
}
break;
case ShaderLanguage::TYPE_VEC2:
if (array_size > 0) {
PackedVector2Array array;
for (int i = 0; i < array_size; i++) {
array.push_back(Vector2(0.0f, 0.0f));
}
value = Variant(array);
} else {
VariantInitializer<Vector2>::init(&value);
VariantDefaultInitializer<Vector2>::init(&value);
}
break;
case ShaderLanguage::TYPE_VEC3:
if (array_size > 0) {
if (p_hint == ShaderLanguage::ShaderNode::Uniform::HINT_SOURCE_COLOR) {
PackedColorArray array;
for (int i = 0; i < array_size; i++) {
array.push_back(Color(0.0f, 0.0f, 0.0f));
}
value = Variant(array);
} else {
PackedVector3Array array;
for (int i = 0; i < array_size; i++) {
array.push_back(Vector3(0.0f, 0.0f, 0.0f));
}
value = Variant(array);
}
} else {
if (p_hint == ShaderLanguage::ShaderNode::Uniform::HINT_SOURCE_COLOR) {
VariantInitializer<Color>::init(&value);
VariantDefaultInitializer<Color>::init(&value);
} else {
VariantInitializer<Vector3>::init(&value);
VariantDefaultInitializer<Vector3>::init(&value);
}
}
break;
case ShaderLanguage::TYPE_VEC4:
if (array_size > 0) {
if (p_hint == ShaderLanguage::ShaderNode::Uniform::HINT_SOURCE_COLOR) {
PackedColorArray array;
for (int i = 0; i < array_size; i++) {
array.push_back(Color(0.0f, 0.0f, 0.0f, 0.0f));
}
value = Variant(array);
} else {
PackedVector4Array array;
for (int i = 0; i < array_size; i++) {
array.push_back(Vector4(0.0f, 0.0f, 0.0f, 0.0f));
}
value = Variant(array);
}
} else {
if (p_hint == ShaderLanguage::ShaderNode::Uniform::HINT_SOURCE_COLOR) {
VariantInitializer<Color>::init(&value);
VariantDefaultInitializer<Color>::init(&value);
} else {
VariantInitializer<Vector4>::init(&value);
VariantDefaultInitializer<Vector4>::init(&value);
}
}
break;
case ShaderLanguage::TYPE_MAT2:
if (array_size > 0) {
PackedFloat32Array array;
for (int i = 0; i < array_size; i++) {
for (int j = 0; j < 4; j++) {
array.push_back(0.0f);
}
}
value = Variant(array);
} else {
VariantInitializer<Transform2D>::init(&value);
VariantDefaultInitializer<Transform2D>::init(&value);
}
break;
case ShaderLanguage::TYPE_MAT3: {
if (array_size > 0) {
PackedFloat32Array array;
for (int i = 0; i < array_size; i++) {
for (int j = 0; j < 9; j++) {
array.push_back(0.0f);
}
}
value = Variant(array);
} else {
VariantInitializer<Basis>::init(&value);
VariantDefaultInitializer<Basis>::init(&value);
}
break;
}
case ShaderLanguage::TYPE_MAT4: {
if (array_size > 0) {
PackedFloat32Array array;
for (int i = 0; i < array_size; i++) {
for (int j = 0; j < 16; j++) {
array.push_back(0.0f);
}
}
value = Variant(array);
} else {
VariantInitializer<Projection>::init(&value);
VariantDefaultInitializer<Projection>::init(&value);
}
break;
}
default: {
} break;
}
return value;
}

PropertyInfo ShaderLanguage::uniform_to_property_info(const ShaderNode::Uniform &p_uniform) {
PropertyInfo pi;
switch (p_uniform.type) {
Expand Down
1 change: 1 addition & 0 deletions servers/rendering/shader_language.h
Original file line number Diff line number Diff line change
Expand Up @@ -824,6 +824,7 @@ class ShaderLanguage {
static bool is_float_type(DataType p_type);
static bool is_sampler_type(DataType p_type);
static Variant constant_value_to_variant(const Vector<Scalar> &p_value, DataType p_type, int p_array_size, ShaderLanguage::ShaderNode::Uniform::Hint p_hint = ShaderLanguage::ShaderNode::Uniform::HINT_NONE);
static Variant get_default_datatype_value(DataType p_type, int p_array_size, ShaderLanguage::ShaderNode::Uniform::Hint p_hint);
static PropertyInfo uniform_to_property_info(const ShaderNode::Uniform &p_uniform);
static uint32_t get_datatype_size(DataType p_type);
static uint32_t get_datatype_component_count(DataType p_type);
Expand Down