Skip to content

GPU terrain: integrate hybrid meshing and mod compatibility - #4

Merged
Vorith03 merged 86 commits into
forge-1.20.1from
gpu-terrain-continuation-20260917
Sep 18, 2026
Merged

Vorith03 merged 86 commits into
forge-1.20.1from
gpu-terrain-continuation-20260917

Conversation

@Vorith03

@Vorith03 Vorith03 commented Sep 17, 2026 •

Copy link
Copy Markdown
Owner

Primary Forge 1.20.1 development/integration branch for the current Phase 7 GPU-terrain work.

This branch now consolidates the previously separate GPU-terrain, compatibility, and still-relevant validation work.

Production terrain changes include:

  • non-blocking GPU terrain dispatch/completion with exact-generation publication and fail-closed CPU recovery;
  • submitted-output allocation pinning through frame completion;
  • authoritative Block.shouldRenderFace qualification for CPU omission;
  • fresh GPU-first REPLACE and conservative hybrid APPEND ownership;
  • atomic APPEND rebuild staging and complete dirty-transition retention;
  • bounded descriptor/output handling and persistent draw handoff.

Compatibility changes now integrated here include:

  • preserved MainTarget and LevelRenderer vanilla mixin call sites needed by Immersive Portals;
  • IP framebuffer compatibility mode and conservative raw-GL fallbacks;
  • IP shader-source transformation, clipping uniform/pipeline validation, reload lifecycle, and helper shader loading;
  • std140-correct mat3 handling for converted legacy shaders;
  • existing FTB Library/Pick Up Notifier compatibility coverage.

Validation consolidation:

  • current async completion and dirty-transition coverage supersedes the old isolated validation branches;
  • the real Minecraft/Forge face-policy oracle from the validation workstream is now run by the main smoke path.

All accelerated terrain behavior remains experimental/default-off and must retain CPU fallback for unsupported Forge content, fluids, block entities, translucent/tripwire paths, stale generations, overflow, and failed GPU work.

Do not merge into forge-1.20.1 until the current combined CI suite is green and the repository gates have been rechecked against live state.

@Vorith03 Vorith03 changed the title GPU terrain: defer input recovery outside area lock GPU terrain: harden recovery and dispatch after input submission Sep 17, 2026

Copy link
Copy Markdown
Owner Author

Validation finding — host readback synchronization gap (blocking correctness issue before RX/RADV runtime testing):

GpuTerrainSectionMesher.submitDispatch() copies the GPU result into a HOST_VISIBLE|HOST_COHERENT readback buffer, then both the synchronous path (vkWaitForFences) and the new nonblocking path (vkGetFenceStatus) map/read it. The command buffer currently has compute-write -> transfer-read synchronization, but after vkCmdCopyBuffer(... -> readbackBuffer) there is no transfer-write -> host-read memory dependency.

The Vulkan spec explicitly says waiting for / polling a signaled fence is an execution dependency only and does not by itself guarantee device memory accesses are visible to the host. HOST_COHERENT removes the need for vkInvalidateMappedMemoryRanges, but it does not remove the need to make the device writes available/visible to HOST_READ.

Suggested narrow fix in the same helper command buffer, after all copies into readbackBuffer and before submission/fence signal:

VkBufferMemoryBarrier.Buffer barrier = VkBufferMemoryBarrier.calloc(1, stack);
barrier.get(0).sType$Default()
        .srcAccessMask(VK_ACCESS_TRANSFER_WRITE_BIT)
        .dstAccessMask(VK_ACCESS_HOST_READ_BIT)
        .srcQueueFamilyIndex(VK_QUEUE_FAMILY_IGNORED)
        .dstQueueFamilyIndex(VK_QUEUE_FAMILY_IGNORED)
        .buffer(readbackBuffer).offset(0L).size(totalReadbackBytes);

vkCmdPipelineBarrier(commandBuffer.getHandle(),
        VK_PIPELINE_STAGE_TRANSFER_BIT,
        VK_PIPELINE_STAGE_HOST_BIT,
        0, null, barrier, null);

For validation/capture mode, cover the full readback range containing both the result/descriptors and copied vertex payload. This should fix both the synchronous oracle and dispatchAsync()/pollCompletions() paths without adding any wait.

Spec references:

I have not changed PR #4 from the validation lane; production thread should own the fix.

@Vorith03 Vorith03 changed the title GPU terrain: harden recovery and dispatch after input submission GPU terrain: harden async dispatch and completion lifecycle Sep 17, 2026

Copy link
Copy Markdown
Owner Author

Additional corroboration: VulkanMod already uses the exact required dependency in RenderTargetManager.copyColorToBuffer() for screenshot readback: VK_ACCESS_TRANSFER_WRITE_BIT -> VK_ACCESS_HOST_READ_BIT with VK_PIPELINE_STAGE_TRANSFER_BIT -> VK_PIPELINE_STAGE_HOST_BIT before the HOST_COHERENT buffer is mapped. The section mesher should mirror that established pattern after its final readback copies.

Copy link
Copy Markdown
Owner Author

Validation status at current head 010902fc: the newer delta only changes GpuTerrainDrawHandoff; GpuTerrainSectionMesher.submitDispatch() still ends immediately after the readback copies with no TRANSFER_WRITE -> HOST_READ memory dependency. The post-submit ordering and async exactly-once lifecycle coverage are otherwise green, but I’m still treating this readback visibility gap as the remaining terrain synchronization blocker before RX/RADV validation. The durable checkpoint should not describe this slice as hazard-free until that dependency lands.

Copy link
Copy Markdown
Owner Author

Validation escalation: this is not isolated to the draft PR. The current forge-1.20.1 head/checkpoint also lacks the required device→host dependency in GpuTerrainSectionMesher.submitDispatch(): after the final vkCmdCopyBuffer(... -> readbackBuffer) operations, the helper command buffer is submitted without a TRANSFER_WRITE -> HOST_READ barrier. The current AGENT_STATUS.md therefore overstates readiness where it says the first RX 6900 XT functional test is warranted.

I rechecked the current Vulkan spec: fence wait / successful vkGetFenceStatus is only an execution dependency; it explicitly does not guarantee device memory accesses are visible to the host. HOST_COHERENT removes the separate invalidate requirement only after writes have been made available to the host domain. A destination access mask containing VK_ACCESS_HOST_READ_BIT performs that device→host domain operation.

Gate consequence: do not request the RX/RADV functional test yet. First land the narrow readback barrier after all copies, covering the full readback range in validation mode; then rerun the terrain lifecycle/Vulkan startup smokes. CI xCollateral#606 otherwise remains healthy through the terrain-related gates and fails only at the separately owned Immersive Portals smoke.

Copy link
Copy Markdown
Owner Author

Validation finding on the new hybrid planner (3a984a33): visibleCpuBlockModel is not a sufficient neighbor-safety fact.

ChunkTask ultimately calls renderBatched(..., checkSides=true). Minecraft's model tessellation therefore uses Block.shouldRenderFace(currentState, level, pos, direction, neighborPos), whose result can depend on currentState.skipRendering(neighborState, direction) and the neighbor's occlusion shape/can-occlude semantics. That is independent of whether the neighbor itself has visible block-model geometry.

Current GpuTerrainHybridMask.unsafeNeighbor() treats an unqualified neighbor as safe whenever visibleCpuBlockModel[index] == false (unless fluid/BE), and the new unit test explicitly asserts "Invisible non-fluid CPU states must not poison adjacent qualified cubes". That assertion is too strong for Forge/modded semantics: an invisible CPU-owned state can still change whether an adjacent qualified cube's face should render, while section_mesher_probe.comp only predicts the face from SOLID_RENDER.

Suggested bounded fix, without redesigning the shader/ABI: during worker-side hybrid planning, for each candidate face compare the GPU predicate (!neighborSolidRender) with the authoritative CPU Block.shouldRenderFace(...) result for that exact state/neighbor/direction. Keep the candidate GPU-owned only when all six decisions match; otherwise demote it to CPU. This preserves arbitrary Forge face semantics while still avoiding full renderBatched tessellation for the accepted subset. The test should cover a deliberate mismatch instead of assuming every invisible non-fluid neighbor is safe.

This is separate from the already-reported TRANSFER_WRITE -> HOST_READ readback barrier blocker; RX/RADV testing should remain gated on that synchronization fix as well.

Copy link
Copy Markdown
Owner Author

Additional hybrid integration constraint (fail-closed, not a corruption bug): Plan.filteredSnapshot() cannot simply replace the current production snapshot at dispatch.

It clears GPU_FULL_CUBE for CPU-owned cells while preserving their real state IDs/flags. But GpuTerrainSectionMesherBridge.qualify() currently requires every non-GPU_FULL_CUBE state to have RenderShape.INVISIBLE; a boundary-demoted or exception-adjacent visible full cube therefore makes qualify(filteredSnapshot) return null. The current staged preflight/face-count match was also computed under the all-or-nothing qualification contract.

When this planner is wired into production, pair the filtered payload with a hybrid-aware qualification/staged-preflight result that counts only GPU-owned faces and explicitly permits CPU-owned visible states. Do not reuse the existing all-or-nothing qualify() contract unchanged. A focused test should also assert that filteredSnapshot() preserves state IDs, SOLID_RENDER/other semantic flags and halo bits while clearing only GPU_FULL_CUBE on CPU-owned cells.

This one safely fails closed today because the hybrid planner is not yet production-wired; it is an integration guardrail for the next slice.

Copy link
Copy Markdown
Owner Author

Hybrid activation guardrail on the new APPEND draw-plan work (43c81b0/9a2e33e/06c96249): today GpuTerrainDrawHandoff.plan(APPEND) deliberately returns the supplied CPU command alone when GPU residency is missing/stale, and RegionBatchLayoutTest calls that a fail-closed CPU fallback. That is valid only while the supplied CPU command is still a complete section fallback.

Once hybrid CPU tessellation actually omits GPU-owned cubes and the CPU command contains exceptions only, the same missing/stale-residency path would draw only those exceptions and leave the GPU-owned portion absent until recovery. In other words, APPEND cannot simultaneously mean CPU exceptions only and rely on cpuOnly(cpu) as a complete fallback.

Please treat this as an activation gate, not a defect in the currently unwired groundwork: before production enables APPEND with CPU omission, make the transition atomic or preserve a complete previous CPU fallback until exact GPU residency publishes. Tests should distinguish complete CPU fallback from hybrid exception command rather than using the latter as the stale/missing-residency oracle. Fresh GPU-first hybrid sections may intentionally have no complete prior fallback, but that visual-gap/recovery behavior should be explicit and bounded rather than described as fail-closed CPU-only rendering.

This is independent of the still-open transfer->host readback barrier and per-face Block.shouldRenderFace equivalence findings.

@Vorith03 Vorith03 changed the title GPU terrain: harden async dispatch and completion lifecycle GPU terrain: integrate hybrid meshing and mod compatibility Sep 18, 2026
@Vorith03
Vorith03 merged commit ac2a9a2 into forge-1.20.1 Sep 18, 2026
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant