From 58ecb8ade537b26844151ab97fa78e9bf35606f3 Mon Sep 17 00:00:00 2001 From: Bryce Hutchings Date: Tue, 23 Sep 2025 16:58:39 -0700 Subject: [PATCH] Fix D3D12 using the wrong clip space projection matrix. Remove error-prone/unnecessary graphicsApi parameter. --- .../platform/openxr_d3d12_extension.cpp | 2 +- .../platform/openxr_metal_extension.mm | 3 +-- .../platform/openxr_opengl_extension.cpp | 2 +- .../platform/openxr_vulkan_extension.cpp | 3 +-- modules/openxr/openxr_util.cpp | 24 ++++++++----------- modules/openxr/openxr_util.h | 15 +++--------- 6 files changed, 17 insertions(+), 32 deletions(-) diff --git a/modules/openxr/extensions/platform/openxr_d3d12_extension.cpp b/modules/openxr/extensions/platform/openxr_d3d12_extension.cpp index 22f939214a2..f78732723cf 100644 --- a/modules/openxr/extensions/platform/openxr_d3d12_extension.cpp +++ b/modules/openxr/extensions/platform/openxr_d3d12_extension.cpp @@ -273,7 +273,7 @@ bool OpenXRD3D12Extension::get_swapchain_image_data(XrSwapchain p_swapchain, int bool OpenXRD3D12Extension::create_projection_fov(const XrFovf p_fov, double p_z_near, double p_z_far, Projection &r_camera_matrix) { OpenXRUtil::XrMatrix4x4f matrix; - OpenXRUtil::XrMatrix4x4f_CreateProjectionFov(&matrix, OpenXRUtil::GRAPHICS_D3D, p_fov, (float)p_z_near, (float)p_z_far); + OpenXRUtil::XrMatrix4x4f_CreateProjectionFov(&matrix, p_fov, (float)p_z_near, (float)p_z_far); for (int j = 0; j < 4; j++) { for (int i = 0; i < 4; i++) { diff --git a/modules/openxr/extensions/platform/openxr_metal_extension.mm b/modules/openxr/extensions/platform/openxr_metal_extension.mm index b5ed48a1745..4c0db956dc9 100644 --- a/modules/openxr/extensions/platform/openxr_metal_extension.mm +++ b/modules/openxr/extensions/platform/openxr_metal_extension.mm @@ -298,9 +298,8 @@ void OpenXRMetalExtension::cleanup_swapchain_graphics_data(void **p_swapchain_gr } bool OpenXRMetalExtension::create_projection_fov(const XrFovf p_fov, double p_z_near, double p_z_far, Projection &r_camera_matrix) { - // Even though this is a Metal renderer we're using OpenGL coordinate systems. OpenXRUtil::XrMatrix4x4f matrix; - OpenXRUtil::XrMatrix4x4f_CreateProjectionFov(&matrix, OpenXRUtil::GRAPHICS_OPENGL, p_fov, (float)p_z_near, (float)p_z_far); + OpenXRUtil::XrMatrix4x4f_CreateProjectionFov(&matrix, p_fov, (float)p_z_near, (float)p_z_far); for (int j = 0; j < 4; j++) { for (int i = 0; i < 4; i++) { diff --git a/modules/openxr/extensions/platform/openxr_opengl_extension.cpp b/modules/openxr/extensions/platform/openxr_opengl_extension.cpp index c0e56cf59a8..28c3e17708a 100644 --- a/modules/openxr/extensions/platform/openxr_opengl_extension.cpp +++ b/modules/openxr/extensions/platform/openxr_opengl_extension.cpp @@ -288,7 +288,7 @@ bool OpenXROpenGLExtension::get_swapchain_image_data(XrSwapchain p_swapchain, in bool OpenXROpenGLExtension::create_projection_fov(const XrFovf p_fov, double p_z_near, double p_z_far, Projection &r_camera_matrix) { OpenXRUtil::XrMatrix4x4f matrix; - OpenXRUtil::XrMatrix4x4f_CreateProjectionFov(&matrix, OpenXRUtil::GRAPHICS_OPENGL, p_fov, (float)p_z_near, (float)p_z_far); + OpenXRUtil::XrMatrix4x4f_CreateProjectionFov(&matrix, p_fov, (float)p_z_near, (float)p_z_far); for (int j = 0; j < 4; j++) { for (int i = 0; i < 4; i++) { diff --git a/modules/openxr/extensions/platform/openxr_vulkan_extension.cpp b/modules/openxr/extensions/platform/openxr_vulkan_extension.cpp index 48b8db5c990..06bd37aae11 100644 --- a/modules/openxr/extensions/platform/openxr_vulkan_extension.cpp +++ b/modules/openxr/extensions/platform/openxr_vulkan_extension.cpp @@ -417,9 +417,8 @@ bool OpenXRVulkanExtension::get_swapchain_image_data(XrSwapchain p_swapchain, in } bool OpenXRVulkanExtension::create_projection_fov(const XrFovf p_fov, double p_z_near, double p_z_far, Projection &r_camera_matrix) { - // Even though this is a Vulkan renderer we're using OpenGL coordinate systems. OpenXRUtil::XrMatrix4x4f matrix; - OpenXRUtil::XrMatrix4x4f_CreateProjectionFov(&matrix, OpenXRUtil::GRAPHICS_OPENGL, p_fov, (float)p_z_near, (float)p_z_far); + OpenXRUtil::XrMatrix4x4f_CreateProjectionFov(&matrix, p_fov, (float)p_z_near, (float)p_z_far); for (int j = 0; j < 4; j++) { for (int i = 0; i < 4; i++) { diff --git a/modules/openxr/openxr_util.cpp b/modules/openxr/openxr_util.cpp index 59f29206c0e..0ffcf1135bb 100644 --- a/modules/openxr/openxr_util.cpp +++ b/modules/openxr/openxr_util.cpp @@ -78,7 +78,7 @@ String OpenXRUtil::make_xr_version_string(XrVersion p_version) { return version; } -// Copied from OpenXR xr_linear.h private header, so we can still link against +// Based on the OpenXR xr_linear.h private header, so we can still link against // system-provided packages without relying on our `thirdparty` code. // Copyright (c) 2017 The Khronos Group Inc. @@ -87,25 +87,22 @@ String OpenXRUtil::make_xr_version_string(XrVersion p_version) { // SPDX-License-Identifier: Apache-2.0 // Creates a projection matrix based on the specified dimensions. -// The projection matrix transforms -Z=forward, +Y=up, +X=right to the appropriate clip space for the graphics API. +// The projection matrix transforms -Z=forward, +Y=up, +X=right to the appropriate clip space for Godot (OpenGL convention). // The far plane is placed at infinity if farZ <= nearZ. // An infinite projection matrix is preferred for rasterization because, except for // things *right* up against the near plane, it always provides better precision: // "Tightening the Precision of Perspective Rendering" // Paul Upchurch, Mathieu Desbrun // Journal of Graphics Tools, Volume 16, Issue 1, 2012 -void OpenXRUtil::XrMatrix4x4f_CreateProjection(XrMatrix4x4f *result, GraphicsAPI graphicsApi, const float tanAngleLeft, - const float tanAngleRight, const float tanAngleUp, float const tanAngleDown, - const float nearZ, const float farZ) { +void OpenXRUtil::XrMatrix4x4f_CreateProjection(XrMatrix4x4f *result, const float tanAngleLeft, const float tanAngleRight, + const float tanAngleUp, float const tanAngleDown, const float nearZ, const float farZ) { const float tanAngleWidth = tanAngleRight - tanAngleLeft; - // Set to tanAngleDown - tanAngleUp for a clip space with positive Y down (Vulkan). - // Set to tanAngleUp - tanAngleDown for a clip space with positive Y up (OpenGL / D3D / Metal). - const float tanAngleHeight = graphicsApi == GRAPHICS_VULKAN ? (tanAngleDown - tanAngleUp) : (tanAngleUp - tanAngleDown); + // Set to tanAngleUp - tanAngleDown for a clip space with positive Y up. + const float tanAngleHeight = (tanAngleUp - tanAngleDown); - // Set to nearZ for a [-1,1] Z clip space (OpenGL / OpenGL ES). - // Set to zero for a [0,1] Z clip space (Vulkan / D3D / Metal). - const float offsetZ = (graphicsApi == GRAPHICS_OPENGL || graphicsApi == GRAPHICS_OPENGL_ES) ? nearZ : 0; + // Set to nearZ for a [-1,1] Z clip space. + const float offsetZ = nearZ; if (farZ <= nearZ) { // place the far plane at infinity @@ -153,13 +150,12 @@ void OpenXRUtil::XrMatrix4x4f_CreateProjection(XrMatrix4x4f *result, GraphicsAPI } // Creates a projection matrix based on the specified FOV. -void OpenXRUtil::XrMatrix4x4f_CreateProjectionFov(XrMatrix4x4f *result, GraphicsAPI graphicsApi, const XrFovf fov, - const float nearZ, const float farZ) { +void OpenXRUtil::XrMatrix4x4f_CreateProjectionFov(XrMatrix4x4f *result, const XrFovf fov, const float nearZ, const float farZ) { const float tanLeft = std::tan(fov.angleLeft); const float tanRight = std::tan(fov.angleRight); const float tanDown = std::tan(fov.angleDown); const float tanUp = std::tan(fov.angleUp); - XrMatrix4x4f_CreateProjection(result, graphicsApi, tanLeft, tanRight, tanUp, tanDown, nearZ, farZ); + XrMatrix4x4f_CreateProjection(result, tanLeft, tanRight, tanUp, tanDown, nearZ, farZ); } diff --git a/modules/openxr/openxr_util.h b/modules/openxr/openxr_util.h index 9e2cc66bdfc..7c2a912fb77 100644 --- a/modules/openxr/openxr_util.h +++ b/modules/openxr/openxr_util.h @@ -52,16 +52,7 @@ public: float m[16]; } XrMatrix4x4f; - typedef enum GraphicsAPI { - GRAPHICS_VULKAN, - GRAPHICS_OPENGL, - GRAPHICS_OPENGL_ES, - GRAPHICS_D3D - } GraphicsAPI; - - static void XrMatrix4x4f_CreateProjection(XrMatrix4x4f *result, GraphicsAPI graphicsApi, const float tanAngleLeft, - const float tanAngleRight, const float tanAngleUp, float const tanAngleDown, - const float nearZ, const float farZ); - static void XrMatrix4x4f_CreateProjectionFov(XrMatrix4x4f *result, GraphicsAPI graphicsApi, const XrFovf fov, - const float nearZ, const float farZ); + static void XrMatrix4x4f_CreateProjection(XrMatrix4x4f *result, const float tanAngleLeft, const float tanAngleRight, + const float tanAngleUp, float const tanAngleDown, const float nearZ, const float farZ); + static void XrMatrix4x4f_CreateProjectionFov(XrMatrix4x4f *result, const XrFovf fov, const float nearZ, const float farZ); };