Skip to content

Optimize orthonormal basis generation using Pixar 2017 method - #335

Open
RohithPariki wants to merge 3 commits into
microsoft:mainfrom
RohithPariki:feature/pixar-onb
Open

Optimize orthonormal basis generation using Pixar 2017 method#335
RohithPariki wants to merge 3 commits into
microsoft:mainfrom
RohithPariki:feature/pixar-onb

Conversation

@RohithPariki

Copy link
Copy Markdown

Resolves upstream issue #73.

Implemented XMVector3OrthogonalBasis utilizing the Pixar 2017 method ("Building an Orthonormal Basis, Revisited" by Duff et al.) to compute a tangent and bitangent from a unit normal. This ensures numeric stability across all edge cases (particularly along the Z-axis, which causes singularities in naive cross-product derivations) while remaining branchless.

changes:

  • Added XMVector3OrthogonalBasis to DirectXMath.h
  • Implemented XMVector3OrthogonalBasis in DirectXMathVector.inl
  • Maintained scalar compatibility with #if defined(_XM_NO_INTRINSICS_)
  • Optimized SIMD intrinsic execution pathway (reusing vectors and leveraging XMVectorAndInt/XMVectorOrInt for fast copysign replacement).

how i verified.

I verified the implementation by writing a test script against the critical edge cases (vectors perfectly aligned with the axes, which often cause singularities in branchless ONB generation).

#include <iostream>
#include <DirectXMath.h>
using namespace DirectX;

void VerifyONB(float x, float y, float z) {
    XMVECTOR N = XMVectorSet(x, y, z, 0);
    XMVECTOR T, B;
    
    // Generate Orthogonal Basis
    XMVector3OrthogonalBasis(&T, &B, N);
    
    // Verify Orthogonality
    float dot_NT = XMVectorGetX(XMVector3Dot(N, T));
    float dot_NB = XMVectorGetX(XMVector3Dot(N, B));
    float dot_TB = XMVectorGetX(XMVector3Dot(T, B));
    
    XMFLOAT4 nf, tf, bf;
    XMStoreFloat4(&nf, N); XMStoreFloat4(&tf, T); XMStoreFloat4(&bf, B);
    
    std::cout << "Normal: (" << nf.x << ", " << nf.y << ", " << nf.z << ")\n";
    std::cout << "Tangent: (" << tf.x << ", " << tf.y << ", " << tf.z << ")\n";
    std::cout << "Bitangent: (" << bf.x << ", " << bf.y << ", " << bf.z << ")\n";
    std::cout << "Verification [N.T, N.B, T.B]: " << dot_NT << ", " << dot_NB << ", " << dot_TB << "\n\n";
}

int main() {
    VerifyONB(0, 0, 1);
    VerifyONB(0, 0, -1);
    VerifyONB(1, 0, 0);
    VerifyONB(0.57735f, 0.57735f, 0.57735f); // Arbitrary unit vector
    return 0;
}

the output :

Normal: (0, 0, 1)
Tangent: (1, -0, -0)
Bitangent: (-0, 1, -0)
Verification [N.T, N.B, T.B]: 0, 0, 0

Normal: (0, 0, -1)
Tangent: (1, -0, 0)
Bitangent: (0, -1, -0)
Verification [N.T, N.B, T.B]: 0, 0, 0

Normal: (1, 0, 0)
Tangent: (0, -0, -1)
Bitangent: (-0, 1, -0)
Verification [N.T, N.B, T.B]: 0, 0, 0

Normal: (0.57735, 0.57735, 0.57735)
Tangent: (0.788675, -0.211325, -0.57735)
Bitangent: (-0.211325, 0.788675, -0.57735)
Verification [N.T, N.B, T.B]: 3.35524e-07, 3.35524e-07, -1.10278e-07

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@walbourn walbourn self-assigned this Aug 4, 2026
Comment thread Inc/DirectXMathVector.inl Outdated
// pTangent = (B1X, B1Y, B1Z, 0)
XMVECTOR B1_XY = XMVectorMergeXY(B1X, B1Y);
XMVECTOR B1_Z0 = XMVectorMergeXY(B1Z, Zero);
*pTangent = XMVectorPermute<XM_PERMUTE_0X, XM_PERMUTE_0Y, XM_PERMUTE_1X, XM_PERMUTE_1Y>(B1_XY, B1_Z0);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should this be if (pTangent) { *pTangen t==...} so the parameter can be _Out_opt_?

Comment thread Inc/DirectXMathVector.inl Outdated
assert(pBitangent != nullptr);

#if defined(_XM_NO_INTRINSICS_)
float sign = copysignf(1.0f, Normal.vector4_f32[2]);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Might have to investigate how far back copysignf can be found in MSVC. There are some compilation scenarios for DirectXMath that may require a different solution.

@RohithPariki
RohithPariki requested a review from walbourn August 4, 2026 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants