Skip to content

Plat 9853 resource descriptor set removal - #90

Draft
JohnGreenFC wants to merge 3 commits into
mainfrom
PLAT-9853-resource-descriptor-set-removal-2
Draft

Plat 9853 resource descriptor set removal#90
JohnGreenFC wants to merge 3 commits into
mainfrom
PLAT-9853-resource-descriptor-set-removal-2

Conversation

@JohnGreenFC

@JohnGreenFC JohnGreenFC commented Sep 11, 2026

Copy link
Copy Markdown

Summary

We have a auto register resource leak when you jump between various scenes in the game, resources that are auto registered on the GPU can "leak" and keep references around that stop it from being cleaned up resulting in larger 1GB+ memory leaks depending on the scenario. This task removed the descriptor set system that has been causing the issue and we now directly bind resources every frame.

AI assistance disclosure

AI used to take old first pass of this task that became out dated and update it based on recent trinity changes.

Type of change

  • [x ] Bug fix
  • New feature
  • Refactor / cleanup (no behaviour change)
  • Documentation
  • Build, CI, or tooling
  • Breaking change (public API or ABI)
  • Other (describe below)

Linked issue (optional)

Plat 9853

What changed

  • Removed resource descriptor sets from DX11, DX12 and Metal.
  • Added direct resource bindings for all GPU objects in replacement of resource descriptor sets.

Testing

I ran probe tests and I'm game test to confirm things look correct and that the memory leak is now gone.

Platforms tested

  • Windows
  • [ x] macOS
  • Not applicable

Checklist

  • [ x] I've read CONTRIBUTING.md.
  • [ x] My commits follow the commit-message style described there.
  • [ x] I've added or updated tests where it made sense.
  • [ x] I've updated docs / inline API comments for any behaviour change.
  • [ x] My CLA / ICLA is signed (the bot will let you know if it isn't).

@JohnGreenFC
WIP PLAT-9853: direct resource bindings (preliminary, pre-perf-tuning…
70f3ad1 
… checkpoint)
@JohnGreenFC
PLAT-9853: Metal direct resource bindings replace resource sets
bedc341
@JohnGreenFC
Skip invalid sampler registers in static resource bindings
f09a2a9 
Bindless sampler handles appear as fake sampler registers (100+) in the
effect description. Binding them failed and stopped the remaining static
bindings, including the bindless texture heap view, from being applied,
so shaders like specialfx/ubershader read the wrong textures.
Copilot AI lite review requested due to automatic review settings September 11, 2026 10:33
Copilot AI reviewed Sep 11, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Unresolved critical binding, indexing, transition, UAV, and material-binding issues remain, along with moderate Metal resource-lifetime issues.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Removes GPU descriptor sets and migrates DX11, DX12, and Metal rendering paths to direct per-frame resource bindings to address scene-transition resource retention.

Changes:

  • Adds direct SRV, UAV, sampler, and static-resource bindings.
  • Updates backend contexts, register maps, APIs, stubs, and state handling.
  • Migrates materials, sprites, ray tracing, upscaling, and tests.
File summaries
File Review focus
trinityal/tests/Rendering.cpp Rendering test migration
trinityal/tests/Raytracing.cpp Ray-tracing test migration
trinityal/tests/Compute.cpp Compute test migration
trinityal/stub/Tr2ShaderProgramALStub.h Stub shader-program API
trinityal/stub/Tr2ResourceSetALStub.h Stub resource-set API removal
trinityal/stub/Tr2ResourceSetALStub.cpp Stub resource-set implementation
trinityal/stub/Tr2RenderContextStub.h Stub render-context API
trinityal/stub/Tr2RenderContextStub.cpp Stub render-context implementation
trinityal/src/upscaling/Tr2Fsr1Upscaling.cpp Direct FSR1 bindings
trinityal/src/Tr2StaticResourceBindingsAL.cpp Static resource-binding support
trinityal/src/Tr2ResourceSetAL.cpp Resource-set implementation removal
trinityal/src/Tr2RegisterMapAL.cpp Register-map implementation and global-index capacity
trinityal/metal/Tr2ShaderProgramALMetal.mm Metal shader-program bindings
trinityal/metal/Tr2ShaderProgramALMetal.h Metal shader-program API
trinityal/metal/Tr2ShaderALMetal.h Metal shader API
trinityal/metal/Tr2RtPipelineStateALMetal.h Metal ray-tracing pipeline API
trinityal/metal/Tr2ResourceSetALMetal.mm Metal resource-set removal
trinityal/metal/Tr2ResourceSetALMetal.h Metal resource-set API
trinityal/metal/Tr2RenderContextMetal.mm Direct Metal bindings; moderate UAV and scene-lifetime findings remain
trinityal/metal/Tr2RenderContextMetal.h Metal render-context API
trinityal/include/upscaling/Tr2Fsr1Upscaling.h FSR1 API
trinityal/include/TrinityAL.h Public rendering API
trinityal/include/Tr2TextureAL.h Texture API
trinityal/include/Tr2StaticResourceBindingsAL.h Static binding API
trinityal/include/Tr2ShaderProgramAL.h Shader-program API
trinityal/include/Tr2SamplerStateAL.h Sampler API
trinityal/include/Tr2RtShaderTableAL.h Ray-tracing shader-table API
trinityal/include/Tr2RtPipelineStateAL.h Ray-tracing pipeline API
trinityal/include/Tr2ResourceSetAL.h Resource-set API removal
trinityal/include/Tr2RegisterMapAL.h Register-map API
trinityal/include/Tr2BufferAL.h Buffer API
trinityal/dx12/util/DescriptorStateCacheDx12.h DX12 descriptor-state cache API
trinityal/dx12/util/DescriptorStateCacheDx12.cpp DX12 descriptor-state cache
trinityal/dx12/upscaling/Tr2XessUpscaling.cpp DX12 XeSS bindings
trinityal/dx12/Tr2TextureALDx12.h DX12 texture API
trinityal/dx12/Tr2TextureALDx12.cpp DX12 texture bindings
trinityal/dx12/Tr2StreamlineALDx12.h DX12 Streamline integration
trinityal/dx12/Tr2ShaderProgramALDx12.h DX12 shader-program API
trinityal/dx12/Tr2SamplerStateALDx12.h DX12 sampler API
trinityal/dx12/Tr2ResourceSetALDx12.h DX12 resource-set API removal
trinityal/dx12/Tr2ResourceSetALDx12.cpp DX12 resource-set implementation
trinityal/dx12/Tr2RenderContextDx12.h DX12 render-context API
trinityal/dx12/Tr2RenderContextDx12.cpp Direct DX12 bindings; critical indexing, transition, and UAV findings remain
trinityal/dx12/Tr2PrimaryRenderContextDx12.cpp Primary DX12 render integration
trinityal/dx12/Tr2BufferALDx12.h DX12 buffer API
trinityal/dx11/Tr2TextureALDx11.h DX11 texture API
trinityal/dx11/Tr2ShaderProgramALDx11.h DX11 shader-program API
trinityal/dx11/Tr2SamplerStateALDx11.h DX11 sampler API
trinityal/dx11/Tr2ResourceSetALDx11.h DX11 resource-set API removal
trinityal/dx11/Tr2ResourceSetALDx11.cpp DX11 resource-set implementation
trinityal/dx11/Tr2RenderContextDx11.h DX11 render-context API
trinityal/dx11/Tr2BufferALDx11.h DX11 buffer API
trinityal/CMakeLists.txt Build configuration
trinity/TriVariable.h Variable API updates
trinity/TriVariable.cpp Variable implementation updates
trinity/Tr2VolumetricsRenderer.cpp Volumetrics binding migration
trinity/Tr2LightManager.cpp Light-manager binding migration
trinity/Sprite2d/Tr2Sprite2dScene.cpp Sprite scene binding migration
trinity/Sprite2d/Tr2Sprite2dDisplayList.h Sprite display-list API
trinity/Sprite2d/Tr2Sprite2dDisplayList.cpp Sprite display-list binding migration
trinity/Shader/Tr2Material.h Material API
trinity/Shader/Tr2Material.cpp Material binding migration; critical override static-binding finding remains
trinity/Shader/Tr2EffectDescription.h Effect-description API
trinity/Shader/Tr2EffectDescription.cpp Effect static-binding setup
trinity/Shader/Tr2Effect.cpp Effect binding migration
trinity/Shader/Parameter/TriVariableParameter.h Variable-parameter API
trinity/Shader/Parameter/TriVariableParameter.cpp Variable-parameter implementation
trinity/Shader/Parameter/TriTextureParameter.h Texture-parameter API
trinity/Shader/Parameter/TriTextureParameter.cpp Texture-parameter implementation
trinity/Shader/Parameter/Tr2TextureAnimationParameter.h Texture-animation parameter API
trinity/Shader/Parameter/Tr2TextureAnimationParameter.cpp Texture-animation implementation
trinity/Shader/Parameter/Tr2RuntimeTextureParameter.h Runtime-texture parameter API
trinity/Shader/Parameter/Tr2RuntimeTextureParameter.cpp Runtime-texture implementation
trinity/Shader/Parameter/Tr2GeometryBufferParameter.h Geometry-buffer parameter API
trinity/Shader/Parameter/Tr2GeometryBufferParameter.cpp Geometry-buffer implementation
trinity/Raytracing/Tr2RaytracingManager.cpp Ray-tracing binding migration
trinity/Raytracing/Tr2RaytracingGeometry.cpp Ray-tracing resource migration
trinity/ITr2EffectValue.h Effect-value API
trinity/Eve/SpaceObject/Children/EveCloudEditableVolume.h Cloud-volume API
trinity/Eve/SpaceObject/Children/EveCloudEditableVolume.cpp Cloud-volume binding migration
trinity/Eve/SpaceObject/Children/EveChildMesh.cpp Child-mesh binding migration
Review details

Suppressed comments (4)

trinityal/dx12/Tr2RenderContextDx12.cpp:1146

  • The direct API accepts an arbitrary mip level, but this path indexes m_uav without checking resource->mip. An out-of-range persisted/runtime UAV mip can read past the vector and crash or corrupt binding state; check the mip against m_uav.size() and use the null UAV when it is invalid.
    trinityal/dx12/Tr2RenderContextDx12.cpp:1202
  • A Sampler::SAMPLER entry can contain an invalid/default Tr2SamplerStateAL, but this branch dereferences m_sampler without checking it. The direct binding API and sampler setup can then crash during UseResourceBindings; guard with IsValid() and bind the null sampler for invalid handles, consistently with the other backends.
    trinityal/metal/Tr2RenderContextMetal.mm:1463
  • GetUAVMetalTexture returns nil for an out-of-range mip, but this code clears missingTextureMask and marks the nil texture range as valid unconditionally. The dummy texture is therefore not installed, leaving a nil UAV bound for an invalid runtime mip; only clear the mask and update the range when the returned texture is non-nil.
    trinityal/metal/Tr2RenderContextMetal.mm:874
  • EndScene now resets only the shader wrapper, but the new pending binding vectors retain Tr2TextureAL/Tr2BufferAL/Tr2SamplerStateAL objects and neither EndScene nor Metal Present calls ResetResourceBindings. If a scene ends without another binding batch, resources from that scene remain strongly referenced by the render context, delaying their release across scene changes and undermining the leak fix. Discard the resource bindings at scene end.
  • Files reviewed: 82/82 changed files
  • Comments generated: 5
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread trinity/Shader/Tr2Material.cpp
Comment on lines +245 to +246
renderContext.ResetResourceBindings();

Comment thread trinityal/dx11/Tr2RenderContextDx11.cpp
Comment on lines +2236 to +2238
if( index < registerMap.srvCount && index < Tr2RegisterMapAL::MAX_RESOURCES_IN_STAGE )
{
m_sortedSRVs[index] = &resource;
Comment thread trinityal/dx12/Tr2RenderContextDx12.cpp
Comment on lines +1031 to +1033
if( index < registerMap.srvCount && index < Tr2RegisterMapAL::MAX_RESOURCES_IN_STAGE )
{
m_sortedSRVs[index] = &resource;
Comment thread trinityal/dx12/Tr2RenderContextDx12.cpp
Comment on lines +1060 to +1070
D3D12_RESOURCE_BARRIER inTransitions[Tr2RegisterMapAL::MAX_RESOURCES_IN_STAGE];
ID3D12Resource* transitioned[Tr2RegisterMapAL::MAX_RESOURCES_IN_STAGE];
uint32_t inCount = 0;
uint32_t transitionedCount = 0;

auto AddTransition = [&]( ID3D12Resource* res, D3D12_RESOURCE_STATES defaultState, D3D12_RESOURCE_STATES expectedState ) {
// TODO: verify state
if( ( defaultState & expectedState ) == 0 && defaultState != D3D12_RESOURCE_STATE_RAYTRACING_ACCELERATION_STRUCTURE )
{
auto found = std::find( transitioned, transitioned + transitionedCount, res );
if( found == transitioned + transitionedCount && transitionedCount < Tr2RegisterMapAL::MAX_RESOURCES_IN_STAGE )
Comment thread trinityal/dx12/Tr2RenderContextDx12.cpp
uint32_t mapIndex = registerMap.uavs[reg.stage][reg.index];
const Resource* resource = mapIndex < Tr2RegisterMapAL::MAX_RESOURCES_IN_STAGE ? m_sortedUAVs[mapIndex] : nullptr;

std::shared_ptr<UnorderedAccessViewDx12> uav;
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@JohnGreenFC