Conversation
WalkthroughThis change integrates RenderDoc frame debugging capabilities into the application. It introduces the RenderDoc API header and implements a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MenuUI
participant RenderDoc
participant State
participant Overlay
participant Plugin
User->>MenuUI: Open Advanced Settings
MenuUI->>State: Get frameDebugging flag
MenuUI->>RenderDoc: Query availability/status
MenuUI->>User: Show frame debugging controls and status
User->>MenuUI: Click "Create Capture"
MenuUI->>RenderDoc: TriggerCapture()
RenderDoc->>RenderDoc: Perform capture if available
Plugin->>State: Load()
State->>Plugin: Return frameDebugging flag
Plugin->>RenderDoc: Initialize() if frameDebugging enabled
Overlay->>RenderDoc: Query status/disk usage
Overlay->>User: Show warnings, capture info, and performance impact
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 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
Documentation and Community
|
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/XSEPlugin.cpp (1)
177-180: Consider adding error handling for RenderDoc initialization.While the initialization is correctly placed and gated by the
frameDebuggingflag, consider handling potential initialization failures to prevent crashes if the RenderDoc DLL is missing or incompatible.// Initialize RenderDoc DLL for debugging, which automatically hooks the game if (state->frameDebugging) - RenderDoc::GetSingleton()->Initialize(); + if (!RenderDoc::GetSingleton()->Initialize()) { + logger::warn("Failed to initialize RenderDoc - frame debugging will be disabled"); + state->frameDebugging = false; + }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package/SKSE/Plugins/Renderdoc/renderdoc.dllis excluded by!**/*.dll
📒 Files selected for processing (9)
include/Renderdoc/renderdoc_app.h(1 hunks)package/SKSE/Plugins/Renderdoc/LICENSE.md(1 hunks)src/FrameAnnotations.cpp(8 hunks)src/Menu.cpp(6 hunks)src/RenderDoc.cpp(1 hunks)src/RenderDoc.h(1 hunks)src/State.cpp(3 hunks)src/State.h(1 hunks)src/XSEPlugin.cpp(2 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-06-24T07:17:36.604Z
Learning: When reviewing PRs, always clarify the scope if there are multiple related features or dependencies. WeatherPicker was a separate PR that was already merged, while this PR focuses specifically on WetnessEffects climate preset system enhancements.
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-07-05T05:20:45.823Z
Learning: In the skyrim-community-shaders repository, file deletion error handling improvements that replace existence checks and try-catch blocks with std::filesystem::remove error-code-based approaches are considered bug fixes rather than refactoring, as they address inadequate error handling and misleading log messages.
package/SKSE/Plugins/Renderdoc/LICENSE.md (1)
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-07-05T05:20:45.823Z
Learning: In the skyrim-community-shaders repository, file deletion error handling improvements that replace existence checks and try-catch blocks with std::filesystem::remove error-code-based approaches are considered bug fixes rather than refactoring, as they address inadequate error handling and misleading log messages.
src/State.cpp (1)
Learnt from: davo0411
PR: #1070
File: src/State.cpp:79-83
Timestamp: 2025-05-30T11:44:15.542Z
Learning: In the Skyrim Community Shaders project, the smoothDrawCalls array in the State class is declared as type double in src/State.h, which is the correct floating-point type for performing exponential moving average smoothing calculations.
src/XSEPlugin.cpp (1)
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-07-01T18:01:07.079Z
Learning: In the skyrim-community-shaders project, simple scalar constants in HLSL shaders use #define (e.g., #define NTHREADS 128), while more complex constants use static const within namespaces (e.g., Math namespace in Math.hlsli). For epsilon standardization, #define is the appropriate choice since epsilon values are simple scalar constants.
src/FrameAnnotations.cpp (5)
Learnt from: alandtse
PR: #577
File: src/Features/WetnessEffects.h:36-36
Timestamp: 2025-06-08T11:25:14.536Z
Learning: In the skyrim-community-shaders project, boolean flags in C++ structs that interface with HLSL shaders use uint type instead of bool for compatibility reasons. This ensures consistent size, alignment, and cross-platform compatibility when passing data to shader constant buffers.
Learnt from: alandtse
PR: #577
File: src/Features/WetnessEffects.h:36-36
Timestamp: 2025-06-08T11:25:14.536Z
Learning: In the skyrim-community-shaders project, boolean flags in C++ structs that interface with HLSL shaders use uint type instead of bool for compatibility reasons. This ensures consistent 4-byte size, proper 16-byte alignment in constant buffers, and cross-platform compatibility when passing data between C++ and HLSL shaders.
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-07-05T05:20:45.823Z
Learning: In the skyrim-community-shaders repository, file deletion error handling improvements that replace existence checks and try-catch blocks with std::filesystem::remove error-code-based approaches are considered bug fixes rather than refactoring, as they address inadequate error handling and misleading log messages.
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-07-01T18:01:07.079Z
Learning: In the skyrim-community-shaders project, simple scalar constants in HLSL shaders use #define (e.g., #define NTHREADS 128), while more complex constants use static const within namespaces (e.g., Math namespace in Math.hlsli). For epsilon standardization, #define is the appropriate choice since epsilon values are simple scalar constants.
Learnt from: davo0411
PR: #1070
File: src/State.cpp:79-83
Timestamp: 2025-05-30T11:44:15.542Z
Learning: In the Skyrim Community Shaders project, the smoothDrawCalls array in the State class is declared as type double in src/State.h, which is the correct floating-point type for performing exponential moving average smoothing calculations.
src/Menu.cpp (1)
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-07-05T05:20:45.823Z
Learning: In the skyrim-community-shaders repository, file deletion error handling improvements that replace existence checks and try-catch blocks with std::filesystem::remove error-code-based approaches are considered bug fixes rather than refactoring, as they address inadequate error handling and misleading log messages.
🔇 Additional comments (9)
include/Renderdoc/renderdoc_app.h (1)
1-742: LGTM! Standard RenderDoc API header.This is the official RenderDoc in-application API header (v1.6.0) which is appropriate for integrating frame capture functionality.
src/State.h (1)
140-140: LGTM! Appropriate rename for RenderDoc integration.The rename from
frameAnnotationstoframeDebuggingbetter reflects the expanded functionality with RenderDoc integration.package/SKSE/Plugins/Renderdoc/LICENSE.md (1)
1-26: LGTM! Proper licensing for RenderDoc integration.The MIT license file appropriately covers the RenderDoc components with current copyright information.
src/FrameAnnotations.cpp (1)
14-14: LGTM! Consistent flag renaming throughout.All occurrences of
frameAnnotationshave been correctly replaced withframeDebuggingto match the State.h rename.Also applies to: 33-33, 75-75, 153-153, 265-265, 284-284, 313-313, 927-927
src/XSEPlugin.cpp (1)
7-7: LGTM! Header include for RenderDoc integration.src/State.cpp (1)
109-109: LGTM!The renaming from
frameAnnotationstoframeDebuggingis consistently applied across all usages in this file.Also applies to: 239-239, 372-372
src/RenderDoc.h (1)
1-1: Add header guards or pragma onceThe header file is missing include guards to prevent multiple inclusions.
Add this at the beginning of the file:
+#pragma once + #include <d3d11.h>⛔ Skipped due to learnings
Learnt from: soda3000 PR: doodlum/skyrim-community-shaders#1123 File: src/Menu.cpp:1407-1408 Timestamp: 2025-06-09T22:27:55.011Z Learning: When checking for missing constants or definitions in C++ code, always verify both the source file (.cpp) and the corresponding header file (.h) before flagging missing definitions, as constants and static members are typically declared in header files.Learnt from: alandtse PR: doodlum/skyrim-community-shaders#0 File: :0-0 Timestamp: 2025-07-01T18:01:07.079Z Learning: In the skyrim-community-shaders project, simple scalar constants in HLSL shaders use #define (e.g., #define NTHREADS 128), while more complex constants use static const within namespaces (e.g., Math namespace in Math.hlsli). For epsilon standardization, #define is the appropriate choice since epsilon values are simple scalar constants.src/Menu.cpp (2)
1510-1557: Well-implemented frame debugging UIThe UI properly handles all RenderDoc states with appropriate user feedback and controls.
1723-1809: Effective performance impact warningsThe overlay warnings ensure users are aware of RenderDoc's severe performance impact in all UI states.
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
|
Instead of jamming this into menu, which I'm in the middle of refactoring since it's unmaintainable, why not make this a core feature? |
Hrmm, we can but I was actually modifying the original frame annotations setting that was placed here, this is just an extension of that. |
|
Just realized the CI was broken at the time. Can you pull from dev (merge is ok) and see if the ci has any issues? |
|
Please resolve conflicts |
alandtse
changed the title
- Add RenderDoc wrapper class (RenderDoc.h/cpp) and renderdoc_app.h API header - Rename frameAnnotations to frameDebugging throughout codebase - Add RenderDoc UI integration to AdvancedSettingsRenderer - Add RenderDoc warnings to overlay renderer - Initialize RenderDoc in XSEPlugin when frame debugging is enabled - Include RenderDoc binary (renderdoc.dll) and LICENSE in package - RenderDoc captures saved to Data/SKSE/Plugins/CommunityShaders/Captures/ Co-authored-by: alandtse <7086117+alandtse@users.noreply.github.com>
4 participants
File->Attach to Running Instanceunder localhost.Capture API supports adding comments which bake text in the capture. In the future we may want to add comments with important information to help us diagnose issues.
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Improvements
Documentation