Support passing raw data to multimesh in RenderingServer

[huwpascoe]

Closes godotengine/godot-proposals#10650

and here's a test project with some torus mixing between COLOR and INSTANCE_CUSTOM. (It has a hacky check to determine if it's running OpenGL or Vulkan so keep that in mind.)
testproj.zip

[aaronfranke]

Your PR is based off of the 4.3 branch for 4.3.x (4.3.1 etc), but your PR is targeting the master branch for 4.x (4.4 etc).

[huwpascoe]

Your PR is based off of the 4.3 branch for 4.3.x (4.3.1 etc), but your PR is targeting the master branch for 4.x (4.4 etc).

Okay rebase finished. Seems to still be working.

[FireCatMagic]

I tested this PR, and it seems to crash whenever I set the buffer. The stack looks as it would expect for this crash

here's the project, just run it and itll crash
New Compressed (zipped) Folder.zip

[huwpascoe]

Okay, fixed a typo where it was multiplying by sizeof(float) when it shouldn't. Oh and the actual crash was updating a buffer that didn't exist yet.

[FireCatMagic]

Awesome work!

Although I tested the new commit and I'm having trouble using the raw data in a test shader, in compatibility.

Here's the shader:

shader_type spatial;
render_mode specular_disabled, unshaded, cull_disabled;

vec4 uint8_color_to_float_color(uvec4 color) {
	return vec4((float(color.r) / 255.0), (float(color.g) / 255.0), (float(color.b) / 255.0), (float(color.a) / 255.0));
}

uvec2 split_uint(uint value) {
	uvec2 split;
	split[0] = (value >> 0u) & 255u;
	split[1] = (value >> 8u) & 255u;
	return split;
}

void vertex() {
	// These should each only fill up bits 0-15, leaving 16-31 unused
	uvec4 packed = uvec4(floatBitsToUint(INSTANCE_CUSTOM.r), floatBitsToUint(INSTANCE_CUSTOM.g), floatBitsToUint(INSTANCE_CUSTOM.b), floatBitsToUint(INSTANCE_CUSTOM.a));
	
	uvec4 color1 = uvec4(split_uint(packed[0]), split_uint(packed[1]));
	uvec4 color2 = uvec4(split_uint(packed[2]), split_uint(packed[3]));

	vec4 float_color1 = uint8_color_to_float_color(color1);
	vec4 float_color2 = uint8_color_to_float_color(color2);

	if (VERTEX.y > 3.0f) {
		COLOR.rgb = float_color1.rgb;
	} else {
		COLOR.rgb = float_color2.rgb;
	}
}

void fragment() {
	ALBEDO = COLOR.rgb;
	//ALPHA = COLOR.a;
}

Maybe I'm doing something wrong somewhere else? here's the test project

New Compressed (zipped) Folder.zip

[huwpascoe]

I was hoping it wouldn't come to this: it clobbers the data in the end anyway.
Good news however, it doesn't happen at glVertexAttrib, it happens in the shader.

https://github.com/godotengine/godot/blob/master/drivers/gles3/shaders/scene.glsl#L412-L414

vec4 instance_custom;
instance_custom.xy = unpackHalf2x16(instance_color_custom_data.z);
instance_custom.zw = unpackHalf2x16(instance_color_custom_data.w);

So you could modify that section for a custom build and that should do it.

[FireCatMagic]

I probably will end up having to do some build modifications sadly then if I wanna do what I’m trying to do :/
But good job on this pr still!! Even if i personally can’t use it atm for my use case I am sure it’ll be of use to other people who are working low level

although one last consideration,
do you think INSTANCE_CUSTOM_RAW is something even possible for another proposal? to go alongside INSTANCE_CUSTOM in the vertex shader? it’d return lowp uint[16], the size of the data buffer in forward, when using compatibility it’d return a 16 size array still but the last 8 would be zeroed out.

at this point though this becomes a very specific use case which goes against some of the engine principles unless there is other people who would also support this level of flexibility.
so if INSTANCE_CUSTOM_RAW were to be contributed, it probably would be good to make it less specific to one use case though, and by that I mean possibly adding stuff like COLOR_RAW alongside it? so the pr is more of a general purpose addition
i am not the most versed in rendering stuff, it might be stupid in the first place to even suggest access to byte arrays in shader code

[FireCatMagic]

Actually wait, that might not even be necessary.
Assuming that the float16 values are cast to float32 to be used in INSTANCE_CUSTOM inside the shader,

I could just write my own cast function that "casts" the float32 bits back into the float16 format, and then from there the bits should remain the same as they were input?
I'll try this out later today when I have time

[huwpascoe]

Adding raw variants of uniforms is certainly architecture breaking, so no I won't be attempting that.

I have personal interest in the multimesh system so making this PR was a good opportunity to investigate the inner workings while potentially contributing to the engine, no worries there.

Yes, maybe it is reversible. Putting INSTANCE_CUSTOM back through packHalf2x16 could potentially reconstruct the data.

Edit: Just tried, only partially reversible. If every rgba component is >=4, then it does replicate the original color. But if any component < 4 it goes crazy. This could have different results depending on GPU implementation so not reliable.

[FireCatMagic]

I tried, it does the same thing, but it works for the most part besides the small rgb components, which seems great enough to me. For me it is only the G channel that makes it act all weird. Any clue on why it does this?

Edit: Testing further, i noticed it does it for the three values from 129, 130, and 131 as well as 1, 2, and 3

[huwpascoe]

Did a little test

uniform uint testval;
...
uint v = min(testval, 0xFFFFu);
uint tst = packHalf2x16(unpackHalf2x16(v));
if (v != tst) { COLOR.rgb = vec3(1.0, 0.0, 1.0); }

And scrolling through the uniforms I find that 0x83FF is the bitmask of invalid ranges.

So we look at this in float toy:

Which maps exactly to the exponent.

The exponent being 0 in float16 means either it's exactly zero, or if the mantissa is non-zero, subnormal (I guess like is_zero_approx() in godot speak). Which the driver apparently responds by normalizing it to 0xFFFF (NaN).

Data is lost when the exponent is zero.

So let's apply the mask to the whole uint32: 0x83FF83FF. And there is the answer.

If (green & 0x7F) < 0x04 and red > 0x00: red and green will = 0xFF
If (alpha & 0x7F) < 0x04 and blue > 0x00: blue and alpha will = 0xFF

[FireCatMagic]

That's very interesting how that works, thanks for that test
I guess the last thing I could possibly do is skip the built in packHalf2x16 function and write my own function that converts a float32 value into a float16, bit by bit and stored in an int, while having special checks to ensure no data is lost

[huwpascoe]

The data's already lost back when INSTANCE_CUSTOM is unpacked, so it has to be accounted for when creating the multimesh buffer. Two options come to mind

• Disallow odd-index bytes from having value of < 4
• Sacrifice 1 bit of every odd indexed byte to always be set, like p_byte |= 0x40. giving 30bits out of 32 to work with.

[FireCatMagic]

Thanks for all that!

if ((value == 0) || (value == 1) || (value == 2) || (value == 3)):
	bytes[index] = 4
elif ((value == 128) || (value == 129) || (value == 130) || (value == 131)):
	bytes[index] = 132

Slightly limits colors but in such a way it doesn't matter at all! I'm very satisified with all this, thanks for all your help! Working with bits and packing stuff into bits is unironically so fun to me

[FireCatMagic]

Any chance on this making it into any of the next 4.4 devs? sorry if this is an annoying question, i'm not exactly sure of how the process goes

[AThousandShips]

This needs the rendering team to take a look and review it, so when it gets approved depends entirely on their decision, including their opinion on if this is something that should be added, and then their views on the implementation