unify(shroud): Merge W3DShroud code from Zero Hour and implicitly fix black terrain for one frame on far camera jumps

[Mr-Sheerlock]

• Closes Far camera jumps via W3DView::lookAt will render terrain black for 1 frame in Generals #2689

The shroud destination texture in Generals was allocated at visible terrain size
rather than full map size. The shroud render() function uses getDrawOrgX/Y to
determine which region of the shroud to upload to the GPU each frame. Because
the shroud renders before updateViews() in W3DDisplay::draw() ,the draw origin
is still stale from the previous frame when the camera jumps far, such as when
clicking the radar and scrolling. This causes the shroud to upload the wrong region, making
newly visible terrain appear black for one frame.

Unifies the Generals W3DShroud destination texture allocation with Zero Hour,
which allocates at full map size and renders the full shroud every frame. This
makes the shroud correct regardless of the draw origin state at render time.

The tradeoff is an increase in GPU upload bandwidth per frame, but it matches the W3DShroud
implementation in Zero Hour.

[greptile-apps]

Greptile Summary

This PR fixes a one-frame black terrain flicker in Generals that occurred on long camera jumps (e.g. clicking the radar). The shroud destination texture was previously allocated at visible-terrain size, and render() used the stale draw origin from the previous frame to determine which shroud region to upload — causing incorrect data to be sent to the GPU before updateViews() ran. The fix unifies Generals with the Zero Hour approach: allocate the texture at full map size and upload the entire shroud every frame, making it draw-origin-agnostic.

• init(): Destination texture is now sized to m_numCellsX × m_numCellsY (full map) instead of the visible-terrain footprint, matching Zero Hour's allocation strategy.
• render(): visStartX/Y are forced to 0 and visEndX/Y to m_numCellsX/Y, so the full shroud is always uploaded regardless of the draw origin state; getBorderSize() calls are replaced with the inline variant throughout.

Confidence Score: 5/5

Safe to merge — the fix is a straightforward allocation and upload-region change that matches the proven Zero Hour implementation, with no new logic paths introduced.

The change correctly addresses the root cause by making the shroud upload draw-origin-agnostic. The tradeoff (higher GPU bandwidth per frame) is explicitly documented in the PR description and aligns with the Zero Hour strategy. No correctness issues were found; remaining notes are cleanup opportunities around dead intermediate calculations and stale member assignments.

No files require special attention.

Important Files Changed

Filename	Overview
Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DShroud.cpp	Fixes shroud black-frame bug by allocating the destination texture at full map size (matching Zero Hour) and rendering the entire shroud each frame; logic is correct, with some cleanup opportunities around now-dead intermediate calculations and stale member-variable assignments.

Sequence Diagram

sequenceDiagram
    participant Display as W3DDisplay::draw()
    participant Shroud as W3DShroud::render()
    participant GPU as GPU Upload

    note over Display,GPU: Before fix — stale draw origin causes wrong region
    Display->>Shroud: render() [draw origin still from prev frame]
    Shroud->>Shroud: "visStartX/Y = f(getDrawOrgX/Y) [stale!]"
    Shroud->>GPU: Upload wrong shroud region → black terrain for 1 frame
    Display->>Display: updateViews() [draw origin updated here]

    note over Display,GPU: After fix — full shroud uploaded every frame
    Display->>Shroud: render()
    Shroud->>Shroud: "visStartX=0, visEndX=m_numCellsX (full map)"
    Shroud->>GPU: Upload entire shroud — correct regardless of draw origin
    Display->>Display: updateViews()

Loading

Prompt To Fix All With AI

Fix the following 4 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 4
Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DShroud.cpp:628-630
These two inline timestamps carry a pre-2026 date. Per the project's convention, newly introduced comments should reference the current year (2026) rather than preserving the old EA timestamp.

```suggestion
	// Do it all [3/11/2026]
	visStartX = 0;
	visStartY = 0;
```

### Issue 2 of 4
Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DShroud.cpp:635-637
Same pre-2026 timestamp on the second "Do it all" override comment; both instances are new to this file and should use 2026.

```suggestion
	// Do it all [3/11/2026]
	visEndX = m_numCellsX;
	visEndY = m_numCellsY;
```

### Issue 3 of 4
Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DShroud.cpp:639-653
**Unreachable clamp blocks after full-map override**

After the "Do it all" lines unconditionally set `visEndX = m_numCellsX` and `visEndY = m_numCellsY`, both conditions `visEndX > m_numCellsX` and `visEndY > m_numCellsY` are always false — making the two clamp blocks dead code. Similarly, the `getDrawOrgX/Y`-based start calculations above them (and their `< 0` guards) are computed but immediately discarded. Consider removing these stale branches and the pre-override calculations to reduce noise for future maintainers.

### Issue 4 of 4
Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DShroud.cpp:128-129
**Stale `m_numMaxVisibleCellsX`/`m_numMaxVisibleCellsY` assignments**

Lines 128-129 set `dstTextureWidth` and `dstTextureHeight` to the visible-size formula, but both are immediately overridden two lines later by `m_numCellsX`/`m_numCellsY`. The only lasting effect is that `m_numMaxVisibleCellsX` and `m_numMaxVisibleCellsY` receive the old visible-size values. Those members are never read anywhere in the Generals codebase, so these assignments are now misleading dead stores. Consider setting `m_numMaxVisibleCellsX = m_numCellsX` and `m_numMaxVisibleCellsY = m_numCellsY` here to keep the member semantics consistent with the new full-map approach, or removing the members entirely.

_{Reviews (2): Last reviewed commit: "Add Comments and blank line for merge pu..." | Re-trigger Greptile}

[Mr-Sheerlock]

A better solution might involve setting the camera transform only if the distance to target position exceeds a certain threshold. This way we can avoid calculating the camera transform multiple times for every frame and maintain the smaller shroud sent to the GPU.

[xezon]

Just eyeballing, it does not look like it is a perfect merge. Comments and blank line not merged.

[Mr-Sheerlock]

Just eyeballing, it does not look like it is a perfect merge. Comments and blank line not merged.

@xezon The file is as close as possible to the ZH version save for the header and 2 comments in the ZH version. This is why the blanks are left intentionally. Does this make it unfit for merge?

[xezon]

We generally merge everything, including comments and empty lines, except for the game title in the first line of the file. Reason being, it reduces merge conflicts and gives a almost perfect diff.

[Mr-Sheerlock]

A better solution might involve setting the camera transform only if the distance to target position exceeds a certain threshold....

I think this probably goes against the strive to decouple the CPU logic and drawing processes.
A better solution I tried to implement was to separate W3DShroud::render() into two separate methods:

1- W3DShroud::updateVisibility() : Calculates currently visible shroud bounds based on logic state data. This replaces the current render function call.
2- W3DShroud::updateShroudTexture(): Updates video memory surface with currently visible shroud data.

and then, call updateShroudTexture inside W3DDisplay as follows:

do {
    if (DX8Wrapper...)
    {
        // 1. Calculations settle matrices and coordinates here
        updateViews();

        // 2. Refresh the shroud bounds on GPU before drawing calls begin
        if (TheTerrainRenderObject && TheTerrainRenderObject->getShroud())
        {
            TheTerrainRenderObject->getShroud()->updateShroudTexture();
        }
       // rest of the drawing
    }

In my trials, this fixed the Generals bug.
This would require replication to zero hour later on.

[Mr-Sheerlock]

We generally merge everything, including comments and empty lines, except for the game title in the first line of the file. Reason being, it reduces merge conflicts and gives a almost perfect diff.

@xezon Thank you for the review. I made the changes and now the only difference with ZH's W3DShroud is the header. Awaiting further reviews.

[xezon]

Looking good