feat(ui): UI flair, better themes & settings, font support #1530
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughAdds a full theming system: many JSON theme presets, ThemeManager with discovery/load/save/validation, font-role system and catalog, background blur (GPU shaders + pipeline), UI font/style integration, path helpers, font license files, and assorted UI/overlay/rendering updates. All major changes are configuration, UI, font, or rendering support. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Plugin as XSEPlugin
participant State as State
participant Menu as Menu
participant TM as ThemeManager
Note over Plugin: Plugin load sequence
Plugin->>State: Load()
Plugin->>State: LoadTheme()
State->>TM: LoadTheme(SettingsTheme.json)
TM-->>State: theme JSON
Plugin->>Menu: CreateDefaultThemes()
Menu->>TM: CreateDefaultThemeFiles()
Plugin->>TM: DiscoverThemes()
TM-->>Plugin: theme list
Note over Menu,TM: First ImGui init
Menu->>TM: ForceApplyDefaultTheme()
Menu->>TM: SetupImGuiStyle(theme)
TM-->>Menu: style applied
sequenceDiagram
autonumber
participant User as User
participant Menu as Menu
participant TM as ThemeManager
Note over User,Menu: User selects or creates theme preset
User->>Menu: LoadThemePreset(name)
Menu->>TM: LoadTheme(name, out json)
TM-->>Menu: theme data
Menu->>Menu: BuildFontSignature(baseSize)
Menu->>TM: ReloadFont(per-role settings)
TM-->>Menu: fonts loaded
Menu->>TM: SetupImGuiStyle(theme)
TM-->>Menu: style applied
Menu->>State: SaveTheme()
sequenceDiagram
autonumber
participant Overlay as OverlayRenderer
participant Menu as Menu
participant TM as ThemeManager
Note over Overlay: Per-frame overlay work
Overlay->>Menu: BuildFontSignature(currentSize)
Menu-->>Overlay: signature
Overlay->>Overlay: compare signature/size with cached
alt changed
Overlay->>TM: ReloadFont(...)
TM-->>Overlay: fonts updated
end
Overlay->>TM: RenderBackgroundBlur()
TM-->>Overlay: blurred texture
Overlay->>Overlay: render ImGui draw data
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
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: 9
🧹 Nitpick comments (14)
src/XSEPlugin.cpp (1)
165-171: Consider ordering: create/discover themes before loading user theme.If LoadTheme depends on default/theme files being present or registered, call CreateDefaultThemes() and DiscoverThemes() first, then LoadTheme. Also wrap in a try/catch to avoid aborting load on non-critical theme issues.
Apply this diff:
- state->Load(); - state->LoadTheme(); // Load theme settings from SettingsTheme.json - - // Initialize theme system - create default themes and discover existing ones - globals::menu->CreateDefaultThemes(); // Creates JSON files if they don't exist - auto themeManager = ThemeManager::GetSingleton(); - themeManager->DiscoverThemes(); // Discover all available themes + state->Load(); + try { + // Initialize theme system - ensure defaults exist, discover, then load user theme + globals::menu->CreateDefaultThemes(); // Creates JSON files if they don't exist + auto themeManager = ThemeManager::GetSingleton(); + themeManager->DiscoverThemes(); // Discover all available themes + state->LoadTheme(); // Load theme settings from SettingsTheme.json + } catch (const std::exception& e) { + logger::warn("Theme initialization failed: {}", e.what()); + }src/Utils/FileSystem.cpp (1)
76-85: Theme path helpers follow existing conventions; consider centralizing file/dir names.Paths look correct. Optional: define shared constexprs for "SettingsTheme.json" and "Themes" to avoid string drift across callers.
src/Menu/ThemeManager.h (1)
9-10: Avoid global using-alias in headersExposing using json = nlohmann::json; in the global namespace of a public header can leak into all includers and cause collisions.
Apply this diff:
-using json = nlohmann::json; +// Prefer fully-qualified nlohmann::json in declarations, or move alias inside the class: +// using json = nlohmann::json; // if you keep it, place it inside ThemeManagerAnd update declarations to use nlohmann::json (or move the alias inside ThemeManager).
src/Utils/UI.cpp (4)
24-25: Duplicate includestd::format is included twice.
Apply this diff:
-#include <format> #include <format>
14-17: Duplicate Menu.h includeMenu.h is included both as "Menu.h" and "../Menu.h". Keep only one to avoid fragile include path dependencies.
Apply this diff:
#include "UI.h" -#include "Menu.h" #include "Menu.h" @@ -#include "../Menu.h"Adjust to whichever include path is canonical in this project.
1335-1387: Make ButtonWithFlash ID-safe and clamp flash colors
- Keying by label risks cross-window collisions. Use ImGuiID to align with ImGui’s ID stack.
- Brightening by +0.2 can exceed 1.0; clamp to [0,1] to avoid overflow artifacts.
Apply this diff:
-bool ButtonWithFlash(const char* label, const ImVec2& size, int flashDurationMs) +bool ButtonWithFlash(const char* label, const ImVec2& size, int flashDurationMs) { - static std::unordered_map<std::string, std::chrono::steady_clock::time_point> flashTimers; - - std::string buttonId = std::string(label); + static std::unordered_map<ImGuiID, std::chrono::steady_clock::time_point> flashTimers; + // Use the same ID that ImGui will use for the Button in this context. + ImGuiID buttonId = ImGui::GetID(label); auto now = std::chrono::steady_clock::now(); @@ - auto it = flashTimers.find(buttonId); + auto it = flashTimers.find(buttonId); @@ - // Style the button with flash effect if active. + // Style the button with flash effect if active. bool styleChanged = false; if (hasActiveFlash) { // Use subtle white overlay similar to action icon hover effect ImVec4 normalButton = ImGui::GetStyleColorVec4(ImGuiCol_Button); - ImVec4 flashColor = ImVec4( - normalButton.x + 0.2f, // Brighten slightly - normalButton.y + 0.2f, - normalButton.z + 0.2f, - normalButton.w - ); - ImVec4 flashHovered = ImVec4(flashColor.x * 1.1f, flashColor.y * 1.1f, flashColor.z * 1.1f, flashColor.w); - ImVec4 flashActive = ImVec4(flashColor.x * 0.9f, flashColor.y * 0.9f, flashColor.z * 0.9f, flashColor.w); + auto clamp01 = [](float v) { return std::clamp(v, 0.0f, 1.0f); }; + ImVec4 flashColor{ + clamp01(normalButton.x + 0.2f), + clamp01(normalButton.y + 0.2f), + clamp01(normalButton.z + 0.2f), + normalButton.w + }; + ImVec4 flashHovered{ + clamp01(flashColor.x * 1.1f), + clamp01(flashColor.y * 1.1f), + clamp01(flashColor.z * 1.1f), + flashColor.w + }; + ImVec4 flashActive{ + clamp01(flashColor.x * 0.9f), + clamp01(flashColor.y * 0.9f), + clamp01(flashColor.z * 0.9f), + flashColor.w + }; @@ - if (clicked) { - flashTimers[buttonId] = now; - } + if (clicked) { + flashTimers[buttonId] = now; + }
1214-1334: Leverage AdjustColorForContrast in contrast-aware widgetsOnce AdjustColorForContrast is implemented, consider using it in ContrastSelectable and ContrastSelectableWithColor to refine color adjustments beyond binary black/white fallbacks.
Example:
- ImVec4 selectedTextColor = GetContrastingTextColor(selectedBgColor, 0.5f); + ImVec4 selectedTextColor = AdjustColorForContrast(style.Colors[ImGuiCol_Text], selectedBgColor, 3.0f);src/Menu/ThemeManager.cpp (3)
95-99: Remove unused lambda to avoid dead code.getContrastingTextColor is defined but never used.
- auto getContrastingTextColor = [&](const ImVec4& bgColor) { - float luminance = calculateLuminance(bgColor); - return luminance > 0.5f ? ImVec4(0.0f, 0.0f, 0.0f, 1.0f) : ImVec4(1.0f, 1.0f, 1.0f, 1.0f); - };
128-133: Use proper selection highlight alpha; current values are fully opaque.ImGuiCol_TextSelectedBg is the selection background color, not the text color. Setting it to fully opaque black/white can harm readability. Prefer a semi‑transparent highlight.
- if (calculateLuminance(colors[ImGuiCol_HeaderActive]) > 0.5f) { - colors[ImGuiCol_TextSelectedBg] = ImVec4(0.0f, 0.0f, 0.0f, 1.0f); // Black text on light selection - } else { - colors[ImGuiCol_TextSelectedBg] = ImVec4(1.0f, 1.0f, 1.0f, 1.0f); // White text on dark selection - } + const float highlightAlpha = 0.35f; + if (calculateLuminance(colors[ImGuiCol_HeaderActive]) > 0.5f) { + colors[ImGuiCol_TextSelectedBg] = ImVec4(0.0f, 0.0f, 0.0f, highlightAlpha); + } else { + colors[ImGuiCol_TextSelectedBg] = ImVec4(1.0f, 1.0f, 1.0f, highlightAlpha); + }
145-152: Avoid non-atomic static guard for reentrancy.isReloading is a non-atomic static; if ReloadFont is ever called off-thread, this can race. If single-threaded by design, add a comment. Otherwise, use std::atomic_bool.
src/Menu.cpp (4)
35-37: Remove duplicate include.Util.h is included twice.
#include "TruePBR.h" #include "Util.h" -#include "Util.h" #include "Utils/UI.h"
174-186: Avoid operator[] on input JSON to prevent accidental insertion.operator[] will create a "Theme" key if missing. Use contains()/find() to avoid mutating input.
void Menu::LoadTheme(json& o_json) { - if (o_json["Theme"].is_object()) { - settings.Theme = o_json["Theme"]; + if (o_json.contains("Theme") && o_json["Theme"].is_object()) { + settings.Theme = o_json.at("Theme"); // Validate the loaded font and fallback to default if necessary if (!Util::ValidateFont(settings.Theme.FontName)) { logger::warn("Font '{}' not found, falling back to default font '{}'", settings.Theme.FontName, ThemeSettings{}.FontName); settings.Theme.FontName = ThemeSettings{}.FontName; } } }
281-285: Optional: pre-check font file existence for clearer logs.Add an exists() check before AddFontFromFileTTF to log path issues explicitly.
- if (!imgui_io.Fonts->AddFontFromFileTTF(fontPath.string().c_str(), - std::round(fontSize), &font_config)) { - logger::warn("Menu::Init() - Failed to load custom font. Using default font."); + if (!std::filesystem::exists(fontPath) || + !imgui_io.Fonts->AddFontFromFileTTF(fontPath.string().c_str(), std::round(fontSize), &font_config)) { + logger::warn("Menu::Init() - Failed to load custom font '{}'. Using default font.", fontPath.string()); imgui_io.Fonts->AddFontDefault(); }
531-538: Avoid relying on const_cast inside ReloadFont; update cache here.Set cachedFontName locally when applying pending font, and remove the const_cast in ThemeManager::ReloadFont in a follow-up.
if (pendingFontReload) { pendingFontReload = false; settings.Theme.FontName = pendingFontName; + cachedFontName = pendingFontName; ThemeManager::ReloadFont(*this, cachedFontSize); pendingFontName.clear(); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
package/SKSE/Plugins/CommunityShaders/Themes/Amber.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/Default.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/DragonBlood.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/DwemerBronze.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/Forest.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/HighContrast.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/Light.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/Mystic.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/NordicFrost.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/Ocean.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/README.md(1 hunks)src/Menu.cpp(11 hunks)src/Menu.h(4 hunks)src/Menu/FeatureListRenderer.cpp(3 hunks)src/Menu/HomePageRenderer.cpp(0 hunks)src/Menu/MenuHeaderRenderer.cpp(2 hunks)src/Menu/OverlayRenderer.cpp(1 hunks)src/Menu/SettingsTabRenderer.cpp(6 hunks)src/Menu/SettingsTabRenderer.h(1 hunks)src/Menu/ThemeManager.cpp(2 hunks)src/Menu/ThemeManager.h(3 hunks)src/SettingsOverrideManager.cpp(3 hunks)src/SettingsOverrideManager.h(0 hunks)src/State.cpp(4 hunks)src/State.h(2 hunks)src/Utils/FileSystem.cpp(2 hunks)src/Utils/FileSystem.h(2 hunks)src/Utils/UI.cpp(7 hunks)src/Utils/UI.h(2 hunks)src/XSEPlugin.cpp(2 hunks)
💤 Files with no reviewable changes (2)
- src/Menu/HomePageRenderer.cpp
- src/SettingsOverrideManager.h
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{cpp,cxx,cc,c,h,hpp,hxx,hlsl,hlsli,fx,fxh,py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not include TODO/FIXME placeholders; provide complete, working solutions
Files:
src/Menu/SettingsTabRenderer.hsrc/Menu/OverlayRenderer.cppsrc/State.hsrc/State.cppsrc/SettingsOverrideManager.cppsrc/Menu/SettingsTabRenderer.cppsrc/Utils/FileSystem.cppsrc/XSEPlugin.cppsrc/Menu.hsrc/Menu/ThemeManager.hsrc/Utils/FileSystem.hsrc/Menu.cppsrc/Utils/UI.hsrc/Menu/MenuHeaderRenderer.cppsrc/Menu/FeatureListRenderer.cppsrc/Utils/UI.cppsrc/Menu/ThemeManager.cpp
src/**/*.{cpp,cxx,cc,h,hpp,hxx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{cpp,cxx,cc,h,hpp,hxx}: Ensure SE/AE/VR runtime compatibility; use runtime detection patterns (e.g., REL::RelocateMember())
Include robust error handling and resource management with graceful degradation in the plugin code
Files:
src/Menu/SettingsTabRenderer.hsrc/Menu/OverlayRenderer.cppsrc/State.hsrc/State.cppsrc/SettingsOverrideManager.cppsrc/Menu/SettingsTabRenderer.cppsrc/Utils/FileSystem.cppsrc/XSEPlugin.cppsrc/Menu.hsrc/Menu/ThemeManager.hsrc/Utils/FileSystem.hsrc/Menu.cppsrc/Utils/UI.hsrc/Menu/MenuHeaderRenderer.cppsrc/Menu/FeatureListRenderer.cppsrc/Utils/UI.cppsrc/Menu/ThemeManager.cpp
🧠 Learnings (2)
📚 Learning: 2025-07-01T18:01:07.079Z
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.
Applied to files:
src/State.h
📚 Learning: 2025-05-30T11:44:15.542Z
Learnt from: davo0411
PR: doodlum/skyrim-community-shaders#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.
Applied to files:
src/State.h
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build plugin and addons
🔇 Additional comments (19)
src/SettingsOverrideManager.cpp (2)
281-285: LGTM: Centralized overrides path.Switching to Util::PathHelpers::GetOverridesPath() improves consistency and portability.
405-409: LGTM: Centralized tracking path.Using Util::PathHelpers::GetAppliedOverridesPath() avoids hardcoded paths and aligns with the new helpers.
src/Menu/OverlayRenderer.cpp (1)
87-92: Verify font reload triggers and member updates
- Confirm
Menu::cachedFontNameexists and is accessible at this callsite.- Ensure
ThemeManager::ReloadFontupdatesmenu.cachedFontNameso name-only changes don’t re-trigger reloads on subsequent frames.package/SKSE/Plugins/CommunityShaders/Themes/Ocean.json (1)
40-96: Validate FullPalette length and component ranges. Ensure the palette has exactly ImGuiCol_COUNT RGBA entries and that every channel is within [0,1] to prevent out-of-bounds indexing and rendering issues at load.package/SKSE/Plugins/CommunityShaders/Themes/Mystic.json (1)
40-96: Validate FullPalette consistency. Confirm every theme’s FullPalette has the same number of entries as Ocean.json and that each RGBA array contains exactly four numeric values between 0.0 and 1.0.src/Utils/FileSystem.h (2)
89-100: New declarations read clean and align with docstrings.No issues spotted.
163-168: Real path helper mirrors ShadersRealPath appropriately.No issues spotted.
package/SKSE/Plugins/CommunityShaders/Themes/DragonBlood.json (1)
1-98: JSON looks well‑formed; verify FullPalette length and schema consistency across all themes.Same validation as Amber.json.
Re-use the validation script provided in Amber.json comment.
package/SKSE/Plugins/CommunityShaders/Themes/DwemerBronze.json (1)
1-98: JSON looks well‑formed; verify FullPalette length and schema consistency across all themes.Same validation as Amber.json.
Re-use the validation script provided in Amber.json comment.
src/Utils/FileSystem.cpp (1)
154-158: Manual verification required: ensure real vs. VFS paths in ThemeManager
No occurrences of GetThemesPath/GetThemesRealPath were found—please verify that ThemeManager and all save/export flows use GetThemesRealPath() for writes and GetThemesPath() for in-game reads under the MO2 VFS.package/SKSE/Plugins/CommunityShaders/Themes/Amber.json (1)
1-98: Verify FullPalette consistency and schema in theme JSONs
Confirm all theme JSONs underpackage/SKSE/Plugins/CommunityShaders/Themesshare the same FullPalette length, use RGBA values in the 0.0–1.0 range, and have correctly sized style arrays to prevent runtime loader mismatches.src/Utils/UI.h (1)
112-121: API additions look goodNew helpers fit the existing Util surface and default parameters are sane.
src/Utils/UI.cpp (1)
19-19: Verify single TU defines STB_IMAGE_IMPLEMENTATIONManual search didn’t locate any occurrences; confirm that
#define STB_IMAGE_IMPLEMENTATIONappears in only one translation unit. If you find multiple definitions, move it to a dedicated .cpp.src/Menu/ThemeManager.h (1)
122-124: Ensure SaveTheme sanitizes themeName and enforces bounds
- Reject “..”, absolute paths or other path‐traversal, enforce “.json” extension and reasonable length
- Confirm MAX_THEMES and MAX_FILE_SIZE limits are applied before writing to disk
src/Menu/ThemeManager.cpp (2)
300-318: Theme discovery flow looks solid.Good bounds on file size and theme count, robust exception handling, and alphabetical sort by displayName.
214-231: No reordering needed: code already follows recommended sequence (Fonts->Build(), then ImGui_ImplDX11_InvalidateDeviceObjects(), then ImGui_ImplDX11_CreateDeviceObjects()).Likely an incorrect or invalid review comment.
src/Menu.cpp (3)
287-290: Good: cache the actually loaded font size.This aligns cached state with the effective value after dynamic sizing/clamping.
347-349: LGTM: centralizing style setup each frame.Applying ThemeManager::SetupImGuiStyle here keeps UI consistent with live theme edits.
206-236: Sync or trigger font reload on theme load
Unable to locate anycachedFontSizeor reload‐trigger logic in the codebase; please verify thatLoadThemePreseteither updates a cached FontSize or sets a flag to scheduleReloadFont, so changes tosettings.Theme.FontSizeactually take effect.
src/SettingsOverrideManager.cpp
Outdated
| #include "SettingsOverrideManager.h" | ||
|
|
||
| #include "FeatureIssues.h" | ||
| #include "Utils/FileSystem.h" |
There was a problem hiding this comment.
Include the correct header for PathHelpers (avoid transitive includes).
This file uses Util::PathHelpers but includes Utils/FileSystem.h. If PathHelpers isn’t re-exported there, this risks brittle include-order dependencies or build failures. Include the PathHelpers header directly and drop unused FileSystem.h.
Apply this diff:
-#include "Utils/FileSystem.h"
+#include "Utils/PathHelpers.h"🤖 Prompt for AI Agents
In src/SettingsOverrideManager.cpp around line 4, the file currently includes
"Utils/FileSystem.h" but uses Util::PathHelpers; replace that transitive include
with the direct header for PathHelpers (e.g. include the correct PathHelpers
header from the Util/ or Utils/ path used in the project) and remove the unused
FileSystem.h include to avoid brittle include-order dependencies and potential
build failures.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
package/SKSE/Plugins/CommunityShaders/Themes/Amber.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/Default.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/DragonBlood.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/DwemerBronze.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/Forest.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/HighContrast.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/Light.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/Mystic.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/NordicFrost.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/Ocean.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/README.md(1 hunks)src/Menu.cpp(9 hunks)src/Menu.h(4 hunks)src/Menu/FeatureListRenderer.cpp(3 hunks)src/Menu/HomePageRenderer.cpp(0 hunks)src/Menu/MenuHeaderRenderer.cpp(2 hunks)src/Menu/OverlayRenderer.cpp(1 hunks)src/Menu/SettingsTabRenderer.cpp(6 hunks)src/Menu/ThemeManager.cpp(2 hunks)src/Menu/ThemeManager.h(3 hunks)src/State.cpp(4 hunks)src/Utils/FileSystem.cpp(2 hunks)src/Utils/FileSystem.h(2 hunks)src/Utils/UI.cpp(7 hunks)src/Utils/UI.h(2 hunks)src/XSEPlugin.cpp(2 hunks)
💤 Files with no reviewable changes (1)
- src/Menu/HomePageRenderer.cpp
✅ Files skipped from review due to trivial changes (1)
- package/SKSE/Plugins/CommunityShaders/Themes/HighContrast.json
🚧 Files skipped from review as they are similar to previous changes (12)
- src/Menu/MenuHeaderRenderer.cpp
- package/SKSE/Plugins/CommunityShaders/Themes/README.md
- package/SKSE/Plugins/CommunityShaders/Themes/Mystic.json
- package/SKSE/Plugins/CommunityShaders/Themes/Light.json
- package/SKSE/Plugins/CommunityShaders/Themes/Amber.json
- package/SKSE/Plugins/CommunityShaders/Themes/NordicFrost.json
- src/Utils/FileSystem.cpp
- src/Menu/FeatureListRenderer.cpp
- package/SKSE/Plugins/CommunityShaders/Themes/DragonBlood.json
- package/SKSE/Plugins/CommunityShaders/Themes/Ocean.json
- src/Utils/FileSystem.h
- src/XSEPlugin.cpp
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{cpp,cxx,cc,c,h,hpp,hxx,hlsl,hlsli,fx,fxh,py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not include TODO/FIXME placeholders; provide complete, working solutions
Files:
src/Menu/OverlayRenderer.cppsrc/Utils/UI.hsrc/Menu.cppsrc/Menu.hsrc/State.cppsrc/Menu/ThemeManager.cppsrc/Utils/UI.cppsrc/Menu/SettingsTabRenderer.cppsrc/Menu/ThemeManager.h
src/**/*.{cpp,cxx,cc,h,hpp,hxx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{cpp,cxx,cc,h,hpp,hxx}: Ensure SE/AE/VR runtime compatibility; use runtime detection patterns (e.g., REL::RelocateMember())
Include robust error handling and resource management with graceful degradation in the plugin code
Files:
src/Menu/OverlayRenderer.cppsrc/Utils/UI.hsrc/Menu.cppsrc/Menu.hsrc/State.cppsrc/Menu/ThemeManager.cppsrc/Utils/UI.cppsrc/Menu/SettingsTabRenderer.cppsrc/Menu/ThemeManager.h
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build plugin and addons
🔇 Additional comments (5)
src/Menu/OverlayRenderer.cpp (1)
86-92: Font reload now reacts to preset swaps.Combining the size epsilon with the cached-name check plugs the gap where switching presets left the old font bound. Nice catch.
src/Menu/SettingsTabRenderer.cpp (2)
197-212: Reserve combo backing storage before pushing pointers
itemsis storingconst char*returned fromdisplayNames.back().c_str(). Any reallocation ofdisplayNamesinvalidates every pointer already copied intoitems, and the nextImGui::Combocall dereferences garbage. Reserve the final capacity up front (themes.size() + 1) before the first push so no reallocation can occur while populating the vectors.- // Clear and rebuild the lists - displayNames.clear(); - items.clear(); + // Clear and rebuild the lists + displayNames.clear(); + items.clear(); + + const auto totalItems = themes.size() + 1; + displayNames.reserve(totalItems); + items.reserve(totalItems);
238-252: Clamp theme combo index before calling ImGui::ComboWhen the preset the user selected isn’t present (or the list is empty),
currentItemstill equals 1 whileitems.size()is 1.ImGui::Comboexpects an index within[0, items.size()-1]; feeding 1 triggers an assert in debug and reads past the array in release. Clamp the value before invoking ImGui so the index is always valid.- // Theme preset dropdown - if (ImGui::Combo("##ThemePreset", ¤tItem, items.data(), static_cast<int>(items.size()))) { + // Theme preset dropdown + if (!items.empty()) { + currentItem = std::clamp(currentItem, 0, static_cast<int>(items.size()) - 1); + } + + if (ImGui::Combo("##ThemePreset", ¤tItem, items.data(), static_cast<int>(items.size()))) {src/Menu/ThemeManager.h (1)
3-37: Include headers for std::unique_ptr and std::time_tThis header exposes
std::unique_ptr(LoadThemeFile) andstd::time_t(ThemeInfo::lastModified) but doesn’t include<memory>or<ctime>. In a TU that includes ThemeManager.h before any other standard headers, the build breaks. Add the missing includes so the header is self-contained.-#include <filesystem> -#include <imgui.h> -#include <nlohmann/json.hpp> -#include <string> -#include <vector> +#include <filesystem> +#include <imgui.h> +#include <memory> +#include <nlohmann/json.hpp> +#include <string> +#include <vector> +#include <ctime>src/Utils/UI.cpp (1)
1434-1444: Block path traversal before loading fonts
fontNamestill accepts absolute paths or../segments, so a crafted preset can point outside the Fonts directory (e.g."..\\..\\Windows\\Fonts\\arial.ttf"). We’d end up reading arbitrary files on disk, exactly the exploit flagged in the prior review. Please treat the value as a bare filename, reject separators/parent paths, and confirm the resolved path lives underGetFontsPath()before checking the extension.- auto fontsPath = Util::PathHelpers::GetFontsPath(); - auto fontPath = fontsPath / fontName; - - // Check if the font file exists and is a regular file - if (std::filesystem::exists(fontPath) && std::filesystem::is_regular_file(fontPath)) { - // Validate extension is .ttf or .otf - auto extension = fontPath.extension().string(); - std::transform(extension.begin(), extension.end(), extension.begin(), ::tolower); - return extension == ".ttf" || extension == ".otf"; - } + const auto fontsPath = Util::PathHelpers::GetFontsPath(); + if (!std::filesystem::exists(fontsPath) || !std::filesystem::is_directory(fontsPath)) { + return false; + } + + std::filesystem::path namePath{ fontName }; + if (namePath.is_absolute() || namePath.has_parent_path() || fontName.find_first_of("\\/") != std::string::npos) { + logger::warn("ValidateFont: Rejecting font outside fonts directory: '{}'", fontName); + return false; + } + + const auto fontPath = (fontsPath / namePath).lexically_normal(); + if (!std::filesystem::exists(fontPath) || !std::filesystem::is_regular_file(fontPath)) { + return false; + } + + const auto fontsCanonical = std::filesystem::weakly_canonical(fontsPath); + const auto fontCanonical = std::filesystem::weakly_canonical(fontPath); + auto fontsCanonicalStr = fontsCanonical.generic_string(); + if (!fontsCanonicalStr.empty() && fontsCanonicalStr.back() != '/') { + fontsCanonicalStr.push_back('/'); + } + const auto fontCanonicalStr = fontCanonical.generic_string(); + if (fontCanonicalStr.rfind(fontsCanonicalStr, 0) != 0) { + logger::warn("ValidateFont: Rejecting font outside fonts directory: '{}'", fontName); + return false; + } + + auto extension = fontPath.extension().string(); + std::transform(extension.begin(), extension.end(), extension.begin(), ::tolower); + return extension == ".ttf" || extension == ".otf";
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
|
✅ A pre-release build is available for this PR: |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/Menu/SettingsTabRenderer.cpp (1)
218-243: Critical: Clamp combo index to prevent out-of-bounds access.When
themesis empty or the selected preset isn't found,currentItemcan be out of bounds (e.g.,currentItem=1whileitems.size()=1). ImGui::Combo will read out of bounds, causing crashes in debug builds and undefined behavior in release builds.Apply this diff to clamp the index before calling ImGui::Combo:
+ // Clamp currentItem to valid range before rendering combo + if (!items.empty()) { + currentItem = std::clamp(currentItem, 0, static_cast<int>(items.size()) - 1); + } + // Theme preset dropdown if (ImGui::Combo("##ThemePreset", ¤tItem, items.data(), static_cast<int>(items.size()))) {
🧹 Nitpick comments (2)
src/Menu/ThemeManager.cpp (2)
30-40: Time conversion may be fragile across platforms.The clock arithmetic to convert
file_time_typetosystem_clockrelies on implementation-defined behavior and may not be portable across all standard library implementations. Consider using a more robust conversion or documenting the assumption.Alternative approach using a more explicit conversion:
std::time_t GetFileModTime(const std::filesystem::path& filePath) { try { - auto fileTime = std::filesystem::last_write_time(filePath); - auto systemTime = std::chrono::time_point_cast<std::chrono::system_clock::duration>( - fileTime - std::filesystem::file_time_type::clock::now() + std::chrono::system_clock::now()); - return std::chrono::system_clock::to_time_t(systemTime); + auto ftime = std::filesystem::last_write_time(filePath); + // C++20 approach (if available): use std::chrono::file_clock::to_sys + // Fallback: compute offset between clocks once and reuse + auto sctp = std::chrono::time_point_cast<std::chrono::system_clock::duration>( + ftime - std::filesystem::file_time_type::clock::now() + + std::chrono::system_clock::now()); + return std::chrono::system_clock::to_time_t(sctp); } catch (...) { return 0; } }
559-568: Consider more robust theme validation.The validation only checks for the presence of a
Themeobject. Consider validating required fields within the theme data (e.g.,UseSimplePalette,Palettestructure) to catch malformed themes earlier and provide better error messages to users.Example enhanced validation:
bool ThemeManager::ValidateThemeData(const json& themeData) const { // Basic validation - ensure Theme object exists if (!themeData.contains("Theme") || !themeData["Theme"].is_object()) { return false; } - // Could add more detailed validation here if needed + const auto& theme = themeData["Theme"]; + + // Validate UseSimplePalette field exists and is boolean + if (!theme.contains("UseSimplePalette") || !theme["UseSimplePalette"].is_boolean()) { + logger::warn("Theme missing or invalid UseSimplePalette field"); + return false; + } + + // Validate Palette structure exists + if (!theme.contains("Palette") || !theme["Palette"].is_object()) { + logger::warn("Theme missing or invalid Palette structure"); + return false; + } + return true; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
package/SKSE/Plugins/CommunityShaders/Themes/Default.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/README.md(1 hunks)src/Menu/SettingsTabRenderer.cpp(6 hunks)src/Menu/ThemeManager.cpp(2 hunks)src/SettingsOverrideManager.cpp(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package/SKSE/Plugins/CommunityShaders/Themes/Default.json
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{cpp,cxx,cc,c,h,hpp,hxx,hlsl,hlsli,fx,fxh,py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not include TODO/FIXME placeholders; provide complete, working solutions
Files:
src/Menu/ThemeManager.cppsrc/Menu/SettingsTabRenderer.cppsrc/SettingsOverrideManager.cpp
src/**/*.{cpp,cxx,cc,h,hpp,hxx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{cpp,cxx,cc,h,hpp,hxx}: Ensure SE/AE/VR runtime compatibility; use runtime detection patterns (e.g., REL::RelocateMember())
Include robust error handling and resource management with graceful degradation in the plugin code
Files:
src/Menu/ThemeManager.cppsrc/Menu/SettingsTabRenderer.cppsrc/SettingsOverrideManager.cpp
🧬 Code graph analysis (3)
src/Menu/ThemeManager.cpp (3)
src/State.cpp (5)
i(189-189)LoadTheme(831-858)LoadTheme(831-831)SaveTheme(860-884)SaveTheme(860-860)src/Utils/FileSystem.cpp (4)
GetFontsPath(46-49)GetFontsPath(46-46)GetThemesPath(76-79)GetThemesPath(76-76)src/Menu.cpp (6)
DiscoverThemes(200-204)DiscoverThemes(200-200)LoadTheme(174-186)LoadTheme(174-174)SaveTheme(188-198)SaveTheme(188-188)
src/Menu/SettingsTabRenderer.cpp (4)
src/Menu/ThemeManager.h (1)
GetSingleton(86-90)src/Utils/UI.h (1)
HoverTooltipWrapper(54-63)src/Utils/UI.cpp (6)
HoverTooltipWrapper(34-41)HoverTooltipWrapper(43-49)ButtonWithFlash(1334-1385)ButtonWithFlash(1334-1334)DiscoverFonts(1387-1426)DiscoverFonts(1387-1387)src/Utils/FileSystem.cpp (4)
GetThemesRealPath(149-152)GetThemesRealPath(149-149)GetFontsPath(46-49)GetFontsPath(46-46)
src/SettingsOverrideManager.cpp (1)
src/Utils/FileSystem.cpp (4)
GetOverridesPath(81-84)GetOverridesPath(81-81)GetAppliedOverridesPath(86-89)GetAppliedOverridesPath(86-86)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build plugin and addons
🔇 Additional comments (8)
src/SettingsOverrideManager.cpp (2)
281-284: LGTM! Path centralization improves maintainability.The change from a hardcoded path constant to the centralized
Util::PathHelpers::GetOverridesPath()improves maintainability and aligns with the broader path helper refactoring effort across the codebase.Note: Approval assumes the include at line 4 is verified to be correct.
405-408: LGTM! Consistent path centralization.Similar to
GetOverridesDirectory(), this change to useUtil::PathHelpers::GetAppliedOverridesPath()centralizes path computation and eliminates hardcoded constants.Note: Approval assumes the include at line 4 is verified to be correct.
src/Menu/ThemeManager.cpp (1)
194-194: LGTM! Font size resolution properly implemented.The code correctly uses
ResolveFontSize(menu)to compute the effective font size (handling auto-sizing with the 0 sentinel) and caches that resolved value. This addresses the previous review concerns about caching the unclampedthemeSettings.FontSize.Also applies to: 258-258
src/Menu/SettingsTabRenderer.cpp (3)
206-207: LGTM! Capacity reservation prevents pointer invalidation.Reserving capacity before populating
displayNamesanditemsensures the vector won't reallocate, keeping all theconst char*pointers initemsvalid. This addresses the previous review concern.
410-413: Good use of deferred font reload pattern.Setting
pendingFontReloadandpendingFontNameavoids reloading fonts during ImGui frame rendering, which prevents crashes. This is the correct approach for handling font changes.
449-477: LGTM! Font combo has proper bounds checking.The font selection combo properly validates array bounds before accessing
availableFontsand handles the empty fonts case gracefully with user-friendly messaging.package/SKSE/Plugins/CommunityShaders/Themes/README.md (2)
106-106: MAX_THEMES matches README TheMAX_THEMESconstant in ThemeManager is set to 100, matching the documentation.
94-94: File size limit accurately documented: The README’s 1 MB limit matchesThemeManager::MAX_FILE_SIZE(1024 × 1024 bytes).
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
src/Utils/UI.cpp (1)
1507-1529: Path traversal vulnerability remains unresolved.This is the same security issue flagged in the previous review. The function still accepts
fontNamevalues containing path separators or.., enabling directory traversal outside the intended Fonts directory.As noted in the prior review, apply the suggested diff to constrain validation to files within the Fonts directory by rejecting paths with separators and verifying the canonical path falls under the fonts directory.
src/Menu/OverlayRenderer.cpp (1)
84-84: Use const-reference binding for compatibility.This is the same issue flagged in the previous review. If
Menu::GetTheme()returns by value,auto&will fail to bind.Apply the previously suggested fix:
- auto& currentTheme = menu.GetTheme(); + const auto& currentTheme = menu.GetTheme();src/Menu/SettingsTabRenderer.cpp (1)
243-257: Clamp combo index before rendering
currentItemstill defaults to1even when only the “+ Create New” entry exists, soImGui::Comboreceives an index equal toitems.size(). ImGui asserts (and may read past the buffer in release) as soon as the themes directory is empty or discovery fails. Clamp the index before the call (or bail when the list is empty) so we never hand ImGui an out-of-range value.- // Theme preset dropdown - if (ImGui::Combo("##ThemePreset", ¤tItem, items.data(), static_cast<int>(items.size()))) { + // Theme preset dropdown + if (!items.empty()) { + currentItem = std::clamp(currentItem, 0, static_cast<int>(items.size()) - 1); + } + + if (ImGui::Combo("##ThemePreset", ¤tItem, items.data(), static_cast<int>(items.size()))) {
🧹 Nitpick comments (1)
src/Utils/UI.cpp (1)
1339-1390: Consider periodic cleanup for the flash timer map.The static
flashTimersmap accumulates entries per button ID and only removes them when expired entries are accessed. If button labels/IDs change frequently (e.g., dynamic content), entries could accumulate.Consider adding a periodic cleanup or size limit to prevent unbounded growth in rare edge cases:
bool ButtonWithFlash(const char* label, const ImVec2& size, int flashDurationMs) { static std::unordered_map<std::string, std::chrono::steady_clock::time_point> flashTimers; + static const size_t MAX_ENTRIES = 100; // Reasonable limit for UI std::string buttonId = std::string(label); auto now = std::chrono::steady_clock::now(); + // Periodic cleanup if map grows too large + if (flashTimers.size() > MAX_ENTRIES) { + auto it = flashTimers.begin(); + while (it != flashTimers.end()) { + auto elapsed = std::chrono::duration_cast<std::chrono::milliseconds>(now - it->second); + if (elapsed.count() >= flashDurationMs) { + it = flashTimers.erase(it); + } else { + ++it; + } + } + } + // Check if this button has active flash bool hasActiveFlash = false;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
package/SKSE/Plugins/CommunityShaders/Themes/Amber.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/Default.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/DragonBlood.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/DwemerBronze.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/Forest.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/HighContrast.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/Light.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/Mystic.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/NordicFrost.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/Ocean.json(1 hunks)src/FeatureIssues.cpp(2 hunks)src/Features/Menu/Shaders/GaussianBlur_Horizontal.hlsl(1 hunks)src/Features/Menu/Shaders/GaussianBlur_Vertical.hlsl(1 hunks)src/Menu.cpp(12 hunks)src/Menu.h(5 hunks)src/Menu/FeatureListRenderer.cpp(6 hunks)src/Menu/OverlayRenderer.cpp(2 hunks)src/Menu/SettingsTabRenderer.cpp(6 hunks)src/Menu/ThemeManager.cpp(4 hunks)src/Menu/ThemeManager.h(3 hunks)src/State.cpp(4 hunks)src/Utils/UI.cpp(11 hunks)src/Utils/UI.h(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- package/SKSE/Plugins/CommunityShaders/Themes/Default.json
- package/SKSE/Plugins/CommunityShaders/Themes/Forest.json
- package/SKSE/Plugins/CommunityShaders/Themes/Mystic.json
- package/SKSE/Plugins/CommunityShaders/Themes/HighContrast.json
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{cpp,cxx,cc,c,h,hpp,hxx,hlsl,hlsli,fx,fxh,py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not include TODO/FIXME placeholders; provide complete, working solutions
Files:
src/State.cppsrc/Menu/OverlayRenderer.cppsrc/Features/Menu/Shaders/GaussianBlur_Horizontal.hlslsrc/Features/Menu/Shaders/GaussianBlur_Vertical.hlslsrc/Menu.cppsrc/Utils/UI.hsrc/FeatureIssues.cppsrc/Menu/ThemeManager.cppsrc/Utils/UI.cppsrc/Menu.hsrc/Menu/FeatureListRenderer.cppsrc/Menu/ThemeManager.hsrc/Menu/SettingsTabRenderer.cpp
src/**/*.{cpp,cxx,cc,h,hpp,hxx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{cpp,cxx,cc,h,hpp,hxx}: Ensure SE/AE/VR runtime compatibility; use runtime detection patterns (e.g., REL::RelocateMember())
Include robust error handling and resource management with graceful degradation in the plugin code
Files:
src/State.cppsrc/Menu/OverlayRenderer.cppsrc/Menu.cppsrc/Utils/UI.hsrc/FeatureIssues.cppsrc/Menu/ThemeManager.cppsrc/Utils/UI.cppsrc/Menu.hsrc/Menu/FeatureListRenderer.cppsrc/Menu/ThemeManager.hsrc/Menu/SettingsTabRenderer.cpp
🧬 Code graph analysis (10)
src/State.cpp (2)
src/Utils/FileSystem.cpp (10)
GetSettingsUserPath(56-59)GetSettingsUserPath(56-56)GetSettingsTestPath(61-64)GetSettingsTestPath(61-61)GetSettingsThemePath(71-74)GetSettingsThemePath(71-71)GetSettingsDefaultPath(66-69)GetSettingsDefaultPath(66-66)GetCommunityShaderPath(31-34)GetCommunityShaderPath(31-31)src/Menu.cpp (4)
LoadTheme(196-208)LoadTheme(196-196)SaveTheme(210-220)SaveTheme(210-210)
src/Menu/OverlayRenderer.cpp (1)
src/Menu/ThemeManager.cpp (2)
RenderBackgroundBlur(197-293)RenderBackgroundBlur(197-197)
src/Menu.cpp (4)
src/Menu/ThemeManager.cpp (14)
CleanupBlurResources(1314-1358)CleanupBlurResources(1314-1314)LoadTheme(532-569)LoadTheme(532-532)SaveTheme(571-619)SaveTheme(571-572)DiscoverThemes(464-518)DiscoverThemes(464-464)ForceApplyDefaultTheme(295-341)ForceApplyDefaultTheme(295-295)SetupImGuiStyle(45-169)SetupImGuiStyle(45-45)ReloadFont(343-461)ReloadFont(343-343)src/State.cpp (6)
Save(395-462)Save(395-395)LoadTheme(831-864)LoadTheme(831-831)SaveTheme(866-890)SaveTheme(866-866)src/Utils/UI.cpp (2)
ValidateFont(1507-1529)ValidateFont(1507-1507)src/Utils/FileSystem.cpp (2)
GetFontsPath(46-49)GetFontsPath(46-46)
src/Utils/UI.h (1)
src/Utils/UI.cpp (18)
ButtonWithFlash(1339-1390)ButtonWithFlash(1339-1339)FeatureToggle(1392-1464)FeatureToggle(1392-1392)DiscoverFonts(1466-1505)DiscoverFonts(1466-1466)ValidateFont(1507-1529)ValidateFont(1507-1507)CalculateLuminance(1221-1234)CalculateLuminance(1221-1221)GetContrastingTextColor(1236-1247)GetContrastingTextColor(1236-1236)CalculateContrastRatio(1249-1259)CalculateContrastRatio(1249-1249)ContrastSelectable(1261-1297)ContrastSelectable(1261-1261)ContrastSelectableWithColor(1299-1336)ContrastSelectableWithColor(1299-1299)
src/Menu/ThemeManager.cpp (4)
src/Menu/FeatureListRenderer.cpp (4)
menu(247-264)menu(247-247)menu(332-338)menu(332-332)src/State.cpp (5)
i(189-189)LoadTheme(831-864)LoadTheme(831-831)SaveTheme(866-890)SaveTheme(866-866)src/Utils/FileSystem.cpp (4)
GetFontsPath(46-49)GetFontsPath(46-46)GetThemesPath(76-79)GetThemesPath(76-76)src/Menu.cpp (6)
DiscoverThemes(222-226)DiscoverThemes(222-222)LoadTheme(196-208)LoadTheme(196-196)SaveTheme(210-220)SaveTheme(210-210)
src/Utils/UI.cpp (2)
src/Menu/FeatureListRenderer.cpp (4)
menu(247-264)menu(247-247)menu(332-338)menu(332-332)src/Utils/FileSystem.cpp (2)
GetFontsPath(46-49)GetFontsPath(46-46)
src/Menu.h (4)
src/Menu.cpp (10)
LoadTheme(196-208)LoadTheme(196-196)SaveTheme(210-220)SaveTheme(210-210)DiscoverThemes(222-226)DiscoverThemes(222-222)LoadThemePreset(228-258)LoadThemePreset(228-228)CreateDefaultThemes(260-265)CreateDefaultThemes(260-260)src/Menu/ThemeManager.cpp (6)
LoadTheme(532-569)LoadTheme(532-532)SaveTheme(571-619)SaveTheme(571-572)DiscoverThemes(464-518)DiscoverThemes(464-464)src/State.cpp (4)
LoadTheme(831-864)LoadTheme(831-831)SaveTheme(866-890)SaveTheme(866-866)src/Menu/ThemeManager.h (1)
ThemeManager(22-218)
src/Menu/FeatureListRenderer.cpp (2)
src/Utils/UI.cpp (8)
ContrastSelectable(1261-1297)ContrastSelectable(1261-1261)ContrastSelectableWithColor(1299-1336)ContrastSelectableWithColor(1299-1299)HoverTooltipWrapper(34-41)HoverTooltipWrapper(43-49)FeatureToggle(1392-1464)FeatureToggle(1392-1392)src/SettingsOverrideManager.h (1)
GetSingleton(42-46)
src/Menu/ThemeManager.h (3)
src/Menu/ThemeManager.cpp (40)
ResolveFontSize(767-785)ResolveFontSize(767-767)SetupImGuiStyle(45-169)SetupImGuiStyle(45-45)ReloadFont(343-461)ReloadFont(343-343)ApplyBackgroundBlur(171-195)ApplyBackgroundBlur(171-171)RenderBackgroundBlur(197-293)RenderBackgroundBlur(197-197)ForceApplyDefaultTheme(295-341)ForceApplyDefaultTheme(295-295)InitializeBlurShaders(789-1073)InitializeBlurShaders(789-789)CreateBlurTextures(1075-1161)CreateBlurTextures(1075-1075)PerformGaussianBlur(1163-1312)PerformGaussianBlur(1163-1163)CleanupBlurResources(1314-1358)CleanupBlurResources(1314-1314)DiscoverThemes(464-518)DiscoverThemes(464-464)GetThemeNames(520-530)GetThemeNames(520-520)LoadTheme(532-569)LoadTheme(532-532)SaveTheme(571-619)SaveTheme(571-572)GetThemeInfo(621-627)GetThemeInfo(621-621)RefreshThemes(629-633)RefreshThemes(629-629)GetThemesDirectory(635-638)GetThemesDirectory(635-635)CreateDefaultThemeFiles(640-702)CreateDefaultThemeFiles(640-640)LoadThemeFile(704-754)LoadThemeFile(704-704)ValidateThemeData(756-765)ValidateThemeData(756-756)src/Menu.cpp (7)
Menu(142-170)DiscoverThemes(222-226)DiscoverThemes(222-222)LoadTheme(196-208)LoadTheme(196-196)SaveTheme(210-220)SaveTheme(210-210)src/Menu.h (2)
Menu(14-241)GetSingleton(21-25)
src/Menu/SettingsTabRenderer.cpp (4)
src/Menu.h (1)
GetSingleton(21-25)src/Menu/ThemeManager.h (1)
GetSingleton(97-101)src/Utils/UI.cpp (6)
HoverTooltipWrapper(34-41)HoverTooltipWrapper(43-49)ButtonWithFlash(1339-1390)ButtonWithFlash(1339-1339)DiscoverFonts(1466-1505)DiscoverFonts(1466-1466)src/Utils/FileSystem.cpp (4)
GetThemesRealPath(149-152)GetThemesRealPath(149-149)GetFontsPath(46-49)GetFontsPath(46-46)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Validate shader compilation (VR, .github/configs/shader-validation-vr.yaml)
- GitHub Check: Validate shader compilation (Flatrim, .github/configs/shader-validation.yaml)
- GitHub Check: Build plugin and addons
🔇 Additional comments (11)
package/SKSE/Plugins/CommunityShaders/Themes/DwemerBronze.json (1)
1-102: Looks consistent with the theme schema. Metadata and palette entries line up with the expected format, RGBA values stay within range, and the full palette count matches ImGui’s color enumeration.src/Utils/UI.cpp (1)
1216-1337: LGTM: Well-implemented color utilities.The new
ColorUtilsnamespace provides WCAG-compliant luminance and contrast calculations with proper gamma correction. The contrast-aware selectable helpers will improve text readability across different themes.src/FeatureIssues.cpp (1)
1461-1462: LGTM: Theme palette field updated.The change from
Palette.BordertoPalette.FrameBorderaligns with the updated theme structure and maintains consistent styling for the test UI buttons.Also applies to: 1484-1485
src/Menu/OverlayRenderer.cpp (1)
199-202: LGTM: Background blur integration.The blur rendering is correctly positioned after
ImGui::Render()but beforeImGui_ImplDX11_RenderDrawData(), ensuring blur is applied only behind visible ImGui windows.src/State.cpp (3)
155-168: LGTM: Return type updated for path helpers.Changing
GetConfigPathto return by value aligns with thePathHelpersAPI and avoids lifetime issues with string references.
831-864: LGTM: Theme loading with preset prioritization.The logic correctly prioritizes theme preset selection over
SettingsTheme.json, preventing unintended overrides during first-time setup. The early return at lines 834-836 ensures preset-based themes remain active.
866-890: LGTM: Theme persistence with proper error handling.The
SaveThemeimplementation includes directory creation, file validation, and structured JSON output with proper formatting. Error handling covers filesystem and I/O failures.src/Menu/FeatureListRenderer.cpp (4)
255-263: LGTM: Contrast-aware menu selections.The Feature Issues item correctly maintains error coloring while other items use
ContrastSelectablefor improved readability. This ensures semantic colors are preserved while enhancing contrast.
318-321: LGTM: Semantic color with contrast adjustment.Using
ContrastSelectableWithColorpreserves the semantictextColor(disabled, error, success, etc.) while ensuring readability against selection backgrounds. This is better than the previous explicit push/pop approach.
437-472: LGTM: Restore defaults control.The bottom-right positioned restore icon/button is a standard UX pattern. The icon fallback to text button ensures functionality when the icon isn't loaded, and the conditional rendering prevents clutter for disabled/unloaded features.
562-626: LGTM: Toggle control improves clarity.Replacing the Enable/Disable button with
FeatureToggleprovides clearer visual feedback for the boot-time setting. The updated tooltip text accurately describes the toggle semantics and restart requirement.
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
src/Menu/MenuHeaderRenderer.cpp (1)
13-47: Code duplication: RoleFontGuard identical to FontRoleGuard in VR.cpp.This RoleFontGuard implementation (lines 15-46) is nearly identical to the FontRoleGuard in src/Features/VR.cpp (lines 28-57). See my comment on VR.cpp regarding extracting this to a shared utility.
src/Menu/SettingsTabRenderer.cpp (1)
293-309: Clamp the theme combo index before calling ImGui::Combo
currentItemis still initialized to1even whenthemesis empty. In that scenarioitemsonly contains the “+ Create New” entry (size == 1), so passingcurrentItem == 1intoImGui::Combolets it read past the bounds ofitems, triggering ImGui assertions or undefined behaviour. ClampcurrentIteminto the valid range (0 … items.size()-1) before the combo call. This was already flagged previously but is still unresolved.- // Theme preset dropdown - if (ComboWithFont("##ThemePreset", ¤tItem, items.data(), static_cast<int>(items.size()), Menu::FontRole::Subtitle)) { + // Theme preset dropdown + if (!items.empty()) { + currentItem = std::clamp(currentItem, 0, static_cast<int>(items.size()) - 1); + } + if (ComboWithFont("##ThemePreset", ¤tItem, items.data(), static_cast<int>(items.size()), Menu::FontRole::Subtitle)) {src/Menu/ThemeManager.cpp (1)
421-444: Harden font path validation
effective.Filecan still carry absolute or..segments. BecauseValidateFontonly checks existence, a crafted theme JSON can escape the Fonts directory and cause us to callAddFontFromFileTTFon arbitrary files. That is a path-traversal vector and was already flagged earlier. Please constrain fonts to the Fonts directory by rejecting absolute/parent paths and comparing canonical paths.Here is the fix I recommend applying in
Util::ValidateFont:- auto fontsPath = Util::PathHelpers::GetFontsPath(); - std::filesystem::path relative(fontName); - std::filesystem::path directPath = fontsPath / relative; + const auto fontsPath = Util::PathHelpers::GetFontsPath(); + if (!std::filesystem::exists(fontsPath) || !std::filesystem::is_directory(fontsPath)) { + return false; + } + + std::filesystem::path namePath = std::filesystem::path(fontName); + if (namePath.is_absolute() || namePath.has_parent_path() || fontName.find_first_of("\\/") != std::string::npos) { + return false; + } + + auto fontPath = (fontsPath / namePath).lexically_normal(); + + auto fontsCanonical = std::filesystem::weakly_canonical(fontsPath); + auto fileCanonical = std::filesystem::weakly_canonical(fontPath); + if (fileCanonical.native().rfind(fontsCanonical.native(), 0) != 0) { + return false; + }This keeps presets from escaping the Fonts directory while retaining the existing extension checks and logging.
src/Menu.cpp (1)
301-304: Queue the font reload instead of short-circuiting itBy copying the preset’s
FontNameintocachedFontNameyou prevent the reload detector from seeing a delta, so the new typeface never loads.DrawOverlay()only callsThemeManager::ReloadFontwhenpendingFontReloadis set. Queue that flag here instead of pre-populating the cache.Suggested change:
settings.SelectedThemePreset = themeName; - // Update cached values for font reload detection - cachedFontName = settings.Theme.FontName; + pendingFontReload = true;
🧹 Nitpick comments (2)
src/Menu.h (1)
111-131: Font caching fields exposed for external access.Moving these font caching fields to public is intentional to support ThemeManager and OverlayRenderer access. The lambda-based initialization of
cachedFontFilesByRoleis functional but adds complexity. Consider whether simpler brace initialization might be clearer:-std::array<std::string, static_cast<size_t>(FontRole::Count)> cachedFontFilesByRole = []() { - std::array<std::string, static_cast<size_t>(FontRole::Count)> files{}; - auto setFile = [&files](FontRole role, std::string value) { - files[static_cast<size_t>(role)] = std::move(value); - }; - setFile(FontRole::Body, "Jost/Jost-Regular.ttf"); - setFile(FontRole::Heading, "Jost/Jost-Regular.ttf"); - setFile(FontRole::Subheading, "Jost/Jost-Regular.ttf"); - setFile(FontRole::Subtitle, "Jost/Jost-Regular.ttf"); - return files; -}(); +std::array<std::string, static_cast<size_t>(FontRole::Count)> cachedFontFilesByRole = { + "Jost/Jost-Regular.ttf", // Body + "Jost/Jost-Regular.ttf", // Heading + "Jost/Jost-Regular.ttf", // Subheading + "Jost/Jost-Regular.ttf" // Subtitle +};src/Features/VR.cpp (1)
26-64: Consider extracting FontRoleGuard to a shared utility.This FontRoleGuard implementation is nearly identical to RoleFontGuard in MenuHeaderRenderer.cpp (lines 15-47) and FontRoleGuard in SettingsTabRenderer.cpp (as shown in the relevant snippets). Having the same RAII wrapper duplicated across multiple files violates DRY principles and increases maintenance burden.
Consider moving this to a shared utility header such as
src/Utils/UI.hor creating a dedicatedsrc/Utils/FontHelpers.h:// In src/Utils/FontHelpers.h namespace Util::Fonts { class RoleGuard { public: explicit RoleGuard(Menu::FontRole role); ~RoleGuard(); RoleGuard(const RoleGuard&) = delete; RoleGuard& operator=(const RoleGuard&) = delete; [[nodiscard]] ImFont* Get() const { return font_; } private: ImFont* font_ = nullptr; }; }Then all three files can use the shared implementation.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (63)
package/Interface/CommunityShaders/Fonts/Bitter/Bitter-Light.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Bitter/Bitter-Regular.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Bitter/Bitter-SemiBold.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/CrimsonPro/CrimsonPro-Light.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/CrimsonPro/CrimsonPro-Regular.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/CrimsonPro/CrimsonPro-SemiBold.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Crimson_Pro/CrimsonPro-Light.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Crimson_Pro/CrimsonPro-Regular.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Crimson_Pro/CrimsonPro-SemiBold.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/IBMPlexMono/IBMPlexMono-Light.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/IBMPlexMono/IBMPlexMono-Regular.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/IBMPlexMono/IBMPlexMono-SemiBold.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/IBMPlexSans/IBMPlexSans-Light.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/IBMPlexSans/IBMPlexSans-Regular.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/IBMPlexSans/IBMPlexSans-SemiBold.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/IBMPlexSans/IBMPlexSans_Condensed-Light.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/IBMPlexSans/IBMPlexSans_Condensed-Regular.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/IBMPlexSans/IBMPlexSans_Condensed-SemiBold.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/IBMPlexSerif/IBMPlexSerif-Light.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/IBMPlexSerif/IBMPlexSerif-Regular.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/IBMPlexSerif/IBMPlexSerif-SemiBold.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/IBM_Plex_Sans/IBMPlexSans-Light.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/IBM_Plex_Sans/IBMPlexSans-Regular.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/IBM_Plex_Sans/IBMPlexSans-SemiBold.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/IBM_Plex_Sans/IBMPlexSans_Condensed-Light.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/IBM_Plex_Sans/IBMPlexSans_Condensed-Regular.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/IBM_Plex_Sans/IBMPlexSans_Condensed-SemiBold.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/IBM_Plex_Sans/IBMPlexSans_SemiCondensed-Light.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/IBM_Plex_Sans/IBMPlexSans_SemiCondensed-Regular.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/IBM_Plex_Sans/IBMPlexSans_SemiCondensed-SemiBold.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Inter/Inter_24pt-Light.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Inter/Inter_24pt-Regular.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Inter/Inter_24pt-SemiBold.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Jost/Jost-Light.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Jost/Jost-Regular.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Jost/Jost-SemiBold.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Merriweather/Merriweather_24pt-Light.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Merriweather/Merriweather_24pt-Regular.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Merriweather/Merriweather_24pt-SemiBold.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Merriweather/Merriweather_24pt_SemiCondensed-Light.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Merriweather/Merriweather_24pt_SemiCondensed-Regular.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Merriweather/Merriweather_24pt_SemiCondensed-SemiBold.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Rajdhani/Rajdhani-Light.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Rajdhani/Rajdhani-Regular.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Roboto/Roboto-Bold.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Roboto/Roboto-Regular.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Roboto/Roboto-SemiBold.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Roboto/Roboto-Thin.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Roboto/Roboto_Condensed-Light.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Roboto/Roboto_Condensed-Regular.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Roboto/Roboto_Condensed-SemiBold.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/RobotoSlab/RobotoSlab-Light.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/RobotoSlab/RobotoSlab-Regular.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/RobotoSlab/RobotoSlab-SemiBold.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Rubik/Rubik-Light.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Rubik/Rubik-Regular.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Rubik/Rubik-SemiBold.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Sanguis/Sanguis.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Sovngarde/SovngardeBold.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Sovngarde/SovngardeLight.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/WorkSans/WorkSans-Light.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/WorkSans/WorkSans-Regular.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/WorkSans/WorkSans-SemiBold.ttfis excluded by!**/*.ttf
📒 Files selected for processing (35)
package/Interface/CommunityShaders/Fonts/Bitter/OFL.txt(1 hunks)package/Interface/CommunityShaders/Fonts/CrimsonPro/OFL.txt(1 hunks)package/Interface/CommunityShaders/Fonts/IBMPlexMono/OFL.txt(1 hunks)package/Interface/CommunityShaders/Fonts/IBMPlexSans/OFL.txt(1 hunks)package/Interface/CommunityShaders/Fonts/IBMPlexSerif/OFL.txt(1 hunks)package/Interface/CommunityShaders/Fonts/Inter/OFL.txt(1 hunks)package/Interface/CommunityShaders/Fonts/Merriweather/OFL.txt(1 hunks)package/Interface/CommunityShaders/Fonts/Rajdhani/OFL.txt(1 hunks)package/Interface/CommunityShaders/Fonts/Roboto/OFL.txt(1 hunks)package/Interface/CommunityShaders/Fonts/Rubik/OFL.txt(1 hunks)package/Interface/CommunityShaders/Fonts/WorkSans/OFL.txt(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/Amber.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/Default.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/DragonBlood.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/DwemerBronze.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/Forest.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/HighContrast.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/Light.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/NordicFrost.json(1 hunks)package/SKSE/Plugins/CommunityShaders/Themes/Ocean.json(1 hunks)src/Features/PerformanceOverlay.cpp(3 hunks)src/Features/PerformanceOverlay.h(1 hunks)src/Features/VR.cpp(4 hunks)src/Menu.cpp(11 hunks)src/Menu.h(8 hunks)src/Menu/FeatureListRenderer.cpp(7 hunks)src/Menu/Fonts.cpp(1 hunks)src/Menu/Fonts.h(1 hunks)src/Menu/MenuHeaderRenderer.cpp(5 hunks)src/Menu/OverlayRenderer.cpp(2 hunks)src/Menu/SettingsTabRenderer.cpp(10 hunks)src/Menu/SettingsTabRenderer.h(1 hunks)src/Menu/ThemeManager.cpp(4 hunks)src/Utils/UI.cpp(11 hunks)src/Utils/UI.h(3 hunks)
✅ Files skipped from review due to trivial changes (11)
- package/Interface/CommunityShaders/Fonts/Bitter/OFL.txt
- package/Interface/CommunityShaders/Fonts/Rajdhani/OFL.txt
- package/Interface/CommunityShaders/Fonts/CrimsonPro/OFL.txt
- package/Interface/CommunityShaders/Fonts/Inter/OFL.txt
- package/Interface/CommunityShaders/Fonts/Rubik/OFL.txt
- package/Interface/CommunityShaders/Fonts/WorkSans/OFL.txt
- package/Interface/CommunityShaders/Fonts/Merriweather/OFL.txt
- package/Interface/CommunityShaders/Fonts/IBMPlexSerif/OFL.txt
- package/Interface/CommunityShaders/Fonts/IBMPlexSans/OFL.txt
- package/Interface/CommunityShaders/Fonts/IBMPlexMono/OFL.txt
- package/Interface/CommunityShaders/Fonts/Roboto/OFL.txt
🚧 Files skipped from review as they are similar to previous changes (7)
- src/Menu/SettingsTabRenderer.h
- src/Menu/OverlayRenderer.cpp
- package/SKSE/Plugins/CommunityShaders/Themes/DragonBlood.json
- package/SKSE/Plugins/CommunityShaders/Themes/DwemerBronze.json
- package/SKSE/Plugins/CommunityShaders/Themes/HighContrast.json
- package/SKSE/Plugins/CommunityShaders/Themes/Light.json
- package/SKSE/Plugins/CommunityShaders/Themes/Default.json
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{cpp,cxx,cc,c,h,hpp,hxx,hlsl,hlsli,fx,fxh,py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not include TODO/FIXME placeholders; provide complete, working solutions
Files:
src/Features/PerformanceOverlay.hsrc/Menu/MenuHeaderRenderer.cppsrc/Menu/FeatureListRenderer.cppsrc/Menu/Fonts.cppsrc/Menu/SettingsTabRenderer.cppsrc/Features/VR.cppsrc/Features/PerformanceOverlay.cppsrc/Menu.cppsrc/Menu/ThemeManager.cppsrc/Menu.hsrc/Utils/UI.hsrc/Utils/UI.cppsrc/Menu/Fonts.h
src/**/*.{cpp,cxx,cc,h,hpp,hxx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{cpp,cxx,cc,h,hpp,hxx}: Ensure SE/AE/VR runtime compatibility; use runtime detection patterns (e.g., REL::RelocateMember())
Include robust error handling and resource management with graceful degradation in the plugin code
Files:
src/Features/PerformanceOverlay.hsrc/Menu/MenuHeaderRenderer.cppsrc/Menu/FeatureListRenderer.cppsrc/Menu/Fonts.cppsrc/Menu/SettingsTabRenderer.cppsrc/Features/VR.cppsrc/Features/PerformanceOverlay.cppsrc/Menu.cppsrc/Menu/ThemeManager.cppsrc/Menu.hsrc/Utils/UI.hsrc/Utils/UI.cppsrc/Menu/Fonts.h
🧬 Code graph analysis (12)
src/Features/PerformanceOverlay.h (1)
src/Features/PerformanceOverlay.cpp (2)
DrawABTestSection(1143-1295)DrawABTestSection(1143-1143)
src/Menu/MenuHeaderRenderer.cpp (3)
src/Menu.h (2)
GetSingleton(70-74)nodiscard(402-403)src/Menu/SettingsTabRenderer.cpp (1)
nodiscard(49-49)src/Utils/UI.cpp (6)
DrawAlignedTextWithLogo(298-335)DrawAlignedTextWithLogo(298-298)DrawSharpText(267-296)DrawSharpText(267-267)ButtonWithFlash(1339-1390)ButtonWithFlash(1339-1339)
src/Menu/FeatureListRenderer.cpp (5)
src/Menu.h (1)
GetSingleton(70-74)src/SettingsOverrideManager.h (1)
GetSingleton(42-46)src/Utils/UI.cpp (8)
ContrastSelectable(1261-1297)ContrastSelectable(1261-1261)ContrastSelectableWithColor(1299-1336)ContrastSelectableWithColor(1299-1299)HoverTooltipWrapper(34-41)HoverTooltipWrapper(43-49)FeatureToggle(1392-1464)FeatureToggle(1392-1392)src/FeatureIssues.cpp (2)
IsObsoleteFeature(717-721)IsObsoleteFeature(717-717)src/Utils/UI.h (1)
HoverTooltipWrapper(56-65)
src/Menu/SettingsTabRenderer.cpp (6)
src/Menu.h (2)
GetSingleton(70-74)GetFontRoleDefaultScale(59-62)src/Menu/ThemeManager.h (1)
GetSingleton(97-101)src/Utils/UI.cpp (4)
HoverTooltipWrapper(34-41)HoverTooltipWrapper(43-49)ButtonWithFlash(1339-1390)ButtonWithFlash(1339-1339)src/Utils/FileSystem.cpp (2)
GetThemesRealPath(149-152)GetThemesRealPath(149-149)src/Menu/ThemeManager.cpp (2)
ResolveFontSize(862-880)ResolveFontSize(862-862)src/Menu/Fonts.cpp (2)
DiscoverFontCatalog(364-454)DiscoverFontCatalog(364-364)
src/Features/VR.cpp (3)
src/Menu/FeatureListRenderer.cpp (12)
menu(295-312)menu(295-295)menu(380-386)menu(380-380)FontRoleGuard(32-44)FontRoleGuard(32-32)FontRoleGuard(46-51)FontRoleGuard(53-53)label(314-323)label(314-314)BeginTabItemWithFont(71-75)BeginTabItemWithFont(71-71)src/Menu.h (1)
GetSingleton(70-74)src/Menu/SettingsTabRenderer.cpp (6)
FontRoleGuard(25-37)FontRoleGuard(25-25)FontRoleGuard(39-44)FontRoleGuard(46-46)BeginTabItemWithFont(66-70)BeginTabItemWithFont(66-66)
src/Features/PerformanceOverlay.cpp (1)
src/Utils/FileSystem.cpp (4)
GetDataPath(10-29)GetDataPath(10-10)LoadJsonDiff(192-237)LoadJsonDiff(192-192)
src/Menu.cpp (3)
src/Menu.h (4)
FontRoleSettings(347-347)Style(251-269)Menu(23-124)GetSingleton(70-74)src/State.cpp (7)
i(189-189)Save(395-462)Save(395-395)LoadTheme(831-864)LoadTheme(831-831)SaveTheme(866-890)SaveTheme(866-866)src/Menu/Fonts.cpp (8)
BuildFontSignature(119-133)BuildFontSignature(119-119)GetDefaultRole(114-117)GetDefaultRole(114-114)NormalizeFontRoles(86-112)NormalizeFontRoles(86-86)ValidateFont(469-514)ValidateFont(469-469)
src/Menu/ThemeManager.cpp (5)
src/State.cpp (5)
i(189-189)LoadTheme(831-864)LoadTheme(831-831)SaveTheme(866-890)SaveTheme(866-866)src/Menu.h (2)
GetSingleton(70-74)GetFontRoleDefaultScale(59-62)src/Menu/ThemeManager.h (1)
GetSingleton(97-101)src/Utils/FileSystem.cpp (4)
GetFontsPath(46-49)GetFontsPath(46-46)GetThemesPath(76-79)GetThemesPath(76-76)src/Menu.cpp (6)
GetDefaultFontRole(167-170)GetDefaultFontRole(167-167)LoadTheme(238-254)LoadTheme(238-238)SaveTheme(256-269)SaveTheme(256-256)
src/Menu.h (3)
src/Menu.cpp (17)
Menu(172-200)ResolveFontRole(152-160)ResolveFontRole(152-152)LoadTheme(238-254)LoadTheme(238-238)SaveTheme(256-269)SaveTheme(256-256)DiscoverThemes(271-275)DiscoverThemes(271-271)LoadThemePreset(277-310)LoadThemePreset(277-277)CreateDefaultThemes(312-317)CreateDefaultThemes(312-312)BuildFontSignature(162-165)BuildFontSignature(162-162)GetDefaultFontRole(167-170)GetDefaultFontRole(167-167)src/Menu/Fonts.cpp (2)
BuildFontSignature(119-133)BuildFontSignature(119-119)src/Menu/ThemeManager.h (1)
ThemeManager(22-218)
src/Utils/UI.h (1)
src/Utils/UI.cpp (14)
ButtonWithFlash(1339-1390)ButtonWithFlash(1339-1339)FeatureToggle(1392-1464)FeatureToggle(1392-1392)CalculateLuminance(1221-1234)CalculateLuminance(1221-1221)GetContrastingTextColor(1236-1247)GetContrastingTextColor(1236-1236)CalculateContrastRatio(1249-1259)CalculateContrastRatio(1249-1249)ContrastSelectable(1261-1297)ContrastSelectable(1261-1261)ContrastSelectableWithColor(1299-1336)ContrastSelectableWithColor(1299-1299)
src/Utils/UI.cpp (1)
src/Menu/FeatureListRenderer.cpp (4)
menu(295-312)menu(295-295)menu(380-386)menu(380-380)
src/Menu/Fonts.h (3)
src/Menu.cpp (3)
Menu(172-200)BuildFontSignature(162-165)BuildFontSignature(162-162)src/Menu.h (2)
Menu(23-124)FontRoleSettings(347-347)src/Menu/Fonts.cpp (14)
NormalizeFontRoles(86-112)NormalizeFontRoles(86-86)GetDefaultRole(114-117)GetDefaultRole(114-114)BuildFontSignature(119-133)BuildFontSignature(119-119)DiscoverFontCatalog(364-454)DiscoverFontCatalog(364-364)FormatFontDisplayName(297-321)FormatFontDisplayName(297-297)DiscoverFonts(457-467)DiscoverFonts(457-457)ValidateFont(469-514)ValidateFont(469-469)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build plugin and addons
- GitHub Check: Validate shader compilation (VR, .github/configs/shader-validation-vr.yaml)
- GitHub Check: Validate shader compilation (Flatrim, .github/configs/shader-validation.yaml)
🔇 Additional comments (9)
package/SKSE/Plugins/CommunityShaders/Themes/Ocean.json (1)
1-131: LGTM! Well-structured theme configuration.This theme JSON follows the established pattern for theme files in the PR. The Ocean Blue theme provides a complete configuration including fonts, colors, and styling. As a data-only file, it integrates cleanly with the theming system introduced in this PR.
src/Menu.h (1)
26-62: LGTM! Clean font role abstraction.The FontRole enum with associated descriptors and static accessor methods provides a well-structured way to manage different font roles across the UI. The constexpr methods enable compile-time optimization where possible.
src/Features/VR.cpp (1)
239-262: LGTM! Font-aware tab rendering.The BeginTabItemWithFont helper cleanly applies the Subheading font role to VR tab items, ensuring consistent typography across the VR settings UI.
src/Features/PerformanceOverlay.h (1)
166-166: LGTM! API simplification.Removing the
showCollapsibleSectionsparameter simplifies the DrawABTestSection interface. Based on the implementation in PerformanceOverlay.cpp, the A/B test results are now rendered unconditionally when available, with inline toggle controls for the settings diff. This is a cleaner, more straightforward design.src/Menu/MenuHeaderRenderer.cpp (2)
87-120: LGTM! Consistent heading font application.The RoleFontGuard ensures the Heading font role is applied consistently across all header rendering paths (with logo, without logo, and fallback scenarios).
138-141: LGTM! Theme persistence integrated with save actions.Adding
SaveTheme()calls alongsideSave()ensures that theme changes are persisted when the user saves settings. The use ofButtonWithFlashalso provides better visual feedback for the save action.Also applies to: 223-226
package/SKSE/Plugins/CommunityShaders/Themes/Amber.json (1)
1-131: LGTM! Warm Amber theme configuration.The Warm Amber theme provides a cohesive warm color palette inspired by Nordic taverns and hearth fires. The configuration is well-structured and consistent with the other theme files in this PR.
package/SKSE/Plugins/CommunityShaders/Themes/NordicFrost.json (1)
1-131: LGTM! Nordic Frost theme with mixed fonts.The Nordic Frost theme provides a cool, icy aesthetic with an interesting font pairing: Crimson Pro for body text and Sovngarde Bold for headings. The larger size scale (1.1) for headings creates good visual hierarchy.
package/SKSE/Plugins/CommunityShaders/Themes/Forest.json (1)
1-131: LGTM! Forest Green theme configuration.The Forest Green theme delivers a natural, woodland aesthetic with a cohesive green color palette. Like Nordic Frost, it uses Sovngarde Bold for headings to create strong visual contrast against the Merriweather body font.
|
@coderabbitai |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 7748 🏁 Script executed: Length of output: 3759 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 1189 🏁 Script executed: Length of output: 4581 🏁 Script executed: Length of output: 2797 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 3874 🏁 Script executed: Length of output: 553 🏁 Script executed: Length of output: 2075 🏁 Script executed: Length of output: 95 🏁 Script executed: Length of output: 8385 🏁 Script executed: Length of output: 1272 🏁 Script executed: Length of output: 2878 🏁 Script executed: Length of output: 2659 🏁 Script executed: Length of output: 404 🏁 Script executed: Length of output: 1615 🏁 Script executed: Length of output: 595 🏁 Script executed: Length of output: 2361 🏁 Script executed: Length of output: 2898 🏁 Script executed: Length of output: 79 🏁 Script executed: Length of output: 249 🏁 Script executed: Length of output: 154 🏁 Script executed: Length of output: 5719 🏁 Script executed: Length of output: 120 🏁 Script executed: Length of output: 627 🏁 Script executed: Length of output: 1165 @davo0411: I've completed a comprehensive assessment of PR #1530. Here's my detailed analysis: Overall AssessmentDesign Quality: Medium - The theme system is functional and feature-rich but has significant architectural and safety concerns that should be addressed before merge. Implementation Competence: Medium-High - The code demonstrates good understanding of ImGui and D3D11, but contains risky patterns (const_cast, static state, manual resource management) that could lead to runtime issues. Critical Issues (Important)1. Thread Safety ViolationsFiles:
Impact: Potential race conditions, crashes in multithreaded scenarios, data corruption Recommendation: Add mutexes or use thread-local storage; consider redesign to avoid static state 2. Resource Leak RisksFiles:
Impact: GPU memory leaks, resource exhaustion, crashes on cleanup Recommendation: Use 3. const_cast Design ViolationsFiles: auto* nonConstMenu = const_cast<Menu*>(&menu);
auto& mutableMenu = const_cast<Menu&>(menu);Impact: Violates const correctness, masks design issues, undefined behavior risk Recommendation: Refactor API to accept non-const references or separate read/write operations 4. Font Atlas Rebuilding SafetyFiles:
Impact: Crashes during font reload, GPU device loss, rendering corruption Recommendation: Defer font changes to a safe point between frames; add more robust state validation 5. Path Traversal VulnerabilityFiles:
Impact: Reading arbitrary files, potential information disclosure Recommendation: Canonicalize paths and validate they remain within fonts directory Medium Priority Issues6. Performance: Repeated File System ScansFiles:
Impact: UI lag, disk thrashing, poor user experience Recommendation: Cache results with file watch or manual refresh button 7. Weak Theme ValidationFiles: bool ThemeManager::ValidateThemeData(const json& themeData) const
{
if (!themeData.contains("Theme") || !themeData["Theme"].is_object()) {
return false;
}
return true;
}Impact: Malformed themes cause crashes, missing required fields accepted Recommendation: Validate schema comprehensively (required fields, value ranges, types) 8. Static Blur Shader StateFiles: static inline ID3D11VertexShader* blurVertexShader = nullptr;
static inline ID3D11PixelShader* blurHorizontalPixelShader = nullptr;
// ... 10+ more static GPU resourcesImpact: Shared state across all instances, no cleanup on error paths, leak risks Recommendation: Move to instance members with proper RAII wrappers 9. Error Handling: Silent FailuresFiles: Multiple
Impact: Users unaware of configuration problems, debugging difficult Recommendation: Surface critical errors to UI, provide actionable messages 10. Missing Input ValidationFiles:
Impact: Resource exhaustion, crashes, DoS potential Recommendation: Add comprehensive input validation with reasonable limits Lower Priority Issues (Not Important but Should Fix)11. Code Duplication
12. Magic Numbers Throughoutfloat contrastBoost = 1.0f + (blurIntensity * 0.05f); // Why 0.05?
textDisabled.w = 0.3f; // Why 0.3?
const float SIGMA = 0.5f; // Why 0.5?Recommendation: Extract to named constants with comments explaining rationale 13. Large Function Complexity
Recommendation: Break into smaller, testable functions 14. Inconsistent Error Logging
15. Missing Documentation
16. Test Coverage Gaps
Design Architecture AssessmentStrengths:✅ Feature-rich: Comprehensive theming with fonts, colors, blur effects Weaknesses:❌ Tight coupling: ThemeManager directly modifies Menu state Architectural Recommendations:
Security Considerations
Recommendations PriorityBefore Merge (Critical):
Short Term (High Priority): Medium Term (Good to Have): Overall Grade: C+ / MediumThe PR demonstrates solid technical skills and delivers valuable functionality, but contains enough safety and design issues that it should not be merged without addressing the critical concerns. The theme system is well-conceived but needs refinement in implementation quality, particularly around resource management, thread safety, and input validation. Estimated Refactoring Effort: 2-3 days to address critical issues, 1 week for comprehensive cleanup. |
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/Features/Menu/Shaders/GaussianBlur_Vertical.hlsl (1)
99-103: Jitter asymmetry fixed.The previous review flagged that
float(i % 2)caused asymmetric jitter for negative indices. The code now correctly usesabs(i) % 2, which ensures symmetric sub-pixel jitter (±0.025 offset) for both positive and negative sample indices, properly centering the kernel at zero.
🧹 Nitpick comments (1)
src/Features/Menu/Shaders/GaussianBlur_Vertical.hlsl (1)
117-118: Consider whether gamma correction is necessary.The gamma correction step
pow(abs(result.rgb), 0.95f)is documented as reducing scanline perception, but it slightly alters color accuracy. For a blur effect, maintaining exact color fidelity might be more important than the subtle perceptual benefit.If scanline artifacts are not visible in practice, consider removing this step for more accurate color reproduction:
- // Apply slight gamma correction to reduce scanline perception - result.rgb = pow(abs(result.rgb), 0.95f) * sign(result.rgb); - return result;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/Features/Menu/Shaders/GaussianBlur_Horizontal.hlsl(1 hunks)src/Features/Menu/Shaders/GaussianBlur_Vertical.hlsl(1 hunks)src/Menu.cpp(11 hunks)src/Menu.h(8 hunks)src/Menu/Fonts.cpp(1 hunks)src/Menu/Fonts.h(1 hunks)src/Menu/ThemeManager.cpp(4 hunks)src/Menu/ThemeManager.h(3 hunks)src/Utils/UI.cpp(11 hunks)src/Utils/UI.h(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/Features/Menu/Shaders/GaussianBlur_Horizontal.hlsl
- src/Menu/ThemeManager.h
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{cpp,cxx,cc,c,h,hpp,hxx,hlsl,hlsli,fx,fxh,py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not include TODO/FIXME placeholders; provide complete, working solutions
Files:
src/Utils/UI.hsrc/Menu/Fonts.hsrc/Menu/Fonts.cppsrc/Menu.cppsrc/Utils/UI.cppsrc/Menu.hsrc/Menu/ThemeManager.cppsrc/Features/Menu/Shaders/GaussianBlur_Vertical.hlsl
src/**/*.{cpp,cxx,cc,h,hpp,hxx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{cpp,cxx,cc,h,hpp,hxx}: Ensure SE/AE/VR runtime compatibility; use runtime detection patterns (e.g., REL::RelocateMember())
Include robust error handling and resource management with graceful degradation in the plugin code
Files:
src/Utils/UI.hsrc/Menu/Fonts.hsrc/Menu/Fonts.cppsrc/Menu.cppsrc/Utils/UI.cppsrc/Menu.hsrc/Menu/ThemeManager.cpp
🧬 Code graph analysis (6)
src/Utils/UI.h (2)
src/Utils/UI.cpp (16)
ButtonWithFlash(1358-1414)ButtonWithFlash(1358-1358)FeatureToggle(1416-1488)FeatureToggle(1416-1416)CalculateLuminance(1222-1235)CalculateLuminance(1222-1222)GetContrastingTextColor(1237-1248)GetContrastingTextColor(1237-1237)CalculateContrastRatio(1250-1260)CalculateContrastRatio(1250-1250)AdjustBackgroundForTextContrast(1262-1278)AdjustBackgroundForTextContrast(1262-1263)ContrastSelectable(1280-1316)ContrastSelectable(1280-1280)ContrastSelectableWithColor(1318-1355)ContrastSelectableWithColor(1318-1318)src/Menu/FeatureListRenderer.cpp (2)
label(314-323)label(314-314)
src/Menu/Fonts.h (3)
src/Menu.cpp (3)
Menu(172-200)BuildFontSignature(162-165)BuildFontSignature(162-162)src/Menu.h (2)
Menu(23-160)FontRoleSettings(383-383)src/Menu/Fonts.cpp (18)
NormalizeFontRoles(88-114)NormalizeFontRoles(88-88)GetDefaultRole(116-119)GetDefaultRole(116-116)BuildFontSignature(121-135)BuildFontSignature(121-121)DiscoverFontCatalog(418-520)DiscoverFontCatalog(418-418)DiscoverFontCatalog(523-526)DiscoverFontCatalog(523-523)FormatFontDisplayName(347-371)FormatFontDisplayName(347-347)DiscoverFonts(529-539)DiscoverFonts(529-529)ValidateFont(541-596)ValidateFont(541-541)IsPathWithinDirectory(141-158)IsPathWithinDirectory(141-141)
src/Menu.cpp (4)
src/Menu.h (4)
FontRoleSettings(383-383)Style(287-305)Menu(23-160)GetSingleton(105-109)src/State.cpp (7)
i(189-189)Save(395-462)Save(395-395)LoadTheme(831-864)LoadTheme(831-831)SaveTheme(866-890)SaveTheme(866-866)src/Menu/Fonts.cpp (8)
BuildFontSignature(121-135)BuildFontSignature(121-121)GetDefaultRole(116-119)GetDefaultRole(116-116)NormalizeFontRoles(88-114)NormalizeFontRoles(88-88)ValidateFont(541-596)ValidateFont(541-541)src/Menu/ThemeManager.cpp (12)
CleanupBlurResources(1549-1572)CleanupBlurResources(1549-1549)LoadTheme(754-791)LoadTheme(754-754)SaveTheme(793-841)SaveTheme(793-794)ForceApplyDefaultTheme(367-413)ForceApplyDefaultTheme(367-367)ReloadFont(415-683)ReloadFont(415-415)SetupImGuiStyle(126-231)SetupImGuiStyle(126-126)
src/Utils/UI.cpp (1)
src/Menu/FeatureListRenderer.cpp (4)
menu(295-312)menu(295-295)menu(380-386)menu(380-380)
src/Menu.h (4)
src/Utils/Serialize.h (1)
nlohmann(2-18)src/Menu.cpp (17)
Menu(172-200)ResolveFontRole(152-160)ResolveFontRole(152-152)LoadTheme(238-254)LoadTheme(238-238)SaveTheme(256-269)SaveTheme(256-256)DiscoverThemes(271-279)DiscoverThemes(271-271)LoadThemePreset(281-318)LoadThemePreset(281-281)CreateDefaultThemes(320-325)CreateDefaultThemes(320-320)BuildFontSignature(162-165)BuildFontSignature(162-162)GetDefaultFontRole(167-170)GetDefaultFontRole(167-167)src/State.cpp (4)
LoadTheme(831-864)LoadTheme(831-831)SaveTheme(866-890)SaveTheme(866-866)src/Menu/Fonts.cpp (2)
BuildFontSignature(121-135)BuildFontSignature(121-121)
src/Menu/ThemeManager.cpp (5)
src/Menu/Fonts.cpp (2)
IsPathWithinDirectory(141-158)IsPathWithinDirectory(141-141)src/State.cpp (5)
i(189-189)LoadTheme(831-864)LoadTheme(831-831)SaveTheme(866-890)SaveTheme(866-866)src/Utils/UI.cpp (4)
CalculateLuminance(1222-1235)CalculateLuminance(1222-1222)AdjustBackgroundForTextContrast(1262-1278)AdjustBackgroundForTextContrast(1262-1263)src/Utils/FileSystem.cpp (4)
GetFontsPath(46-49)GetFontsPath(46-46)GetThemesPath(76-79)GetThemesPath(76-76)src/Menu.cpp (6)
GetDefaultFontRole(167-170)GetDefaultFontRole(167-167)LoadTheme(238-254)LoadTheme(238-238)SaveTheme(256-269)SaveTheme(256-256)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Validate shader compilation (Flatrim, .github/configs/shader-validation.yaml)
- GitHub Check: Validate shader compilation (VR, .github/configs/shader-validation-vr.yaml)
- GitHub Check: Build plugin and addons
| // Uniforms | ||
| cbuffer BlurBuffer : register(b0) | ||
| { | ||
| float4 TexelSize; // x = 1/width, y = 1/height, z = blur strength, w = unused | ||
| int4 BlurParams; // x = samples, y = unused, z = unused, w = unused | ||
| };Gaussian Blur Shader | ||
| // Based on Unrimp rendering engine's separable blur implementation | ||
| // Credits: Christian Ofenberg and the Unrimp project (https://github.com/cofenberg/unrimp) | ||
| // License: MIT License | ||
| // Used for ImGui background blur effects - Second pass (vertical) | ||
|
|
||
| // Uniforms | ||
| cbuffer BlurBuffer : register(b0) | ||
| { | ||
| float4 TexelSize; // x = 1/width, y = 1/height, z = blur strength, w = unused | ||
| int4 BlurParams; // x = samples, y = unused, z = unused, w = unused | ||
| }; |
There was a problem hiding this comment.
Remove duplicate header and constant buffer declaration.
Lines 31-47 contain a duplicate header comment and a second cbuffer BlurBuffer declaration. HLSL will reject duplicate constant buffer names in the same shader, causing a compilation error.
Remove the duplicate section:
-// Uniforms
-cbuffer BlurBuffer : register(b0)
-{
- float4 TexelSize; // x = 1/width, y = 1/height, z = blur strength, w = unused
- int4 BlurParams; // x = samples, y = unused, z = unused, w = unused
-};Gaussian Blur Shader
-// Based on Unrimp rendering engine's separable blur implementation
-// Credits: Christian Ofenberg and the Unrimp project (https://github.com/cofenberg/unrimp)
-// License: MIT License
-// Used for ImGui background blur effects - Second pass (vertical)
-
-// Uniforms
-cbuffer BlurBuffer : register(b0)
-{
- float4 TexelSize; // x = 1/width, y = 1/height, z = blur strength, w = unused
- int4 BlurParams; // x = samples, y = unused, z = unused, w = unused
-};
-
SamplerState LinearSampler : register(s0);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Uniforms | |
| cbuffer BlurBuffer : register(b0) | |
| { | |
| float4 TexelSize; // x = 1/width, y = 1/height, z = blur strength, w = unused | |
| int4 BlurParams; // x = samples, y = unused, z = unused, w = unused | |
| };Gaussian Blur Shader | |
| // Based on Unrimp rendering engine's separable blur implementation | |
| // Credits: Christian Ofenberg and the Unrimp project (https://github.com/cofenberg/unrimp) | |
| // License: MIT License | |
| // Used for ImGui background blur effects - Second pass (vertical) | |
| // Uniforms | |
| cbuffer BlurBuffer : register(b0) | |
| { | |
| float4 TexelSize; // x = 1/width, y = 1/height, z = blur strength, w = unused | |
| int4 BlurParams; // x = samples, y = unused, z = unused, w = unused | |
| }; | |
| SamplerState LinearSampler : register(s0); |
🤖 Prompt for AI Agents
In src/Features/Menu/Shaders/GaussianBlur_Vertical.hlsl around lines 31 to 47,
there is a duplicated header comment and a second cbuffer BlurBuffer declaration
which will cause HLSL compilation errors; remove the duplicate block (the
repeated header comment and the second cbuffer declaration) so only the original
single cbuffer BlurBuffer and its comment remain, ensuring there is just one
constant buffer definition for BlurBuffer in the file.
| static inline constexpr std::array<FontRoleDescriptor, static_cast<size_t>(FontRole::Count)> FontRoleDescriptors = { | ||
| FontRoleDescriptor{ "Body", "Body Text", 1.0f }, | ||
| FontRoleDescriptor{ "Heading", "Headings", 1.0f }, | ||
| FontRoleDescriptor{ "Subheading", "Subheadings", 1.0f }, | ||
| FontRoleDescriptor{ "Subtitle", "Subtitles", 1.0f } | ||
| }; |
There was a problem hiding this comment.
Fill in the missing font-role descriptors and defaults.
FontRoleDescriptors is sized for all roles but only initializes the first four. As soon as a theme JSON specifies "Caption" or "Monospace", ResolveFontRole() will never match (empty key), the signature builder writes duplicate empty keys, and we silently fall back to the body font. The default ThemeSettings::FontRoles array mirrors the same bug, so the shipped defaults never seed those roles either. Please add proper descriptors (stable key/display name/default scale) and populate the default role settings (family/style/file/scale) for Caption and Monospace so the new roles actually work.
Also applies to: 229-245
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/Utils/UI.cpp (1)
22-31: Include<array>to satisfystd::arrayusage
InitializeMenuIconsnow relies onstd::array, yet<array>isn’t included here. Without the direct include the build fails once PCH/transitive coverage changes. Add the header with the other standard includes.#include <format> #include <functional> #include <iomanip> +#include <array> #include <mutex> #include <sstream> #include <stb_image.h>src/Menu.cpp (1)
172-190: Release uiIcons.search in destructorThe search icon is not released.
uiIcons.logo.Release(); +uiIcons.search.Release(); uiIcons.featureSettingRevert.Release();
♻️ Duplicate comments (2)
src/Menu/ThemeManager.h (1)
3-9: Add the missing standard headers
ThemeManager.hstill usesstd::unique_ptrandstd::time_twithout including<memory>and<ctime>, so the header isn’t self-contained and will break in TU’s lacking transitive includes. Please include both headers at the top.#include <d3d11.h> #include <filesystem> #include <imgui.h> #include <mutex> #include <nlohmann/json.hpp> +#include <memory> +#include <ctime> #include <string> #include <vector>src/Menu.cpp (1)
271-279: Good: ensure DiscoverThemes() is called before returning namesThis prevents empty lists on first call.
🧹 Nitpick comments (4)
src/Menu/ThemeManager.cpp (4)
1310-1338: Recreate textures when backbuffer format changesCreateBlurTextures() only checks width/height; if swap chain format changes (e.g., sRGB toggles), textures aren’t recreated.
- Track and compare the DXGI_FORMAT as well; recreate when it differs.
Also applies to: 1380-1382
303-365: Prefer RAII for temporary COM pointers and always restore pipeline stateOMGetRenderTargets(), GetResource(), QueryInterface() use raw pointers with manual Release and many early returns → leak risk and fragile state.
- Use winrt::com_ptr for currentRTV/currentRT/currentTexture.
- Save/restore VS/PS/IA topology alongside OM/RS/viewport to avoid state leakage.
I can provide a patch if you want to switch these to com_ptr.
341-359: Per‑window full blur is O(Nwindows) passes; consider batchingRunning a full two‑pass blur per visible window scales poorly. Compute the union (or minimal set) of rects and blur once, or blur full frame then composite per‑window.
This will significantly cut draw calls and shader work.
595-600: Duplicate/inconsistent cache updates in ReloadFontYou set cachedFontSize twice and cachedFontName from both body role and themeSettings. Pick one source and remove duplication.
- menu.cachedFontName = const_cast<Menu&>(menu).GetFontRoleSettings(Menu::FontRole::Body).File; - cachedFontSize = fontSize; + menu.cachedFontName = themeSettings.FontName; + cachedFontSize = fontSize;Also applies to: 677-681
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/Features/Menu/Shaders/GaussianBlur_Horizontal.hlsl(1 hunks)src/Features/Menu/Shaders/GaussianBlur_Vertical.hlsl(1 hunks)src/Menu.cpp(11 hunks)src/Menu.h(8 hunks)src/Menu/Fonts.cpp(1 hunks)src/Menu/Fonts.h(1 hunks)src/Menu/ThemeManager.cpp(4 hunks)src/Menu/ThemeManager.h(3 hunks)src/Utils/UI.cpp(11 hunks)src/Utils/UI.h(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/Menu/Fonts.cpp
- src/Features/Menu/Shaders/GaussianBlur_Horizontal.hlsl
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{cpp,cxx,cc,c,h,hpp,hxx,hlsl,hlsli,fx,fxh,py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not include TODO/FIXME placeholders; provide complete, working solutions
Files:
src/Menu.cppsrc/Menu/ThemeManager.hsrc/Menu/Fonts.hsrc/Menu/ThemeManager.cppsrc/Features/Menu/Shaders/GaussianBlur_Vertical.hlslsrc/Utils/UI.hsrc/Utils/UI.cppsrc/Menu.h
src/**/*.{cpp,cxx,cc,h,hpp,hxx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{cpp,cxx,cc,h,hpp,hxx}: Ensure SE/AE/VR runtime compatibility; use runtime detection patterns (e.g., REL::RelocateMember())
Include robust error handling and resource management with graceful degradation in the plugin code
Files:
src/Menu.cppsrc/Menu/ThemeManager.hsrc/Menu/Fonts.hsrc/Menu/ThemeManager.cppsrc/Utils/UI.hsrc/Utils/UI.cppsrc/Menu.h
🧬 Code graph analysis (7)
src/Menu.cpp (4)
src/Menu.h (4)
FontRoleSettings(383-383)Style(287-305)Menu(23-160)GetSingleton(105-109)src/State.cpp (7)
i(189-189)Save(395-462)Save(395-395)LoadTheme(831-864)LoadTheme(831-831)SaveTheme(866-890)SaveTheme(866-866)src/Menu/Fonts.cpp (8)
BuildFontSignature(121-135)BuildFontSignature(121-121)GetDefaultRole(116-119)GetDefaultRole(116-116)NormalizeFontRoles(88-114)NormalizeFontRoles(88-88)ValidateFont(541-596)ValidateFont(541-541)src/Menu/ThemeManager.cpp (12)
CleanupBlurResources(1549-1572)CleanupBlurResources(1549-1549)LoadTheme(754-791)LoadTheme(754-754)SaveTheme(793-841)SaveTheme(793-794)ForceApplyDefaultTheme(367-413)ForceApplyDefaultTheme(367-367)ReloadFont(415-683)ReloadFont(415-415)SetupImGuiStyle(126-231)SetupImGuiStyle(126-126)
src/Menu/ThemeManager.h (1)
src/Menu/ThemeManager.cpp (40)
ResolveFontSize(990-1008)ResolveFontSize(990-990)SetupImGuiStyle(126-231)SetupImGuiStyle(126-126)ReloadFont(415-683)ReloadFont(415-415)ApplyBackgroundBlur(233-258)ApplyBackgroundBlur(233-233)RenderBackgroundBlur(260-365)RenderBackgroundBlur(260-260)ForceApplyDefaultTheme(367-413)ForceApplyDefaultTheme(367-367)InitializeBlurShaders(1012-1304)InitializeBlurShaders(1012-1012)CreateBlurTextures(1306-1382)CreateBlurTextures(1306-1306)PerformGaussianBlur(1384-1547)PerformGaussianBlur(1384-1384)CleanupBlurResources(1549-1572)CleanupBlurResources(1549-1549)DiscoverThemes(686-740)DiscoverThemes(686-686)GetThemeNames(742-752)GetThemeNames(742-742)LoadTheme(754-791)LoadTheme(754-754)SaveTheme(793-841)SaveTheme(793-794)GetThemeInfo(843-849)GetThemeInfo(843-843)RefreshThemes(851-855)RefreshThemes(851-851)GetThemesDirectory(857-860)GetThemesDirectory(857-857)CreateDefaultThemeFiles(862-924)CreateDefaultThemeFiles(862-862)LoadThemeFile(926-983)LoadThemeFile(926-926)ValidateThemeData(985-988)ValidateThemeData(985-985)
src/Menu/Fonts.h (3)
src/Menu.cpp (3)
Menu(172-200)BuildFontSignature(162-165)BuildFontSignature(162-162)src/Menu.h (2)
Menu(23-160)FontRoleSettings(383-383)src/Menu/Fonts.cpp (18)
NormalizeFontRoles(88-114)NormalizeFontRoles(88-88)GetDefaultRole(116-119)GetDefaultRole(116-116)BuildFontSignature(121-135)BuildFontSignature(121-121)DiscoverFontCatalog(418-520)DiscoverFontCatalog(418-418)DiscoverFontCatalog(523-526)DiscoverFontCatalog(523-523)FormatFontDisplayName(347-371)FormatFontDisplayName(347-347)DiscoverFonts(529-539)DiscoverFonts(529-529)ValidateFont(541-596)ValidateFont(541-541)IsPathWithinDirectory(141-158)IsPathWithinDirectory(141-141)
src/Menu/ThemeManager.cpp (5)
src/Menu/Fonts.cpp (2)
IsPathWithinDirectory(141-158)IsPathWithinDirectory(141-141)src/State.cpp (3)
i(189-189)LoadTheme(831-864)LoadTheme(831-831)src/Utils/UI.cpp (4)
CalculateLuminance(1222-1235)CalculateLuminance(1222-1222)AdjustBackgroundForTextContrast(1262-1278)AdjustBackgroundForTextContrast(1262-1263)src/Utils/FileSystem.cpp (4)
GetFontsPath(46-49)GetFontsPath(46-46)GetThemesPath(76-79)GetThemesPath(76-76)src/Menu.cpp (6)
GetDefaultFontRole(167-170)GetDefaultFontRole(167-167)DiscoverThemes(271-279)DiscoverThemes(271-271)LoadTheme(238-254)LoadTheme(238-238)
src/Utils/UI.h (1)
src/Utils/UI.cpp (16)
ButtonWithFlash(1358-1414)ButtonWithFlash(1358-1358)FeatureToggle(1416-1488)FeatureToggle(1416-1416)CalculateLuminance(1222-1235)CalculateLuminance(1222-1222)GetContrastingTextColor(1237-1248)GetContrastingTextColor(1237-1237)CalculateContrastRatio(1250-1260)CalculateContrastRatio(1250-1250)AdjustBackgroundForTextContrast(1262-1278)AdjustBackgroundForTextContrast(1262-1263)ContrastSelectable(1280-1316)ContrastSelectable(1280-1280)ContrastSelectableWithColor(1318-1355)ContrastSelectableWithColor(1318-1318)
src/Utils/UI.cpp (1)
src/Menu/FeatureListRenderer.cpp (4)
menu(295-312)menu(295-295)menu(380-386)menu(380-380)
src/Menu.h (4)
src/Menu.cpp (17)
Menu(172-200)ResolveFontRole(152-160)ResolveFontRole(152-152)LoadTheme(238-254)LoadTheme(238-238)SaveTheme(256-269)SaveTheme(256-256)DiscoverThemes(271-279)DiscoverThemes(271-271)LoadThemePreset(281-318)LoadThemePreset(281-281)CreateDefaultThemes(320-325)CreateDefaultThemes(320-320)BuildFontSignature(162-165)BuildFontSignature(162-162)GetDefaultFontRole(167-170)GetDefaultFontRole(167-167)src/State.cpp (4)
LoadTheme(831-864)LoadTheme(831-831)SaveTheme(866-890)SaveTheme(866-866)src/Menu/Fonts.cpp (2)
BuildFontSignature(121-135)BuildFontSignature(121-121)src/Menu/ThemeManager.h (1)
ThemeManager(126-323)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Validate shader compilation (Flatrim, .github/configs/shader-validation.yaml)
- GitHub Check: Validate shader compilation (VR, .github/configs/shader-validation-vr.yaml)
- GitHub Check: Build plugin and addons
🔇 Additional comments (1)
src/Menu.cpp (1)
592-598: Good: defer font reload before any ImGui opsReloading fonts here avoids atlas rebuild during a frame. This looks correct.
| { | ||
| float4 TexelSize; // x = 1/width, y = 1/height, z = blur strength, w = unused | ||
| int4 BlurParams; // x = samples, y = unused, z = unused, w = unused | ||
| };Gaussian Blur Shader | ||
| // Based on Unrimp rendering engine's separable blur implementation | ||
| // Credits: Christian Ofenberg and the Unrimp project (https://github.com/cofenberg/unrimp) | ||
| // License: MIT License | ||
| // Used for ImGui background blur effects - Second pass (vertical) | ||
|
|
||
| // Uniforms | ||
| cbuffer BlurBuffer : register(b0) | ||
| { | ||
| float4 TexelSize; // x = 1/width, y = 1/height, z = blur strength, w = unused | ||
| int4 BlurParams; // x = samples, y = unused, z = unused, w = unused | ||
| }; |
There was a problem hiding this comment.
Fix duplicated BlurBuffer block and stray text
Line [36] emits };Gaussian Blur Shader followed by a second BlurBuffer declaration. That stray text breaks HLSL parsing, and the duplicated constant buffer definition (same register) will make the shader fail to compile. Drop the extra text and keep only a single BlurBuffer definition.
-cbuffer BlurBuffer : register(b0)
-{
- float4 TexelSize; // x = 1/width, y = 1/height, z = blur strength, w = unused
- int4 BlurParams; // x = samples, y = unused, z = unused, w = unused
-};Gaussian Blur Shader
-// Based on Unrimp rendering engine's separable blur implementation
-// Credits: Christian Ofenberg and the Unrimp project (https://github.com/cofenberg/unrimp)
-// License: MIT License
-// Used for ImGui background blur effects - Second pass (vertical)
-
-// Uniforms
-cbuffer BlurBuffer : register(b0)
-{
- float4 TexelSize; // x = 1/width, y = 1/height, z = blur strength, w = unused
- int4 BlurParams; // x = samples, y = unused, z = unused, w = unused
-};
+cbuffer BlurBuffer : register(b0)
+{
+ float4 TexelSize; // x = 1/width, y = 1/height, z = blur strength, w = unused
+ int4 BlurParams; // x = samples, y = unused, z = unused, w = unused
+};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { | |
| float4 TexelSize; // x = 1/width, y = 1/height, z = blur strength, w = unused | |
| int4 BlurParams; // x = samples, y = unused, z = unused, w = unused | |
| };Gaussian Blur Shader | |
| // Based on Unrimp rendering engine's separable blur implementation | |
| // Credits: Christian Ofenberg and the Unrimp project (https://github.com/cofenberg/unrimp) | |
| // License: MIT License | |
| // Used for ImGui background blur effects - Second pass (vertical) | |
| // Uniforms | |
| cbuffer BlurBuffer : register(b0) | |
| { | |
| float4 TexelSize; // x = 1/width, y = 1/height, z = blur strength, w = unused | |
| int4 BlurParams; // x = samples, y = unused, z = unused, w = unused | |
| }; | |
| cbuffer BlurBuffer : register(b0) | |
| { | |
| float4 TexelSize; // x = 1/width, y = 1/height, z = blur strength, w = unused | |
| int4 BlurParams; // x = samples, y = unused, z = unused, w = unused | |
| }; |
🤖 Prompt for AI Agents
In src/Features/Menu/Shaders/GaussianBlur_Vertical.hlsl around lines 33 to 47,
remove the stray text "};Gaussian Blur Shader" and delete the duplicated
BlurBuffer constant buffer declaration so only one cbuffer BlurBuffer :
register(b0) block remains; ensure the final file contains a single, correctly
formatted BlurBuffer cbuffer with TexelSize and BlurParams and no extraneous
text between/around the braces.
| bool IsEnabled = false; | ||
| std::unordered_map<std::string, int> Menu::categoryCounts; |
There was a problem hiding this comment.
Remove stray global IsEnabled shadowing Menu::IsEnabled
This global duplicates/shadows the class member and can cause confusion/ODR issues.
-bool IsEnabled = false;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| bool IsEnabled = false; | |
| std::unordered_map<std::string, int> Menu::categoryCounts; | |
| std::unordered_map<std::string, int> Menu::categoryCounts; |
🤖 Prompt for AI Agents
In src/Menu.cpp around lines 149-150 there is a stray global "bool IsEnabled =
false;" that duplicates/shadows the class/static member and risks ODR/confusion;
remove that global declaration and, if the static member needs a
definition/initialization in this translation unit, add a definition using the
class scope (e.g. define "bool Menu::IsEnabled = false;" in this file) so only
the intended Menu::IsEnabled exists.
| static inline constexpr std::array<FontRoleDescriptor, static_cast<size_t>(FontRole::Count)> FontRoleDescriptors = { | ||
| FontRoleDescriptor{ "Body", "Body Text", 1.0f }, | ||
| FontRoleDescriptor{ "Heading", "Headings", 1.0f }, | ||
| FontRoleDescriptor{ "Subheading", "Subheadings", 1.0f }, | ||
| FontRoleDescriptor{ "Subtitle", "Subtitles", 1.0f } | ||
| }; | ||
|
|
There was a problem hiding this comment.
FontRoleDescriptors missing entries (Caption, Monospace) → bad defaults
Enum has 6 roles but descriptors define only 4. For missing roles defaultScale is 0, breaking size resolution and logs.
Add the two descriptors:
static inline constexpr std::array<FontRoleDescriptor, static_cast<size_t>(FontRole::Count)> FontRoleDescriptors = {
FontRoleDescriptor{ "Body", "Body Text", 1.0f },
FontRoleDescriptor{ "Heading", "Headings", 1.0f },
FontRoleDescriptor{ "Subheading", "Subheadings", 1.0f },
- FontRoleDescriptor{ "Subtitle", "Subtitles", 1.0f }
+ FontRoleDescriptor{ "Subtitle", "Subtitles", 1.0f },
+ FontRoleDescriptor{ "Caption", "Captions", 0.9f },
+ FontRoleDescriptor{ "Monospace", "Monospace", 1.0f }
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| static inline constexpr std::array<FontRoleDescriptor, static_cast<size_t>(FontRole::Count)> FontRoleDescriptors = { | |
| FontRoleDescriptor{ "Body", "Body Text", 1.0f }, | |
| FontRoleDescriptor{ "Heading", "Headings", 1.0f }, | |
| FontRoleDescriptor{ "Subheading", "Subheadings", 1.0f }, | |
| FontRoleDescriptor{ "Subtitle", "Subtitles", 1.0f } | |
| }; | |
| static inline constexpr std::array<FontRoleDescriptor, static_cast<size_t>(FontRole::Count)> FontRoleDescriptors = { | |
| FontRoleDescriptor{ "Body", "Body Text", 1.0f }, | |
| FontRoleDescriptor{ "Heading", "Headings", 1.0f }, | |
| FontRoleDescriptor{ "Subheading","Subheadings",1.0f }, | |
| FontRoleDescriptor{ "Subtitle", "Subtitles", 1.0f }, | |
| FontRoleDescriptor{ "Caption", "Captions", 0.9f }, | |
| FontRoleDescriptor{ "Monospace", "Monospace", 1.0f } | |
| }; |
🤖 Prompt for AI Agents
In src/Menu.h around lines 77 to 83, the static FontRoleDescriptors array only
defines 4 descriptors while the FontRole enum has 6 entries, leaving Caption and
Monospace without descriptors (causing defaultScale 0). Add FontRoleDescriptor
entries for "Caption" (display name e.g. "Captions") and "Monospace" (display
name e.g. "Monospace") with a defaultScale of 1.0f so the array covers all
FontRole values and size resolution/logging work correctly.
| std::array<std::string, static_cast<size_t>(FontRole::Count)> cachedFontFilesByRole = []() { | ||
| std::array<std::string, static_cast<size_t>(FontRole::Count)> files{}; | ||
| auto setFile = [&files](FontRole role, std::string value) { | ||
| files[static_cast<size_t>(role)] = std::move(value); | ||
| }; | ||
| setFile(FontRole::Body, "Jost/Jost-Regular.ttf"); | ||
| setFile(FontRole::Heading, "Jost/Jost-Regular.ttf"); | ||
| setFile(FontRole::Subheading, "Jost/Jost-Regular.ttf"); | ||
| setFile(FontRole::Subtitle, "Jost/Jost-Regular.ttf"); | ||
| return files; | ||
| }(); | ||
| mutable std::array<float, static_cast<size_t>(FontRole::Count)> cachedFontPixelSizesByRole = {}; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Initialize cachedFontFilesByRole for all roles
Two roles are left default-empty.
setFile(FontRole::Body, "Jost/Jost-Regular.ttf");
setFile(FontRole::Heading, "Jost/Jost-Regular.ttf");
setFile(FontRole::Subheading, "Jost/Jost-Regular.ttf");
setFile(FontRole::Subtitle, "Jost/Jost-Regular.ttf");
+ setFile(FontRole::Caption, "Jost/Jost-Light.ttf");
+ setFile(FontRole::Monospace, "IBMPlexMono/IBMPlexMono-Regular.ttf");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| std::array<std::string, static_cast<size_t>(FontRole::Count)> cachedFontFilesByRole = []() { | |
| std::array<std::string, static_cast<size_t>(FontRole::Count)> files{}; | |
| auto setFile = [&files](FontRole role, std::string value) { | |
| files[static_cast<size_t>(role)] = std::move(value); | |
| }; | |
| setFile(FontRole::Body, "Jost/Jost-Regular.ttf"); | |
| setFile(FontRole::Heading, "Jost/Jost-Regular.ttf"); | |
| setFile(FontRole::Subheading, "Jost/Jost-Regular.ttf"); | |
| setFile(FontRole::Subtitle, "Jost/Jost-Regular.ttf"); | |
| return files; | |
| }(); | |
| mutable std::array<float, static_cast<size_t>(FontRole::Count)> cachedFontPixelSizesByRole = {}; | |
| std::array<std::string, static_cast<size_t>(FontRole::Count)> cachedFontFilesByRole = []() { | |
| std::array<std::string, static_cast<size_t>(FontRole::Count)> files{}; | |
| auto setFile = [&files](FontRole role, std::string value) { | |
| files[static_cast<size_t>(role)] = std::move(value); | |
| }; | |
| setFile(FontRole::Body, "Jost/Jost-Regular.ttf"); | |
| setFile(FontRole::Heading, "Jost/Jost-Regular.ttf"); | |
| setFile(FontRole::Subheading, "Jost/Jost-Regular.ttf"); | |
| setFile(FontRole::Subtitle, "Jost/Jost-Regular.ttf"); | |
| setFile(FontRole::Caption, "Jost/Jost-Light.ttf"); | |
| setFile(FontRole::Monospace, "IBMPlexMono/IBMPlexMono-Regular.ttf"); | |
| return files; | |
| }(); | |
| mutable std::array<float, static_cast<size_t>(FontRole::Count)> cachedFontPixelSizesByRole = {}; |
🤖 Prompt for AI Agents
In src/Menu.h around lines 150-161, the lambda that initializes
cachedFontFilesByRole leaves two enum indices as empty strings; update the
initializer so every FontRole index is assigned a valid path. Either add setFile
calls for the two missing roles (use the appropriate font paths) or, after the
explicit setFile calls, iterate over files and set any empty entries to a
sensible default (e.g., "Jost/Jost-Regular.ttf") so no role remains
default-empty.
| std::array<FontRoleSettings, static_cast<size_t>(FontRole::Count)> FontRoles = []() { | ||
| std::array<FontRoleSettings, static_cast<size_t>(FontRole::Count)> roles{}; | ||
| auto setRole = [&roles](FontRole role, std::string family, std::string style, std::string file, float sizeScale) { | ||
| auto index = static_cast<size_t>(role); | ||
| roles[index].Family = std::move(family); | ||
| roles[index].Style = std::move(style); | ||
| roles[index].File = std::move(file); | ||
| roles[index].SizeScale = sizeScale; | ||
| }; | ||
|
|
||
| setRole(FontRole::Body, "Jost", "Regular", "Jost/Jost-Regular.ttf", 1.0f); | ||
| setRole(FontRole::Heading, "Jost", "Regular", "Jost/Jost-Regular.ttf", 1.0f); | ||
| setRole(FontRole::Subheading, "Jost", "Regular", "Jost/Jost-Regular.ttf", 1.0f); | ||
| setRole(FontRole::Subtitle, "Jost", "Regular", "Jost/Jost-Regular.ttf", 1.0f); | ||
|
|
||
| return roles; | ||
| }(); | ||
|
|
There was a problem hiding this comment.
Provide defaults for FontRoles (Caption, Monospace)
Defaults for last two roles are missing; they remain empty and force fallbacks.
setRole(FontRole::Body, "Jost", "Regular", "Jost/Jost-Regular.ttf", 1.0f);
setRole(FontRole::Heading, "Jost", "Regular", "Jost/Jost-Regular.ttf", 1.0f);
setRole(FontRole::Subheading, "Jost", "Regular", "Jost/Jost-Regular.ttf", 1.0f);
setRole(FontRole::Subtitle, "Jost", "Regular", "Jost/Jost-Regular.ttf", 1.0f);
+ setRole(FontRole::Caption, "Jost", "Light", "Jost/Jost-Light.ttf", 0.9f);
+ setRole(FontRole::Monospace, "IBMPlexMono", "Regular", "IBMPlexMono/IBMPlexMono-Regular.ttf", 1.0f);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| std::array<FontRoleSettings, static_cast<size_t>(FontRole::Count)> FontRoles = []() { | |
| std::array<FontRoleSettings, static_cast<size_t>(FontRole::Count)> roles{}; | |
| auto setRole = [&roles](FontRole role, std::string family, std::string style, std::string file, float sizeScale) { | |
| auto index = static_cast<size_t>(role); | |
| roles[index].Family = std::move(family); | |
| roles[index].Style = std::move(style); | |
| roles[index].File = std::move(file); | |
| roles[index].SizeScale = sizeScale; | |
| }; | |
| setRole(FontRole::Body, "Jost", "Regular", "Jost/Jost-Regular.ttf", 1.0f); | |
| setRole(FontRole::Heading, "Jost", "Regular", "Jost/Jost-Regular.ttf", 1.0f); | |
| setRole(FontRole::Subheading, "Jost", "Regular", "Jost/Jost-Regular.ttf", 1.0f); | |
| setRole(FontRole::Subtitle, "Jost", "Regular", "Jost/Jost-Regular.ttf", 1.0f); | |
| return roles; | |
| }(); | |
| std::array<FontRoleSettings, static_cast<size_t>(FontRole::Count)> FontRoles = []() { | |
| std::array<FontRoleSettings, static_cast<size_t>(FontRole::Count)> roles{}; | |
| auto setRole = [&roles](FontRole role, std::string family, std::string style, std::string file, float sizeScale) { | |
| auto index = static_cast<size_t>(role); | |
| roles[index].Family = std::move(family); | |
| roles[index].Style = std::move(style); | |
| roles[index].File = std::move(file); | |
| roles[index].SizeScale = sizeScale; | |
| }; | |
| setRole(FontRole::Body, "Jost", "Regular", "Jost/Jost-Regular.ttf", 1.0f); | |
| setRole(FontRole::Heading, "Jost", "Regular", "Jost/Jost-Regular.ttf", 1.0f); | |
| setRole(FontRole::Subheading, "Jost", "Regular", "Jost/Jost-Regular.ttf", 1.0f); | |
| setRole(FontRole::Subtitle, "Jost", "Regular", "Jost/Jost-Regular.ttf", 1.0f); | |
| setRole(FontRole::Caption, "Jost", "Light", "Jost/Jost-Light.ttf", 0.9f); | |
| setRole(FontRole::Monospace, "IBMPlexMono","Regular", "IBMPlexMono/IBMPlexMono-Regular.ttf", 1.0f); | |
| return roles; | |
| }(); |
🤖 Prompt for AI Agents
In src/Menu.h around lines 229 to 246 the FontRoles array initializer omits
defaults for FontRole::Caption and FontRole::Monospace leaving them empty; add
setRole calls for those two roles (e.g. setRole(FontRole::Caption, "Jost",
"Regular", "Jost/Jost-Regular.ttf", 0.85f); setRole(FontRole::Monospace,
"RobotoMono", "Regular", "RobotoMono/RobotoMono-Regular.ttf", 1.0f)) so all
entries in the std::array are populated with sensible defaults consistent with
the other roles.
| #include <algorithm> | ||
| #include <cstring> | ||
| #include <string> | ||
| #include <string_view> | ||
| #include <vector> | ||
|
|
There was a problem hiding this comment.
Include <filesystem> for path declarations
This header declares functions taking std::filesystem::path, but it doesn’t include <filesystem>. Without it, any TU that includes Fonts.h without a transitive <filesystem> will fail to compile. Add the missing header near the top.
#include "../Menu.h"
#include <algorithm>
#include <cstring>
+#include <filesystem>
#include <string>
#include <string_view>
#include <vector>🤖 Prompt for AI Agents
In src/Menu/Fonts.h around lines 4 to 9, the header declares functions using
std::filesystem::path but does not include <filesystem>; add #include
<filesystem> near the other standard includes at the top of the file so any TU
including Fonts.h will have the filesystem declarations available.
| if (isThemeCorrupted) { | ||
| logger::warn("Theme appears corrupted, attempting to reload Default.json to prevent ImGui defaults"); | ||
| // Note: This violates const correctness but is necessary for emergency theme recovery | ||
| // TODO: Refactor to use separate recovery path that doesn't require const_cast | ||
| if (const_cast<Menu*>(&menu)->LoadThemePreset("Default")) { | ||
| logger::info("Successfully reloaded Default.json theme"); | ||
| } else { | ||
| logger::error("Failed to reload Default.json - ImGui may show hardcoded defaults"); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Avoid const_cast in recovery path
Mutating Menu from a const reference is unsafe and risks UB. Don’t const_cast here.
- Call a non-mutating fallback instead (e.g., ForceApplyDefaultTheme()) or change SetupImGuiStyle(Menu&) to accept non-const and perform recovery upstream before styling.
Example minimal fix:
- if (const_cast<Menu*>(&menu)->LoadThemePreset("Default")) {
+ if (ThemeManager::ForceApplyDefaultTheme(), true) { // apply colors without mutating Menu hereCommittable suggestion skipped: line range outside the PR's diff.
| static bool initialized = false; | ||
| static bool initializationFailed = false; | ||
|
|
||
| if (initialized || initializationFailed) { | ||
| return initialized; | ||
| } |
There was a problem hiding this comment.
Blur init flags survive Cleanup; shaders never re‑init
InitializeBlurShaders() uses static initialized/initializationFailed locals that are not reset by CleanupBlurResources(), so after cleanup blur can’t reinitialize.
Replace locals with file‑scope flags and reset them in Cleanup; or derive readiness from com_ptrs.
Proposed fix:
@@ (near top file-scope helpers)
namespace {
+ // Track blur init across calls and allow reset from CleanupBlurResources()
+ std::atomic<bool> g_blurShadersInitialized{false};
+ std::atomic<bool> g_blurInitFailed{false};
}
@@ bool ThemeManager::InitializeBlurShaders()
- static bool initialized = false;
- static bool initializationFailed = false;
-
- if (initialized || initializationFailed) {
- return initialized;
- }
+ if (g_blurShadersInitialized.load() || g_blurInitFailed.load()) {
+ return g_blurShadersInitialized.load();
+ }
@@ on any compile/create failure branches
- initializationFailed = true;
+ g_blurInitFailed = true;
return false;
@@ on success
- initialized = true;
+ g_blurShadersInitialized = true;
logger::info("Gaussian blur shaders initialized successfully with Unrimp architecture");
return true;
@@ void ThemeManager::CleanupBlurResources()
currentBlurIntensity = 0.0f;
+ g_blurShadersInitialized = false;
+ g_blurInitFailed = false;Also applies to: 1187-1191, 1204-1206, 1211-1215, 1228-1230, 1247-1252, 1264-1269, 1284-1289, 1291-1294, 1568-1572
🤖 Prompt for AI Agents
In src/Menu/ThemeManager.cpp around lines 1016-1021 (and similarly at 1187-1191,
1204-1206, 1211-1215, 1228-1230, 1247-1252, 1264-1269, 1284-1289, 1291-1294,
1568-1572), the InitializeBlurShaders() functions use function-local static
booleans (initialized / initializationFailed) that are not cleared by
CleanupBlurResources(), preventing reinitialization after cleanup; replace these
local static flags with file-scope flags (or remove them entirely and instead
test actual resource com_ptr readiness) and ensure CleanupBlurResources() resets
those file-scope flags (or releases/clears the com_ptrs) so subsequent calls can
attempt initialization again; update all listed locations consistently to use
the same file-scope flags or readiness checks and reset them in the cleanup
path.
| // First pass: Horizontal blur (source -> blur texture 1) | ||
| context->OMSetRenderTargets(1, &rtv1Ptr, nullptr); | ||
| context->PSSetShader(blurHorizontalPixelShader.get(), nullptr, 0); | ||
| context->PSSetShaderResources(0, 1, &sourceSRV); | ||
| context->Draw(3, 0); // Draw fullscreen triangle | ||
|
|
||
| // Second pass: Vertical blur (blur texture 1 -> blur texture 2) | ||
| context->OMSetRenderTargets(1, &rtv2Ptr, nullptr); | ||
| context->PSSetShader(blurVerticalPixelShader.get(), nullptr, 0); | ||
| ID3D11ShaderResourceView* nullSRV = nullptr; | ||
| auto srv1Ptr = blurSRV1.get(); | ||
| auto srv2Ptr = blurSRV2.get(); | ||
| context->PSSetShaderResources(0, 1, &nullSRV); // Clear previous SRV | ||
| context->PSSetShaderResources(0, 1, &srv1Ptr); | ||
| context->Draw(3, 0); | ||
|
|
||
| // Final composition: Blend blurred result back to main render target (only in menu area) | ||
| context->RSSetViewports(1, &originalViewport); | ||
| context->OMSetRenderTargets(1, &targetRTV, nullptr); | ||
|
|
||
| // Enable scissor test to limit blur to menu area | ||
| ID3D11RasterizerState* originalRS = nullptr; | ||
| context->RSGetState(&originalRS); | ||
|
|
||
| // Create rasterizer state with scissor test enabled for final composition | ||
| ID3D11RasterizerState* scissorRS = nullptr; | ||
| D3D11_RASTERIZER_DESC rsDesc = {}; | ||
| if (originalRS) { | ||
| originalRS->GetDesc(&rsDesc); | ||
| } else { | ||
| rsDesc.FillMode = D3D11_FILL_SOLID; | ||
| rsDesc.CullMode = D3D11_CULL_BACK; | ||
| rsDesc.FrontCounterClockwise = FALSE; | ||
| rsDesc.DepthBias = 0; | ||
| rsDesc.DepthBiasClamp = 0.0f; | ||
| rsDesc.SlopeScaledDepthBias = 0.0f; | ||
| rsDesc.DepthClipEnable = TRUE; | ||
| rsDesc.MultisampleEnable = FALSE; | ||
| rsDesc.AntialiasedLineEnable = FALSE; | ||
| } | ||
| rsDesc.ScissorEnable = TRUE; | ||
|
|
||
| device->CreateRasterizerState(&rsDesc, &scissorRS); | ||
| if (scissorRS) { | ||
| context->RSSetState(scissorRS); | ||
| } | ||
|
|
||
| // Set blend state for proper compositing | ||
| float blendFactor[4] = { 1.0f, 1.0f, 1.0f, currentBlurIntensity * 0.8f }; | ||
| context->OMSetBlendState(blurBlendState.get(), blendFactor, 0xFFFFFFFF); | ||
|
|
||
| context->PSSetShaderResources(0, 1, &nullSRV); // Clear previous SRV | ||
| context->PSSetShaderResources(0, 1, &srv2Ptr); | ||
| context->Draw(3, 0); | ||
|
|
||
| // Restore original state |
There was a problem hiding this comment.
Final “composition” pass applies extra blur; make it pass‑through
You reuse the vertical blur PS for composition without changing samples → third blur pass is applied. Set sampleCount=1 for pass‑through before the final draw.
// Final composition: Blend blurred result back to main render target (only in menu area)
context->RSSetViewports(1, &originalViewport);
context->OMSetRenderTargets(1, &targetRTV, nullptr);
+ // Use pass-through composition: 1 sample => no extra blur on final pass
+ constants.blurParams[0] = 1;
+ context->UpdateSubresource(blurConstantBuffer.get(), 0, nullptr, &constants, 0, 0);
+ context->PSSetShader(blurVerticalPixelShader.get(), nullptr, 0);
@@
context->PSSetShaderResources(0, 1, &nullSRV); // Clear previous SRV
context->PSSetShaderResources(0, 1, &srv2Ptr);
context->Draw(3, 0);Also consider saving/restoring VS/PS and IA topology to avoid leaking pipeline state.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // First pass: Horizontal blur (source -> blur texture 1) | |
| context->OMSetRenderTargets(1, &rtv1Ptr, nullptr); | |
| context->PSSetShader(blurHorizontalPixelShader.get(), nullptr, 0); | |
| context->PSSetShaderResources(0, 1, &sourceSRV); | |
| context->Draw(3, 0); // Draw fullscreen triangle | |
| // Second pass: Vertical blur (blur texture 1 -> blur texture 2) | |
| context->OMSetRenderTargets(1, &rtv2Ptr, nullptr); | |
| context->PSSetShader(blurVerticalPixelShader.get(), nullptr, 0); | |
| ID3D11ShaderResourceView* nullSRV = nullptr; | |
| auto srv1Ptr = blurSRV1.get(); | |
| auto srv2Ptr = blurSRV2.get(); | |
| context->PSSetShaderResources(0, 1, &nullSRV); // Clear previous SRV | |
| context->PSSetShaderResources(0, 1, &srv1Ptr); | |
| context->Draw(3, 0); | |
| // Final composition: Blend blurred result back to main render target (only in menu area) | |
| context->RSSetViewports(1, &originalViewport); | |
| context->OMSetRenderTargets(1, &targetRTV, nullptr); | |
| // Enable scissor test to limit blur to menu area | |
| ID3D11RasterizerState* originalRS = nullptr; | |
| context->RSGetState(&originalRS); | |
| // Create rasterizer state with scissor test enabled for final composition | |
| ID3D11RasterizerState* scissorRS = nullptr; | |
| D3D11_RASTERIZER_DESC rsDesc = {}; | |
| if (originalRS) { | |
| originalRS->GetDesc(&rsDesc); | |
| } else { | |
| rsDesc.FillMode = D3D11_FILL_SOLID; | |
| rsDesc.CullMode = D3D11_CULL_BACK; | |
| rsDesc.FrontCounterClockwise = FALSE; | |
| rsDesc.DepthBias = 0; | |
| rsDesc.DepthBiasClamp = 0.0f; | |
| rsDesc.SlopeScaledDepthBias = 0.0f; | |
| rsDesc.DepthClipEnable = TRUE; | |
| rsDesc.MultisampleEnable = FALSE; | |
| rsDesc.AntialiasedLineEnable = FALSE; | |
| } | |
| rsDesc.ScissorEnable = TRUE; | |
| device->CreateRasterizerState(&rsDesc, &scissorRS); | |
| if (scissorRS) { | |
| context->RSSetState(scissorRS); | |
| } | |
| // Set blend state for proper compositing | |
| float blendFactor[4] = { 1.0f, 1.0f, 1.0f, currentBlurIntensity * 0.8f }; | |
| context->OMSetBlendState(blurBlendState.get(), blendFactor, 0xFFFFFFFF); | |
| context->PSSetShaderResources(0, 1, &nullSRV); // Clear previous SRV | |
| context->PSSetShaderResources(0, 1, &srv2Ptr); | |
| context->Draw(3, 0); | |
| // Restore original state | |
| // Final composition: Blend blurred result back to main render target (only in menu area) | |
| context->RSSetViewports(1, &originalViewport); | |
| context->OMSetRenderTargets(1, &targetRTV, nullptr); | |
| // Use pass-through composition: 1 sample => no extra blur on final pass | |
| constants.blurParams[0] = 1; | |
| context->UpdateSubresource(blurConstantBuffer.get(), 0, nullptr, &constants, 0, 0); | |
| context->PSSetShader(blurVerticalPixelShader.get(), nullptr, 0); | |
| // Enable scissor test to limit blur to menu area | |
| ID3D11RasterizerState* originalRS = nullptr; | |
| context->RSGetState(&originalRS); | |
| // Create rasterizer state with scissor test enabled for final composition | |
| ID3D11RasterizerState* scissorRS = nullptr; | |
| D3D11_RASTERIZER_DESC rsDesc = {}; | |
| if (originalRS) { | |
| originalRS->GetDesc(&rsDesc); | |
| } else { | |
| rsDesc.FillMode = D3D11_FILL_SOLID; | |
| rsDesc.CullMode = D3D11_CULL_BACK; | |
| rsDesc.FrontCounterClockwise = FALSE; | |
| rsDesc.DepthBias = 0; | |
| rsDesc.DepthBiasClamp = 0.0f; | |
| rsDesc.SlopeScaledDepthBias = 0.0f; | |
| rsDesc.DepthClipEnable = TRUE; | |
| rsDesc.MultisampleEnable = FALSE; | |
| rsDesc.AntialiasedLineEnable = FALSE; | |
| } | |
| rsDesc.ScissorEnable = TRUE; | |
| device->CreateRasterizerState(&rsDesc, &scissorRS); | |
| if (scissorRS) { | |
| context->RSSetState(scissorRS); | |
| } | |
| // Set blend state for proper compositing | |
| float blendFactor[4] = { 1.0f, 1.0f, 1.0f, currentBlurIntensity * 0.8f }; | |
| context->OMSetBlendState(blurBlendState.get(), blendFactor, 0xFFFFFFFF); | |
| context->PSSetShaderResources(0, 1, &nullSRV); // Clear previous SRV | |
| context->PSSetShaderResources(0, 1, &srv2Ptr); | |
| context->Draw(3, 0); | |
| // Restore original state |
🤖 Prompt for AI Agents
In src/Menu/ThemeManager.cpp around lines 1474-1529, the final composition
reuses the vertical blur pixel shader and thus applies a third unwanted blur;
before the final Draw bind a pass-through pixel shader (or switch the sampler
used by the PS to one with sampleCount = 1) so the composition simply samples
the already-blurred texture without additional taps, and ensure you Save and
Restore the VS and PS and the IA topology (input assembler primitive/topology)
around the blur/composite sequence to avoid leaking pipeline state; also
save/restore the original rasterizer and blend states if not already restored.
Will update my description. The blur shaders are for the behind-the-menu blur effect I have made. A title something along the lines of ui modernisation or something is more fitting |
davo0411
changed the title
|
+6000 lines is a lot to review. I'll defer to someone else. |
update to dev
Added collapsible sections for FPS, Draw Calls, VRAM, and A/B Test results in the Performance Overlay. Updated function signatures and logic to support the new collapsible behavior.
2 participants
KEPT AS TO-DO & TRACKING LIST FOR UI CHANGES, I WILL DELETE WHEN DONE.
PR 1:
Settings UI Overhaul & new UI settings (Border controls, scrollbar controls.)
PR 2:
PR 3:
PR 4: