| … | … | @@ -285,6 +285,49 @@ func _batch_still_uses_push_constants() -> void: |
| 285 | 285 | "Offset occupies param0..2, and its default is not baked in") |
| 286 | 286 | |
| 287 | 287 | |
| 288 | ## L. A VOLUME GRAPH CAN SAY WHAT ITS DENSITY IS, and the mechanism is a Group | |
| 289 | ## Output FLOAT -- not VolumeToMesh. | |
| 290 | ## | |
| 291 | ## hyper-games #65 NEXT 5 asked whether volume_nodes.h's last | |
| 292 | ## supports_gpu() == false has a reason or is the next debt. It has one, but the | |
| 293 | ## comment justifying it had gone stale: it said the density function "has no way | |
| 294 | ## to return a node's value", and _density_output_socket has been able to since | |
| 295 | ## the Group Output interface landed. So the honest gate is not "VolumeToMesh is | |
| 296 | ## off" -- it is that the thing it would have been FOR works without it. | |
| 297 | ## | |
| 298 | ## Case F is the negative half (no explicit output -> the displacement, with a | |
| 299 | ## warning). This is the positive half. | |
| 300 | func _volume_density_via_group_output() -> void: | |
| 301 | var g := ProcNodeGraph.new() | |
| 302 | var s1 := g.add_node(SphereSdfNode.new()) | |
| 303 | var s2 := g.add_node(SphereSdfNode.new()) | |
| 304 | var bo := g.add_node(VolumeBooleanNode.new()) | |
| 305 | g.connect_sockets(s1, 0, bo, 0) | |
| 306 | g.connect_sockets(s2, 0, bo, 1) | |
| 307 | var go := GroupOutputNode.new() | |
| 308 | go.set_interface([{"name": "Density", "type": 0}]) # 0 = Float | |
| 309 | var goi := g.add_node(go) | |
| 310 | g.connect_sockets(bo, 0, goi, 0) | |
| 311 | var t: String = g.compile_to_glsl_function("proc_density", false, PackedStringArray()) | |
| 312 | var want := "return n%d_s0;" % bo | |
| 313 | _check("volume density via Group Output", t.find(want) != -1, | |
| 314 | "returns the chain (%s), not the displacement" % want) | |
| 315 | # And the comment above it must follow the branch. It read | |
| 316 | # "SDF: terrain_height - query_y" over a `return n2_s0;`, which describes | |
| 317 | # the wrong thing on the one line that says what the shader computes. | |
| 318 | _check("return comment follows the branch", | |
| 319 | t.find("The graph's OWN density") != -1 | |
| 320 | and t.find("SDF: terrain_height - query_y") == -1, | |
| 321 | "names the Group Output float, not the displacement") | |
| 322 | ||
| 323 | # VolumeToMesh stays GPU-off, and a graph containing it still REFUSES -- | |
| 324 | # case B depends on that, so assert the property rather than leave it | |
| 325 | # implied by one other case. | |
| 326 | var vtm := VolumeToMeshNode.new() | |
| 327 | _check("VolumeToMesh stays GPU-off", not vtm.supports_gpu(), | |
| 328 | "supports_gpu() false -- it hands on geometry, it names no float") | |
| 329 | ||
| 330 | ||
| 288 | 331 | func _init() -> void: |
| 289 | 332 | for a in OS.get_cmdline_user_args(): |
| 290 | 333 | if a == "--write": |
| … | … | @@ -431,6 +474,7 @@ func _init() -> void: |
| 431 | 474 | _push_constant_map_is_sane("NoiseTextureNode") |
| 432 | 475 | _reads_the_right_sockets() |
| 433 | 476 | _batch_still_uses_push_constants() |
| 477 | _volume_density_via_group_output() | |
| 434 | 478 | |
| 435 | 479 | print("\n%d/%d passed" % [_ran - _fail, _ran]) |
| 436 | 480 | quit(1 if _fail > 0 else 0) |
| … | … | @@ -351,22 +351,13 @@ String GpuCompiler::compile_to_function( |
| 351 | 351 | |
| 352 | 352 | // 1. EXPLICIT: a Group Output with a connected float input is the graph |
| 353 | 353 | // saying what it produces. |
| 354 | // ONE rule, ONE implementation. This used to be a second copy of the loop | |
| 355 | // in proc_node_graph.cpp, and a mutation to that one left this one | |
| 356 | // answering the old way -- two texts agreeing by review. | |
| 354 | 357 | if (p_graph != nullptr) { |
| 355 | for (int i = 0; i < p_graph->get_node_count() && ret.is_empty(); i++) { | |
| 356 | Ref<ProcNode> n = p_graph->get_node(i); | |
| 357 | if (n.is_null() || n->get_node_type() != StringName("GroupOutput")) { | |
| 358 | continue; | |
| 359 | } | |
| 360 | for (int s = 0; s < n->get_input_count(); s++) { | |
| 361 | if (n->get_input_def(s).type != SocketType::Float) { | |
| 362 | continue; | |
| 363 | } | |
| 364 | if (p_graph->get_input_source_node(i, s) < 0) { | |
| 365 | continue; | |
| 366 | } | |
| 367 | ret = ctx.in(i, s); | |
| 368 | break; | |
| 369 | } | |
| 358 | int out_node = -1, out_socket = -1; | |
| 359 | if (p_graph->find_density_output(out_node, out_socket)) { | |
| 360 | ret = ctx.in(out_node, out_socket); | |
| 370 | 361 | } |
| 371 | 362 | } |
| 372 | 363 | |
| … | … | @@ -411,7 +402,16 @@ String GpuCompiler::compile_to_function( |
| 411 | 402 | ret = "pos.y - query_y"; |
| 412 | 403 | } |
| 413 | 404 | |
| 414 | result += " // SDF: terrain_height - query_y. Positive below surface, negative above.\n"; | |
| 405 | // The comment has to follow the BRANCH. It said "terrain_height - query_y" | |
| 406 | // unconditionally, including over a `return n2_s0;` that is a Group Output | |
| 407 | // float -- so a volume graph's emitted text described itself as a | |
| 408 | // displacement. A stale comment on the one line that says what the shader | |
| 409 | // computes is worth more than a stale comment anywhere else. | |
| 410 | if (ret == "pos.y - query_y") { | |
| 411 | result += " // SDF: terrain_height - query_y. Positive below surface, negative above.\n"; | |
| 412 | } else { | |
| 413 | result += " // The graph's OWN density, from its Group Output float.\n"; | |
| 414 | } | |
| 415 | 415 | result += " return " + ret + ";\n"; |
| 416 | 416 | result += "}\n"; |
| 417 | 417 |
| … | … | @@ -462,9 +462,9 @@ String ProcNodeGraph::compile_to_glsl_function(const String &p_function_name, |
| 462 | 462 | // Which value the graph's density IS, by the same rules gpu_compiler.cpp uses |
| 463 | 463 | // for the emitted function. Returns the node and socket of an explicit Group |
| 464 | 464 | // Output float, or false for "the displacement of pos.y". |
| 465 | static bool _density_output_socket(const ProcNodeGraph &p_graph, int &r_node, int &r_socket) { | |
| 466 | for (int i = 0; i < p_graph.get_node_count(); i++) { | |
| 467 | Ref<ProcNode> n = p_graph.get_node(i); | |
| 465 | bool ProcNodeGraph::find_density_output(int &r_node, int &r_socket) const { | |
| 466 | for (int i = 0; i < get_node_count(); i++) { | |
| 467 | Ref<ProcNode> n = get_node(i); | |
| 468 | 468 | if (n.is_null() || n->get_node_type() != StringName("GroupOutput")) { |
| 469 | 469 | continue; |
| 470 | 470 | } |
| … | … | @@ -472,7 +472,7 @@ static bool _density_output_socket(const ProcNodeGraph &p_graph, int &r_node, in |
| 472 | 472 | if (n->get_input_def(s).type != SocketType::Float) { |
| 473 | 473 | continue; |
| 474 | 474 | } |
| 475 | if (p_graph.get_input_source_node(i, s) < 0) { | |
| 475 | if (get_input_source_node(i, s) < 0) { | |
| 476 | 476 | continue; |
| 477 | 477 | } |
| 478 | 478 | r_node = i; |
| … | … | @@ -483,6 +483,10 @@ static bool _density_output_socket(const ProcNodeGraph &p_graph, int &r_node, in |
| 483 | 483 | return false; |
| 484 | 484 | } |
| 485 | 485 | |
| 486 | static bool _density_output_socket(const ProcNodeGraph &p_graph, int &r_node, int &r_socket) { | |
| 487 | return p_graph.find_density_output(r_node, r_socket); | |
| 488 | } | |
| 489 | ||
| 486 | 490 | ProcNodeGraph::~ProcNodeGraph() { |
| 487 | 491 | delete _density_plan; |
| 488 | 492 | _density_plan = nullptr; |
| … | … | @@ -173,6 +173,18 @@ public: |
| 173 | 173 | bool p_clamp_octaves = false, |
| 174 | 174 | const PackedStringArray &p_skip_types = PackedStringArray()) const; |
| 175 | 175 | |
| 176 | // WHICH VALUE THE GRAPH'S DENSITY IS -- the one rule, in one place. | |
| 177 | // | |
| 178 | // An explicit Group Output with a CONNECTED FLOAT input is the graph saying | |
| 179 | // what it produces; anything else means "the displacement of pos.y". This | |
| 180 | // was written TWICE -- here for the CPU evaluator and again as a loop in | |
| 181 | // gpu_compiler.cpp for the emitter -- with a comment on one of them saying | |
| 182 | // it followed "the same rules gpu_compiler.cpp uses", which asserts | |
| 183 | // agreement rather than enforcing it. A mutation to one left the other | |
| 184 | // answering the old way, which is how the duplication was found | |
| 185 | // (hyper-games #65 NEXT 5). | |
| 186 | bool find_density_output(int &r_node, int &r_socket) const; | |
| 187 | ||
| 176 | 188 | // THE BATCH PATH'S TEXT, FOR THE GATE. |
| 177 | 189 | // |
| 178 | 190 | // compile_batch's output is otherwise reachable only through a GPU |
| … | … | @@ -1,4 +1,6 @@ |
| 1 | 1 | #include "volume_nodes.h" |
| 2 | ||
| 3 | #include <godot_cpp/variant/utility_functions.hpp> | |
| 2 | 4 | #include "../graph/graph_executor.h" |
| 3 | 5 | |
| 4 | 6 | namespace godot { |
| … | … | @@ -92,10 +94,20 @@ VolumeToMeshNode::VolumeToMeshNode() { |
| 92 | 94 | } |
| 93 | 95 | |
| 94 | 96 | void VolumeToMeshNode::execute(ProcEvalContext &p_ctx, int p_node_idx) { |
| 95 | // VolumeToMesh creates a ProcGeometry with a ProcVolume containing | |
| 96 | // the compiled GLSL chain. The GLSL is compiled by the graph's | |
| 97 | // compile_to_glsl_function() method. For CPU execution, this node | |
| 98 | // creates an empty geometry with a volume placeholder. | |
| 97 | // ⛔ THIS IS A PLACEHOLDER AND IT FAILS SILENTLY. It writes an EMPTY | |
| 98 | // ProcVolume on an empty ProcGeometry -- no mesh, no density, no message -- | |
| 99 | // so a graph ending here bakes to nothing and looks like a graph that did | |
| 100 | // not run. That is the actual debt behind this node, not its | |
| 101 | // supports_gpu() == false (see the header). | |
| 102 | // | |
| 103 | // Say so. The working way to express a volume's density is an explicit | |
| 104 | // Group Output float, which _density_output_socket reads and the emitter | |
| 105 | // returns; this node is not that mechanism. | |
| 106 | UtilityFunctions::push_warning( | |
| 107 | "proc-nodes: Volume to Mesh is a PLACEHOLDER -- it produces an empty " | |
| 108 | "geometry with an empty volume, so this graph will bake to nothing. " | |
| 109 | "To give a graph a density, wire the SDF chain into a Group Output " | |
| 110 | "FLOAT socket instead; that is what the emitter returns."); | |
| 99 | 111 | Ref<ProcGeometry> geo; |
| 100 | 112 | geo.instantiate(); |
| 101 | 113 | Ref<ProcVolume> vol; |
| … | … | @@ -83,10 +83,29 @@ public: |
| 83 | 83 | |
| 84 | 84 | void execute(ProcEvalContext &p_ctx, int p_node_idx) override; |
| 85 | 85 | |
| 86 | // Its output is a GEOMETRY socket, which has no GLSL spelling, and the | |
| 87 | // density function has no way to return a node's value anyway -- it returns | |
| 88 | // how far `pos` moved. Emitting nothing while claiming GPU support is the | |
| 89 | // silent-skip this change exists to remove, so it says false. | |
| 86 | // ⛔ FALSE ON PURPOSE, AND THE REASON WAS RE-CHECKED 2026-09-06 (hyper-games | |
| 87 | // #65 NEXT 5, which asked whether this last supports_gpu() == false has a | |
| 88 | // reason or is the next debt). It has one, and half of what stood here was | |
| 89 | // already out of date: | |
| 90 | // | |
| 91 | // 1. Its output is a GEOMETRY socket, which has no GLSL spelling. | |
| 92 | // | |
| 93 | // 2. ⚠ "the density function has no way to return a node's value" is NO | |
| 94 | // LONGER TRUE. _density_output_socket (proc_node_graph.cpp) picks an | |
| 95 | // explicit Group Output FLOAT, and gpu_compiler emits `return n2_s0;` | |
| 96 | // for it. Measured: an SDF chain wired into a Group Output float emits | |
| 97 | // 486 characters returning the chain, with no VolumeToMesh anywhere. | |
| 98 | // So a volume graph CAN say what its density is -- through a mechanism | |
| 99 | // this node is not, and cannot become, because it hands on geometry | |
| 100 | // rather than naming a float. | |
| 101 | // | |
| 102 | // 3. It has NO CONSUMERS. grep across proc-nodes, dc-terrain and | |
| 103 | // hyper-tps finds VolumeToMesh only in register_types.cpp and in | |
| 104 | // glsl_golden_gate.gd's refusal case. Nothing builds a volume through | |
| 105 | // it, which is consistent with execute() below being a placeholder. | |
| 106 | // | |
| 107 | // Turning this true would require it to emit something, and there is | |
| 108 | // nothing for it to emit. The debt here is not the flag -- see execute(). | |
| 90 | 109 | bool supports_gpu() const override { return false; } |
| 91 | 110 | String compile_glsl(const GlslEmitContext &p_ctx, int p_node_idx) const override; |
| 92 | 111 | }; |