Skip to content

Commit 8d00d9e

Browse files
authored
Fix NGX Lifetime Management and Manual FSR FOV Conversion (#875)
* Clean up DLSS DX12 inputs - Refactored CreateFeature() and EvaluateFeature() into smaller functions with shared utilities and distinct code paths for internal OptiScaler and passthrough features. - Added documentation to clarify internal implementation details and behavior required by the NGX API. - Added more descriptive error messages. - Fixed a potential memory leak in GetParameters(). Per the DLSS Programming Guide (310.5.0), this deprecated API requires the SDK to manage the lifetime of the returned parameter table, as legacy applications do not free this memory. - Identified a memory management issue: NVNGX_Parameter tables are incorrectly handled in Allocate/Destroy and FeatureProvider. C-style free() is being used on tables allocated with 'new' and on native NGX tables, leading to memory leaks and undefined behavior. Using free() here is actually worse than just willfully leaking memory. This needs fixing. * Fix: Correct NGX Parameter table lifetime and deallocation Problem: The previous implementation incorrectly used C-style free() on NVSDK_NGX_Parameter objects within the DLSS feature providers. This was unsafe for several reasons: - Non-trivial types: These structs often contain internal data/tables that free() does not account for, bypassing necessary destructors. - Ambiguous ownership: Tables were inconsistently sourced from internal allocations (OptiScaler), external NGX API calls, or legacy persistent pointers. - Heap/Stack corruption: Using free() on memory managed by the NGX API or on stack-allocated memory from legacy GetParameters() calls. This risked immediate crashes or heap instability. Solution: Introduced a tagging system to explicitly track memory ownership and determine the correct deallocation strategy. When a destruction request is made, the system now checks the table's tag and acts accordingly: 1. InternPersistent: Internal OptiScaler tables for legacy APIs. Lifetime is managed by the SDK; these persist until process exit. 2. InternDynamic: Internal tables explicitly created/destroyed by the client. Now correctly freed using delete with a static_cast to the implementation type. 3. NVPersistent: Native NGX legacy tables. These are ignored as the native SDK manages their lifetime. 4. NVDynamic: Native NGX modern tables. These are now properly forwarded to the native NGX DestroyParameters() function. * Start gathering unique string literals in header - Started organizing string literals being used as unique identifiers for things like feature selection and parameter lookup into a dedicated header. Less error prone and actually works with intellisense. - The actual usages of these strings have yet to be replaced, but I am using them for new code. * Fix incorrect configurable FOV conversion in FSR - The conversions for manually configured horizontal FOV to vertical FOV used in upscaling have been corrected to use the following formula: vFov = 2 * arctan( tan( hFov / 2 ) * ( h / w ) ) - Added convenience variables for global singletons to Eval functions to help readability - Streamlined ffxResolveTypelessFormat usages. More readable. * Update ffxResolveTypelessFormat usage * Run clang formatting * Fix potential NGX handle memory leak if device couldn't be acquired Fixed a preexisting edge case that could cause the NGX handle to be leaked if the D3D12 device could not be acquired. * Fix Vulkan upscaler typo and DX11 init error - Vulkan: Correct default upscaler key from FSR 2.1 to FSR 2.2. - DX11: Removed redundant table initialization in DLSS inputs caused by copy-paste error. * Add NGX table reset check and remove old DLSSD check - Added extra check in NGX table to preserve usage keys for custom tables. - Fixed incorrect feature release log error - Reran clang-format - Removed old DLSSD check for root signature restore - Removed GetIsDlssModuleInited() helper utility
1 parent ec46e0e commit 8d00d9e

31 files changed

+1092
-633
lines changed

OptiScaler/Config.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -367,9 +367,9 @@ class Config
367367
CustomOptional<bool> DontCreateD3D12DeviceForLuma { false };
368368

369369
// Upscalers
370-
CustomOptional<std::string, SoftDefault> Dx11Upscaler { "fsr22" };
371-
CustomOptional<std::string, SoftDefault> Dx12Upscaler { "xess" };
372-
CustomOptional<std::string, SoftDefault> VulkanUpscaler { "fsr22" };
370+
CustomOptional<std::string, SoftDefault> Dx11Upscaler { std::string(OptiKeys::FSR22) };
371+
CustomOptional<std::string, SoftDefault> Dx12Upscaler { std::string(OptiKeys::XeSS) };
372+
CustomOptional<std::string, SoftDefault> VulkanUpscaler { std::string(OptiKeys::FSR22) };
373373

374374
// Output Scaling
375375
CustomOptional<bool> OutputScalingEnabled { false };

OptiScaler/MathUtils.h

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
#pragma once
2+
#include <numbers>
3+
#include <cmath>
4+
5+
namespace OptiMath
6+
{
7+
// Radians per degree. Used to convert from degrees to radians
8+
inline constexpr float DegToRad = std::numbers::pi_v<float> / 180.0f;
9+
// Degrees per radian. Used to convert from radians to degrees
10+
inline constexpr float RadToDeg = 180.0f / std::numbers::pi_v<float>;
11+
12+
// Converts from an angle in degrees to radians
13+
[[nodiscard]] inline constexpr float GetRadiansFromDeg(const float deg) { return deg * DegToRad; }
14+
15+
// Converts from an angle in radians to degrees
16+
[[nodiscard]] inline constexpr float GetDegreesFromRad(const float rad) { return rad * RadToDeg; }
17+
18+
/**
19+
* @brief Calculates the vertical field of view for a camera from its horizontal FOV and its
20+
* viewport dimensions according to the formula: vFov = 2 * arctan( tan( hFov / 2 ) * ( h / w ) ).
21+
* @param hFovRad: Horizontal field of view
22+
* @param width: Width of the camera viewport
23+
* @param height: Height of the camera viewport
24+
* @return Vertical FOV in radians
25+
*/
26+
[[nodiscard]] inline float GetVerticalFovFromHorizontal(const float hFovRad, const float width, const float height)
27+
{
28+
if (width <= 0.0f)
29+
return 0.0f;
30+
return 2.0f * std::atan(std::tan(hFovRad * 0.5f) * (height / width));
31+
}
32+
33+
/**
34+
* @brief Calculates the horizontal field of view for a camera from its vertical FOV and its
35+
* viewport dimensions according to the formula: hFov = 2 * arctan( tan( vFov / 2 ) * ( w / h ) ).
36+
* @param vFovRad: Vertical field of view in radians
37+
* @param width: Width of the camera viewport
38+
* @param height: Height of the camera viewport
39+
* @return Horizontal FOV in radians
40+
*/
41+
[[nodiscard]] inline float GetHorizontalFovFromVertical(const float vFovRad, const float width, const float height)
42+
{
43+
if (height <= 0.0f)
44+
return 0.0f;
45+
return 2.0f * std::atan(std::tan(vFovRad * 0.5f) * (width / height));
46+
}
47+
} // namespace OptiMath

OptiScaler/NVNGX_Parameter.h

Lines changed: 92 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
#pragma once
22
#include "SysUtils.h"
3-
43
#include "Config.h"
5-
64
#include <ankerl/unordered_dense.h>
75

86
// Use real NVNGX params encapsulated in custom one
@@ -18,6 +16,24 @@
1816
#define LOG_PARAM(msg, ...)
1917
#endif
2018

19+
/** @brief Indicates the lifetime management required by an NGX parameter table. */
20+
namespace NGX_AllocTypes
21+
{
22+
// Key used to get/set enum from table
23+
constexpr std::string_view AllocKey = "OptiScaler.ParamAllocType";
24+
25+
constexpr uint32_t Unknown = 0;
26+
// Standard behavior in modern DLSS. Created with NGX Allocate(). Freed with Destroy().
27+
constexpr uint32_t NVDynamic = 1;
28+
// Legacy DLSS. Lifetime managed internally by the SDK.
29+
constexpr uint32_t NVPersistent = 2;
30+
// OptiScaler implementation used internally with new/delete.
31+
constexpr uint32_t InternDynamic = 3;
32+
// OptiScaler implementation for legacy applications. Must maintain a persistent instance
33+
// for the lifetime of the application.
34+
constexpr uint32_t InternPersistent = 4;
35+
} // namespace NGX_AllocTypes
36+
2137
/// @brief Calculates the resolution scaling ratio override based on the provided quality level and current
2238
/// configuration.
2339
/// @param input The performance quality value (e.g. Quality, Balanced, Performance).
@@ -670,6 +686,15 @@ struct NVNGX_Parameters : public NVSDK_NGX_Parameter
670686
{
671687
std::string Name;
672688

689+
NVNGX_Parameters(std::string_view name, bool isPersistent) : Name(name)
690+
{
691+
// Old flag used to indicate custom table. Obsolete?
692+
Set("OptiScaler", 1);
693+
// New tracking flag
694+
Set(NGX_AllocTypes::AllocKey.data(),
695+
isPersistent ? NGX_AllocTypes::InternPersistent : NGX_AllocTypes::InternDynamic);
696+
}
697+
673698
#ifdef ENABLE_ENCAPSULATED_PARAMS
674699
NVSDK_NGX_Parameter* OriginalParam = nullptr;
675700
#endif // ENABLE_ENCAPSULATED_PARAMS
@@ -934,8 +959,16 @@ struct NVNGX_Parameters : public NVSDK_NGX_Parameter
934959
void Reset() override
935960
{
936961
if (!m_values.empty())
962+
{
963+
// Preserve usage type if set
964+
uint32_t allocType = NGX_AllocTypes::Unknown;
965+
NVSDK_NGX_Result result = Get(NGX_AllocTypes::AllocKey.data(), &allocType);
937966
m_values.clear();
938967

968+
if (result != NVSDK_NGX_Result_Fail)
969+
Set(NGX_AllocTypes::AllocKey.data(), allocType);
970+
}
971+
939972
LOG_DEBUG("Start");
940973

941974
InitNGXParameters(this);
@@ -981,13 +1014,64 @@ struct NVNGX_Parameters : public NVSDK_NGX_Parameter
9811014
}
9821015
};
9831016

984-
/// @brief Allocates and populates a new NGX param map.
985-
inline static NVNGX_Parameters* GetNGXParameters(std::string InName)
1017+
/**
1018+
* @brief Allocates and populates a new custom NGX param map. The persistence flag indicates
1019+
* whether the table should be destroyed when NGX DestroyParameters() is used.
1020+
*/
1021+
inline static NVNGX_Parameters* GetNGXParameters(std::string_view name, bool isPersistent)
9861022
{
987-
auto params = new NVNGX_Parameters();
988-
params->Name = InName;
1023+
auto params = new NVNGX_Parameters(name, isPersistent);
9891024
InitNGXParameters(params);
990-
params->Set("OptiScaler", 1);
991-
9921025
return params;
9931026
}
1027+
1028+
/**
1029+
* @brief Sets a custom tracking tag to indicate the memory management strategy required by
1030+
* the table, indicated by NGX_AllocTypes.
1031+
*/
1032+
inline static void SetNGXParamAllocType(NVSDK_NGX_Parameter& params, uint32_t allocType)
1033+
{
1034+
params.Set(NGX_AllocTypes::AllocKey.data(), allocType);
1035+
}
1036+
1037+
/**
1038+
* @brief Attempts to safely delete an NGX parameter table. Dynamically allocated NGX tables use the NGX API.
1039+
* OptiScaler tables use delete. Persistent tables are not freed.
1040+
*/
1041+
template <typename PFN_DestroyNGXParameters>
1042+
static inline bool TryDestroyNGXParameters(NVSDK_NGX_Parameter* InParameters, PFN_DestroyNGXParameters NVFree = nullptr)
1043+
{
1044+
if (InParameters == nullptr)
1045+
return false;
1046+
1047+
uint32_t allocType = NGX_AllocTypes::Unknown;
1048+
NVSDK_NGX_Result result = InParameters->Get(NGX_AllocTypes::AllocKey.data(), &allocType);
1049+
1050+
// Key not set. Either a bug, or the client application called Reset() on the table before destroying.
1051+
// Derived type unknown if this happens. Not safe to delete. Leaking is the best option.
1052+
if (result == NVSDK_NGX_Result_Fail)
1053+
{
1054+
LOG_WARN("Destroy called on NGX table with unset alloc type. Leaking.");
1055+
return false;
1056+
}
1057+
1058+
if (allocType == NGX_AllocTypes::NVDynamic)
1059+
{
1060+
if (NVFree != nullptr)
1061+
{
1062+
LOG_INFO("Calling NVFree");
1063+
result = NVFree(InParameters);
1064+
LOG_INFO("Calling NVFree result: {0:X}", (UINT) result);
1065+
return true;
1066+
}
1067+
else
1068+
return false;
1069+
}
1070+
else if (allocType == NGX_AllocTypes::InternDynamic)
1071+
{
1072+
delete static_cast<NVNGX_Parameters*>(InParameters);
1073+
return true;
1074+
}
1075+
1076+
return false;
1077+
}

OptiScaler/OptiScaler.vcxproj

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<?xml version="1.0" encoding="utf-8"?>
1+
<?xml version="1.0" encoding="utf-8"?>
22
<Project DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
33
<ItemGroup Label="ProjectConfigurations">
44
<ProjectConfiguration Include="Debug|Win32">
@@ -340,6 +340,8 @@ copy NUL "$(SolutionDir)x64\Release\a\!! EXTRACT ALL FILES TO GAME FOLDER !!" /Y
340340
<ClInclude Include="inputs\FSR2_Dx11.h" />
341341
<ClInclude Include="inputs\FSR2_Vk.h" />
342342
<ClInclude Include="inputs\XeSS_Dx11.h" />
343+
<ClInclude Include="MathUtils.h" />
344+
<ClInclude Include="OptiTexts.h" />
343345
<ClInclude Include="shaders\depth_invert\DI_Common.h" />
344346
<ClInclude Include="shaders\depth_invert\DI_Dx12.h" />
345347
<ClInclude Include="shaders\depth_invert\precompiled\DI_Shader.h" />

OptiScaler/OptiScaler.vcxproj.filters

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -695,7 +695,7 @@
695695
<ClInclude Include="shaders\hud_copy\HudCopy_Dx12.h">
696696
<Filter>Header Files</Filter>
697697
</ClInclude>
698-
<ClInclude Include="shaders\hud_copy\precompile\HudCopy_Shader.h">
698+
<ClInclude Include="MathUtils.h">
699699
<Filter>Header Files</Filter>
700700
</ClInclude>
701701
<ClInclude Include="shaders\output_scaling\precompile\bcds_bicubic_Shader_Vk.h">
@@ -728,6 +728,9 @@
728728
<ClInclude Include="shaders\render_ui\RUI_Dx12.h">
729729
<Filter>Header Files</Filter>
730730
</ClInclude>
731+
<ClInclude Include="OptiTexts.h">
732+
<Filter>Header Files</Filter>
733+
</ClInclude>
731734
<ClInclude Include="shaders\output_scaling\precompile\bcds_kaiser2_Shader.h">
732735
<Filter>Header Files</Filter>
733736
</ClInclude>

OptiScaler/OptiTexts.h

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
#pragma once
2+
3+
/**
4+
* @brief Common strings and identifiers used internally by OptiScaler
5+
*/
6+
namespace OptiKeys
7+
{
8+
using CString = const char[];
9+
10+
// Application name provided to upscalers
11+
inline constexpr CString ProjectID = "OptiScaler";
12+
13+
// ID code used for the Vulkan input provider
14+
inline constexpr CString VkProvider = "OptiVk";
15+
// ID code used for the DX11 input provider
16+
inline constexpr CString Dx11Provider = "OptiDx11";
17+
// ID code used for the DX12 input provider
18+
inline constexpr CString Dx12Provider = "OptiDx12";
19+
20+
// ID code used for the XeSS upscaler backend
21+
inline constexpr CString XeSS = "xess";
22+
// ID code used for the XeSS upscaler backend used with the DirectX 11 on 12 compatibility layer
23+
inline constexpr CString XeSS_11on12 = "xess_12";
24+
// ID code used for the FSR 2.1.x upscaler backend
25+
inline constexpr CString FSR21 = "fsr21";
26+
// ID code used for the FSR 2.1.x upscaler backend used with the DirectX 11 on 12 compatibility layer
27+
inline constexpr CString FSR21_11on12 = "fsr21_12";
28+
// ID code used for the FSR 2.2.x upscaler backend
29+
inline constexpr CString FSR22 = "fsr22";
30+
// ID code used for the FSR 2.2.x upscaler backend used with the DirectX 11 on 12 compatibility layer
31+
inline constexpr CString FSR22_11on12 = "fsr22_12";
32+
// ID code used for the FSR 3.1+ upscaler backend
33+
inline constexpr CString FSR31 = "fsr31";
34+
// ID code used for the FSR 3.1+ upscaler backend used with the DirectX 11 on 12 compatibility layer
35+
inline constexpr CString FSR31_11on12 = "fsr31_12";
36+
// ID code used for the DLSS upscaler backend
37+
inline constexpr CString DLSS = "dlss";
38+
// ID code used for the DLSS-D/Ray Reconstruction upscaler+denoiser backend
39+
inline constexpr CString DLSSD = "dlssd";
40+
41+
inline constexpr CString FSR_UpscaleWidth = "FSR.upscaleSize.width";
42+
inline constexpr CString FSR_UpscaleHeight = "FSR.upscaleSize.height";
43+
44+
inline constexpr CString FSR_NearPlane = "FSR.cameraNear";
45+
inline constexpr CString FSR_FarPlane = "FSR.cameraFar";
46+
inline constexpr CString FSR_CameraFovVertical = "FSR.cameraFovAngleVertical";
47+
inline constexpr CString FSR_FrameTimeDelta = "FSR.frameTimeDelta";
48+
inline constexpr CString FSR_ViewSpaceToMetersFactor = "FSR.viewSpaceToMetersFactor";
49+
inline constexpr CString FSR_TransparencyAndComp = "FSR.transparencyAndComposition";
50+
inline constexpr CString FSR_Reactive = "FSR.reactive";
51+
52+
} // namespace OptiKeys

OptiScaler/SysUtils.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,3 +183,5 @@ inline static void to_lower_in_place(std::string& string)
183183
{
184184
std::transform(string.begin(), string.end(), string.begin(), ::tolower);
185185
}
186+
187+
#include "OptiTexts.h"

OptiScaler/inputs/FG/FSR3_Dx12_FG.cpp

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
#include "Config.h"
55
#include "Util.h"
6+
#include "MathUtils.h"
67

78
#include "resource.h"
89
#include "NVNGX_Parameter.h"
@@ -23,6 +24,7 @@
2324
#include "fsr3/ffx_frameinterpolation.h"
2425

2526
const UINT fgContext = 0x1337;
27+
using namespace OptiMath;
2628

2729
// Swapchain create
2830
typedef FFX_API
@@ -1250,21 +1252,25 @@ void FSR3FG::SetUpscalerInputs(ID3D12GraphicsCommandList* InCmdList, NVSDK_NGX_P
12501252

12511253
float tempCameraNear = 0.0f;
12521254
float tempCameraFar = 0.0f;
1253-
InParameters->Get("FSR.cameraNear", &tempCameraNear);
1254-
InParameters->Get("FSR.cameraFar", &tempCameraFar);
12551255

1256-
if (!Config::Instance()->FsrUseFsrInputValues.value_or_default() ||
1257-
(tempCameraNear == 0.0f && tempCameraFar == 0.0f))
1256+
auto& state = State::Instance();
1257+
auto& cfg = *Config::Instance();
1258+
const auto& ngxParams = *InParameters;
1259+
1260+
ngxParams.Get(OptiKeys::FSR_NearPlane, &tempCameraNear);
1261+
ngxParams.Get(OptiKeys::FSR_FarPlane, &tempCameraFar);
1262+
1263+
if (!cfg.FsrUseFsrInputValues.value_or_default() || (tempCameraNear == 0.0f && tempCameraFar == 0.0f))
12581264
{
12591265
if (feature->DepthInverted())
12601266
{
1261-
cameraFar = Config::Instance()->FsrCameraNear.value_or_default();
1262-
cameraNear = Config::Instance()->FsrCameraFar.value_or_default();
1267+
cameraFar = cfg.FsrCameraNear.value_or_default();
1268+
cameraNear = cfg.FsrCameraFar.value_or_default();
12631269
}
12641270
else
12651271
{
1266-
cameraFar = Config::Instance()->FsrCameraFar.value_or_default();
1267-
cameraNear = Config::Instance()->FsrCameraNear.value_or_default();
1272+
cameraFar = cfg.FsrCameraFar.value_or_default();
1273+
cameraNear = cfg.FsrCameraNear.value_or_default();
12681274
}
12691275
}
12701276
else
@@ -1273,20 +1279,23 @@ void FSR3FG::SetUpscalerInputs(ID3D12GraphicsCommandList* InCmdList, NVSDK_NGX_P
12731279
cameraFar = tempCameraFar;
12741280
}
12751281

1276-
if (!Config::Instance()->FsrUseFsrInputValues.value_or_default() ||
1277-
InParameters->Get("FSR.cameraFovAngleVertical", &cameraVFov) != NVSDK_NGX_Result_Success)
1282+
if (!cfg.FsrUseFsrInputValues.value_or_default() ||
1283+
ngxParams.Get(OptiKeys::FSR_CameraFovVertical, &cameraVFov) != NVSDK_NGX_Result_Success)
12781284
{
1279-
if (Config::Instance()->FsrVerticalFov.has_value())
1280-
cameraVFov = Config::Instance()->FsrVerticalFov.value() * 0.0174532925199433f;
1281-
else if (Config::Instance()->FsrHorizontalFov.value_or_default() > 0.0f)
1282-
cameraVFov = 2.0f * atan((tan(Config::Instance()->FsrHorizontalFov.value() * 0.0174532925199433f) * 0.5f) /
1283-
(float) feature->TargetHeight() * (float) feature->TargetWidth());
1285+
if (cfg.FsrVerticalFov.has_value())
1286+
cameraVFov = GetRadiansFromDeg(cfg.FsrVerticalFov.value());
1287+
else if (cfg.FsrHorizontalFov.value_or_default() > 0.0f)
1288+
{
1289+
const float hFovRad = GetRadiansFromDeg(cfg.FsrHorizontalFov.value());
1290+
cameraVFov =
1291+
GetVerticalFovFromHorizontal(hFovRad, (float) feature->TargetWidth(), (float) feature->TargetHeight());
1292+
}
12841293
else
1285-
cameraVFov = 1.0471975511966f;
1294+
cameraVFov = GetRadiansFromDeg(60);
12861295
}
12871296

1288-
if (!Config::Instance()->FsrUseFsrInputValues.value_or_default())
1289-
InParameters->Get("FSR.viewSpaceToMetersFactor", &meterFactor);
1297+
if (!cfg.FsrUseFsrInputValues.value_or_default())
1298+
ngxParams.Get(OptiKeys::FSR_ViewSpaceToMetersFactor, &meterFactor);
12901299

12911300
State::Instance().lastFsrCameraFar = cameraFar;
12921301
State::Instance().lastFsrCameraNear = cameraNear;

0 commit comments

Comments
 (0)