Texture streaming#34
Open
jiayev wants to merge 10 commits into
Open
Conversation
📝 WalkthroughWalkthroughAdds configurable texture streaming via a ChangesTexture Streaming & Async Upload
Sequence DiagramsequenceDiagram
participant Game as Game Engine
participant Hook as CreateTextureAndSRV::thunk
participant TM as TextureManager
participant D3D12 as D3D12 Device
participant Queue as PendingUploadQueue
participant API as LogTextureMemoryStats API
rect rgba(100, 149, 237, 0.5)
note over Hook,D3D12: Streaming texture creation
Game->>Hook: CreateTextureAndSRV(data, desc)
Hook->>Hook: selectStreamingMipBias(ExperimentalSettings, dims)
Hook->>D3D12: CreateCommittedResource(residentMips, residentDims)
Hook->>TM: RegisterResidentMipOffset(resource, mipBias)
Hook->>D3D12: upload resident mips from a_data[mipBias offset]
Hook->>D3D12: createEventQuery + executeCommandList
Hook->>Queue: push(cmdList, eventQuery) for async retirement
end
rect rgba(144, 238, 144, 0.5)
note over TM,API: Descriptor registration and stats logging
Hook->>TM: GetDescriptor(d3d11res, d3d12res, type)
TM->>TM: lookup+erase residentMipOffset from map
TM->>TM: construct TextureReference(texture, descMgr, residentMipOffset)
Game->>API: LogTextureMemoryStats()
API->>TM: LogMemoryStats()
TM-->>API: logger::info(streamed vs standard counts/sizes)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Core/TextureManager.cpp`:
- Around line 97-103: The standardBytes counter is accumulating all textures
without excluding streamed ones, so streamed textures are being counted in both
standard and streamed categories. Modify the code to only add to standardBytes
when the texture is not a streamed texture by adding a condition that excludes
textures where residentMipOffset is greater than 0. This will ensure the
standard and streamed byte counts are mutually exclusive and the log breakdown
is accurate. Apply the same fix to the similar code block referenced at lines
112-119.
- Around line 24-31: The RegisterResidentMipOffset function in
TextureManager.cpp stores raw IUnknown pointers as keys in the
g_ResidentMipOffsets map, but these entries are only removed when GetDescriptor
consumes them. If a registered resource is destroyed without reaching descriptor
creation, the stale pointer key persists in the map. When another COM object
later reuses the same memory address, the old entry will incorrectly apply a
stale mip offset to the new object. Implement proper cleanup by registering a
release callback or using COM object lifecycle tracking (such as implementing
IUnknown::Release wrapping or using weak_ptr alternatives) to remove entries
from g_ResidentMipOffsets when resources are actually destroyed, rather than
deferring cleanup until descriptor consumption.
In `@src/Hooks.cpp`:
- Around line 113-155: The GetTextureStreamingMipBias function determines
texture streaming bias but never uses the TextureBudgetMB setting from
ExperimentalSettings, making the memory budget configuration ineffective.
Integrate TextureBudgetMB into the bias calculation by computing an estimated
memory footprint for the texture at different mip levels, then adjust the
desiredBias upward as needed to keep memory consumption within the configured
budget limit. This adjustment should happen after the initial bias is determined
by TextureStreamingMode but before the final clamping operations, ensuring the
budget constraint overrides mode-based decisions when memory limits would be
exceeded.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a30ea6a0-e526-4e30-869d-897c421e9496
📒 Files selected for processing (10)
AGENTS.mdextern/RTXDI-Librarysrc/API.cppsrc/API.hsrc/Core/MaterialBase.cppsrc/Core/Texture.hsrc/Core/TextureManager.cppsrc/Core/TextureManager.hsrc/Hooks.cppsrc/Types/Settings.h
💤 Files with no reviewable changes (1)
- AGENTS.md
Comment on lines
+24
to
31
| void TextureManager::RegisterResidentMipOffset(IUnknown* resource, uint32_t mipOffset) | ||
| { | ||
| if (!resource || mipOffset == 0) | ||
| return; | ||
|
|
||
| std::scoped_lock lock(g_ResidentMipOffsetsMutex); | ||
| g_ResidentMipOffsets[resource] = mipOffset; | ||
| } |
There was a problem hiding this comment.
Resident mip-offset registry can retain stale pointer entries.
Offsets are only removed on GetDescriptor consumption. If a registered resource never reaches descriptor creation, the raw-pointer key persists and can later be reused by another COM object, applying an incorrect mip offset.
Suggested mitigation
void TextureManager::ReleaseTexture(RE::BSGraphics::Texture* texture)
{
if (!texture)
return;
IUnknown* key = nullptr;
@@
`#endif`
key = texture->texture;
+ {
+ std::scoped_lock lock(g_ResidentMipOffsetsMutex);
+ g_ResidentMipOffsets.erase(key);
+ }
+
m_Textures.erase(key);
m_NormalMaps.erase(key);
}Also applies to: 172-179
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Core/TextureManager.cpp` around lines 24 - 31, The
RegisterResidentMipOffset function in TextureManager.cpp stores raw IUnknown
pointers as keys in the g_ResidentMipOffsets map, but these entries are only
removed when GetDescriptor consumes them. If a registered resource is destroyed
without reaching descriptor creation, the stale pointer key persists in the map.
When another COM object later reuses the same memory address, the old entry will
incorrectly apply a stale mip offset to the new object. Implement proper cleanup
by registering a release callback or using COM object lifecycle tracking (such
as implementing IUnknown::Release wrapping or using weak_ptr alternatives) to
remove entries from g_ResidentMipOffsets when resources are actually destroyed,
rather than deferring cleanup until descriptor consumption.
Comment on lines
+97
to
+103
| standardBytes += texture->size; | ||
|
|
||
| if (texture->residentMipOffset > 0) { | ||
| streamedTextures++; | ||
| streamedBytes += texture->size; | ||
| } | ||
| } |
There was a problem hiding this comment.
standard memory in the log currently includes streamed textures.
This makes the breakdown non-exclusive and easy to misread. Either rename the label or log non-streamed bytes explicitly.
Suggested fix
- uint64_t standardBytes = 0;
+ uint64_t standardBytes = 0;
+ uint64_t nonStreamedBytes = 0;
@@
standardBytes += texture->size;
if (texture->residentMipOffset > 0) {
streamedTextures++;
streamedBytes += texture->size;
+ } else {
+ nonStreamedBytes += texture->size;
}
@@
- "TextureManager - RT texture memory: total={} MiB, standard={} MiB ({} textures), normal={} MiB ({} textures), streamed={} MiB ({} textures)",
+ "TextureManager - RT texture memory: total={} MiB, standard={} MiB ({} textures), normal={} MiB ({} textures), streamed={} MiB ({} textures)",
(standardBytes + normalBytes) / (1024 * 1024),
- standardBytes / (1024 * 1024),
+ nonStreamedBytes / (1024 * 1024),Also applies to: 112-119
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Core/TextureManager.cpp` around lines 97 - 103, The standardBytes counter
is accumulating all textures without excluding streamed ones, so streamed
textures are being counted in both standard and streamed categories. Modify the
code to only add to standardBytes when the texture is not a streamed texture by
adding a condition that excludes textures where residentMipOffset is greater
than 0. This will ensure the standard and streamed byte counts are mutually
exclusive and the log breakdown is accurate. Apply the same fix to the similar
code block referenced at lines 112-119.
Comment on lines
+113
to
+155
| uint32_t GetTextureStreamingMipBias(const ExperimentalSettings& settings, uint32_t width, uint32_t height, uint32_t mipLevels, DXGI_FORMAT format) | ||
| { | ||
| if (settings.TextureStreamingMode == TextureStreamingMode::Off || mipLevels <= 1) | ||
| return 0; | ||
|
|
||
| const uint32_t maxDimension = std::max(width, height); | ||
| if (maxDimension < 1024) | ||
| return 0; | ||
|
|
||
| if (IsCriticalStreamingFormat(format)) | ||
| return 0; | ||
|
|
||
| uint32_t desiredBias = 0; | ||
| const bool alphaCapable = IsAlphaCapableFormat(format); | ||
| switch (settings.TextureStreamingMode) { | ||
| case TextureStreamingMode::Conservative: | ||
| if (alphaCapable) | ||
| return 0; | ||
|
|
||
| desiredBias = maxDimension >= 4096 ? 1 : 0; | ||
| break; | ||
| case TextureStreamingMode::Balanced: | ||
| if (alphaCapable) | ||
| return 0; | ||
|
|
||
| desiredBias = maxDimension >= 4096 ? 2 : 1; | ||
| break; | ||
| case TextureStreamingMode::Aggressive: | ||
| desiredBias = maxDimension >= 4096 ? 3 : (maxDimension >= 2048 ? 2 : 1); | ||
| if (alphaCapable) | ||
| desiredBias = std::min(desiredBias, 1u); | ||
| break; | ||
| default: | ||
| break; | ||
| } | ||
|
|
||
| desiredBias = std::min(desiredBias, settings.TextureMaxMipBias); | ||
|
|
||
| if (!IsBlockCompressedFormat(format)) | ||
| desiredBias = std::min(desiredBias, 1u); | ||
|
|
||
| return std::min(desiredBias, mipLevels - 1); | ||
| } |
There was a problem hiding this comment.
TextureBudgetMB is currently a no-op in streaming decisions.
The streaming bias path reads mode and max bias, but never uses ExperimentalSettings::TextureBudgetMB, so memory budget configuration doesn’t influence residency behavior.
Also applies to: 327-341
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Hooks.cpp` around lines 113 - 155, The GetTextureStreamingMipBias
function determines texture streaming bias but never uses the TextureBudgetMB
setting from ExperimentalSettings, making the memory budget configuration
ineffective. Integrate TextureBudgetMB into the bias calculation by computing an
estimated memory footprint for the texture at different mip levels, then adjust
the desiredBias upward as needed to keep memory consumption within the
configured budget limit. This adjustment should happen after the initial bias is
determined by TextureStreamingMode but before the final clamping operations,
ensuring the budget constraint overrides mode-based decisions when memory limits
would be exceeded.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Chores