bugfix(water): Fix river borders appearing darkened under shroud

[afc-afc0]

Fixes #2364

Description River borders appear darkened when under shroud, creating visual artifacts that force map makers to place rocks and other objects to hide them.

Root Cause

River water rendering in drawRiverWater() applies the shroud as a separate multiplicative pass on the framebuffer after the
water is already alpha-blended onto the terrain. This causes double-darkening at transparent river edges:

1. Terrain renders with its own shroud: pixel = terrain × shroud
2. River alpha-blends on top: pixel = α × water + (1-α) × terrain × shroud
3. River's multiplicative shroud pass darkens the entire framebuffer: pixel = shroud × (α × water + (1-α) × terrain × shroud)

The terrain contribution at river borders becomes terrain × shroud² instead of terrain × shroud — visibly darker than
surrounding terrain. This is only noticeable under shroud (when shroud < 1.0), since 1.0² = 1.0.

Note: Trapezoid water (lakes/ponds) avoids this by applying shroud in texture stage 3 of the main pass when pixel shaders are
available. River water cannot do this because stage 3 is already used for the river alpha edge texture.

Fix

Applies shroud per-vertex by looking up the shroud level at each river vertex's world position and multiplying it into the vertex
diffuse color before alpha blending. This ensures only the water's color contribution is shroud-modulated:

pixel = α × (water × shroud) + (1-α) × (terrain × shroud) = shroud × (α × water + (1-α) × terrain)

The separate multiplicative shroud pass is removed, which also eliminates one draw call per river.

Testing

Tested with the minimal reproduction map (Ariver.zip from #2364) — river borders no longer show dark outlines under shroud.

[greptile-apps]

Greptile Summary

This PR fixes the river-border double-darkening bug by replacing the post-draw multiplicative shroud pass with per-vertex shroud modulation: each river vertex's world position is converted to a shroud cell index and the resulting scale factor is multiplied into the vertex diffuse color before the draw call. This also removes one draw call per river. The approach is mathematically correct and consistent with how the rest of the engine handles shroud on vertex-colored geometry.

• Missing lower-bounds guard: getShroudLevel(x, y) only checks x < m_numCellsX && y < m_numCellsY — it does not guard x >= 0 && y >= 0. A negative cellX/cellY (possible if a river vertex ever lands at a near-zero or negative world coordinate) would satisfy the upper-bound check and read memory before the shroud buffer. Adding cellX >= 0 && cellY >= 0 before the call in both vertex blocks would close this gap.
• Code duplication: The inner-vertex and outer-vertex shroud blocks are identical; a shared inline lambda would reduce the maintenance surface.

Confidence Score: 5/5

Safe to merge; both remaining findings are P2 style/safety suggestions that do not affect correctness in any realistic scenario.

The fix is mathematically correct and well-described. The only open issues are a defensive lower-bounds guard on a cell-index lookup (negative world coordinates are not possible for river vertices in practice) and a code-duplication note. Neither blocks merge.

Core/GameEngineDevice/Source/W3DDevice/GameClient/Water/W3DWater.cpp — the two shroud cell-index blocks should ideally add cellX >= 0 && cellY >= 0 guards.

Important Files Changed

Filename	Overview
Core/GameEngineDevice/Source/W3DDevice/GameClient/Water/W3DWater.cpp	Removes the separate multiplicative shroud render pass for river water and replaces it with per-vertex shroud modulation; logic is sound but the shroud cell-index calculation lacks a lower-bounds guard against potential negative values.

Sequence Diagram

sequenceDiagram
    participant R as drawRiverWater()
    participant VB as Vertex Buffer
    participant S as W3DShroud
    participant DX as DX8Wrapper

    note over R: Before fix
    R->>VB: Fill vertices (diffuse = unshrouded color)
    R->>DX: Draw_Triangles() first pass, alpha-blend water
    R->>S: getShroudTexture()
    R->>DX: Draw_Triangles() second pass, multiplicative shroud over entire framebuffer
    note over DX: terrain x shroud^2 at border edges

    note over R: After fix (this PR)
    R->>S: getShroud()
    loop per vertex (inner + outer)
        R->>S: getShroudLevel(cellX, cellY)
        S-->>R: level 0-255
        R->>VB: diffuse = shade x (level/255) | alpha
    end
    R->>DX: Draw_Triangles() single pass, shroud already in vertex color
    note over DX: terrain x shroud^1 at border edges (correct)

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: Core/GameEngineDevice/Source/W3DDevice/GameClient/Water/W3DWater.cpp
Line: 2842-2850

Comment:
**Missing lower-bounds guard before `getShroudLevel`**

`W3DShroud::getShroudLevel` checks `x < m_numCellsX && y < m_numCellsY` but has no `x >= 0 && y >= 0` guard; a negative index would satisfy the upper-bounds check and produce a buffer underread (`m_srcTextureData + x*2` where `x*2` is negative). River vertices always have non-negative world coordinates in normal gameplay, but an explicit guard here would be more defensive:

```suggestion
			if (shroud) {
				Int cellX = REAL_TO_INT_FLOOR(x / shroud->getCellWidth());
				Int cellY = REAL_TO_INT_FLOOR(y / shroud->getCellHeight());
				if (cellX >= 0 && cellY >= 0) {
					W3DShroudLevel level = shroud->getShroudLevel(cellX, cellY);
					Real shroudScale = (Real)level / 255.0f;
					vb->diffuse = REAL_TO_INT(shadeB * shroudScale) | (REAL_TO_INT(shadeG * shroudScale) << 8) | (REAL_TO_INT(shadeR * shroudScale) << 16) | (diffuse & 0xff000000);
				} else {
					vb->diffuse = diffuse;
				}
			} else {
				vb->diffuse = diffuse;
			}
```

The same guard should be applied to the identical outer-vertex block below.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: Core/GameEngineDevice/Source/W3DDevice/GameClient/Water/W3DWater.cpp
Line: 2858-2868

Comment:
**Duplicated shroud-application block**

The inner-vertex and outer-vertex shroud blocks are byte-for-byte identical. Extracting a small inline lambda or helper (or a single local function capturing `shroud`, `shadeR/G/B`, `diffuse`) would eliminate the duplication and make a future change to the shroud logic a single-site update. The outer-vertex block also needs the same lower-bounds guard noted on the inner-vertex block above.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "bugfix(water): Fix river borders appeari..." | Re-trigger Greptile}

[stephanmeesters]

Did a little pre- post comparison, the results are interesting, this is using the map Winding River:

2026-04-13.00-23-21.mp4

You can try by adding this commit:
stephanmeesters@c63851d