feat(tracy): add additional zones#1760
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds a new Feature::ForEachLoadedFeature(template) to centralize iteration over loaded features with optional Tracy profiling, and updates multiple call sites to use it. Adds and adjusts profiling zones in Deferred.cpp, State.cpp, ShaderCache.cpp, and ScreenSpaceShadows.cpp; no exported API changes. Changes
Sequence Diagram(s)(omitted — changes are internal iteration and profiling refactors without a multi-component sequential flow that benefits from a diagram) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related issues
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
There was a problem hiding this comment.
Pull request overview
This PR refactors Tracy profiling integration by introducing a new ForEachLoadedFeature helper method that adds automatic CPU profiling zones when iterating over loaded features. It also fixes incorrect Tracy context references in Deferred.cpp and simplifies profiling code in ScreenSpaceShadows.cpp.
Changes:
- Introduced
Feature::ForEachLoadedFeature()template method that automatically adds Tracy CPU profiling zones - Replaced manual feature iteration loops with the new helper in XSEPlugin.cpp, State.cpp, and Deferred.cpp
- Added profiling zones to State::Draw() and ShaderCache compilation
- Fixed incorrect
globals::game::graphicsState->tracyCtxreferences to useglobals::state->tracyCtxin Deferred.cpp
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Feature.h | Added new ForEachLoadedFeature() template method with Tracy CPU profiling support |
| src/XSEPlugin.cpp | Replaced manual feature iteration with ForEachLoadedFeature() for PostPostLoad, DataLoaded, and Load methods |
| src/State.cpp | Replaced manual feature iteration with ForEachLoadedFeature() for Reset and SetupResources; added profiling zones to Draw() |
| src/ShaderCache.cpp | Added Tracy profiling zones to shader compilation tasks |
| src/Features/ScreenSpaceShadows.cpp | Simplified Tracy profiling code by removing unnecessary string formatting |
| src/Deferred.cpp | Fixed incorrect tracyCtx references and replaced manual feature iteration with ForEachLoadedFeature() |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Features/ScreenSpaceShadows.cpp (1)
103-108: PR metadata: conventional title + issue keywordThe PR title isn’t in conventional-commit format. Consider something like
fix(tracing): standardize tracy zones(≤50 chars). If this fixes a bug, add a keyword reference (e.g.,Fixes#123``). As per coding guidelines, ...
🧹 Nitpick comments (2)
src/ShaderCache.cpp (1)
2648-2650: MissingTRACY_ENABLEpreprocessor guards.The Tracy macros
ZoneScopedandZoneTextare used without#ifdef TRACY_ENABLEguards. While Tracy macros typically expand to no-ops when profiling is disabled at compile time, for consistency with the pattern established inFeature.h(lines 192-203), consider adding explicit guards.Suggested fix
void ShaderCompilationTask::Perform() const { +#ifdef TRACY_ENABLE ZoneScoped; ZoneText(GetString().c_str(), GetString().size()); +#endif if (shaderClass == ShaderClass::Vertex) {src/Feature.h (1)
187-206: Well-designed centralized feature iteration with profiling support.The
ForEachLoadedFeaturetemplate provides a clean abstraction for iterating over loaded features with optional Tracy profiling. The use ofthread_localforzoneNameis appropriate for minimizing allocations in multi-threaded scenarios.Minor nit: There's an extra blank line at line 202 before
#endif.Optional formatting fix
`#else` callback(feature); - `#endif` } },
|
✅ A pre-release build is available for this PR: |
Summary by CodeRabbit
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.