MultiMesh set_as_bulk_array is an absolute mesh

DMClVG commented 3 years ago

Godot version

3.3.3

System information

OS: Ubuntu latest GPU: GTX 1050

Issue description

Basically, I've been trying to render a cube using a MultiMeshInstance. There are multiple ways to transform an instance in an MultiMesh, one being the set_instance_transform function that works well and fine, and the second being set_as_bulk_array which on the other hand I can't seem to get working at all. I premuse it's because I don't have the floats in the array in the right order, but there is no documentation anywhere on how the transform should be layed out in memory. (aside from this https://docs.godotengine.org/en/stable/tutorials/misc/binary_serialization_api.html#transform, which also does not work in this case??)

With everything I've tried the only array that wouldn't distort or move my cube was [-1, 0, 0, 1, 0, 0, 1, 0, 0, 1, 0, 0], which is what I was trying to achieve, but I have no idea why my other array inside the code for supposedly an identity (unmoving) matrix doesn't work, much less how these floats influence the transformation matrix.

Here are some screenshots For transform [1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0] (lots of nothing)

For transform [0, 0, 1, 0, 1, 0, 1, 0, 0, 0, 0, 0] (not a cube)

For transform [-1, 0, 0, 1, 0, 0, 1, 0, 0, 1, 0, 0] (working, but why?????)

Can anyone help and tell me what I'm doing wrong? (and also update the docs)

Steps to reproduce

Create Spatial with a MultiMeshInstance child
Add this script to the Spatial
```
extends Spatial
```

func _ready(): var meshInstance = $MultiMeshInstance

var mesh = MultiMesh.new()

mesh.mesh = CubeMesh.new() # render cube

    # omitting colors and metadata
mesh.transform_format = MultiMesh.TRANSFORM_3D
mesh.color_format = MultiMesh.COLOR_NONE
mesh.custom_data_format = MultiMesh.CUSTOM_DATA_NONE

mesh.instance_count = 1 # draw one instance (for example)

var transforms = PoolRealArray([1, 0, 0,    0, 1, 0,     0, 0, 1,     0, 0, 0]) # transform matrix for instance 0
mesh.set_as_bulk_array(transforms) # use the transform on the instance

meshInstance.multimesh = mesh
pass


- Run game

### Minimal reproduction project

_No response_

Calinou commented 3 years ago

This may be due to matrices being transposed compared to how they're usually written in the math field (or something along those lines).

cc @clayjohn

DMClVG commented 3 years ago

This may be due to matrices being transposed compared to how they're usually written in the math field (or something along those lines).

To my knowledge identity matrices are supposed to remain the same on transposition. Nonetheless, I did try many other rearrangements, but I still haven't got it figured out yet.

Also I now realised the transform written inside of the code can also be found here in the docs https://docs.godotengine.org/fr/stable/classes/class_transform.html#constants, which makes it even more confusing on why this had to be different. And again the MultiMesh doc has very little information. See here https://docs.godotengine.org/fr/stable/classes/class_multimesh.html#class-multimesh-method-set-as-bulk-array

kleonc commented 3 years ago

Just checked the source code, the memory layout is the same for both GLES3 and GLES2 renderers (3.3.3).

General layout is: [instance_0_data][instance_1_data][instance_2_data](...)

Each instance takes (xform_floats + color_floats + custom_data_floats) * sizeof(float) consecutive bytes in the memory and for each such memory block:

first xform_floats floats represent transform,
next color_floats floats represent color,
next custom_data_floats floats represent custom data.

These numbers depend on the formats set for the given MultiMesh: https://github.com/godotengine/godot/blob/b973f997ff56bd9ad4c0a521e123cc4aee13135d/drivers/gles3/rasterizer_storage_gles3.cpp#L4652-L4672

You can check the layout for each part (transform/color/custom_data) in the implementation of the multimesh_instance_set_X method. For example here you can see the layout for a Transform (3D): https://github.com/godotengine/godot/blob/b973f997ff56bd9ad4c0a521e123cc4aee13135d/drivers/gles3/rasterizer_storage_gles3.cpp#L4804-L4833

So in your "working" example you've kinda set (pseudo code):

-1 = p_transform.basis.elements[0][0];
 0 = p_transform.basis.elements[0][1];
 0 = p_transform.basis.elements[0][2];
 1 = p_transform.origin.x;
 0 = p_transform.basis.elements[1][0];
 0 = p_transform.basis.elements[1][1];
 1 = p_transform.basis.elements[1][2];
 0 = p_transform.origin.y;
 0 = p_transform.basis.elements[2][0];
 1 = p_transform.basis.elements[2][1];
 0 = p_transform.basis.elements[2][2];
 0 = p_transform.origin.z;

so the basis vectors are (-1, 0, 0), (0, 0, 1), (0, 1, 0) and thus everything "works" (they're orthonormal).

DMClVG commented 3 years ago

Oh, so to put it simply, each origin coordinate is shoved in between every matrix row? Not only is it strange but it's undocumented aswell. Maybe a more straightforward basis + origin would've caused less headache and been more consistent with the rest of the engine.

Anyhow, its working now

Thanks for the help!

kleonc commented 3 years ago

Maybe a more straightforward basis + origin would've caused less headache and been more consistent with the rest of the engine.

Probably yes. But I'd guess it's rather about performance / being shader friendly: https://github.com/godotengine/godot/blob/b973f997ff56bd9ad4c0a521e123cc4aee13135d/drivers/gles3/shaders/scene.glsl#L52-L54 https://github.com/godotengine/godot/blob/b973f997ff56bd9ad4c0a521e123cc4aee13135d/drivers/gles3/shaders/scene.glsl#L324-L325

godotengine / godot