CNA-BUG-001: Plane::Transform(plane, matrix) transposes the inverse in place through a non-aliasing-safe Matrix::Transpose

CNA snapshot 009d40f5  ·  Known Issues › Current bugs  ·  source links pinned to 009d40f5

Plane::Transform with a Matrix passes one object as both source and destination of Matrix::Transpose, which is not aliasing-safe, so any transform whose inverse is not symmetric (a translation, most rotations) yields a wrong plane.

Identifier

CNA-BUG-001

Category

Bug

Subsystem

Math & geometry

Status

Open

Verified against

CNA 009d40f5 (009d40f5dd085c4e674d3479675fac84b12b3e0a)

Severity

High (a triage suggestion, not a project priority)

Evidence basis

Reproduced: executed for this entry (the Evidence section names exactly what was run)

Tests touching this area

Yes: see Current tests

Affected contract

Plane::Transform(Plane, Matrix) and Plane::Transform(const Plane&, const Matrix&, Plane&); C ABI cna_plane_transform_matrix

Expected behaviour

A plane is transformed by the inverse-transpose of the matrix: the plane (Normal, D) taken as a row vector is multiplied by transpose(inverse(M)). The genuine XNA 4.0 implementation inverts the matrix and then reads rows of the inverse directly, so it never transposes (read from the IL of the genuine XNA 4.0 Microsoft.Xna.Framework assembly; not executed). FNA's Plane.Transform (read at FNA commit 1358793, 2026-08-01) makes the same call CNA makes, Matrix.Transpose(ref transformedMatrix, out transformedMatrix), and is correct because FNA's Matrix.Transpose builds the transposed value in a local and assigns it at the end; the XNA IL of Matrix.Transpose likewise loads all sixteen fields into locals before writing. For M = Matrix::CreateTranslation(1, 0, 0) and the plane y = 1 (Normal (0, 1, 0), D = -1), a translation along X leaves the plane unchanged.

Actual behaviour at TARGET

Plane.cpp computes Matrix::Invert(matrix, transformedMatrix) and then calls Matrix::Transpose(transformedMatrix, transformedMatrix) with the same object as input and output. Matrix::Transpose(const Matrix&, Matrix&) in Matrix.cpp assigns the sixteen fields in declaration order while reading the source, so result.M12 = matrix.M21 is written before result.M21 = matrix.M12 reads it back. Every upper-triangle cell receives its lower-triangle mirror and every lower-triangle cell is left unchanged: the call symmetrises the matrix from its lower triangle, which equals the transpose only when the inverse is already symmetric.

For the example above the inverse has M41 = -1 and M14 = 0; after the aliased call both are -1, and Vector4::Transform then yields Normal (1, 1, 0), D = -1: a pure translation tilts the normal by 45 degrees and leaves it unnormalised. Pure rotations are affected too: the plane y = 2 transformed by CreateRotationZ(PiOver2) comes out with Normal (1, 0, 0) instead of (-1, 0, 0), i.e. on the wrong side.

Source locations

• modules/math/src/Plane.cpp — Plane::Transform(const Plane&, const Matrix&, Plane&): the aliased Matrix::Transpose(transformedMatrix, transformedMatrix) call
• modules/math/src/Matrix.cpp — Matrix::Transpose(const Matrix&, Matrix&): field-by-field copy that reads the source after writing the destination
• modules/math/src/Vector4.cpp — Vector4::Transform(const Vector4&, const Matrix&, Vector4&): the row-vector product applied to the corrupted matrix
• modules/c-api/src/CnaCApiGeometry.cpp — cna_plane_transform_matrix forwards to the value overload
• modules/math/tests/Microsoft/Xna/Framework/PlaneTests.cpp — PlaneTest.TransformByIdentityMatrixUnchanged and TransformByIdentityMatrixOutRef use the identity only
• modules/c-api/tests/pure_c/GeometrySmoke.c — validate_plane: transforms a plane with D = 0 along its own normal, which hides the defect

Evidence

Checked by reading at 009d40f5; the audit's own executed probe follows below. The failure follows mechanically from the two functions. A search of the TARGET tree for Transpose( call sites finds Plane.cpp as the only caller that passes one object for both parameters: BasicEffect.cpp, SkinnedEffect.cpp and EnvironmentMapEffect.cpp transpose into a second local, and every other caller uses the value-returning Matrix::Transpose(Matrix), which writes a distinct object. The worked numbers were first derived from the assignment order of the two functions with a small arithmetic model outside CNA; the executed probe below ran CNA's own code and reproduces them. The intended contract was taken from the XNA 4.0 IL and from FNA, whose structure CNA follows.

Independent re-verification: Reproduced by the audit at 009d40f5, not by CNA's own tests. The TARGET math sources were compiled with g++ -O0 into a probe that calls Plane::Transform and compares it with the XNA formula. Translation (1,0,0) on the plane (0,1,0, D=-1) gave Normal (1,1,0), D=-1 against XNA's (0,1,0), D=-1. The plane y=2 rotated by CreateRotationZ(PiOver2) gave Normal (1,0,0) against XNA's (-1,0,0). Identity and pure scale matched XNA, and the C ABI smoke case (plane y=0, translation along Y) also matched, which confirms it hides the defect. The sharp-runtime types came from a sibling checkout that TARGET does not pin. The search of Transpose call sites, and the reading of XNA IL and FNA, were done by reading.

Focused reproduction

Illustrative snippet; the executed probe is described under Evidence. The expected values follow from the code as described above.

#include "Microsoft/Xna/Framework/Matrix.hpp"
#include "Microsoft/Xna/Framework/Plane.hpp"
using namespace Microsoft::Xna::Framework;

Plane p(Vector3(0.0f, 1.0f, 0.0f), -1.0f);                       // the plane y = 1
Plane q = Plane::Transform(p, Matrix::CreateTranslation(1.0f, 0.0f, 0.0f));
// XNA: q.Normal == (0, 1, 0), q.D == -1
// TARGET code: q.Normal == (1, 1, 0), q.D == -1

Matrix m = Matrix::CreateTranslation(1.0f, 2.0f, 3.0f);
Matrix t;
Matrix::Transpose(m, t);            // distinct objects: correct
Matrix::Transpose(m, m);            // aliased: m is now symmetric, not transposed

Current tests

PlaneTests.cpp has two matrix-transform cases, PlaneTest.TransformByIdentityMatrixUnchanged and TransformByIdentityMatrixOutRef, both with the identity matrix, whose inverse is symmetric: the one input class on which symmetrisation and transposition agree. The C ABI smoke test GeometrySmoke.c transforms the plane y = 0 by a translation along Y; the wrongly copied term is multiplied by D = 0, so it also passes with the defect. MatrixTests.cpp tests Transpose on distinct objects only, and no XNA oracle covers Plane.

Regression test

A PlaneTests case that transforms a plane with a non-zero D by non-symmetric matrices (the translation and rotation cases above, and a rotation combined with a translation) and compares the result with the inverse-transpose computed through two distinct Matrix objects; plus a MatrixTests case calling Matrix::Transpose(m, m) on a non-symmetric matrix if the fix is made in Transpose itself rather than in Plane::Transform.

Blast radius

Affected: both Plane::Transform overloads taking a Matrix and the C ABI route cna_plane_transform_matrix, for every matrix whose inverse is not symmetric: any translation, most rotations, and rotation combined with scale or translation. Identity and pure (axis-aligned) scale matrices are unaffected. Not affected: Plane::Transform(Plane, Quaternion) (it rotates the normal with Vector3::Transform), BoundingFrustum (it derives its planes from the matrix directly), and the effect classes, which compute their world-inverse-transpose with distinct objects. At TARGET no module other than the C ABI calls the Matrix overload, so rendering, culling and content are not affected; the damage lands in games and bindings that transform planes themselves.

Workaround

Transpose into a separate matrix and transform the plane as a vector: Matrix it = Matrix::Transpose(Matrix::Invert(m)); Plane q(Vector4::Transform(Vector4(p.Normal, p.D), it)); The value-returning Matrix::Transpose writes a distinct object and is safe.