Skip to content

Conversation

@xezon
Copy link

@xezon xezon commented Jan 3, 2026

This change implements Matrix4x4::Inverse and Matrix3D::Get_inverse so that we do not need to use D3DXMatrixInverse for Matrix4x4 and Matrix3D anymore (there are 4 callers, verified to produce same result). Chat GPT was used to help implement the text book math.

Eliminates all unsafe C casts (that I was able to find) between D3DMATRIX, D3DXMATRIX and Matrix4x4, Matrix3D.

The following new helpers functions help converting between the matrices:

void To_D3DMATRIX(_D3DMATRIX& dxm, const Matrix3D& m);
_D3DMATRIX To_D3DMATRIX(const Matrix3D& m);
D3DXMATRIX To_D3DXMATRIX(const Matrix3D& m);

void To_D3DMATRIX(_D3DMATRIX& dxm, const Matrix4x4& m);
_D3DMATRIX To_D3DMATRIX(const Matrix4x4& m);
D3DXMATRIX To_D3DXMATRIX(const Matrix4x4& m);

void To_Matrix4x4(Matrix4x4& m, const _D3DMATRIX& dxm);
Matrix4x4 To_Matrix4x4(const _D3DMATRIX& dxm);

TODO

  • Replicate in Generals

…inverse and replace unsafe Matrix4x4 to D3DMATRIX casts with conversion functions that apply the required transpose
@xezon xezon added this to the Code foundation build up milestone Jan 3, 2026
@xezon xezon requested a review from bobtista January 3, 2026 13:51
@xezon xezon added Major Severity: Minor < Major < Critical < Blocker Refactor Edits the code with insignificant behavior changes, is never user facing Rendering Is Rendering related labels Jan 3, 2026
@xezon
Copy link
Author

xezon commented Jan 3, 2026

Generals will fail to compile until replicated.

@xezon xezon changed the title refactor(math): Implement Matrix4x4::Inverse, Matrix3D::Get_inverse and replace unsafe Matrix4x4 to D3DMATRIX casts with conversion functions that apply the required transpose refactor(math): Implement Matrix4x4::Inverse, Matrix3D::Get_Inverse and replace unsafe Matrix4x4 to D3DMATRIX casts with conversion functions that apply the required transpose Jan 3, 2026
@xezon
Copy link
Author

xezon commented Jan 12, 2026

@greptileai

@greptile-apps
Copy link

greptile-apps bot commented Jan 12, 2026

Greptile Overview

Greptile Summary

Implemented custom matrix inversion for Matrix4x4 and Matrix3D using the adjugate/determinant method, eliminating dependency on D3DXMatrixInverse for these types. Added conversion functions (To_D3DMATRIX, To_D3DXMATRIX, To_Matrix4x4) that properly handle the transpose required when converting between column-major engine matrices and row-major Direct3D matrices.

Key Changes

  • Matrix Inversion: Implemented Matrix4x4::Inverse and Matrix3D::Get_Inverse using cofactor expansion with comprehensive comments explaining the mathematical approach
  • Conversion Functions: Added helper functions that apply the required transpose when converting between matrix formats, eliminating all unsafe C-style casts
  • DX8Wrapper Refactoring: Changed internal storage from Matrix4x4 to D3DMATRIX to avoid redundant conversions and simplified the transform API
  • Cast Elimination: Removed all dangerous casts like (D3DMATRIX*)&matrix4x4 throughout the codebase, replacing them with proper conversion function calls
  • Const-correctness: Improved const-correctness in Matrix3x3::Multiply and Matrix3D::Multiply

Notes

The implementation correctly handles the fundamental difference between the engine's column-major, column-vector convention and Direct3D's row-major convention by applying transpose operations during conversion.

Confidence Score: 5/5

  • This PR is safe to merge with high confidence
  • The matrix inversion implementation uses standard textbook algorithms (adjugate/determinant method), conversion functions properly handle row-major vs column-major differences with transpose, all unsafe casts have been systematically eliminated, and the PR author verified the results match D3DXMatrixInverse for all 4 callers
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
Core/Libraries/Source/WWVegas/WWMath/matrix4.h 5/5 Implemented Matrix4x4::Inverse using adjugate/determinant method and added conversion function declarations for D3DMATRIX interop
Core/Libraries/Source/WWVegas/WWMath/matrix4.cpp 5/5 Implemented matrix inversion and conversion functions (To_D3DMATRIX, To_Matrix4x4) with proper transpose handling
Core/Libraries/Source/WWVegas/WWMath/matrix3d.h 5/5 Added Matrix3D::Get_Inverse static method declaration and conversion function declarations for D3DMATRIX interop
Core/Libraries/Source/WWVegas/WWMath/matrix3d.cpp 5/5 Implemented Matrix3D::Get_Inverse using adjugate method, replaced D3DXMatrixInverse call, added conversion functions, removed unsafe cast
GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/dx8wrapper.h 5/5 Changed internal storage from Matrix4x4 to D3DMATRIX, simplified _Set_DX8_Transform to take D3DMATRIX directly
GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/dx8wrapper.cpp 5/5 Updated transform handling to use conversion functions, removed all unsafe casts, simplified matrix operations
Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DShaderManager.cpp 5/5 Replaced all Matrix4x4 to D3DXMATRIX casts with proper types, removed unsafe pointer casts throughout shader code

Sequence Diagram

sequenceDiagram
    participant App as Application Code
    participant M4 as Matrix4x4/Matrix3D
    participant Conv as Conversion Functions
    participant DX8 as DX8Wrapper
    participant D3D as Direct3D API

    Note over M4: Column-major storage<br/>Row[col][row]
    Note over D3D: Row-major storage<br/>m[row][col]

    App->>M4: Create/manipulate matrix
    M4->>M4: Perform operations (e.g., Inverse)
    
    alt Set Transform
        App->>DX8: Set_Transform(transform, Matrix4x4)
        DX8->>Conv: To_D3DMATRIX(Matrix4x4)
        Note over Conv: Transposes during conversion<br/>dxm.m[i][j] = m[j][i]
        Conv-->>DX8: D3DMATRIX
        DX8->>DX8: Store in render_state
        DX8->>DX8: _Set_DX8_Transform(D3DMATRIX)
        DX8->>D3D: SetTransform(D3DMATRIX*)
    end

    alt Get Transform
        App->>DX8: Get_Transform(transform, Matrix4x4&)
        DX8->>DX8: Retrieve D3DMATRIX from cache
        DX8->>Conv: To_Matrix4x4(D3DMATRIX)
        Note over Conv: Transposes during conversion<br/>m[i][j] = dxm.m[j][i]
        Conv-->>DX8: Matrix4x4
        DX8-->>App: Matrix4x4
    end

    alt Shader Texture Transform
        App->>DX8: _Get_DX8_Transform(D3DTS_VIEW, D3DXMATRIX&)
        DX8->>D3D: GetTransform(D3DXMATRIX*)
        D3D-->>DX8: D3DXMATRIX (row-major)
        DX8-->>App: D3DXMATRIX
        App->>App: D3DXMatrixInverse(&inv, &det, &curView)
        App->>App: Matrix operations in D3DXMATRIX space
        App->>DX8: _Set_DX8_Transform(D3DTS_TEXTURE0, D3DXMATRIX)
        DX8->>D3D: SetTransform(D3DMATRIX*)
    end
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

@xezon
Copy link
Author

xezon commented Jan 12, 2026

@greptileai

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

* *
* HISTORY: *
* 8/7/98 GTH : Created. *
* 01/03/2026 TheSuperHackers : Implemented with assistance of Chat GPT. *

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the comment is appropriate. ChatGPT is a tool, not an author and attributing tools in code creates legal ambiguity without adding value. GPL already covers licensing and authorship should remain limited to human contributors. Perhaps, to address AI use, it should belong in the project policy not in code comments?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. Tweaked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Major Severity: Minor < Major < Critical < Blocker Refactor Edits the code with insignificant behavior changes, is never user facing Rendering Is Rendering related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Properly implement transpose between Matrix4 and D3DMatrix

2 participants