f-compute-shaders #14

Ade9 · 2026-09-16T21:41:11+03:00

Ade9 commented

2026-09-16 21:41:11 +03:00

Compute shader implementation speeds up generation greatly.

Ade9 added 3 commits

2026-09-16 21:41:11 +03:00

Initial Compute Shader for Plate Generation. 47ca1265e6

1030% faster comp shader 9ab7191c95

readme update b0caa8c8ad

beepster reviewed

2026-09-16 22:33:10 +03:00

shaders/compute/PlateExpansion.glsl Outdated

					
				@ -0,0 +32,4 @@

							if (platebuffer.plateid[localpoint] != -1 && platebuffer.plateid[neighborbuffer.neighbors[localpoint*6+neighbor]] == -1)

							{

								// Neighbor unclaimed, and we can claim it.

								platebuffer.plateid[neighborbuffer.neighbors[localpoint*6+neighbor]] = platebuffer.plateid[localpoint];

(lines 32-35)

⚫️ [PROBLEM] ⚫️ [PROBLEM] Unsynchronized test-and-claim on shared plate buffer — nondeterministic plate ownership

Lines 32-35 are a classic read-then-write race on a storage buffer shared by all workgroups: one workgroup can read plateid[n] == -1 and write plate X while another workgroup does the same for plate Y on the same target vertex, and there are no memoryBarrier()/groupMemoryBarrier() calls or atomics anywhere in main(). The sequential CPU version this mirrors made the test-and-claim deterministic. Here plate assignment varies across runs/GPUs: a vertex can be claimed by whichever workgroup's write happens to win, propagation depth varies per dispatch, and since this buffer feeds the read-back that drives BorderSearch/EdgeStress/Height, the whole planet becomes unreproducible run-to-run. Use atomicCompSwap on the owner slot so exactly one claimant wins.

int n = neighborbuffer.neighbors[localpoint*6+neighbor];
if (n == -1) { continue; }
int myPlate = platebuffer.plateid[localpoint];
if (myPlate != -1) {
    // atomicCompSwap guarantees only one workgroup wins the claim
    atomicCompSwap(platebuffer.plateid[n], -1, myPlate);
}

*(lines 32-35)* ⚫️ [PROBLEM] ⚫️ [PROBLEM] Unsynchronized test-and-claim on shared plate buffer — nondeterministic plate ownership Lines 32-35 are a classic read-then-write race on a storage buffer shared by all workgroups: one workgroup can read `plateid[n] == -1` and write plate X while another workgroup does the same for plate Y on the same target vertex, and there are no `memoryBarrier()`/`groupMemoryBarrier()` calls or atomics anywhere in `main()`. The sequential CPU version this mirrors made the test-and-claim deterministic. Here plate assignment varies across runs/GPUs: a vertex can be claimed by whichever workgroup's write happens to win, propagation depth varies per dispatch, and since this buffer feeds the read-back that drives BorderSearch/EdgeStress/Height, the whole planet becomes unreproducible run-to-run. Use `atomicCompSwap` on the owner slot so exactly one claimant wins. ```glsl int n = neighborbuffer.neighbors[localpoint*6+neighbor]; if (n == -1) { continue; } int myPlate = platebuffer.plateid[localpoint]; if (myPlate != -1) { // atomicCompSwap guarantees only one workgroup wins the claim atomicCompSwap(platebuffer.plateid[n], -1, myPlate); } ```

(lines 25-35)

⚫️ [PROBLEM] ⚫️ [PROBLEM] No bounds check on localpoint — tail invocations read/write outside the storage buffers

localpoint (line 25) is derived from gl_WorkGroupID.x * gl_WorkGroupSize.x with no comparison against the actual vertex count, but the C# side dispatches xgroups = ceil(points.Length / 25.0) (PlanetHelper.cs:355), so the last workgroup iterates indices past points.Length - 1. Those tail invocations read neighbors[localpoint*6+neighbor] out of bounds, and when that garbage read is not -1 they read plateid[localpoint] and even write plateid[garbage_neighbor] (line 35) — an out-of-bounds write to unrelated buffer memory. On the D3D12/Vulkan drivers this PR now targets (the gl_compatibility fallback was removed in project.godot), that is undefined behaviour: spurious plate assignments or validation-device aborts in debug builds. Pass the vertex count into the shader (push constant or uniform) and guard before any indexing.

layout(push_constant) uniform Limits { int vertexCount; } limits;
// ...
int localpoint = int(gl_WorkGroupID.x) * int(gl_WorkGroupSize.x) + point;
if (localpoint >= limits.vertexCount) { continue; }

*(lines 25-35)* ⚫️ [PROBLEM] ⚫️ [PROBLEM] No bounds check on localpoint — tail invocations read/write outside the storage buffers `localpoint` (line 25) is derived from `gl_WorkGroupID.x * gl_WorkGroupSize.x` with no comparison against the actual vertex count, but the C# side dispatches `xgroups = ceil(points.Length / 25.0)` (`PlanetHelper.cs:355`), so the last workgroup iterates indices past `points.Length - 1`. Those tail invocations read `neighbors[localpoint*6+neighbor]` out of bounds, and when that garbage read is not -1 they read `plateid[localpoint]` and even write `plateid[garbage_neighbor]` (line 35) — an out-of-bounds write to unrelated buffer memory. On the D3D12/Vulkan drivers this PR now targets (the `gl_compatibility` fallback was removed in `project.godot`), that is undefined behaviour: spurious plate assignments or validation-device aborts in debug builds. Pass the vertex count into the shader (push constant or uniform) and guard before any indexing. ```glsl layout(push_constant) uniform Limits { int vertexCount; } limits; // ... int localpoint = int(gl_WorkGroupID.x) * int(gl_WorkGroupSize.x) + point; if (localpoint >= limits.vertexCount) { continue; } ```

src/Main.cs Outdated

					
				@ -40,2 +43,4 @@

				        AxialTiltChanged(GetNode<LineEdit>("%AxialTilt").Text);

				        UpdateTime();

				        _planetHelper.InitializeGeneration();

⚫️ [PROBLEM] ⚫️ [PROBLEM] Eager init in _Ready is wiped by the first AdvanceStage — seeded plate and color are discarded

InitializeGeneration() here ends with CompleteStage(), which sets StageComplete = true but leaves Stage = NotStarted (PlanetHelper.cs:173). On the first advance, AdvanceStage() moves Stage to Initialization and clears StageComplete, so Process()'s case GenerationStage.Initialization: (PlanetHelper.cs:272) calls InitializeGeneration() a second time — whose first statement is Plates = new(); (PlanetHelper.cs:147). The re-seeded plates and this line's manually assigned land color are thrown away before generation is ever visible. Compounding this, the PR removed ColorVertex(vertex.Id, color) from InitializeGeneration and the only code that would paint the seeded color onto the mesh (the commented block below) never runs, so the initial-color intent is silently lost. Either make the seed stick (e.g. advance Stage inside InitializeGeneration so the duplicate run doesn't wipe Plates) or assign the color at the point where it is actually consumed.

⚫️ [PROBLEM] ⚫️ [PROBLEM] Eager init in _Ready is wiped by the first AdvanceStage — seeded plate and color are discarded `InitializeGeneration()` here ends with `CompleteStage()`, which sets `StageComplete = true` but leaves `Stage = NotStarted` (`PlanetHelper.cs:173`). On the first advance, `AdvanceStage()` moves `Stage` to `Initialization` and clears `StageComplete`, so `Process()`'s `case GenerationStage.Initialization:` (PlanetHelper.cs:272) calls `InitializeGeneration()` a second time — whose first statement is `Plates = new();` (`PlanetHelper.cs:147`). The re-seeded plates and this line's manually assigned land color are thrown away before generation is ever visible. Compounding this, the PR removed `ColorVertex(vertex.Id, color)` from `InitializeGeneration` and the only code that would paint the seeded color onto the mesh (the commented block below) never runs, so the initial-color intent is silently lost. Either make the seed stick (e.g. advance `Stage` inside `InitializeGeneration` so the duplicate run doesn't wipe `Plates`) or assign the color at the point where it is actually consumed.

src/PlanetHelper.cs Outdated

					
				@ -318,0 +307,4 @@

				        int[] points = Enumerable.Range(0, Mdt.GetVertexCount()).ToArray();

				        var sizePoints = points.Length * sizeof(int);

				        int[] neighbors = Vertices.SelectMany(v => v.Neighbours.Count < 6 ? [..v.Neighbours.ToArray(),-1] : v.Neighbours.ToArray()).ToArray();

🚨 [CRITICAL] 🚨 [CRITICAL] Neighbor buffer is not padded to a fixed stride of 6

The shader indexes the neighbor buffer with a fixed stride of 6 (neighborbuffer.neighbors[localpoint*6+neighbor], PlateExpansion.glsl:30/34/37), so the contract requires exactly 6 ints per vertex. But the flattening here appends a single -1 only when a vertex has fewer than 6 neighbours and does NOT truncate when it has more: v.Neighbours.Count < 6 ? [..v.Neighbours.ToArray(),-1] : v.Neighbours.ToArray(). A degree-4 vertex emits 5 entries and a degree-7 vertex emits 7, so every vertex after it is shifted off its 6-slot boundary. Neighbours come from the imported mesh via Mdt.GetVertexEdges, so degree is not guaranteed to be exactly 6. If the total length also ends up shorter than vertexCount*6, the shader reads past the end of the storage buffer. The failure is silent and data-dependent — it will look like wrong/random continent shapes rather than a crash. Pad to exactly 6 slots and assert the invariant before dispatch.

int[] neighbors = new int[Vertices.Count * 6];
Array.Fill(neighbors, -1);
for (int i = 0; i < Vertices.Count; i++)
{
    var n = Vertices[i].Neighbours;
    for (int j = 0; j < Math.Min(6, n.Count); j++)
        neighbors[i * 6 + j] = n[j];
}
if (neighbors.Length != Vertices.Count * 6)
    GD.PushError("Neighbor buffer stride mismatch");

🚨 [CRITICAL] 🚨 [CRITICAL] Neighbor buffer is not padded to a fixed stride of 6 The shader indexes the neighbor buffer with a fixed stride of 6 (`neighborbuffer.neighbors[localpoint*6+neighbor]`, `PlateExpansion.glsl:30/34/37`), so the contract requires exactly 6 ints per vertex. But the flattening here appends a single `-1` only when a vertex has fewer than 6 neighbours and does NOT truncate when it has more: `v.Neighbours.Count < 6 ? [..v.Neighbours.ToArray(),-1] : v.Neighbours.ToArray()`. A degree-4 vertex emits 5 entries and a degree-7 vertex emits 7, so every vertex after it is shifted off its 6-slot boundary. Neighbours come from the imported mesh via `Mdt.GetVertexEdges`, so degree is not guaranteed to be exactly 6. If the total length also ends up shorter than `vertexCount*6`, the shader reads past the end of the storage buffer. The failure is silent and data-dependent — it will look like wrong/random continent shapes rather than a crash. Pad to exactly 6 slots and assert the invariant before dispatch. ```csharp int[] neighbors = new int[Vertices.Count * 6]; Array.Fill(neighbors, -1); for (int i = 0; i < Vertices.Count; i++) { var n = Vertices[i].Neighbours; for (int j = 0; j < Math.Min(6, n.Count); j++) neighbors[i * 6 + j] = n[j]; } if (neighbors.Length != Vertices.Count * 6) GD.PushError("Neighbor buffer stride mismatch"); ```

src/PlanetHelper.cs Outdated

					
				@ -325,0 +369,4 @@

				            {

				                Mdt.SetVertexColor(index, Plates[plate].Color);

				                Vertices[index].PlateId = plate;

				            }

(lines 366-372)

🚨 [CRITICAL] 🚨 [CRITICAL] GPU read-back never repopulates Plates[x].Vertices — area logic runs on seed-only lists

The removed CPU expansion performed plateData.Vertices.Add(expandTo) for every newly claimed vertex. This read-back loop only sets Vertices[index].PlateId and the colour; it never adds the newly-claimed index to Plates[plate].Vertices. Each PlateData.Vertices therefore keeps only the single seed created in InitializeGeneration (new PlateData(i, color, false, [vertex.Id]), PlanetHelper.cs:167), and nothing else repopulates it. Consequences: AssignOceanPlates (areas.Sum(a => a.Vertices.Count * a.PlateExpansion) and the per-plate foreach (int v in areas[i].Vertices) recolouring) sees Vertices.Count == 1 for every plate, so land/ocean selection is driven purely by PlateExpansion, the area readout in Main.cs:255 shows ~0%, and only the seed vertices can be recoloured. Add the newly-claimed index to the owning plate during read-back.

int index = 0;
foreach (int plate in output)
{
    if (plate != -1)
    {
        Mdt.SetVertexColor(index, Plates[plate].Color);
        if (Vertices[index].PlateId == -1)
            Plates[plate].Vertices.Add(index);
        Vertices[index].PlateId = plate;
    }
    index++;
}

*(lines 366-372)* 🚨 [CRITICAL] 🚨 [CRITICAL] GPU read-back never repopulates Plates[x].Vertices — area logic runs on seed-only lists The removed CPU expansion performed `plateData.Vertices.Add(expandTo)` for every newly claimed vertex. This read-back loop only sets `Vertices[index].PlateId` and the colour; it never adds the newly-claimed index to `Plates[plate].Vertices`. Each `PlateData.Vertices` therefore keeps only the single seed created in `InitializeGeneration` (`new PlateData(i, color, false, [vertex.Id])`, `PlanetHelper.cs:167`), and nothing else repopulates it. Consequences: `AssignOceanPlates` (`areas.Sum(a => a.Vertices.Count * a.PlateExpansion)` and the per-plate `foreach (int v in areas[i].Vertices)` recolouring) sees `Vertices.Count == 1` for every plate, so land/ocean selection is driven purely by `PlateExpansion`, the area readout in `Main.cs:255` shows ~0%, and only the seed vertices can be recoloured. Add the newly-claimed index to the owning plate during read-back. ```csharp int index = 0; foreach (int plate in output) { if (plate != -1) { Mdt.SetVertexColor(index, Plates[plate].Color); if (Vertices[index].PlateId == -1) Plates[plate].Vertices.Add(index); Vertices[index].PlateId = plate; } index++; } ```

src/PlanetHelper.cs Outdated

					
				@ -326,0 +380,4 @@

				        _rd.FreeRid(neighborBuffer);

				        AssignOceanPlates(Plates);

				        CompleteStage();

(lines 382-383)

⚫️ [PROBLEM] ⚫️ [PROBLEM] Single dispatch expands plates only one hop, then unconditionally completes the stage

The removed CPU loop was re-entered every Process() tick and only fired AssignOceanPlates/CompleteStage once !availableVerts.Any(), i.e. it kept expanding waves until the sphere filled. Here PlateGeneration() runs a single dispatch (one claim wave around the 12 seeds) and these two calls fire unconditionally at lines 382-383. Result: plates claim roughly one neighbour-hop per seed instead of filling the sphere, so every downstream stage (BorderSearch, EdgeStress, Height, ocean assignment) operates on an almost entirely unclaimed planet, and land/ocean ratios derived from plate sizes are meaningless. Loop the dispatch until no reachable -1 vertex remains (the read-back plate array is the convergence signal) and keep AssignOceanPlates/CompleteStage behind that condition — or move the iteration axis into the shader as its comments intend.

*(lines 382-383)* ⚫️ [PROBLEM] ⚫️ [PROBLEM] Single dispatch expands plates only one hop, then unconditionally completes the stage The removed CPU loop was re-entered every `Process()` tick and only fired `AssignOceanPlates`/`CompleteStage` once `!availableVerts.Any()`, i.e. it kept expanding waves until the sphere filled. Here `PlateGeneration()` runs a single dispatch (one claim wave around the 12 seeds) and these two calls fire unconditionally at lines 382-383. Result: plates claim roughly one neighbour-hop per seed instead of filling the sphere, so every downstream stage (BorderSearch, EdgeStress, Height, ocean assignment) operates on an almost entirely unclaimed planet, and land/ocean ratios derived from plate sizes are meaningless. Loop the dispatch until no reachable `-1` vertex remains (the read-back plate array is the convergence signal) and keep `AssignOceanPlates`/`CompleteStage` behind that condition — or move the iteration axis into the shader as its comments intend.

beepster commented

2026-09-16 22:33:10 +03:00

Hey! Nice ambition on this one — moving plate expansion onto the GPU via Godot's RenderingDevice is a meaty refactor, and the CPU-side RD plumbing (buffers, submit/sync, cleanup) is put together correctly. The catch is that the GPU version doesn't yet reproduce the semantics the CPU loop provided, so the generated planet will come out very differently (and unreproducibly) from before.

a. Summary

This PR offloads tectonic plate expansion from a per-tick CPU loop to a GLSL compute shader (PlateExpansion.glsl), adds eager generation bootstrap in Main._Ready, and switches the octree containment test to explicit inclusive per-component comparisons (Oct.cs). It also drops the gl_compatibility renderer fallback, making a compute-capable driver (D3D12/Vulkan) a hard requirement.

The biggest risks, in order: (1) the neighbor buffer is not padded to the fixed 6-int stride the shader assumes, which silently misaligns every lookup after the first non-6-degree vertex; (2) the read-back never repopulates Plates[x].Vertices, so all area-weighted land/ocean logic and per-plate recolouring now run on seed-only lists; (3) the single dispatch plus unconditional CompleteStage() means plates expand one hop and the stage is "done"; (4) the shader has no bounds guard and no atomic claim, giving out-of-bounds reads/writes and run-to-run nondeterminism. I'd treat this as work-in-progress against a merge until the stride fix, the Vertices repopulation, and the multi-wave dispatch loop are in.

b. Good points

🟢 The RenderingDevice sequence is kept in one synchronous block — ComputeListEnd() → Submit() → Sync() before BufferGetData (src/PlanetHelper.cs:358-361) — so the CPU read-back can never race the dispatch.
🟢 Every per-dispatch Rid (pipeline, uniform set, all three buffers) is freed explicitly right after read-back (src/PlanetHelper.cs:375-379), keeping VRAM flat across repeated generations since MakeGo() constructs a fresh PlanetHelper.
🟢 The inclusive per-component IsInside in src/Oct.cs:140-146 tiles consistently with WhichSubOct/GetSubStart and removes a silent data-loss path where boundary-exact vertices were dropped and could never be returned by SearchNearest (pointer/projection picking).
🟢 GetNeighboringVertices now treats NotStarted like Initialization (src/PlanetHelper.cs:178), which is exactly the boundary the new eager Main._Ready init needed so seeding reads mesh-derived neighbours instead of empty lists.

Main Issues

🚨 Neighbor buffer is not padded to a fixed stride of 6 — src/PlanetHelper.cs:310. The shader indexes neighbors[localpoint*6+neighbor], but a degree-4 vertex emits 5 entries and a degree-7 vertex emits 7, shifting every subsequent vertex off its 6-slot boundary; short totals also cause out-of-bounds reads. Wrong plate propagation with no error.
🚨 GPU read-back never repopulates Plates[x].Vertices — src/PlanetHelper.cs:366-372. Only PlateId and colour are restored; the removed CPU loop's plateData.Vertices.Add(expandTo) has no replacement, so AssignOceanPlates sees Vertices.Count == 1 per plate and the area readout (Main.cs:255) shows ~0%.
⚫️ One dispatch = one expansion hop, then unconditional stage completion — src/PlanetHelper.cs:382-383. The CPU version looped per Process() tick until the sphere filled; here AssignOceanPlates/CompleteStage fire after a single wave, leaving the planet ~unclaimed for all downstream stages.
⚫️ No bounds check on localpoint — shaders/compute/PlateExpansion.glsl:25-35. ceil(vertexCount/25) dispatch leaves the last workgroup indexing past the end; tail invocations can write plateid[garbage_neighbor] — UB on the D3D12/Vulkan drivers this PR now targets.
⚫️ Unsynchronized test-and-claim on the shared plate buffer — shaders/compute/PlateExpansion.glsl:32-35. Concurrent read-then-write with no atomics/barriers makes plate ownership nondeterministic across runs and GPUs; atomicCompSwap is the drop-in fix.
⚫️ Eager init in _Ready is wiped by the first advance — src/Main.cs:46-47. AdvanceStage() transitions to Initialization, which re-runs InitializeGeneration() starting with Plates = new(), discarding the seeded plates and the manually assigned land colour before the user ever sees them.

Additional Notes

src/PlanetHelper.cs:306, 356: the points array / pointBuffer / binding 0 is uploaded but never read by the shader, and yGroups: 50 with local_size_y = 1 launches ~50x more invocations than the shader actually uses — consider dropping both until the y/iteration axis is implemented.
src/PlanetHelper.cs:306: the comment "We use floats in the shader" is stale — the buffers are int.
src/PlanetHelper.cs:139-142: GD.Load<RDShaderFile>(...) / ShaderCreateFromSpirV(...) run unvalidated; a null shaderFile is an immediate NRE and an unsupported device only surfaces later as a cryptic RD error — worth explicit null/valid checks with a clear message, especially now that project.godot removes the gl_compatibility fallback and compute is a hard requirement.
src/Main.cs:52-81: the commented-out CPU expansion block references variables that no longer exist in scope and duplicates logic that now lives in the shader — delete it; git history preserves it.
src/Main.cs:4, 8, 10: the added System.Collections.Generic, System.Threading.Tasks, and Array = System.Array usings are unused in the file (they supported the commented-out block).
src/Oct.cs:55-66: Insert has no depth cap — two nodes at the identical scaled position recurse forever (stack overflow); the widened inclusive IsInside admits boundary-exact points into this path. A coincident-position guard or max-depth bail with GD.PrintErr would make it diagnosable.

Hey! Nice ambition on this one — moving plate expansion onto the GPU via Godot's RenderingDevice is a meaty refactor, and the CPU-side RD plumbing (buffers, submit/sync, cleanup) is put together correctly. The catch is that the GPU version doesn't yet reproduce the semantics the CPU loop provided, so the generated planet will come out very differently (and unreproducibly) from before. ## a. Summary This PR offloads tectonic plate expansion from a per-tick CPU loop to a GLSL compute shader (`PlateExpansion.glsl`), adds eager generation bootstrap in `Main._Ready`, and switches the octree containment test to explicit inclusive per-component comparisons (`Oct.cs`). It also drops the `gl_compatibility` renderer fallback, making a compute-capable driver (D3D12/Vulkan) a hard requirement. The biggest risks, in order: (1) the neighbor buffer is not padded to the fixed 6-int stride the shader assumes, which silently misaligns every lookup after the first non-6-degree vertex; (2) the read-back never repopulates `Plates[x].Vertices`, so all area-weighted land/ocean logic and per-plate recolouring now run on seed-only lists; (3) the single dispatch plus unconditional `CompleteStage()` means plates expand one hop and the stage is "done"; (4) the shader has no bounds guard and no atomic claim, giving out-of-bounds reads/writes and run-to-run nondeterminism. I'd treat this as work-in-progress against a merge until the stride fix, the `Vertices` repopulation, and the multi-wave dispatch loop are in. ## b. Good points - 🟢 The RenderingDevice sequence is kept in one synchronous block — `ComputeListEnd()` → `Submit()` → `Sync()` before `BufferGetData` (`src/PlanetHelper.cs:358-361`) — so the CPU read-back can never race the dispatch. - 🟢 Every per-dispatch Rid (pipeline, uniform set, all three buffers) is freed explicitly right after read-back (`src/PlanetHelper.cs:375-379`), keeping VRAM flat across repeated generations since `MakeGo()` constructs a fresh `PlanetHelper`. - 🟢 The inclusive per-component `IsInside` in `src/Oct.cs:140-146` tiles consistently with `WhichSubOct`/`GetSubStart` and removes a silent data-loss path where boundary-exact vertices were dropped and could never be returned by `SearchNearest` (pointer/projection picking). - 🟢 `GetNeighboringVertices` now treats `NotStarted` like `Initialization` (`src/PlanetHelper.cs:178`), which is exactly the boundary the new eager `Main._Ready` init needed so seeding reads mesh-derived neighbours instead of empty lists. ## Main Issues 1. 🚨 **Neighbor buffer is not padded to a fixed stride of 6** — `src/PlanetHelper.cs:310`. The shader indexes `neighbors[localpoint*6+neighbor]`, but a degree-4 vertex emits 5 entries and a degree-7 vertex emits 7, shifting every subsequent vertex off its 6-slot boundary; short totals also cause out-of-bounds reads. Wrong plate propagation with no error. 2. 🚨 **GPU read-back never repopulates `Plates[x].Vertices`** — `src/PlanetHelper.cs:366-372`. Only `PlateId` and colour are restored; the removed CPU loop's `plateData.Vertices.Add(expandTo)` has no replacement, so `AssignOceanPlates` sees `Vertices.Count == 1` per plate and the area readout (`Main.cs:255`) shows ~0%. 3. ⚫️ **One dispatch = one expansion hop, then unconditional stage completion** — `src/PlanetHelper.cs:382-383`. The CPU version looped per `Process()` tick until the sphere filled; here `AssignOceanPlates`/`CompleteStage` fire after a single wave, leaving the planet ~unclaimed for all downstream stages. 4. ⚫️ **No bounds check on `localpoint`** — `shaders/compute/PlateExpansion.glsl:25-35`. `ceil(vertexCount/25)` dispatch leaves the last workgroup indexing past the end; tail invocations can write `plateid[garbage_neighbor]` — UB on the D3D12/Vulkan drivers this PR now targets. 5. ⚫️ **Unsynchronized test-and-claim on the shared plate buffer** — `shaders/compute/PlateExpansion.glsl:32-35`. Concurrent read-then-write with no atomics/barriers makes plate ownership nondeterministic across runs and GPUs; `atomicCompSwap` is the drop-in fix. 6. ⚫️ **Eager init in `_Ready` is wiped by the first advance** — `src/Main.cs:46-47`. `AdvanceStage()` transitions to `Initialization`, which re-runs `InitializeGeneration()` starting with `Plates = new()`, discarding the seeded plates and the manually assigned land colour before the user ever sees them. ## Additional Notes - `src/PlanetHelper.cs:306, 356`: the `points` array / `pointBuffer` / binding 0 is uploaded but never read by the shader, and `yGroups: 50` with `local_size_y = 1` launches ~50x more invocations than the shader actually uses — consider dropping both until the y/iteration axis is implemented. - `src/PlanetHelper.cs:306`: the comment "We use floats in the shader" is stale — the buffers are `int`. - `src/PlanetHelper.cs:139-142`: `GD.Load<RDShaderFile>(...)` / `ShaderCreateFromSpirV(...)` run unvalidated; a null `shaderFile` is an immediate NRE and an unsupported device only surfaces later as a cryptic RD error — worth explicit null/valid checks with a clear message, especially now that `project.godot` removes the `gl_compatibility` fallback and compute is a hard requirement. - `src/Main.cs:52-81`: the commented-out CPU expansion block references variables that no longer exist in scope and duplicates logic that now lives in the shader — delete it; git history preserves it. - `src/Main.cs:4, 8, 10`: the added `System.Collections.Generic`, `System.Threading.Tasks`, and `Array = System.Array` usings are unused in the file (they supported the commented-out block). - `src/Oct.cs:55-66`: `Insert` has no depth cap — two nodes at the identical scaled position recurse forever (stack overflow); the widened inclusive `IsInside` admits boundary-exact points into this path. A coincident-position guard or max-depth bail with `GD.PrintErr` would make it diagnosable.

Ade9 added 8 commits

2026-09-17 07:51:01 +03:00

Removed commented code 5af4623dc0

version bump 1310a599c8

aesthetic changes 688b7086d7

Synchronize test-and-claim on plate buffer. 089beb748e

fix(compute): guard PlateExpansion tail invocations against OOB buffer access c99f87af32

Fix seeded plates and land color being wiped by duplicate InitializeGeneration on first AdvanceStage 0c3c76ce0f

Fix plate area tracking: repopulate PlateData.Vertices during GPU read-back in PlateGeneration 5055ad41b8

PlateGeneration: drive one GPU claim wave per tick until the read-back plate array is fully claimed before assigning ocean plates and completing the stage 8963ba9122

Ade9 commented

2026-09-17 07:52:17 +03:00

@beepster Please review after changes.

beepster commented

2026-09-17 08:15:22 +03:00

@Ade9

Re-reviewed after the push. All four blockers from my last pass are addressed, and the plumbing looks right. Remaining items are mostly semantics/perf rather than the GPU sequence itself.

Verified fixed

Fixed 6-int stride — EnsurePlateExpansionResources now allocates vertexCount * 6 and Array.Fill(neighbors, -1) before copying, so every row stays on its 6-slot boundary (src/PlanetHelper.cs:452-461). Matches neighbors[localpoint*6+neighbor] in the shader.
Plates[x].Vertices repopulated on read-back — newly GPU-claimed vertices are appended (src/PlanetHelper.cs:386-402), and your reasoning in the comment holds: the shader only ever does -1 -> myPlate, so an owned vertex can't change owners and no stale list entries are possible. AssignOceanPlates' area math and the Main area readout are no longer running on seed-only lists.
Multi-wave loop with real termination — one wave per Process() tick, read-back fed straight back in, unclaimed/newlyClaimed as the convergence signal, and AssignOceanPlates + CompleteStage only after the sphere fills (src/PlanetHelper.cs:404-435). MaxPlateWaves = 4096 plus ClaimOrphans() gives a belt-and-braces exit.
Bounds guard — vertexCount goes in as a push constant and tail invocations bail before any read/write (PlateExpansion.glsl:33-40, src/PlanetHelper.cs:372-374).
atomicCompSwap on the claim — one winner per contested vertex (PlateExpansion.glsl:47).
Eager init no longer wiped — InitializeGeneration sets Stage = Initialization itself (src/PlanetHelper.cs:172-178), so the first AdvanceStage() lands on PlateGeneration rather than re-running the method and clobbering the seeded plates/colours.
Also fixed since last time: yGroups: 1 (no more ~50x phantom invocations), the stale "We use floats" comment, the commented-out CPU expansion block and the unused System.Collections.Generic / System.Threading.Tasks / Array = usings in Main.cs. Caching the pipeline + buffers per vertex count is the right call — no per-hop pipeline creation.

Remaining / new

⚫️ The land seed in _Ready is transient, and plate 0 can't actually be land. Main.cs:46 paints plate 0 land, but PlateGeneration ends with AssignOceanPlates(Plates) (src/PlanetHelper.cs:434), which overwrites IsLandform and Color for every plate. Worse, the mask enumeration only visits multiples of 4:
```
var combs = Enumerable.Range(0, combinations / 4).Select(value => value * 4).ToList();
```
bits 0 and 1 are therefore always clear, so IsLandform = (bestMask & (1 << i)) != 0 is always false for plates 0 and 1. Net effect: the eager land seed survives only until the expansion stage finishes, then flips to ocean. What's the intent here — "plate 0 must start as land" (then AssignOceanPlates needs a forced-land constraint / the quantization needs fixing), or just "show something coloured on the mesh immediately" (then it's working as designed and worth a comment saying so)?
⚫️ Neighbours beyond 6 are dropped silently — Math.Min(vertexNeighbors.Count, neighborsPerVertex) (src/PlanetHelper.cs:460). On the current icosphere-derived mesh that's benign (only the twelve degree-5 vertices, which under-fill rather than overflow), but any mesh with a degree ≥ 7 vertex loses adjacency with no diagnostic, and ClaimOrphans only runs when the GPU stops making progress — so the loss would show up as subtly different plate boundaries, not an error. Cheap fix: GD.PushWarning when vertexNeighbors.Count > 6.
⚫️ pointbuffer / binding 0 is still dead weight — grep for pointbuffer. in shaders/compute/*.glsl returns nothing; the shader never reads it. The binding has to stay to match the SPIR-V layout unless you also delete the block from the shader, but the Enumerable.Range + upload (src/PlanetHelper.cs:448-449, 476-479) is pure waste. Either drop the binding entirely or allocate it without payload.
⚫️ Shader/device bootstrap is still unvalidated — src/PlanetHelper.cs:164-167. A failed GD.Load<RDShaderFile> is an NRE inside the constructor, and a null/unsupported local RenderingDevice surfaces later as an opaque RD error. This matters more now that the gl_compatibility fallback is gone. Worth an explicit null/IsUsingDrawCalls-style guard with a clear "compute-capable driver required" message.
⚫️ project.godot still declares "GL Compatibility" — line 16: config/features=PackedStringArray("4.6", "C#", "GL Compatibility"). The diff removed renderer/rendering_method / .mobile, so the project actually runs forward_plus, but the feature tag still claims GL Compatibility. That tag feeds import-time/feature checks and will mislead anyone reading the project settings. Drop it (or set rendering_method explicitly to forward_plus) so declared features match reality.
Small — misleading log on the wave cap: if (unclaimed > 0) prints wave N claimed nothing (src/PlanetHelper.cs:417-422), but that branch is also reached when the wave was progressing and simply hit MaxPlateWaves. Split the message so a cap hit doesn't point debugging at the wrong thing.
Small — _rd / _shader lifecycle: MakeGo() (src/Main.cs:232) constructs a fresh PlanetHelper, which means a fresh local RenderingDevice per generation, released only whenever GC collects the old one; _shader is never explicitly freed (ReleasePlateExpansionResources frees pipeline/uniform set/buffers only). A deterministic Cleanup() that frees _shader and destroys the local RD would keep repeated MakeGo() clicks from piling up devices.
Perf note — the wall clock is now dominated by the per-tick Submit() + Sync() + BufferGetData(), not by the shader. Since the buffers persist across waves, you could run K waves per tick and pay one sync instead of K — check whether your Godot version exposes a memory barrier inside the compute list first, since consecutive dispatches hitting the same storage buffer need one to be correct. Worth measuring before bothering, though: at ~60 fps a couple hundred waves is only a few seconds.
Still open from last time (not a blocker for this PR's goal): Oct.Insert has no depth cap (src/Oct.cs:41-66), so two nodes at an identical scaled position recurse forever — and the widened inclusive IsInside makes boundary-exact coincident points reachable. A coincident-position guard or max-depth bail with GD.PrintErr would make it diagnosable instead of a stack overflow.

Also confirmed the .mtl deletion is safe — nothing references PlanetLow anywhere in the tree; the scene uses res://assets/PlanetBase.obj (scenes/MainScene.tscn:4).

So: GPU side is good to go. I'd still want #1 resolved (or explicitly declared intentional) before merge, since it changes the generated world; #4/#5 are cheap and are the ones most likely to bite someone else on different hardware.

@Ade9 Re-reviewed after the push. All four blockers from my last pass are addressed, and the plumbing looks right. Remaining items are mostly semantics/perf rather than the GPU sequence itself. ## Verified fixed 1. **Fixed 6-int stride** — `EnsurePlateExpansionResources` now allocates `vertexCount * 6` and `Array.Fill(neighbors, -1)` before copying, so every row stays on its 6-slot boundary (`src/PlanetHelper.cs:452-461`). Matches `neighbors[localpoint*6+neighbor]` in the shader. 2. **`Plates[x].Vertices` repopulated on read-back** — newly GPU-claimed vertices are appended (`src/PlanetHelper.cs:386-402`), and your reasoning in the comment holds: the shader only ever does `-1 -> myPlate`, so an owned vertex can't change owners and no stale list entries are possible. `AssignOceanPlates`' area math and the `Main` area readout are no longer running on seed-only lists. 3. **Multi-wave loop with real termination** — one wave per `Process()` tick, read-back fed straight back in, `unclaimed`/`newlyClaimed` as the convergence signal, and `AssignOceanPlates` + `CompleteStage` only after the sphere fills (`src/PlanetHelper.cs:404-435`). `MaxPlateWaves = 4096` plus `ClaimOrphans()` gives a belt-and-braces exit. 4. **Bounds guard** — `vertexCount` goes in as a push constant and tail invocations bail before any read/write (`PlateExpansion.glsl:33-40`, `src/PlanetHelper.cs:372-374`). 5. **`atomicCompSwap` on the claim** — one winner per contested vertex (`PlateExpansion.glsl:47`). 6. **Eager init no longer wiped** — `InitializeGeneration` sets `Stage = Initialization` itself (`src/PlanetHelper.cs:172-178`), so the first `AdvanceStage()` lands on `PlateGeneration` rather than re-running the method and clobbering the seeded plates/colours. 7. Also fixed since last time: `yGroups: 1` (no more ~50x phantom invocations), the stale "We use floats" comment, the commented-out CPU expansion block and the unused `System.Collections.Generic` / `System.Threading.Tasks` / `Array =` usings in `Main.cs`. Caching the pipeline + buffers per vertex count is the right call — no per-hop pipeline creation. ## Remaining / new 1. ⚫️ **The land seed in `_Ready` is transient, and plate 0 can't actually be land.** `Main.cs:46` paints plate 0 land, but `PlateGeneration` ends with `AssignOceanPlates(Plates)` (`src/PlanetHelper.cs:434`), which overwrites `IsLandform` **and** `Color` for every plate. Worse, the mask enumeration only visits multiples of 4: ```csharp var combs = Enumerable.Range(0, combinations / 4).Select(value => value * 4).ToList(); ``` bits 0 and 1 are therefore always clear, so `IsLandform = (bestMask & (1 << i)) != 0` is always false for plates 0 and 1. Net effect: the eager land seed survives only until the expansion stage finishes, then flips to ocean. What's the intent here — "plate 0 must start as land" (then `AssignOceanPlates` needs a forced-land constraint / the quantization needs fixing), or just "show something coloured on the mesh immediately" (then it's working as designed and worth a comment saying so)? 2. ⚫️ **Neighbours beyond 6 are dropped silently** — `Math.Min(vertexNeighbors.Count, neighborsPerVertex)` (`src/PlanetHelper.cs:460`). On the current icosphere-derived mesh that's benign (only the twelve degree-5 vertices, which under-fill rather than overflow), but any mesh with a degree ≥ 7 vertex loses adjacency with no diagnostic, and `ClaimOrphans` only runs when the GPU stops making progress — so the loss would show up as subtly different plate boundaries, not an error. Cheap fix: `GD.PushWarning` when `vertexNeighbors.Count > 6`. 3. ⚫️ **`pointbuffer` / binding 0 is still dead weight** — grep for `pointbuffer.` in `shaders/compute/*.glsl` returns nothing; the shader never reads it. The binding has to stay to match the SPIR-V layout unless you also delete the block from the shader, but the `Enumerable.Range` + upload (`src/PlanetHelper.cs:448-449`, `476-479`) is pure waste. Either drop the binding entirely or allocate it without payload. 4. ⚫️ **Shader/device bootstrap is still unvalidated** — `src/PlanetHelper.cs:164-167`. A failed `GD.Load<RDShaderFile>` is an NRE inside the constructor, and a null/unsupported local `RenderingDevice` surfaces later as an opaque RD error. This matters more now that the `gl_compatibility` fallback is gone. Worth an explicit null/`IsUsingDrawCalls`-style guard with a clear "compute-capable driver required" message. 5. ⚫️ **`project.godot` still declares `"GL Compatibility"`** — line 16: `config/features=PackedStringArray("4.6", "C#", "GL Compatibility")`. The diff removed `renderer/rendering_method` / `.mobile`, so the project actually runs forward_plus, but the feature tag still claims GL Compatibility. That tag feeds import-time/feature checks and will mislead anyone reading the project settings. Drop it (or set `rendering_method` explicitly to `forward_plus`) so declared features match reality. 6. Small — **misleading log on the wave cap**: `if (unclaimed > 0)` prints `wave N claimed nothing` (`src/PlanetHelper.cs:417-422`), but that branch is also reached when the wave *was* progressing and simply hit `MaxPlateWaves`. Split the message so a cap hit doesn't point debugging at the wrong thing. 7. Small — **`_rd` / `_shader` lifecycle**: `MakeGo()` (`src/Main.cs:232`) constructs a fresh `PlanetHelper`, which means a fresh local `RenderingDevice` per generation, released only whenever GC collects the old one; `_shader` is never explicitly freed (`ReleasePlateExpansionResources` frees pipeline/uniform set/buffers only). A deterministic `Cleanup()` that frees `_shader` and destroys the local RD would keep repeated `MakeGo()` clicks from piling up devices. 8. Perf note — the wall clock is now dominated by the per-tick `Submit()` + `Sync()` + `BufferGetData()`, not by the shader. Since the buffers persist across waves, you could run K waves per tick and pay one sync instead of K — check whether your Godot version exposes a memory barrier inside the compute list first, since consecutive dispatches hitting the same storage buffer need one to be correct. Worth measuring before bothering, though: at ~60 fps a couple hundred waves is only a few seconds. 9. Still open from last time (not a blocker for this PR's goal): `Oct.Insert` has no depth cap (`src/Oct.cs:41-66`), so two nodes at an identical scaled position recurse forever — and the widened inclusive `IsInside` makes boundary-exact coincident points reachable. A coincident-position guard or max-depth bail with `GD.PrintErr` would make it diagnosable instead of a stack overflow. Also confirmed the `.mtl` deletion is safe — nothing references `PlanetLow` anywhere in the tree; the scene uses `res://assets/PlanetBase.obj` (`scenes/MainScene.tscn:4`). So: GPU side is good to go. I'd still want #1 resolved (or explicitly declared intentional) before merge, since it changes the generated world; #4/#5 are cheap and are the ones most likely to bite someone else on different hardware.

Ade9 added 1 commit

2026-09-17 08:46:48 +03:00

fix: address review findings on compute bootstrap, masks, and lifecycle 9f93fe7446

- AssignOceanPlates: enumerate the full mask space (the /4 quantization pinned
  bits 0-1, making plates 0 and 1 impossible to ever be land); rename
  oceanArea/targetOcean to landArea/targetLand to match actual semantics
- Main._Ready: document the plate 0 land seed as display-only; land/ocean is
  re-derived by AssignOceanPlates at stage end
- Warn via GD.PushWarning when a vertex has more than 6 neighbours instead of
  silently truncating adjacency
- Remove the unused PointBuffer (binding 0) from the shader and the C# side;
  bindings 1-2 keep matching the SPIR-V layout
- Validate the compute bootstrap: clear InvalidOperationException for missing
  local RenderingDevice, failed shader load, or invalid compiled shader
- project.godot: drop the stale 'GL Compatibility' feature tag and declare
  renderer/rendering_method=forward_plus explicitly
- PlateGeneration: split the wave-cap log so a MaxPlateWaves hit while still
  progressing is no longer reported as 'claimed nothing'
- Add deterministic PlanetHelper.Cleanup() (frees shader Rid + local
  RenderingDevice); called from MakeGo() and _ExitTree()
- Oct.Insert: add a max-depth bail with GD.PushWarning so coincident
  positions are diagnosable instead of a stack overflow

Ade9 commented

2026-09-17 09:01:39 +03:00

@beepster I have addressed your concerns. Please re-validate 🙂

beepster commented

2026-09-17 09:07:33 +03:00

@Ade9

@Ade9 Re-validated after the push — all nine items from my last pass are resolved (or explicitly declared intentional where the semantics question allowed it). Verified against the current head:

Confirmed fixed

Land-seed / plate-0 semantics (#1) — AssignOceanPlates now enumerates the full mask space (Parallel.For(0, 1 << n, ...) at src/PlanetHelper.cs:937), so bits 0/1 are no longer pinned and plates 0 and 1 can come out as land from the area math. The Main._Ready seed is now documented as transient-by-design (src/Main.cs:44-49): it paints plate 0 green for immediate feedback and AssignOceanPlates legitimately re-derives land/ocean at the end of PlateGeneration. That's the "working as designed + comment says so" branch, which is what I asked for. ✅
Degree > 6 diagnostic (#2) — GD.PushWarning when vertexNeighbors.Count > neighborsPerVertex before the truncating Array.Copy (src/PlanetHelper.cs:513). Adjacency loss is now visible instead of silent. ✅
Dead binding 0 (#3) — the PointBuffer block is gone from the shader (PlateExpansion.glsl:6-8) and is no longer allocated/uploaded; the C# builds bindings 1 and 2 only, with a comment to keep them matched to the SPIR-V layout (src/PlanetHelper.cs:522-523, UniformSetCreate at :558). No more Enumerable.Range + payload for a buffer nothing reads. ✅
RD/shader bootstrap validated (#4) — explicit null check on CreateLocalRenderingDevice, null check on GD.Load<RDShaderFile>, and _shader.IsValid check with _rd.Dispose() before throwing, each with a "compute-capable renderer required" message (src/PlanetHelper.cs:166-189). No more bare NRE on a failed import. ✅
project.godot feature tags (#5) — config/features=PackedStringArray("4.7", "C#") (line 16); the "GL Compatibility" tag is dropped, and rendering_device/driver.windows="d3d12" matches the actual forward_plus + D3D12 setup. Declared features now match reality. ✅
Wave-cap log split (#6) — the no-progress branch (:471) and the cap-hit branch (:479) are now separate messages, so hitting MaxPlateWaves while still progressing no longer masquerades as a reachability bug. ✅
Deterministic GPU lifecycle (#7) — new Cleanup() frees the _shader Rid and disposes the local RenderingDevice, idempotent via the _rd == null guard (src/PlanetHelper.cs:197-214). Called from both MakeGo() (src/Main.cs:244) and _ExitTree() (src/Main.cs:56), so repeated regenerations no longer stack up devices waiting on the GC. ✅
Octree depth cap (#9) — MaxDepth = 48 with a GD.PushWarning bail in Insert (src/Oct.cs:45, :55-59), and the recursive calls pass depth + 1. Coincident-position infinite recursion is now a diagnosable drop instead of a stack overflow. ✅

The previously-verified plumbing (6-int stride with Array.Fill(-1), Plates[x].Vertices repopulation on read-back, one-wave-per-tick convergence loop, push-constant bounds guard, atomicCompSwap claim, InitializeGeneration setting Stage itself) all still holds at src/PlanetHelper.cs:445, :456, :458-481, PlateExpansion.glsl:33-40/:47, :221.

Remaining (non-blocking)

#8 from last time was a perf note and is untouched — still one Submit()/Sync()/BufferGetData() round-trip per tick. Fine to leave; measure first as I said.
New, tiny: Parallel.For(0, 1 << n, ...) is O(2^n) with a lock on every candidate improvement (src/PlanetHelper.cs:937-953). Perfectly fine at _plateCount = 12 (4096 masks), but it becomes pathological if _plateCount ever grows past ~20, and 1 << n overflows at n ≥ 31. Worth a comment capping the exhaustive search or a note that _plateCount must stay small — no change needed for this PR.

The one thing I flagged as a merge condition (#1, since it changes the generated world) is now both fixed in the quantization and declared intentional in the seed comment. LGTM — no further blockers from me.

@Ade9 @Ade9 Re-validated after the push — all nine items from my last pass are resolved (or explicitly declared intentional where the semantics question allowed it). Verified against the current head: ## Confirmed fixed 1. **Land-seed / plate-0 semantics (#1)** — `AssignOceanPlates` now enumerates the *full* mask space (`Parallel.For(0, 1 << n, ...)` at `src/PlanetHelper.cs:937`), so bits 0/1 are no longer pinned and plates 0 and 1 *can* come out as land from the area math. The `Main._Ready` seed is now documented as transient-by-design (`src/Main.cs:44-49`): it paints plate 0 green for immediate feedback and `AssignOceanPlates` legitimately re-derives land/ocean at the end of `PlateGeneration`. That's the "working as designed + comment says so" branch, which is what I asked for. ✅ 2. **Degree > 6 diagnostic (#2)** — `GD.PushWarning` when `vertexNeighbors.Count > neighborsPerVertex` before the truncating `Array.Copy` (`src/PlanetHelper.cs:513`). Adjacency loss is now visible instead of silent. ✅ 3. **Dead binding 0 (#3)** — the `PointBuffer` block is gone from the shader (`PlateExpansion.glsl:6-8`) and is no longer allocated/uploaded; the C# builds bindings 1 and 2 only, with a comment to keep them matched to the SPIR-V layout (`src/PlanetHelper.cs:522-523`, `UniformSetCreate` at `:558`). No more `Enumerable.Range` + payload for a buffer nothing reads. ✅ 4. **RD/shader bootstrap validated (#4)** — explicit null check on `CreateLocalRenderingDevice`, null check on `GD.Load<RDShaderFile>`, and `_shader.IsValid` check with `_rd.Dispose()` before throwing, each with a "compute-capable renderer required" message (`src/PlanetHelper.cs:166-189`). No more bare NRE on a failed import. ✅ 5. **`project.godot` feature tags (#5)** — `config/features=PackedStringArray("4.7", "C#")` (line 16); the `"GL Compatibility"` tag is dropped, and `rendering_device/driver.windows="d3d12"` matches the actual forward_plus + D3D12 setup. Declared features now match reality. ✅ 6. **Wave-cap log split (#6)** — the no-progress branch (`:471`) and the cap-hit branch (`:479`) are now separate messages, so hitting `MaxPlateWaves` while still progressing no longer masquerades as a reachability bug. ✅ 7. **Deterministic GPU lifecycle (#7)** — new `Cleanup()` frees the `_shader` Rid and disposes the local `RenderingDevice`, idempotent via the `_rd == null` guard (`src/PlanetHelper.cs:197-214`). Called from both `MakeGo()` (`src/Main.cs:244`) and `_ExitTree()` (`src/Main.cs:56`), so repeated regenerations no longer stack up devices waiting on the GC. ✅ 8. **Octree depth cap (#9)** — `MaxDepth = 48` with a `GD.PushWarning` bail in `Insert` (`src/Oct.cs:45`, `:55-59`), and the recursive calls pass `depth + 1`. Coincident-position infinite recursion is now a diagnosable drop instead of a stack overflow. ✅ The previously-verified plumbing (6-int stride with `Array.Fill(-1)`, `Plates[x].Vertices` repopulation on read-back, one-wave-per-tick convergence loop, push-constant bounds guard, `atomicCompSwap` claim, `InitializeGeneration` setting `Stage` itself) all still holds at `src/PlanetHelper.cs:445`, `:456`, `:458-481`, `PlateExpansion.glsl:33-40`/`:47`, `:221`. ## Remaining (non-blocking) - **#8 from last time was a perf note and is untouched** — still one `Submit()`/`Sync()`/`BufferGetData()` round-trip per tick. Fine to leave; measure first as I said. - New, tiny: `Parallel.For(0, 1 << n, ...)` is O(2^n) with a `lock` on every candidate improvement (`src/PlanetHelper.cs:937-953`). Perfectly fine at `_plateCount = 12` (4096 masks), but it becomes pathological if `_plateCount` ever grows past ~20, and `1 << n` overflows at n ≥ 31. Worth a comment capping the exhaustive search or a note that `_plateCount` must stay small — no change needed for this PR. The one thing I flagged as a merge condition (#1, since it changes the generated world) is now both fixed in the quantization and declared intentional in the seed comment. LGTM — no further blockers from me.

Ade9 added 10 commits

2026-09-17 19:14:43 +03:00

BorderSearch implemented as compute shader. 3f4ae1b885

EdgeDistanceCalculation as compute shader 24301c8a5f

EdgeStressCalculation implemented as compute shader. cfed7c5a3c

SpreadStress implemented as compute shader. 0e30e70e62

EdgeStressCalculation: model the boundary as rigid rotation, not per-edge chords 62858cca7c

The strain model measured the wrong thing in three ways at once, which is why the
boundaries came back as noise rather than as fault lines.

Velocity. The old per-point velocity was the unit chord of a 1 degree turn times
MovementSpeed, so every point on a plate moved at the same linear speed -- including
points sitting on the plate's own rotation axis, which should not move at all. A plate
whose rotation pole sat on a boundary was shredding it at full strength. Also `p -
rot(p, +1deg)` is anti-parallel to the direction a point actually travels, so every
tension reading was a compression reading and vice versa. Now v = omega x p with
omega = axis * speed / |p|: exactly tangential, MovementSpeed reads as the equatorial
linear speed, and it fades to zero at the rotational poles the way a rigid cap's must.

Frame. Decomposing relative velocity against each mesh edge's own direction made the
strain type depend on how the triangulation happened to lie. A border vertex has a
median of 2 cross-plate contacts (28% have exactly one, so no averaging at all), and
41% of the multi-contact ones hold separating *and* closing contacts at once -- the
signed normal rate cancels while the always-positive shear never does. Now one
measurement per (vertex, other plate) pair, in a single frame built at the vertex from
its radial and that pair's mean tangent-projected contact direction, so the normal and
shear components being compared actually come from the same frame.

Type and magnitude. The majority vote over 1-2 samples was a coin flip, and the
magnitude (mean of the per-edge |relV|) was decoupled from the type, so a vertex could
come back bright and shear-free. The magnitude is now hypot(N, S) of the very averaged
components the type is read from, and the type is a plain ratio test
(TENSION_DOMINANCE_RATIO = 1.0).

HeightCalculation drops the negation of NormalRate that existed to compensate for the
backwards chord, and boundary painting is normalised to the brightest boundary in the
run so a bad draw of RandF(0,1) speeds can neither wash the map out nor dim it.
Colour scaling no longer multiplies alpha along with brightness.

PlanetHelper now keeps a CPU mirror of the whole model and checks every dispatch
against it (VerifyEdgeStressMirror), so parity is verified rather than asserted -- the
old comments claimed the two agreed while they were 180 degrees apart. Measured on GPU
over ~6.5k border vertices per run: worst mirror disagreement 0.00005% of the
brightest measurement, no type mismatches. Against a reference built from the exact
rotation field plus a smoothed local boundary normal, type agreement goes 23% -> 85%
and rms normal error 1.07 -> 0.13. SpreadStress note: ShearRate is now signed, so it
averages the same way NormalRate already did.

SpreadStress: superpose the field instead of claiming it once b46b7469e8

The flood was first-arrival-wins: a vertex that had been written once returned
early, so the second wave to reach it was discarded at the door. Measured on
the shipped field, 0 of 10 242 reached vertices carried influence from more
than one direction -- meeting waves produced a hard seam rather than mixing,
and plate interiors were a pure nearest-border field with no intermixing at all.

Replace the cached travelling value with a relaxed weighted sum:

    out[v] = source[v] + f * mean(in[neighbours])    // per sweep
    value  = out.U / out.D                          // rates = U, weight = D

source is the constant border injection; U accumulates seeded rate components
scaled by attenuation, D the matching weights. The fixed point is
(I - f*M)^-1 * s, so every source reaches every vertex with IDW weight f^d and
the contributions superpose. The border stops being special: it is just where
source != 0, which is what makes total coverage fall out rather than have to be
engineered.

Attenuation changes meaning along the way. Under claim-once it set the *reach*.
Now it sets the *blend width*, and coverage is independent of it -- the reason
raising 0.9 -> 0.96 visibly softened the falloff without ever fixing the seam.

Two bugs found while verifying this, both caught by measuring instead of assuming:

  * SpreadMinInfluence = 1e-12 looked like a divide-by-zero guard but was a
    reach limit. D fades about 0.72 per hop, crossing 1e-12 near 91 hops while
    float32 is sound to about 272. U and D fade together and the ratio keeps
    roughly float32's full relative precision all the way down, so the epsilon
    was silently discarding real, representable far-field answers. Lowered to
    1e-30, which sits just above the float32 normal floor and so can only ever
    catch a denominator that is genuinely zero.

  * One sweep carries influence exactly one hop, so the tolerance-derived sweep
    count was simultaneously capping the blend's geography. At 0.9 retention it
    asks for 88 sweeps while the mesh reaches 92 hops: 1 175 vertices received
    nothing at all, not filtered but never reached. Sweep count is now
    max(toleranceSweeps, requiredReach), with the reach measured by a BFS from
    the seed set and logged, so "how far does this go" is a stated quantity.

Verified on GPU (RTX 5080, forward_plus): full coverage with 0 unreached,
0 errors and 0 warnings, and the EdgeStress CPU mirror still agrees to 0.00004%
with 0 type mismatches. The blended map now shows genuine superposition -- where
compression meets tension the result is near-black cancellation, not a wall.

Type balance moves to roughly 64% shear. That is the expected consequence of
superposition rather than a defect: tension and compression are opposite signs
of the same channel and annihilate at range, leaving shear comparatively
prominent. Left as-is pending a look at how it reads in motion.

Add ADATONIC_AUTORUN back, with a quit on completion b969194697

Restores the headless harness flag: ADATONIC_AUTORUN=1 drives the whole
generation without a keypress. The new part is that it also shuts the process
down on reaching Completed, which is what actually made scripted runs painful --
without it the process sat at Completed with a live window and a warm GPU
context, so every run burned its entire timeout even though the answer had
printed several seconds earlier. A full generation now finishes in 9-17s
instead of being killed at an arbitrary ceiling.

Quit is deferred three frames rather than fired on the tick Completed is first
observed. The transition is reported from the stage timer's own callback, the
last UpdateMesh() that puts finished heights onto the visible mesh happens on
the ticks after it, and GPU work submitted this frame still wants to land before
its buffers are freed.

One caveat recorded in the code because it will otherwise get chased again: a
fully successful run still exits 139. Godot's shutdown destroys the display and
Vulkan contexts and the NVIDIA driver faults inside libGLX_nvidia /
libnvidia-glcore during that teardown. Ran it under gdb to confirm -- the
faulting stack contains no Godot frame and no project frame, so this is not
something our teardown ordering causes. "Autorun complete; quitting." is the
success marker; $? is not.

The alternative was measured rather than guessed. Running Cleanup() and then
System.Environment.Exit(0) does return 0, but skipping Godot's static teardown
puts 37 internal "BUG: Unreferenced static string" errors on stderr on every
run -- rb_ssao, luminance_buffers, _shaped_text_draw and friends, all engine
internals, none of them ours. Clean logs were preferred over a clean exit code.

If a trustworthy exit code is ever needed, try a persistent X server (Xvfb :99
&, then DISPLAY=:99 godot ...) instead of xvfb-run: xvfb-run tears X down around
the process as it exits, which is the likely trigger for the driver fault.

Tweaked colours - added amplitude knob 7ce8da63e6

speedup plate gen e3ce1cefd6

HeightCalculation implemented as compute shader. ca2b84b9b7

Ade9 commented

2026-09-17 19:16:19 +03:00

@beepster Implemented the rest of the stages as compute shaders - and changed some implementations for better ones both visually and logically.

Please review 🙂

@beepster Implemented the rest of the stages as compute shaders - and changed some implementations for better ones both visually and logically. Please review 🙂

beepster commented

2026-09-17 19:19:12 +03:00

@Ade9

Reviewed the compute-shader port. Overall this reads well — the batching story (chain N waves/sweeps per submit, poll a 16-byte progress counter instead of a full read-back) is the right shape, and the CPU fallback paths in PlateGeneration/SeedSpreadStress are honest about what they're papering over.

Things I verified as correct:

ComputeWorkgroupSize = 25 matches local_size_x = 25 in PlateExpansion.glsl, BorderSearch.glsl, EdgeDistance.glsl, EdgeStress.glsl, SpreadStress.glsl and HeightCalculation.glsl.
The shader bindings line up with every Ensure*Resources builder (PlateExpansion 1–3, BorderSearch 1–4, EdgeDistance 1–4, EdgeStress 1–6, SpreadStress 1–4, HeightCalculation 1–8).
SpreadStress's read-back parity is right: sweep 0 runs forward (A→B), so sweeps % 2 == 1 → AccumB, even → AccumA — and the batched loop keeps the global sweep index, so batch boundaries can't flip the parity.
Cleanup()/_ExitTree() and the MakeGo() pre-cleanup close the leaked-RenderingDevice-per-click hole, and the release helpers are idempotent.
ClaimOrphans() parking stranded vertices on the smallest plate keeps the unguarded Plates[PlateId] lookups downstream safe.

A few things worth a look:

Workgroup size vs. warp size. local_size_x = 25 doesn't divide 32, so on NVIDIA each workgroup straddles warp boundaries and ~7 lanes/warp sit idle in these mostly-uniform loops. Bumping to 64 or 128 (in the shaders and ComputeWorkgroupSize) is usually a free win on these full-sphere dispatches. Not a correctness issue.
Land/ocean semantics flipped in AssignOceanPlates. The old code treated set mask bits as ocean and targeted totalArea * _landRatio as the ocean area; the new FindBestLandMask treats set bits as land against targetLand = totalArea * _landRatio. With _landRatio = 0.4f that means the generator now produces ~40% land where it used to produce ~40% ocean. The name matches the new behaviour, so I assume it's intentional — just confirming, because it's an easy silent change to bake into screenshots/tuning.
Unreached vertices go slightly negative after normalisation. In FinishEdgeDistanceCalculation, vertices the flood never reached keep EdgeDistance = -1, but when some vertex reaches a distance > 1 the divide by maxDistance turns that marker into -1/max — a small negative that HeightCalculation then feeds into the coastline term with the landform sign applied. Worth parking unreached vertices explicitly (e.g. skip them in the divide, or clamp the term shader-side) so "no measurement" never becomes a tiny-but-real signal.
Missing read-back length check in PlateGeneration. Buffer.BlockCopy(outputBytes, 0, output, 0, seedBytes.Length) will throw on a short buffer. Every other stage guards this (if (flagBytes.Length < expectedBytes) ... in BorderSearch, same in EdgeDistance/EdgeStress/SpreadStress/HeightCalculation) — worth matching here for consistency.
Minor: leaked shader Rids on a mid-bootstrap compile failure. In LoadComputeShader, the failure path disposes _rd and throws, but shaders already compiled in earlier LoadComputeShader calls from the constructor are never freed. Harmless in practice (the process is failing anyway), just noting it.

None of these block the PR — 2 and 3 are the only ones I'd want a conscious answer on. Nice work on the meet-in-the-middle land split; the truncation-preserving comment on weight[i] is exactly the right thing to call out, since that's what keeps the chosen mask identical to the old enumeration.

@Ade9 Reviewed the compute-shader port. Overall this reads well — the batching story (chain N waves/sweeps per submit, poll a 16-byte progress counter instead of a full read-back) is the right shape, and the CPU fallback paths in `PlateGeneration`/`SeedSpreadStress` are honest about what they're papering over. **Things I verified as correct:** - `ComputeWorkgroupSize = 25` matches `local_size_x = 25` in `PlateExpansion.glsl`, `BorderSearch.glsl`, `EdgeDistance.glsl`, `EdgeStress.glsl`, `SpreadStress.glsl` and `HeightCalculation.glsl`. - The shader bindings line up with every `Ensure*Resources` builder (PlateExpansion 1–3, BorderSearch 1–4, EdgeDistance 1–4, EdgeStress 1–6, SpreadStress 1–4, HeightCalculation 1–8). - `SpreadStress`'s read-back parity is right: sweep 0 runs forward (A→B), so `sweeps % 2 == 1 → AccumB`, even → `AccumA` — and the batched loop keeps the global sweep index, so batch boundaries can't flip the parity. - `Cleanup()`/`_ExitTree()` and the `MakeGo()` pre-cleanup close the leaked-`RenderingDevice`-per-click hole, and the release helpers are idempotent. - `ClaimOrphans()` parking stranded vertices on the smallest plate keeps the unguarded `Plates[PlateId]` lookups downstream safe. **A few things worth a look:** 1. **Workgroup size vs. warp size.** `local_size_x = 25` doesn't divide 32, so on NVIDIA each workgroup straddles warp boundaries and ~7 lanes/warp sit idle in these mostly-uniform loops. Bumping to 64 or 128 (in the shaders *and* `ComputeWorkgroupSize`) is usually a free win on these full-sphere dispatches. Not a correctness issue. 2. **Land/ocean semantics flipped in `AssignOceanPlates`.** The old code treated set mask bits as **ocean** and targeted `totalArea * _landRatio` as the *ocean* area; the new `FindBestLandMask` treats set bits as **land** against `targetLand = totalArea * _landRatio`. With `_landRatio = 0.4f` that means the generator now produces ~40% land where it used to produce ~40% ocean. The name matches the new behaviour, so I assume it's intentional — just confirming, because it's an easy silent change to bake into screenshots/tuning. 3. **Unreached vertices go slightly negative after normalisation.** In `FinishEdgeDistanceCalculation`, vertices the flood never reached keep `EdgeDistance = -1`, but when *some* vertex reaches a distance > 1 the divide by `maxDistance` turns that marker into `-1/max` — a small negative that `HeightCalculation` then feeds into the coastline term with the landform sign applied. Worth parking unreached vertices explicitly (e.g. skip them in the divide, or clamp the term shader-side) so "no measurement" never becomes a tiny-but-real signal. 4. **Missing read-back length check in `PlateGeneration`.** `Buffer.BlockCopy(outputBytes, 0, output, 0, seedBytes.Length)` will throw on a short buffer. Every other stage guards this (`if (flagBytes.Length < expectedBytes) ...` in `BorderSearch`, same in `EdgeDistance`/`EdgeStress`/`SpreadStress`/`HeightCalculation`) — worth matching here for consistency. 5. **Minor: leaked shader Rids on a mid-bootstrap compile failure.** In `LoadComputeShader`, the failure path disposes `_rd` and throws, but shaders already compiled in earlier `LoadComputeShader` calls from the constructor are never freed. Harmless in practice (the process is failing anyway), just noting it. None of these block the PR — 2 and 3 are the only ones I'd want a conscious answer on. Nice work on the meet-in-the-middle land split; the truncation-preserving comment on `weight[i]` is exactly the right thing to call out, since that's what keeps the chosen mask identical to the old enumeration.

This pull request can be merged automatically.

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin f-compute-shaders:f-compute-shaders

git switch f-compute-shaders

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch main

git merge --no-ff f-compute-shaders

git switch f-compute-shaders

git rebase main

git switch main

git merge --ff-only f-compute-shaders

git switch f-compute-shaders

git rebase main

git switch main

git merge --no-ff f-compute-shaders

git switch main

git merge --squash f-compute-shaders