GPUParticles3D parameter local_coords not working with doubles builds of the engine

Godot version

4.0.dev (double precision) ( f2aae8fa5c2e9d9323832fb43c8446c2e518d698 [f2aae8f])

System information

Win10, RTX3060, Vulkan

Issue description

If i set local_coords to false, particles still moves with the GPUParticles3D node, but don't rotate. When local_coords is true, particles moves(with a node) and rotates(around himselves, what is wrong compared to RC2) .

Steps to reproduce

Create scene. Add GPUParticles3D node, set local_coords to fale, create particle material with shader code and move node.

shader_type particles;
uniform float rows = 10.;
uniform float spacing = 2.;
void start() {
    vec3 pos = vec3(0.0, 0.0, 0.0);
    pos.z = float(INDEX);
    pos.x = mod(pos.z, rows);
    pos.z = (pos.z - pos.x) / rows;
    TRANSFORM[3][0] = pos.x*spacing;
    TRANSFORM[3][1] = 0.;
    TRANSFORM[3][2] = pos.z*spacing;
}

void process() {
}

Minimal reproduction project

N/A TestParticles.zip

I can't reproduce on RC2 or on master following your reproduction steps.

By chance is there anything else you did not listed in the steps?

At any rate, you should add an MRP so other users can test with your exact settings/scene setup to see if they face the same issue

I can't reproduce on RC2 or on master following your reproduction steps.

By chance is there anything else you did not listed in the steps?

At any rate, you should add an MRP so other users can test with your exact settings/scene setup to see if they face the same issue

Tried RC2, bug does not reproduce. Saved project, and opened it with my build - bug exist. Again opened with RC2 - no bug. Forgot that i make build with double precision, but i dont think it is matter.

Unfortunately I also ran into this bug, which I would even consider critical in nature.

(Short explanation why I think this is absolutely critical. You can skip this.) I'm currently developing a game where I need the new feature for barycentric coordinates. That means I can't use the Jolt physics engine, since that does some internal reindexing and thus can't tell the caller of a raycast the correct face index to further gather normals and weights. The game also has to handle massive differences in proportions: small objects moving around fast in a big world. Godot's built-in physics engine unfortunately often lacks numeric stability. As an example: It is very hard to make a Raycast3D of length 0.05 actually hit anything. At least in 32-bit precision. To improve the accuracy of the physics engine (which directly impacts a user's gaming experience) it is absolutely necessary to make a 64-bit Godot build. Oh, and as you probably know, cpu particles are not supported anymore. If you want any of the new features, you have to stay with gpu particles. So here we are with Particles not working...

(Back to topic) Given the current 4.3-dev-2 build (352434668), I tried to track down the problem. What I learned so far is that Godot treats particles essentially as a MultiMesh with transform feedback. The problem seems to occur in the GLSL scene shader of the Forward+ and Forward Mobile renderers (Compatibility seems unaffected since it doesn't support precision=double yet). For Forward+ everything happens in ./servers/rendering/renderer_rd/shaders/forward_clustered/scene_forward_clustered.glsl. To make the particles movement work again I commented out the #ifdef macros in lines 308 to 315. This reactivates transform feedback, but the particles' movement is still wrong (it seems like all values are exactly halfed). I also commented out line 400, which makes the particles move correctly again. And I commented out line 401 because I had weird scaling issues and that fixed it for me. With these in total 4 lines changed GPUParticles3D seems to work for me again. However this is very much fiddling around. I don't completely understand how the scene shader works, and just commenting out math-heavy code probably breaks lots of other stuff.

I hope someone more knowledgeable can figure out how to actually fix this. I feel like a proper fix would include a way of knowing inside the scene shader whether this is a multimesh or actually particles? In that case another macro definition could do...?

Thx for reading.

PS: Output of git diff:

diff --git a/servers/rendering/renderer_rd/shaders/forward_clustered/scene_forward_clustered.glsl b/servers/rendering/renderer_rd/shaders/forward_clustered/scene_forward_clustered.glsl
index 930d9814942..21cd8478487 100644
--- a/servers/rendering/renderer_rd/shaders/forward_clustered/scene_forward_clustered.glsl
+++ b/servers/rendering/renderer_rd/shaders/forward_clustered/scene_forward_clustered.glsl
@@ -305,14 +305,14 @@ void vertex_shader(vec3 vertex_input,
 #endif
                //transpose
                matrix = transpose(matrix);
-#if !defined(USE_DOUBLE_PRECISION) || defined(SKIP_TRANSFORM_USED) || defined(VERTEX_WORLD_COORDS_USED) || defined(MODEL_MATRIX_USED)
+//#if !defined(USE_DOUBLE_PRECISION) || defined(SKIP_TRANSFORM_USED) || defined(VERTEX_WORLD_COORDS_USED) || defined(MODEL_MATRIX_USED)
                // Normally we can bake the multimesh transform into the model matrix, but when using double precision
                // we avoid baking it in so we can emulate high precision.
                read_model_matrix = model_matrix * matrix;
-#if !defined(USE_DOUBLE_PRECISION) || defined(SKIP_TRANSFORM_USED) || defined(VERTEX_WORLD_COORDS_USED)
+//#if !defined(USE_DOUBLE_PRECISION) || defined(SKIP_TRANSFORM_USED) || defined(VERTEX_WORLD_COORDS_USED)
                model_matrix = read_model_matrix;
-#endif // !defined(USE_DOUBLE_PRECISION) || defined(SKIP_TRANSFORM_USED) || defined(VERTEX_WORLD_COORDS_USED)
-#endif // !defined(USE_DOUBLE_PRECISION) || defined(SKIP_TRANSFORM_USED) || defined(VERTEX_WORLD_COORDS_USED) || defined(MODEL_MATRIX_USED)
+//#endif // !defined(USE_DOUBLE_PRECISION) || defined(SKIP_TRANSFORM_USED) || defined(VERTEX_WORLD_COORDS_USED)
+//#endif // !defined(USE_DOUBLE_PRECISION) || defined(SKIP_TRANSFORM_USED) || defined(VERTEX_WORLD_COORDS_USED) || defined(MODEL_MATRIX_USED)
                model_normal_matrix = model_normal_matrix * mat3(matrix);
        }

@@ -397,8 +397,8 @@ void vertex_shader(vec3 vertex_input,
        // We add the result to the vertex and ignore the final lost precision.
        vec3 model_origin = model_matrix[3].xyz;
        if (is_multimesh) {
-               vertex = mat3(matrix) * vertex;
-               model_origin = double_add_vec3(model_origin, model_precision, matrix[3].xyz, vec3(0.0), model_precision);
+               //vertex = mat3(matrix) * vertex;
+               //model_origin = double_add_vec3(model_origin, model_precision, matrix[3].xyz, vec3(0.0), model_precision);
        }
        vertex = mat3(inv_view_matrix * modelview) * vertex;
        vec3 temp_precision; // Will be ignored.

I encountered this issue when testing #89165 and am struggling to follow the shader ifdef logic that introduces this bug.

The workaround by @wildlachs seems to disable the entire point of double precision.

The use-case I have is engine fire on a RigidBody3D ship so we need them to track in local space, but there doesn't seem to be a usable workaround to achieve that right now.

So for me the MRP looks like this in a double precision build: local_coords_double_current whereas it looks like this in a single precision build: local_coords_single_current

The rotation is being applied, but it is applied before the TRANSFORM set in the shader. Does this correspond with the issue you are seeing in your project @cridenour ?

From the screenshot in #87339, that issue also shows a transformational order symptom. This seems to be a general multimesh in double precision builds issue where the node rotation is applied before the shader translation, rather than the other way around.

@ecmjohnson yes that does align with what I'm seeing. I haven't tested other multimesh nodes with rotation, but that would make sense.

The underlying issue is that the multimesh instance translation is not having the model rotation applied to it since it is currently being added to model_origin which is added to the vertex later, after the model rotation is applied.

The math explaining this looks like: double-local_coords-issue

A preliminary idea for a fix that I don't think properly respects the double precision is:

diff --git a/servers/rendering/renderer_rd/shaders/forward_clustered/scene_forward_clustered.glsl b/servers/rendering/renderer_rd/shaders/forward_clustered/scene_forward_clustered.glsl
index 4a630b0b0a..144bb4ad3a 100644
--- a/servers/rendering/renderer_rd/shaders/forward_clustered/scene_forward_clustered.glsl
+++ b/servers/rendering/renderer_rd/shaders/forward_clustered/scene_forward_clustered.glsl
@@ -203,6 +203,7 @@ void vertex_shader(vec3 vertex_input,
    mat4 inv_view_matrix = scene_data.inv_view_matrix;

 #ifdef USE_DOUBLE_PRECISION
+   // Additional precision is stored in the bottom row of the transformation matrices.
    vec3 model_precision = vec3(model_matrix[0][3], model_matrix[1][3], model_matrix[2][3]);
    model_matrix[0][3] = 0.0;
    model_matrix[1][3] = 0.0;
@@ -221,6 +222,9 @@ void vertex_shader(vec3 vertex_input,
    }

    mat4 matrix;
+#ifdef USE_DOUBLE_PRECISION
+   vec3 matrix_translation;
+#endif
    mat4 read_model_matrix = model_matrix;

    if (is_multimesh) {
@@ -311,14 +315,13 @@ void vertex_shader(vec3 vertex_input,
 #endif
        //transpose
        matrix = transpose(matrix);
-#if !defined(USE_DOUBLE_PRECISION) || defined(SKIP_TRANSFORM_USED) || defined(VERTEX_WORLD_COORDS_USED) || defined(MODEL_MATRIX_USED)
-       // Normally we can bake the multimesh transform into the model matrix, but when using double precision
-       // we avoid baking it in so we can emulate high precision.
+#ifdef USE_DOUBLE_PRECISION
+       // Strip the translation from the matrix for emulating high precision later.
+       matrix_translation = matrix[3].xyz;
+       matrix[3].xyz = vec3(0.0);
+#endif
        read_model_matrix = model_matrix * matrix;
-#if !defined(USE_DOUBLE_PRECISION) || defined(SKIP_TRANSFORM_USED) || defined(VERTEX_WORLD_COORDS_USED)
        model_matrix = read_model_matrix;
-#endif // !defined(USE_DOUBLE_PRECISION) || defined(SKIP_TRANSFORM_USED) || defined(VERTEX_WORLD_COORDS_USED)
-#endif // !defined(USE_DOUBLE_PRECISION) || defined(SKIP_TRANSFORM_USED) || defined(VERTEX_WORLD_COORDS_USED) || defined(MODEL_MATRIX_USED)
        model_normal_matrix = model_normal_matrix * mat3(matrix);
    }

@@ -398,17 +401,16 @@ void vertex_shader(vec3 vertex_input,
 #if !defined(SKIP_TRANSFORM_USED) && !defined(VERTEX_WORLD_COORDS_USED)

 #ifdef USE_DOUBLE_PRECISION
-   // We separate the basis from the origin because the basis is fine with single point precision.
-   // Then we combine the translations from the model matrix and the view matrix using emulated doubles.
-   // We add the result to the vertex and ignore the final lost precision.
-   vec3 model_origin = model_matrix[3].xyz;
    if (is_multimesh) {
-       vertex = mat3(matrix) * vertex;
-       model_origin = double_add_vec3(model_origin, model_precision, matrix[3].xyz, vec3(0.0), model_precision);
+       // Apply multimesh instance translation which is single precision.
+       // The multimesh instance rotation is already part of modelview matrix to account for billboarding.
+       vertex += matrix_translation;
    }
-   vertex = mat3(inv_view_matrix * modelview) * vertex;
+   // Remove view matrix rotation from vertex for adding model and view translations in double precision.
+   vertex = mat3(inv_view_matrix) * mat3(modelview) * vertex;
    vec3 temp_precision; // Will be ignored.
-   vertex += double_add_vec3(model_origin, model_precision, scene_data.inv_view_matrix[3].xyz, view_precision, temp_precision);
+   vertex += double_add_vec3(model_matrix[3].xyz, model_precision, scene_data.inv_view_matrix[3].xyz, view_precision, temp_precision);
+   // Re-apply view matrix rotation.
    vertex = mat3(scene_data.view_matrix) * vertex;
 #else
    vertex = (modelview * vec4(vertex, 1.0)).xyz;

I think the vertex += matrix_translation; line might need to use double_add_vec3 even though both values don't have extra precision (i.e. prec_a = prec_b = vec3(0.0)) and use the out vec3 out_precision to track the lost precision. I'm not totally clear on double_add_vec3 though...

I regressed this change on #76388 and the modifications made by the material shader code to the model view matrix for billboarding are still respected.

godotengine / godot