-
Notifications
You must be signed in to change notification settings - Fork 151
fix(radar): Fix Radar pixel color format for non A8R8B8G8 surfaces #2170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix(radar): Fix Radar pixel color format for non A8R8B8G8 surfaces #2170
Conversation
Greptile Overview
|
| Filename | Overview |
|---|---|
| Core/Libraries/Source/WWVegas/WW3D2/ww3dformat.cpp | Implemented ARGB_Color_To_WW3D_Color function with format conversion logic for 13 different pixel formats including RGB, luminance, and fixed-point color formats |
| Core/GameEngineDevice/Source/W3DDevice/Common/System/W3DRadar.cpp | Converted ARGB colors to surface-specific formats before pixel writes in radar rendering, terrain building, and shroud updating functions |
Sequence Diagram
sequenceDiagram
participant Radar as W3DRadar
participant Surface as SurfaceClass
participant Convert as ARGB_Color_To_WW3D_Color
participant Draw as Draw_Pixel
Note over Radar: Rendering radar objects/terrain/shroud
Radar->>Surface: Get_Description(surfaceDesc)
Surface-->>Radar: Returns surface format (e.g. R5G6B5)
Radar->>Surface: Lock(&pitch)
Surface-->>Radar: Returns pixel buffer pointer
Note over Radar: For each pixel to draw
Radar->>Radar: Create ARGB color (GameMakeColor)
Radar->>Convert: ARGB_Color_To_WW3D_Color(format, argb)
Note over Convert: Convert ARGB to surface-specific format<br/>(e.g. A8R8G8B8, R5G6B5, L8, etc.)
Convert-->>Radar: Returns format-specific pixel value
Radar->>Draw: Draw_Pixel(x, y, pixelColor, bytesPerPixel, pBits, pitch)
Draw->>Surface: Write pixel to buffer
Note over Radar: After all pixels drawn
Radar->>Surface: Unlock()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additional Comments (1)
-
Core/GameEngineDevice/Source/W3DDevice/Common/System/W3DRadar.cpp, line 1431 (link)style:
m_shroudSurfaceFormatshould be reset toWW3D_FORMAT_UNKNOWNhere for consistency with other cleanupNote: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
4 files reviewed, 1 comment
|
I've reviewed the implementation and verified the bit manipulation logic mathematically - the RGB565/ARGB4444 conversions are correct, and the luminance calculation properly follows ITU-R BT.601 standard (77R + 150G + 29B = 256). The function is applied consistently across all radar rendering paths (renderObjectList, buildTerrainTexture, setShroudLevel) and handles edge cases safely by returning 0 for unsupported formats. While the code path may rarely execute on modern hardware (which typically uses A8R8G8B8), this is solid defensive programming that ensures correctness for legacy systems and alternative DirectX configurations with zero performance penalty. |
|
|
||
| } | ||
|
|
||
| color = ARGB_Color_To_WW3D_Color(surfaceDesc.Format, color); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't know what C++ convention is, but in c# you would distinguish the colors by variable name, so rgbColor and ww3dColor, rather than using 1 variable for both color types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay. Introduced argbColor and pixelColor for clarity.
0ba1ef5 to
64d1bf0
Compare
This change converts ARGB Radar pixel colors into the correct format before writing them into the DirectX surface.
I have not been able to figure out how to get the game to use a non A8R8B8G8 surface on my machine, so I was not able to really confirm that this is a useful change.
But it seems like this is the correct thing to do.
The
ARGB_Color_To_WW3D_Colorfunction was mostly generated by Chad Gippi.