perf(radar): Reduce cost of W3DRadar::renderObjectList (by 80%), W3DRadar::buildTerrainTexture (by 25%), W3DRadar::clearShroud by locking radar surface only once

[xezon]

• Relates to jmarshall2323@9b1c399
• Follow up for perf(radar): Reduce cost of radar pixel draw in PartitionManager::refreshShroudForLocalPlayer by 96% #2072

This change reduces the cost of

• W3DRadar::renderObjectList by around 80%, from 0.5 ms to 0.1 ms (tested with many radar objects)
• W3DRadar::buildTerrainTexture by around 25%, from 0.0127 ms to 0.0164 ms
• W3DRadar::clearShroud - not measured because not important

This is achieved by locking the radar surface only once instead of many times in loops.

W3DRadar::renderObjectList is called every 6 logic frames, and the jump from 0.5 ms to 0.1 ms is nice to improve overall performance (measured on modern machine).

Functions SurfaceClass::DrawPixel and SurfaceClass::DrawHLine were renamed to better fit the WWVegas style.

The NULL's in W3DRadar were changed to nullptr, which was added by #2072.

[greptile-apps]

Greptile Summary

This PR optimizes radar rendering performance by eliminating redundant surface lock/unlock operations. Previously, SurfaceClass::DrawPixel and SurfaceClass::DrawHLine locked and unlocked the D3D surface for every single pixel or line drawn. This PR refactors these methods to accept pre-locked surface parameters (pBits, pitch, bytesPerPixel), allowing callers to lock the surface once and perform many pixel operations before unlocking.

Performance improvements achieved:

• W3DRadar::renderObjectList: ~80% faster (0.5ms → 0.1ms with many radar objects)
• W3DRadar::buildTerrainTexture: ~25% faster (0.0164ms → 0.0127ms)
• W3DRadar::clearShroud: Improved (not measured)

Key changes:

• SurfaceClass::DrawPixel → SurfaceClass::Draw_Pixel with new signature
• SurfaceClass::DrawHLine → SurfaceClass::Draw_H_Line with new signature
• SurfaceClass::Get_Pixel updated to accept pre-locked parameters
• Added SurfaceClass::Get_Bytes_Per_Pixel() helper method
• Updated all callers in W3DRadar.cpp, W3DWater.cpp, and commented code in MapUtil.cpp
• Renamed methods follow WWVegas naming convention with underscores
• Changed NULL to nullptr for C++ best practices

The optimization is architecturally sound - batch locking is a standard D3D best practice. The implementation correctly passes surface data through the call chain and properly pairs Lock/Unlock calls.

Confidence Score: 5/5

• This PR is safe to merge with minimal risk - it's a well-executed performance optimization following D3D best practices
• This is a textbook performance optimization with no logical risks. The change reduces expensive D3D Lock/Unlock operations from per-pixel to per-batch, following standard graphics API best practices. All Lock/Unlock calls are properly paired, memory access patterns are correct (verified pixel offset calculations), and the refactoring is comprehensive across all call sites. The 80% performance improvement in renderObjectList validates the approach. The only code change beyond the optimization is renaming methods to match WWVegas conventions and updating NULL to nullptr, both of which are safe improvements.
• No files require special attention

Important Files Changed

Filename	Overview
Core/Libraries/Source/WWVegas/WW3D2/surfaceclass.h	Added Get_Bytes_Per_Pixel() method and updated Draw_Pixel, Draw_H_Line, and Get_Pixel signatures to accept pre-locked surface parameters for performance optimization
Core/Libraries/Source/WWVegas/WW3D2/surfaceclass.cpp	Refactored Draw_Pixel, Draw_H_Line, and Get_Pixel to work with pre-locked surfaces, replaced individual Lock/Unlock calls with direct memory access using memcpy, added Get_Bytes_Per_Pixel() implementation
Core/GameEngineDevice/Source/W3DDevice/Common/System/W3DRadar.cpp	Core optimization - locks radar surfaces once per function instead of per pixel in renderObjectList, buildTerrainTexture, clearShroud, setShroudLevel, and refreshObjects, achieving 80% and 25% performance improvements

Sequence Diagram

sequenceDiagram
    participant Caller as W3DRadar::renderObjectList
    participant Surface as SurfaceClass
    participant D3D as Direct3D Surface

    Note over Caller,D3D: Before Optimization (Old Approach)
    rect rgb(255, 200, 200)
        loop For each radar object (N objects)
            loop Draw 4 pixels per object
                Caller->>Surface: DrawPixel(x, y, color)
                Surface->>D3D: LockRect()
                D3D-->>Surface: locked bits
                Surface->>D3D: Write pixel
                Surface->>D3D: UnlockRect()
            end
        end
        Note over Caller,D3D: Total: 4N Lock/Unlock operations
    end

    Note over Caller,D3D: After Optimization (New Approach)
    rect rgb(200, 255, 200)
        Caller->>Surface: Lock(&pitch)
        Surface->>D3D: LockRect()
        D3D-->>Surface: pBits, pitch
        Surface-->>Caller: pBits, pitch
        Caller->>Surface: Get_Bytes_Per_Pixel()
        Surface-->>Caller: bytesPerPixel
        loop For each radar object (N objects)
            loop Draw 4 pixels per object
                Caller->>Surface: Draw_Pixel(x, y, color, bytesPerPixel, pBits, pitch)
                Note right of Surface: Direct memory write<br/>using memcpy<br/>(no Lock/Unlock)
            end
        end
        Caller->>Surface: Unlock()
        Surface->>D3D: UnlockRect()
        Note over Caller,D3D: Total: 1 Lock/Unlock operation
    end

Loading

[greptile-apps]

_{5 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[xezon]

@greptileai

[bobtista]

Are we planning on using this commented code? Keeping it for historical reference? Something else?

[xezon]

I do not know. I just updated it for courtesy.

[bobtista]

It's unused, so should be fine, but should we still clamp x and y?

[xezon]

No, because this function does not know size of pBits. It assumes caller makes no mistake. Also it would be bad for performance.

[bobtista]

Is it ok to call file.readInt() again here? (I know it's commented out, but in theory do we want to call it once and use the value twice?)

[bobtista]

Also is [y + x] right? should it be buffer[y * size.x + x] ?

[bobtista]

Actually do we need the buffer at all? It gets filled but then setRawTextureData(buffer) is called without freeing it, which could leak, but it's not clear to me that we need it in the first place.
Int color = file.readInt();
surface->Draw_Pixel(x, y, color, ...);

[xezon]

This code is commented, so do not care. Just touched for courtesy.

[bobtista]

Code LGTM, good find!

I left some nits about commented out code, but looks good to go

[bobtista]

LGTM