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

MultiMesh custom data bits are not accurate #93875

Closed
FireCatMagic opened this issue Jul 2, 2024 · 3 comments
Closed

MultiMesh custom data bits are not accurate #93875

FireCatMagic opened this issue Jul 2, 2024 · 3 comments

Comments

@FireCatMagic
Copy link

Tested versions

4.3 beta2, 4.1.1 stable

System information

Godot v4.3.beta2 - Windows 10.0.19045 - GLES3 (Compatibility) - NVIDIA GeForce RTX 4050 Laptop GPU (NVIDIA; 31.0.15.5186) - 13th Gen Intel(R) Core(TM) i5-13500HX (20 Threads)

Issue description

Godot docs describe the Color variant as being composed of 4 32-bit values, one float for each value.
The custom data property of Multimesh allows us to use a single Color variant as multimesh custom data.
Theoretically, this allows us to pack up to 128 bits of custom data inside if we use bitwise operations and bit copying without conversions.
However, when done in practice, it doesn't actually work because the bits seem to not be preserved.

Steps to reproduce

I've set up an example project that is supposed to achieve the following task:
Since the custom data of multimesh instances accepts 1 Color value, and the docs state that a Color variant is composed of 4 32-bit values, in theory we could pack 4 different colors into 1 singular color by storing each color channel as a uint8 in a 32-bit integer.
It lines up perfectly, with each color being able to be represented as a 32-bit integer.

@tool
extends Node3D

@export var color_1: Color = Color.RED: set = set_color_1
@export var color_2: Color = Color.BLUE: set = set_color_2
@export var color_3: Color = Color.GREEN: set = set_color_3
@export var color_4: Color = Color.PURPLE: set = set_color_4

func set_color_1(color: Color) -> void:
	color_1 = color
	update_multimesh()
func set_color_2(color: Color) -> void:
	color_2 = color
	update_multimesh()
func set_color_3(color: Color) -> void:
	color_3 = color
	update_multimesh()
func set_color_4(color: Color) -> void:
	color_4 = color
	update_multimesh()

func _ready() -> void:
	get_node(^"MultiMeshInstance3D").multimesh.instance_count = 1
	get_node(^"MultiMeshInstance3D").multimesh.set_instance_transform(0, Transform3D())

func update_multimesh() -> void:
	if not is_inside_tree(): return
	var bytes: PackedByteArray = get_packed_colors()
	var packed_color: Color = Color(bytes.decode_float(0), bytes.decode_float(4), bytes.decode_float(8), bytes.decode_float(12))
	get_node(^"MultiMeshInstance3D").multimesh.set_instance_custom_data(0, packed_color)

func get_packed_colors() -> PackedByteArray:
	return [color_1.r8, color_1.g8, color_1.b8, color_1.a8, color_2.r8, color_2.g8, color_2.b8, color_2.a8, color_3.r8, color_3.g8, color_3.b8, color_3.a8, color_4.r8, color_4.g8, color_4.b8, color_4.a8];

The following line creates the byte array of colors:

func get_packed_colors() -> PackedByteArray:
	return [color_1.r8, color_1.g8, color_1.b8, color_1.a8, color_2.r8, color_2.g8, color_2.b8, color_2.a8, color_3.r8, color_3.g8, color_3.b8, color_3.a8, color_4.r8, color_4.g8, color_4.b8, color_4.a8];

which can then be turned into a color as following by extracting 4 floats:

var packed_color: Color = Color(bytes.decode_half(0), bytes.decode_half(4), bytes.decode_half(8), bytes.decode_half(12))

and then passed into the multimesh:

set_instance_custom_data(0, packed_color)

Then the shader splits each color up, and sets the vertex colors:

shader_type spatial;
render_mode specular_disabled, unshaded;

// convert color8 color to float color
vec4 from_color8(uint[4] color) {
	return vec4(float(color[0]) / 255.0f, float(color[1]) / 255.0f, float(color[2]) / 255.0f, float(color[3]) / 255.0f);
}

uint[4] split_uint(uint value) {
	uint[4] split;
	split[0] = (value >> 0u) & 255u;
	split[1] = (value >> 8u) & 255u;
	split[2] = (value >> 16u) & 255u;
	split[3] = (value >> 24u) & 255u;
	return split;
}

void vertex() {
	uint packed1 = floatBitsToUint(INSTANCE_CUSTOM.r);
	uint packed2 = floatBitsToUint(INSTANCE_CUSTOM.g);
	uint packed3 = floatBitsToUint(INSTANCE_CUSTOM.b);
	uint packed4 = floatBitsToUint(INSTANCE_CUSTOM.a);
	
	vec4 color1 = from_color8(split_uint(packed1));
	vec4 color2 = from_color8(split_uint(packed2));
	vec4 color3 = from_color8(split_uint(packed3));
	vec4 color4 = from_color8(split_uint(packed4));
	
	if (VERTEX.y > 0.5) {
		COLOR = VERTEX.z > 0.4 ? color1 : color2;
	} else {
		COLOR = VERTEX.z > 0.4 ? color3 : color4;
	}
}

void fragment() {
	ALBEDO = COLOR.rgb;
	ALPHA = COLOR.a + 0.2f;
}

But then, when actually put to use, the values do not behave as expected - the colors are totally wrong.
Godot_v4 3-beta2_win64_wJu3FPaahR

Minimal reproduction project (MRP)

MultimeshBits.zip

@Lielay9
Copy link
Contributor

Lielay9 commented Jul 2, 2024

The colors are compressed internally to have 16 bit components (Line 1723):

void MeshStorage::multimesh_instance_set_color(RID p_multimesh, int p_index, const Color &p_color) {
MultiMesh *multimesh = multimesh_owner.get_or_null(p_multimesh);
ERR_FAIL_NULL(multimesh);
ERR_FAIL_INDEX(p_index, multimesh->instances);
ERR_FAIL_COND(!multimesh->uses_colors);
_multimesh_make_local(multimesh);
{
// Colors are packed into 2 floats.
float *w = multimesh->data_cache.ptrw();
float *dataptr = w + p_index * multimesh->stride_cache + multimesh->color_offset_cache;
uint16_t val[4] = { Math::make_half_float(p_color.r), Math::make_half_float(p_color.g), Math::make_half_float(p_color.b), Math::make_half_float(p_color.a) };
memcpy(dataptr, val, 2 * 4);
}
_multimesh_mark_dirty(multimesh, p_index, false);
}

So some precision is lost:

var mm: MultiMesh = MultiMesh.new()
mm.use_colors = true
mm.instance_count = 1
var color_in: Color = Color(0.3, 0.3, 0.3, 0.3)
mm.set_instance_color(0, color_in)
print(color_in == mm.get_instance_color(0)) # false;
print(mm.get_instance_color(0)) # (0.2998, 0.2998, 0.2998, 0.2998)

This isn't well documented and has also been a problem for me before when I tried to use the custom data (which is compressed the same way). I think having the option to set the raw data would be ideal, at least for the custom data.

@FireCatMagic FireCatMagic changed the title MultiMesh custom data bits are not accurated MultiMesh custom data bits are not accurate Jul 2, 2024
@FireCatMagic
Copy link
Author

The colors are compressed internally to have 16 bit components (Line 1723):

void MeshStorage::multimesh_instance_set_color(RID p_multimesh, int p_index, const Color &p_color) {
MultiMesh *multimesh = multimesh_owner.get_or_null(p_multimesh);
ERR_FAIL_NULL(multimesh);
ERR_FAIL_INDEX(p_index, multimesh->instances);
ERR_FAIL_COND(!multimesh->uses_colors);
_multimesh_make_local(multimesh);
{
// Colors are packed into 2 floats.
float *w = multimesh->data_cache.ptrw();
float *dataptr = w + p_index * multimesh->stride_cache + multimesh->color_offset_cache;
uint16_t val[4] = { Math::make_half_float(p_color.r), Math::make_half_float(p_color.g), Math::make_half_float(p_color.b), Math::make_half_float(p_color.a) };
memcpy(dataptr, val, 2 * 4);
}
_multimesh_mark_dirty(multimesh, p_index, false);
}

So some precision is lost:

var mm: MultiMesh = MultiMesh.new()
mm.use_colors = true
mm.instance_count = 1
var color_in: Color = Color(0.3, 0.3, 0.3, 0.3)
mm.set_instance_color(0, color_in)
print(color_in == mm.get_instance_color(0)) # false;
print(mm.get_instance_color(0)) # (0.2998, 0.2998, 0.2998, 0.2998)

This isn't well documented and has also been a problem for me before when I tried to use the custom data (which is compressed the same way). I think having the option to set the raw data would be ideal, at least for the custom data.

ah, this makes sense.

It really feels essential that there exists a way to pass in raw data into the custom data in some form.
Maybe even a way to access this in raw form too, to not break compatibility? Like in addition to INSTANCE_CUSTOM, you could use INSTANCE_CUSTOM_RAW and retrieve either a vec2 with the 2 float components, or some other thing.

I can easily pack data into a 64-bit structure if the channels are only 16 bits. But due to the data compression, it doesn't seem like there is actually a way to achieve this at all even if I create my structure beforehand, since no matter what the data is going to get messed up even if the values put inside the color are a half_float

@akien-mga
Copy link
Member

Closed by #94942.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants