OverviewFilesCommitsIssuesDocsReleasesPeers

proc-nodes commits / c627c51

glsl: the emitted density was not the graph's field -- four ways
100/100 on the gate added in d2f27e8, which was 0/100. FOUR DEFECTS, each of which alone broke the density shader for every graph bridged from Blender, and none of which any existing gate could see. 1. GROUP INPUT NEVER DECLARED ITS PARAMETERS. gpu_transparent() was written when the node had ONE output, the Geometry the caller hands in, which the emitted shader carries implicitly as `pos`. Since it started carrying the modifier's exposed knobs on its other outputs -- execute() writes them from output_defs -- a transparent Group Input means every one of those is a variable the shader references and nothing declares: error: 'n0_s19' : undeclared identifier (x16, the mountain) That is every real Blender modifier graph, since a group interface is how a knob is exposed. Transparent now means "MAY emit nothing", not "emits nothing": compile_to_glsl_function includes these and accepts an empty result from them. Still transparent for the BATCH path, whose contiguous runs are built from non-transparent nodes -- putting Group Input in them moved the boundaries and gpu_batch_gate went red with SetPosition reported 'cpu'. 2. GRADIENT TEXTURE DECLARED `gradient_val`, not its sockets, so a consumer reading Fac hit `error: 'n14_s1' : undeclared identifier`. The same function read `pos` and ignored its Vector INPUT, so a graph that transforms the coordinate before the gradient -- the entire point of the socket -- got the transform on the CPU and the raw position on the GPU. 3. THE EMITTER IGNORED THE FRAME. GraphExecutor converts a bridged graph's result out of Blender's frame (_to_godot_frame, gated on is_blender_frame); `grep is_blender_frame src/gpu/` was EMPTY. A Blender graph displaces along Z, so the emitted tail read `pos.y - query_y` on an axis nothing had touched and the density was identically zero -- while the CPU returned a real mountain, because it goes through the converting path. M is applied to the query point on entry and the displacement read along Blender's up. The local stays named query_y in both branches. Renaming it rewrote the emitted text of every Godot-frame graph for no behavioural reason and took glsl_golden_gate red on two assertions that are spellings of a return statement. map_dc_erosion_density is byte-identical at 58953 chars. 4. THE CPU TWIN MIXED FRAMES. evaluate_density_batch compared an M-converted dst against an unconverted src, so with zero displacement it returned src.z - src.y instead of 0. literal_value is factored out of literal() rather than spelled a second time for Group Input's outputs: a value formatter written twice is how the two paths would come to disagree about a number neither of them computes. Against measured baselines, not assumed ones -- op parity 770/770 unchanged, golden 31/32 unchanged, batch 7/8 unchanged (both of those failures predate this work), and the CPU mesh bake bit-identical at sum(z)=1792386.7043. STILL OPEN, caught by the same gate: on a graph that declares its density through a Group Output float, evaluate_density_batch returns 0 at every point. evaluate_socket_value hands back a lazy field promise for a cone rooted at Position, and the scalar branch casts it to zero. The GPU side of that graph is correct. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
c627c5108da3bc916020136527b1fa9f154c6738
by plankatron <fleeky@gmail.com> · 2026-09-08T13:01:55.000Z

Changed files (7) +206 -23

src/gpu/glsl_emit_context.cpp +4 -3
……@@ -29,10 +29,11 @@ String GlslEmitContext::literal(int p_node_idx, int p_socket) const {
2929 if (node.is_null() || p_socket < 0 || p_socket >= node->get_input_count()) {
3030 return String();
3131 }
32 const SocketDef &def = node->get_input_def(p_socket);
33 const Variant v = node->get_input_default(p_socket);
32 return literal_value(node->get_input_def(p_socket).type, node->get_input_default(p_socket));
33}
3434
35 switch (def.type) {
35String GlslEmitContext::literal_value(SocketType p_type, const Variant &v) const {
36 switch (p_type) {
3637 case SocketType::Float:
3738 return String::num((double)v, 9);
3839 case SocketType::Int: {
src/gpu/glsl_emit_context.h +7 -0
……@@ -118,6 +118,13 @@ struct GlslEmitContext {
118118 // a y-scale of 5/21000 = 0.000238 truncated to 0.0002 is a 16% error in the
119119 // vertical mapping, which moved a terrain surface ~1770 m.
120120 String literal(int p_node_idx, int p_socket) const;
121
122 // The same formatter, for a value that is not on an INPUT socket. Group
123 // Input's parameters live on its OUTPUTS -- execute() writes them from
124 // output_defs[i].default_value -- and spelling them a second time here is
125 // how the two paths would come to disagree about a number neither of them
126 // computes.
127 String literal_value(SocketType p_type, const Variant &p_value) const;
121128};
122129
123130} // namespace godot
src/gpu/gpu_compiler.cpp +42 -4
……@@ -226,6 +226,10 @@ String GpuCompiler::compile_batch(
226226 // p_node_indices[i] is the graph-level index (for compile_glsl context)
227227 int graph_idx = (i < p_node_indices.size()) ? p_node_indices[i] : i;
228228 String glsl = node->compile_glsl(ctx, graph_idx);
229 if (glsl.is_empty() && node->gpu_transparent()) {
230 // A boundary node with nothing to declare. Not the lie below.
231 continue;
232 }
229233 if (glsl.is_empty()) {
230234 continue;
231235 }
……@@ -393,7 +397,41 @@ String GpuCompiler::compile_to_function(
393397 // For SDF/density convention: density = ground_height - pos.y
394398 // After displacement, pos.y contains the displaced height, so we use that.
395399 result += "float " + p_function_name + "(vec3 pos) {\n";
396 result += " float query_y = pos.y; // Save original query height before nodes modify pos\n";
400 // ⛔ THE FRAME, WHICH THE EMITTER USED TO IGNORE ENTIRELY.
401 //
402 // GraphExecutor converts a bridged graph's result out of Blender's frame at
403 // the end (_to_godot_frame, gated on is_blender_frame). The emitter had
404 // never heard of that flag -- `grep is_blender_frame src/gpu/` was empty --
405 // so the SAME graph was one field on the CPU and another on the GPU.
406 //
407 // For a heightfield authored in Blender the difference is not subtle. A
408 // Blender graph displaces along Z, so the emitted tail read `pos.y -
409 // query_y` on an axis nothing had touched: measured on the bridged voronoi
410 // mountain, 0/100 sample points agreed and the emitted density was
411 // IDENTICALLY ZERO while the CPU returned a real mountain
412 // (tests/test_emitted_density_parity.cpp).
413 //
414 // The local stays named query_y in BOTH branches on purpose. Renaming it
415 // changed the emitted text of every Godot-frame graph -- the shipping
416 // ones -- for no behavioural reason, and glsl_golden_gate went red on two
417 // assertions that are spellings of a return statement. A frame fix that
418 // rewrites the shader for graphs it does not apply to is a bigger change
419 // than it looks.
420 //
421 // M = (x, y, z) -> (-x, z, y), the same conversion, self-inverse, applied
422 // to the query point on the way IN so every node sees the coordinates the
423 // CPU gives it. The density is then the displacement along BLENDER's up,
424 // which is exactly Godot's y displacement after M -- M(q).y == q.z -- so
425 // the consumer's convention does not change.
426 const bool blender_frame = (p_graph != nullptr && p_graph->is_blender_frame());
427 if (blender_frame) {
428 result += " // Bridged from Blender: M maps the query point into the frame\n";
429 result += " // the graph was authored in. M is self-inverse.\n";
430 result += " pos = vec3(-pos.x, pos.z, pos.y);\n";
431 result += " float query_y = pos.z; // Blender's up, read by the return below\n";
432 } else {
433 result += " float query_y = pos.y; // Save original query height before nodes modify pos\n";
434 }
397435 result += " vec3 normal = vec3(0.0, 1.0, 0.0);\n";
398436 result += body;
399437 // WHICH VALUE THIS FUNCTION RETURNS, chosen in a fixed order and never by a
……@@ -429,7 +467,7 @@ String GpuCompiler::compile_to_function(
429467 if (p_nodes[i]->get_output_def(s).type == SocketType::Geometry) geo_out = true;
430468 }
431469 if (geo_in && geo_out) {
432 ret = "pos.y - query_y";
470 ret = blender_frame ? "pos.z - query_y" : "pos.y - query_y";
433471 break;
434472 }
435473 }
……@@ -452,7 +490,7 @@ String GpuCompiler::compile_to_function(
452490 "proc_density returns the displacement of `pos` -- zero unless a "
453491 "node moved it. A graph computing a density value needs a Group "
454492 "Output to return it.");
455 ret = "pos.y - query_y";
493 ret = blender_frame ? "pos.z - query_y" : "pos.y - query_y";
456494 }
457495
458496 // The comment has to follow the BRANCH. It said "terrain_height - query_y"
……@@ -460,7 +498,7 @@ String GpuCompiler::compile_to_function(
460498 // float -- so a volume graph's emitted text described itself as a
461499 // displacement. A stale comment on the one line that says what the shader
462500 // computes is worth more than a stale comment anywhere else.
463 if (ret == "pos.y - query_y") {
501 if (ret == "pos.y - query_y" || ret == "pos.z - query_y") {
464502 result += " // SDF: terrain_height - query_y. Positive below surface, negative above.\n";
465503 } else {
466504 result += " // The graph's OWN density, from its Group Output float.\n";
src/graph/proc_node_graph.cpp +35 -2
……@@ -436,7 +436,18 @@ String ProcNodeGraph::compile_to_glsl_function(const String &p_function_name,
436436 // is a stale world rather than a wrong one.
437437 // A boundary node carries no operation, so emitting nothing is correct
438438 // for it and is not the silent drop this refusal exists to catch.
439 if (nodes[i]->gpu_transparent()) {
439 //
440 // ⛔ IT IS STILL INCLUDED. Skipping it outright meant Group Input --
441 // which carries the modifier's exposed PARAMETERS on its outputs, and
442 // writes them in execute() -- never declared them, so every consumer
443 // referenced `n0_sN` and the shader would not compile:
444 //
445 // error: 'n0_s19' : undeclared identifier (x16, bridged mountain)
446 //
447 // compile_to_function accepts an empty emission from a transparent node
448 // and refuses it from any other, so a boundary node that has nothing to
449 // say still says nothing.
450 if (nodes[i]->gpu_transparent() && !nodes[i]->supports_gpu()) {
440451 continue;
441452 }
442453 if (!nodes[i]->supports_gpu()) {
……@@ -531,7 +542,29 @@ PackedFloat32Array ProcNodeGraph::evaluate_density_batch(const PackedVector3Arra
531542 Ref<ProcGeometry> in_geo;
532543 in_geo.instantiate();
533544 Ref<ProcMesh> in_mesh = in_geo->ensure_mesh();
534 in_mesh->positions = p_positions;
545 // ⛔ THE QUERY POINT IS IN THE CONSUMER'S FRAME, THE GRAPH IS IN ITS OWN.
546 //
547 // A bridged graph runs in Blender's frame and GraphExecutor converts its
548 // RESULT on the way out (_to_godot_frame). The input was never converted,
549 // so this compared an M-converted dst against an unconverted src and the
550 // two axes did not correspond: with zero displacement the density came back
551 // as src.z - src.y instead of 0.
552 //
553 // M on the way in, self-inverse, matching what compile_to_function now
554 // emits. src stays the caller's own point, so dst.y - src.y below is still
555 // the displacement along the CONSUMER's up.
556 if (is_blender_frame()) {
557 PackedVector3Array conv;
558 conv.resize(n);
559 Vector3 *cw = conv.ptrw();
560 const Vector3 *pr = p_positions.ptr();
561 for (int i = 0; i < n; i++) {
562 cw[i] = Vector3(-pr[i].x, pr[i].z, pr[i].y);
563 }
564 in_mesh->positions = conv;
565 } else {
566 in_mesh->positions = p_positions;
567 }
535568 in_mesh->normals.resize(n);
536569 for (int i = 0; i < n; i++) {
537570 in_mesh->normals.ptrw()[i] = Vector3(0, 1, 0);
src/nodes/input_nodes.cpp +30 -0
……@@ -1,5 +1,6 @@
11#include "input_nodes.h"
22#include "../graph/graph_executor.h"
3#include "../gpu/glsl_emit_context.h"
34
45namespace godot {
56
……@@ -55,6 +56,35 @@ void GroupInputNode::set_interface(const Array &p_spec) {
5556 }
5657}
5758
59String GroupInputNode::compile_glsl(const GlslEmitContext &p_ctx, int p_node_idx) const {
60 // One declaration per PARAMETER output, from output_defs -- the same values
61 // execute() writes. The Geometry output has no spelling and needs none: the
62 // emitted function's `pos` is the geometry, and declare() returns empty for
63 // it, which is the signal to skip rather than an error.
64 String s;
65 for (int i = 0; i < output_defs.size(); i++) {
66 if (output_defs[i].type == SocketType::Geometry) {
67 continue;
68 }
69 const String decl = p_ctx.declare(p_node_idx, i);
70 if (decl.is_empty()) {
71 continue;
72 }
73 // String(" = ") rather than a bare literal: a NAMED const String plus a
74 // char array is an ambiguous overload in godot-cpp (String and
75 // StringName convert equally well), which the prvalue form every
76 // other node happens to use never runs into.
77 s += decl + String(" = ") + p_ctx.literal_value(output_defs[i].type, output_defs[i].default_value) + String(";\n\t");
78 }
79 // A graph whose interface is geometry alone emits nothing here, and an
80 // empty string is how compile_to_function spells "this node lied about
81 // supporting the GPU". Give it a statement that says so instead.
82 if (s.is_empty()) {
83 return "// Group Input: geometry only, carried as `pos`";
84 }
85 return s;
86}
87
5888void GroupInputNode::execute(ProcEvalContext &p_ctx, int p_node_idx) {
5989 // The graph's entry point: the geometry the CALLER handed in. For a graph
6090 // bridged from Blender this is the object's own mesh, which is what its
src/nodes/input_nodes.h +31 -0
……@@ -35,7 +35,38 @@ public:
3535 void set_interface(const Array &p_spec);
3636 Array get_interface() const { return interface_spec; }
3737
38 // ⛔ TRANSPARENT USED TO MEAN "EMITS NOTHING", and that is what broke.
39 //
40 // gpu_transparent() was written when Group Input had ONE output, the
41 // Geometry the caller hands in -- which the emitted shader carries
42 // implicitly as `pos`, so emitting nothing for the node was right. Since
43 // the node started carrying the modifier's exposed PARAMETERS on its other
44 // outputs, a transparent Group Input means every one of those is a variable
45 // the shader references and nothing declares:
46 //
47 // error: 'n0_s19' : undeclared identifier (x16, the bridged
48 // error: 'n0_s6' : undeclared identifier voronoi mountain)
49 //
50 // compile_to_glsl_function skips transparent nodes outright, so the density
51 // shader for EVERY bridged Blender graph with a group interface -- which is
52 // every real modifier graph, since that is how a knob is exposed -- failed
53 // to compile. The refusal machinery could not see it: each node reported GPU
54 // support and emitted real GLSL, and the missing text was the node that had
55 // been excused from emitting.
56 //
57 // Reading the value from output_defs is what execute() does, so the two
58 // paths take the same number from the same place rather than agreeing by
59 // coincidence.
60 // STILL TRANSPARENT, because the BATCH path's runs are built from
61 // non-transparent nodes and putting Group Input into them moved the
62 // boundaries: gpu_batch_gate went red with SetPosition reported 'cpu'.
63 // Transparent now means "may emit nothing", not "emits nothing" --
64 // compile_to_glsl_function includes these and accepts an empty result from
65 // them, which is how Group Input can declare its parameters for the density
66 // shader without joining a batch.
3867 bool gpu_transparent() const override { return true; }
68 bool supports_gpu() const override { return true; }
69 String compile_glsl(const GlslEmitContext &p_ctx, int p_node_idx) const override;
3970
4071 void execute(ProcEvalContext &p_ctx, int p_node_idx) override;
4172
src/nodes/noise_nodes.cpp +57 -14
……@@ -1146,22 +1146,65 @@ void GradientTextureNode::execute(ProcEvalContext &p_ctx, int p_node_idx) {
11461146}
11471147
11481148String GradientTextureNode::compile_glsl(const GlslEmitContext &p_ctx, int p_node_idx) const {
1149 // ⛔ TWO DEFECTS, and each one alone was enough to break a density shader.
1150 //
1151 // 1. It declared `gradient_val`. Consumers name a node's outputs
1152 // n<idx>_s0 and n<idx>_s1, so the bridged voronoi mountain -- which
1153 // reads Fac, output 1 -- emitted
1154 //
1155 // float n15_s0 = pn_safe_pow(n14_s1, n0_s3);
1156 // error: 'n14_s1' : undeclared identifier
1157 //
1158 // and the whole density shader failed to compile. Nothing caught it:
1159 // the node reports GPU support and emits real GLSL, so the refusal in
1160 // compile_to_glsl_function sees a well-behaved node.
1161 //
1162 // 2. It read `pos` and ignored the node's Vector INPUT. execute() reads
1163 // socket 0, so a graph that transforms the coordinate before the
1164 // gradient -- which is the entire point of the socket -- got the
1165 // transform on the CPU and the raw position on the GPU. Two different
1166 // fields, silently, from one graph.
1167 //
1168 // ⚠ An UNCONNECTED Vector now emits the socket's default, matching
1169 // execute(). Blender would substitute the implicit position field there
1170 // instead; that asymmetry is real and is recorded in the blender-bridge
1171 // skill, but it belongs to read_input, not here -- fixing it in one path
1172 // only is how the two came to disagree in the first place.
1173 const String v = String("pn_grad_") + itos(p_node_idx);
1174 const String fac = p_ctx.declare(p_node_idx, 1);
1175 const String col = p_ctx.declare(p_node_idx, 0);
1176 String expr;
11491177 switch (gradient_type) {
1150 case 1:
1151 return "float gradient_val = clamp(pow(max(pos.x, 0.0), 2.0), 0.0, 1.0);";
1152 case 2:
1153 return "float gradient_val = clamp(pos.x, 0.0, 1.0); gradient_val = gradient_val * gradient_val * (3.0 - 2.0 * gradient_val);";
1154 case 3:
1155 return "float gradient_val = clamp((pos.x + pos.y) * 0.5, 0.0, 1.0);";
1156 case 4:
1157 return "float gradient_val = clamp(max(1.0 - length(pos), 0.0), 0.0, 1.0);";
1158 case 5:
1159 return "float gradient_val = clamp(pow(max(1.0 - length(pos), 0.0), 2.0), 0.0, 1.0);";
1160 case 6:
1161 return "float gradient_val = clamp(atan(pos.y, pos.x) / 6.283185307 + 0.5, 0.0, 1.0);";
1162 default:
1163 return "float gradient_val = clamp(pos.x, 0.0, 1.0);";
1178 case 1: // QUADRATIC
1179 expr = String("pow(max(") + v + String(".x, 0.0), 2.0)");
1180 break;
1181 case 2: { // EASING
1182 const String t = String("clamp(") + v + String(".x, 0.0, 1.0)");
1183 expr = t + String(" * ") + t + String(" * (3.0 - 2.0 * ") + t + String(")");
1184 break;
1185 }
1186 case 3: // DIAGONAL
1187 expr = String("(") + v + String(".x + ") + v + String(".y) * 0.5");
1188 break;
1189 case 4: // SPHERICAL
1190 expr = String("max(1.0 - length(") + v + String("), 0.0)");
1191 break;
1192 case 5: // QUADRATIC_SPHERE
1193 expr = String("pow(max(1.0 - length(") + v + String("), 0.0), 2.0)");
1194 break;
1195 case 6: // RADIAL
1196 expr = String("atan(") + v + String(".y, ") + v + String(".x) / 6.283185307 + 0.5");
1197 break;
1198 default: // LINEAR
1199 expr = v + String(".x");
1200 break;
11641201 }
1202 // The clamp is outside the switch because _gradient applies it to every
1203 // type on the way out, including the ones whose own arithmetic already
1204 // bounds the result.
1205 return String("vec3 ") + v + String(" = ") + p_ctx.in(p_node_idx, 0) + String(";\n\t") +
1206 fac + String(" = clamp(") + expr + String(", 0.0, 1.0);\n\t") +
1207 col + String(" = vec4(vec3(") + p_ctx.out(p_node_idx, 1) + String("), 1.0);");
11651208}
11661209
11671210// =========================================================================