Fix typo in MakeObliqueProjection: M(0,2) should be M(0,3)

[CodeReclaimers]

Problem

In Matrix4x4.h line 234, the GTE_USE_VEC_MAT branch of MakeObliqueProjection writes M(0, 2) = zero instead of M(0, 3) = zero. This overwrites the correct value set on line 230 (direction[2] * normal[0]) with zero.

Fix

-        M(0, 2) = zero;
+        M(0, 3) = zero;

Test program

Compiled with -DGTE_USE_VEC_MAT. Verifies M(0,2) retains its correct value, and that projected points match a direct geometric computation.

Before fix:

Test 1 (M(0,2) value):       BUG DETECTED
Test 2 (projection result):   BUG DETECTED

After fix:

Test 1 (M(0,2) value):       passed
Test 2 (projection result):   passed

CMakeLists.txt

cmake_minimum_required(VERSION 3.14)
project(test_issue_1_2 LANGUAGES CXX)

set(CMAKE_CXX_STANDARD 17)
set(CMAKE_CXX_STANDARD_REQUIRED ON)

set(GTE_ROOT "${CMAKE_CURRENT_SOURCE_DIR}/../../GTE")

add_executable(test_issue_1_2 test_issue_1_2.cpp)
target_include_directories(test_issue_1_2 PRIVATE "${GTE_ROOT}")

# This bug only manifests under GTE_USE_VEC_MAT.
target_compile_definitions(test_issue_1_2 PRIVATE GTE_USE_VEC_MAT)

test_issue_1_2.cpp

// Test for Issue 1.2: Matrix4x4.h MakeObliqueProjection copy-paste bug
//
// Bug: In the GTE_USE_VEC_MAT branch, line 234 writes M(0,2) instead of
// M(0,3). This overwrites the correct value of M(0,2) (set on line 230)
// with zero, corrupting the projection matrix.
//
// File: GTE/Mathematics/Matrix4x4.h, line 234
//   M(0, 2) = zero;    // BUG: should be M(0, 3)
//
// Expected fix: Change M(0, 2) to M(0, 3)

#include <cmath>
#include <cstdlib>
#include <iostream>

#include <Mathematics/Matrix4x4.h>
#include <Mathematics/Vector4.h>

// Compute the oblique projection by a direct, unambiguous formula so we
// have a reference to compare against. For the VEC_MAT convention, the
// projected point is p' = p * M, where:
//
//   M = (d . n) * I - n * d^T     (3x3 upper-left block, transposed for VEC_MAT)
//
// with the translation row encoding the origin offset.
//
// Rather than re-derive the full matrix, we just project individual points
// using the geometric definition and verify MakeObliqueProjection gives
// the same result.
//
// The geometric projection of point P onto plane (origin O, normal N)
// along direction D is:
//
//   P' = P + t * D,  where t = N . (O - P) / (N . D)
//
template <typename Real>
gte::Vector4<Real> ProjectPointDirect(
    gte::Vector4<Real> const& point,
    gte::Vector4<Real> const& origin,
    gte::Vector4<Real> const& normal,
    gte::Vector4<Real> const& direction)
{
    Real dotND = gte::Dot(normal, direction);
    Real dotN_OminusP = gte::Dot(normal, origin - point);
    Real t = dotN_OminusP / dotND;
    return point + t * direction;
}

// Multiply point (as homogeneous row vector) by the matrix: result = p * M
template <typename Real>
gte::Vector4<Real> MultiplyVecMat(
    gte::Vector4<Real> const& p,
    gte::Matrix4x4<Real> const& M)
{
    gte::Vector4<Real> result;
    for (int col = 0; col < 4; ++col)
    {
        Real sum = static_cast<Real>(0);
        for (int row = 0; row < 4; ++row)
        {
            sum += p[row] * M(row, col);
        }
        result[col] = sum;
    }
    return result;
}

static bool test_oblique_projection()
{
    using Real = double;

    // Define a plane: origin at (1, 0, 0), normal along x-axis.
    gte::Vector4<Real> origin{ 1.0, 0.0, 0.0, 1.0 };
    gte::Vector4<Real> normal{ 1.0, 0.0, 0.0, 0.0 };
    // Projection direction: not aligned with normal, to exercise all entries.
    gte::Vector4<Real> direction{ 1.0, 1.0, 1.0, 0.0 };

    // Build the projection matrix.
    gte::Matrix4x4<Real> M = gte::MakeObliqueProjection(origin, normal, direction);

    // Test several points.
    gte::Vector4<Real> testPoints[] = {
        { 3.0, 2.0, 1.0, 1.0 },
        { 0.0, 0.0, 0.0, 1.0 },
        { -1.0, 5.0, -3.0, 1.0 },
        { 2.0, -1.0, 4.0, 1.0 },
    };

    Real const dotND = gte::Dot(normal, direction);
    Real const tol = 1e-12;
    bool all_ok = true;

    for (auto const& P : testPoints)
    {
        // Reference: direct geometric projection.
        gte::Vector4<Real> expected = ProjectPointDirect(P, origin, normal, direction);

        // Matrix projection: p' = (p * M) / (-dotND) to account for the
        // homogeneous scaling built into the matrix (the matrix is scaled
        // by -dotND; the w component of the result is -dotND, not 1).
        gte::Vector4<Real> raw = MultiplyVecMat(P, M);
        gte::Vector4<Real> actual;
        for (int i = 0; i < 4; ++i)
            actual[i] = raw[i] / raw[3];

        // Compare.
        Real maxErr = 0.0;
        for (int i = 0; i < 3; ++i)
        {
            Real err = std::fabs(actual[i] - expected[i]);
            if (err > maxErr) maxErr = err;
        }

        if (maxErr > tol)
        {
            all_ok = false;
            std::cout << "  FAIL: P=(" << P[0] << "," << P[1] << "," << P[2] << ")\n"
                      << "    expected=(" << expected[0] << "," << expected[1] << "," << expected[2] << ")\n"
                      << "    actual  =(" << actual[0] << "," << actual[1] << "," << actual[2] << ")\n"
                      << "    maxErr=" << maxErr << "\n";
        }
    }

    return all_ok;
}

static bool test_m02_not_zeroed()
{
    using Real = double;

    // Use direction and normal that make direction[2]*normal[0] nonzero.
    gte::Vector4<Real> origin{ 0.0, 0.0, 0.0, 1.0 };
    gte::Vector4<Real> normal{ 1.0, 0.0, 0.0, 0.0 };
    gte::Vector4<Real> direction{ 1.0, 2.0, 3.0, 0.0 };

    gte::Matrix4x4<Real> M = gte::MakeObliqueProjection(origin, normal, direction);

    // M(0,2) should be direction[2]*normal[0] = 3.0*1.0 = 3.0
    // The bug sets M(0,2) = 0 instead.
    Real expected_m02 = direction[2] * normal[0];  // 3.0
    Real actual_m02 = M(0, 2);

    std::cout << "  M(0,2): expected=" << expected_m02 << " actual=" << actual_m02 << "\n";

    if (std::fabs(actual_m02 - expected_m02) > 1e-15)
    {
        std::cout << "  FAIL: M(0,2) was overwritten to " << actual_m02
                  << " (should be " << expected_m02 << ")\n";
        return false;
    }
    std::cout << "  PASS: M(0,2) has correct value.\n";
    return true;
}

int main()
{
    std::cout << "=== Issue 1.2: MakeObliqueProjection M(0,2) vs M(0,3) typo ===\n";
    std::cout << "File: GTE/Mathematics/Matrix4x4.h, line 234\n";
    std::cout << "Bug: M(0,2) written instead of M(0,3) under GTE_USE_VEC_MAT\n\n";

    std::cout << "Test 1: M(0,2) entry value check\n";
    bool ok1 = test_m02_not_zeroed();

    std::cout << "\nTest 2: Full projection correctness\n";
    bool ok2 = test_oblique_projection();
    if (ok2)
        std::cout << "  PASS: All projected points match direct computation.\n";
    else
        std::cout << "  FAIL: Projected points do not match.\n";

    std::cout << "\n=== Summary ===\n";
    std::cout << "Test 1 (M(0,2) value):       " << (ok1 ? "passed" : "BUG DETECTED") << "\n";
    std::cout << "Test 2 (projection result):   " << (ok2 ? "passed" : "BUG DETECTED") << "\n";

    int bugs = (!ok1 ? 1 : 0) + (!ok2 ? 1 : 0);
    std::cout << "\n" << bugs << " bug(s) detected out of 2 tests.\n";
    return bugs > 0 ? EXIT_FAILURE : EXIT_SUCCESS;
}

🤖 Generated with Claude Code