feat: performance overlay#1070
Conversation
Drawcallui git fixes
|
Taking to WIP until the keypress is fixed. |
|
Hotkey fixed, ready for merge. |
alandtse
left a comment
alandtse
left a comment
There was a problem hiding this comment.
Still need to know if there was a perf impact when the overlay is hidden. But probably can consider moving this in.
WalkthroughA comprehensive performance overlay system has been added, featuring real-time metrics such as FPS, draw calls, VRAM usage, and frame time graphs. Supporting infrastructure includes new methods for frame timing and draw call tracking, as well as user-configurable overlay display and hotkey toggling. No existing logic was removed. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Menu
participant State
participant Upscaling
participant DX12SwapChain
User->>Menu: Presses overlay toggle hotkey
Menu->>Menu: Toggles performance overlay visibility
loop Every frame (if overlay enabled)
Menu->>State: Query drawCalls and smoothDrawCalls
Menu->>Upscaling: IsFrameGenerationActive()
Upscaling->>DX12SwapChain: GetFrameTime()
DX12SwapChain-->>Upscaling: Frame time (if available)
Upscaling-->>Menu: Frame generation status and timing
Menu->>Menu: Update overlay metrics and graphs
Menu->>User: Display performance overlay window
end
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code Graph Analysis (1)src/Menu.cpp (5)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (4)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Nitpick comments (4)
src/DX12SwapChain.cpp (1)
179-200: Consider thread safety and error handling improvements.The frame time calculation logic is correct and uses appropriate high-resolution timing. However, there are some potential improvements to consider:
- Thread Safety: The static variables could be corrupted if this method is called from multiple threads simultaneously.
- Error Handling:
QueryPerformanceCountercan theoretically fail, but there's no error checking.Consider these improvements:
float DX12SwapChain::GetFrameTime() const { - // Calculate frame time based on swap chain presentation - static float lastPresentTime = 0.0f; - static float frameTime = 1.0f / 60.0f; // Default to 60 fps - static LARGE_INTEGER frequency = {}; - static LARGE_INTEGER currentTime = {}; + // Calculate frame time based on swap chain presentation + static std::mutex timingMutex; + static float lastPresentTime = 0.0f; + static float frameTime = 1.0f / 60.0f; // Default to 60 fps + static LARGE_INTEGER frequency = {}; + + std::lock_guard<std::mutex> lock(timingMutex); + LARGE_INTEGER currentTime = {}; - if (frequency.QuadPart == 0) { - QueryPerformanceFrequency(&frequency); + if (frequency.QuadPart == 0) { + if (!QueryPerformanceFrequency(&frequency)) { + return frameTime; // Return default on failure + } } - QueryPerformanceCounter(¤tTime); + if (!QueryPerformanceCounter(¤tTime)) { + return frameTime; // Return last known value on failure + }src/State.cpp (1)
506-509: Micro-cleanup – std::fill is clearer-for (auto& c : drawCalls) - c = 0; -for (auto& c : smoothDrawCalls) - c = 0; +std::fill(std::begin(drawCalls), std::end(drawCalls), 0); +std::fill(std::begin(smoothDrawCalls), std::end(smoothDrawCalls), 0.f);src/Menu.cpp (2)
1225-1260: Timer logic drifts for largeUpdateInterval
deltaTimeis measured from the previous frame, butlastUpdateTimeis
reset every frame. ConsequentlyupdateTimeraccumulates frame time,
not true elapsed wall-time, and will always be ≤UpdateInterval + dt.
If the game is paused or stalls the overlay stops updating.Consider:
updateTimer += std::chrono::duration<float>( currentTime - lastUpdateTime).count(); lastUpdateTime = currentTime; if (updateTimer >= settings.PerfOverlay.UpdateInterval) { … updateTimer -= settings.PerfOverlay.UpdateInterval; // keep remainder }
1431-1444: Enum-driven table – avoid hard-coding shader namesThe draw-call list is manually duplicated; any new
BSShader::Type
adds maintenance burden.for (int t = 1; t < RE::BSShader::Type::Total; ++t) { ImGui::Text("%s: %d", magic_enum::enum_name(static_cast<RE::BSShader::Type>(t)).data(), int(globals::state->smoothDrawCalls[t])); } ImGui::Text("Total: %d", int(globals::state->smoothDrawCalls[ RE::BSShader::Type::Total]));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/DX12SwapChain.cpp(1 hunks)src/DX12SwapChain.h(1 hunks)src/Menu.cpp(12 hunks)src/Menu.h(4 hunks)src/State.cpp(2 hunks)src/State.h(1 hunks)src/Upscaling.cpp(1 hunks)src/Upscaling.h(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build plugin and addons
🔇 Additional comments (6)
src/DX12SwapChain.h (1)
97-98: Clean method declaration with good documentation.The
GetFrameTime()method declaration is well-structured with appropriate const correctness and a clear comment explaining its purpose for frame generation FPS measurement.src/State.h (1)
47-48: Good addition of draw call tracking arrays.The
smoothDrawCallsanddrawCallsarrays are appropriately typed for tracking and smoothing draw call statistics per shader type. The sizing withRE::BSShader::Type::Total + 1suggests proper indexing by shader type with an additional slot.src/Upscaling.h (1)
50-52: Well-designed frame generation query methods.The
IsFrameGenerationActive()andGetFrameGenerationFrameTime()methods have appropriate signatures with const correctness. The return types (bool and float) are suitable for their respective purposes in the performance overlay.src/State.cpp (1)
88-91: Bounds-safety fordrawCallsindex
currentShader->shaderType.get()is blindly used as an array index.
Although engine enums are usually contiguous, a corrupted/unknown shader could supply a value ≥RE::BSShader::Type::Total, causing UB. A simple guard avoids that:if (currentShader) { auto idx = currentShader->shaderType.get(); if (idx > 0 && idx < RE::BSShader::Type::Total) ++drawCalls[idx]; }src/Upscaling.cpp (1)
762-781: Guard against zero refresh-rate / missing swap-chain
GetFrameGenerationFrameTime()returns 0 on failure, but callers use the value to compute
1000 / frameTimewhich will divide by zero.
Consider returningstd::optional<float>or at least guarding downstream computations:auto fgTime = globals::upscaling->GetFrameGenerationFrameTime(); if (fgTime > 0.f) { /* safe maths */ }src/Menu.cpp (1)
1148-1180: Style-stack balance & leak preventionTwo style vars (
WindowBorderSize,ItemSpacing) and one color are pushed.
PopStyleVar()is called twice at 1476 & 1480, andPopStyleColor()once at 1481 – good.However, an early
return;before the Pops (e.g. ifImGui::Begin()fails)
would leak the style stack. Wrap drawing in a RAII helper or restructure:if (!ImGui::Begin("Performance Overlay", …)) { ImGui::PopStyleVar(2); ImGui::PopStyleColor(); return; }
|
✅ A pre-release build is available for this PR: |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/Menu.cpp (1)
1490-1614: Well-structured settings UI with minor suggestion.The performance overlay settings UI is well-organized with clear sections and helpful tooltips. One minor suggestion:
Consider adding validation for the update interval to prevent extremely small values that could cause performance issues:
if (ImGui::SliderFloat("Update Interval", &settings.PerfOverlay.UpdateInterval, 0.001f, 2.0f, "%.2f seconds")) { settings.PerfOverlay.UpdateInterval = std::max(0.001f, settings.PerfOverlay.UpdateInterval); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Menu.cpp(12 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build plugin and addons
🔇 Additional comments (6)
src/Menu.cpp (6)
35-53: LGTM! Comprehensive settings serialization.The JSON serialization for
PerfOverlaySettingsis well-structured and includes all necessary configuration options for the performance overlay feature.
235-237: Good practice: Using float literals.Adding the 'f' suffix to floating-point literals is good practice in C++ as it avoids implicit double-to-float conversions and makes the intended type explicit.
Also applies to: 276-276, 679-679
999-1002: Clean feature integration.The Performance Overlay is properly integrated into the display features list alongside the existing upscaling feature.
1448-1480: Good fix: Proper VRAM error handling.The VRAM usage section now properly addresses the division-by-zero concern from previous reviews by checking both
SUCCEEDED(hr)andvideoMemoryInfo.Budget > 0before performing calculations. The fallback message for unavailable VRAM info is also appropriate.
1931-1933: Consistent hotkey implementation.The overlay toggle key handling follows the same pattern as other hotkeys in the system and is properly integrated into the input processing logic.
Also applies to: 1948-1949
1117-1133: Good consistency improvements in test mode.The test mode timing logic maintains consistency with the float literal improvements and the logic for alternating between test and user configurations appears correct.
|
✅ A pre-release build is available for this PR: |
|
✅ A pre-release build is available for this PR: |
|
I'm still waiting for an answer to my last question to approve and merge this. |
I'm sorry i missed the notification, I'll set github up on my phone so i see it first before asking on discord in future. |
Co-authored-by: Exist <Exist@Yes> Co-authored-by: ThePagi <ThePagi@users.noreply.github.com> Co-authored-by: davo0411 <davo0411@users.noreply.github.com>
3 participants
Adds a performance overlay to CS.
Tracks pre and post FG FPS, Frametime, with graphs.
Tracks draw calls, code from Exist.
Tracks vram usage.
Has various settings for visuals and size. All options toggleable. Can make it a small fps counter, or a full info overlay. Overlay can move around the screen.
(New pr with cleaned commits)
Summary by CodeRabbit
New Features
Enhancements