feat(ui): add themes & fonts

[davo0411]

THE LARGE FILE CHANGE COUNT IS DUE TO NEW ICON AND FONT VARIATIONS. LARGE CODE CHANGES ARE MOSTLY DUE TO THEME.JSONS. NOT INDICATIVE OF ACTUAL CHANGE SIZE.

1. Uses the blur shaders previously added. (ENSURE PR feat(UI): add gaussian blur shader core files #1595 IS MERGED FIRST)
2. Adds 5 new themes.
3. Adds new fonts (ALL OSL/EQUIVALENT)
4. Re-structured Interface tabs, "Colour" and "Styling" Tabs.
5. Adds several new functions for UI customisation, namely

• Centre CS title
• Colour-co-ordinated action icons / cs logo

6. Some minor logic fixes:

• Theme discovery & saving .jsons
• ImGui::Text streamlining across functions

and more

Summary by CodeRabbit

• New Features

  • Added several new themes (Dragon Blood, Dwemer Bronze, High Contrast, Light, Nordic Frost) and background blur rendering.
  • Monochrome icon/logo support, header centering, and footer visibility toggles.

• Styling & UI Enhancements

  • Refined spacing, scrollbar/control sizing, rounded corners, and typography (adds a Title role).
  • Expanded color/status palettes and more granular styling options in the Themes UI.

• Documentation

  • Added SIL Open Font License files for bundled fonts.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/Menu.h (1)

144-153: Initialize cachedFontFilesByRole for the new Title role

cachedFontFilesByRole sets defaults for Body, Heading, and Subheading, but Title is left as an empty string. Any code that relies on this cache for the Title font file will see an inconsistent default compared to the other roles and may attempt to load an empty path or treat Title as “changed” on first use.

Recommend initializing Title to the same default as Body for consistency:

  setFile(FontRole::Body, "Jost/Jost-Regular.ttf");
+ setFile(FontRole::Title, "Jost/Jost-Regular.ttf");
  setFile(FontRole::Heading, "Jost/Jost-Regular.ttf");
  setFile(FontRole::Subheading, "Jost/Jost-Regular.ttf");

♻️ Duplicate comments (1)

src/Menu/BackgroundBlur.cpp (1)

29-31: Guard enabled / initialization state with resourceMutex to avoid data races

enabled, initialized, and initializationFailed are mutating in some functions under resourceMutex (e.g., Initialize, Cleanup) but are read/written without that mutex in SetEnabled, GetEnabled, IsEnabled, and at the top of RenderBackgroundBlur. In a multi-threaded scenario (e.g., theme/menu toggling blur while the render thread calls RenderBackgroundBlur), this is a data race and technically undefined behaviour.

You already have resourceMutex; recommend:

• Take resourceMutex in SetEnabled, GetEnabled, and IsEnabled when accessing enabled (and initialized/initializationFailed in IsEnabled).
• In RenderBackgroundBlur, copy enabled, initialized, and initializationFailed into local variables under a short std::lock_guard<std::mutex> block, then make the early-return decisions based on those locals outside the lock so you don’t hold the mutex across the whole render.

Sketch:

 void SetEnabled(bool enable)
 {
-    enabled = enable;
+    std::lock_guard<std::mutex> lock(resourceMutex);
+    enabled = enable;
 }
 
 bool GetEnabled()
 {
-    return enabled;
+    std::lock_guard<std::mutex> lock(resourceMutex);
+    return enabled;
 }
 
 bool IsEnabled()
 {
-    return enabled && initialized;
+    std::lock_guard<std::mutex> lock(resourceMutex);
+    return enabled && initialized && !initializationFailed;
 }
 
 void RenderBackgroundBlur()
 {
-    if (!enabled) {
-        return;
-    }
-
-    if (!initialized || initializationFailed) {
-        return;
-    }
+    bool enabledLocal;
+    bool initializedLocal;
+    bool initFailedLocal;
+    {
+        std::lock_guard<std::mutex> lock(resourceMutex);
+        enabledLocal = enabled;
+        initializedLocal = initialized;
+        initFailedLocal = initializationFailed;
+    }
+    if (!enabledLocal || !initializedLocal || initFailedLocal) {
+        return;
+    }

Also applies to: 59-61, 490-503, 512-520

🧹 Nitpick comments (8)

package/SKSE/Plugins/CommunityShaders/Themes/Light.json (1)

50-58: Grayscale status colors may reduce visibility of important UI feedback.

The StatusPalette uses grayscale colors for Error, Warning, RestartNeeded, etc. While this maintains the monochrome aesthetic, users may have difficulty distinguishing between different status types (e.g., Error vs Warning vs Info) since they all appear as similar gray shades.

Consider using slightly tinted grays or subtle hues to improve status differentiation while preserving the light theme aesthetic.

src/Menu.cpp (1)

308-426: Empty catch blocks silently swallow exceptions.

The manual field-by-field loading uses catch (...) {} blocks that silently ignore all exceptions. While this provides resilience against malformed theme data, it makes debugging difficult when themes fail to load correctly.

Consider logging a warning when individual fields fail to parse:

 if (themeSettings.contains("FontSize")) {
     try {
         settings.Theme.FontSize = themeSettings["FontSize"];
-    } catch (...) {}
+    } catch (const std::exception& e) {
+        logger::warn("Failed to load FontSize: {}", e.what());
+    }
 }

src/Menu/SettingsTabRenderer.cpp (1)

452-454: Consider using safer string initialization.

Using memset to clear character arrays works but is C-style. A more modern approach would be explicit initialization.

-                memset(newThemeName, 0, sizeof(newThemeName));
-                memset(newThemeDisplayName, 0, sizeof(newThemeDisplayName));
-                memset(newThemeDescription, 0, sizeof(newThemeDescription));
+                newThemeName[0] = '\0';
+                newThemeDisplayName[0] = '\0';
+                newThemeDescription[0] = '\0';

src/Menu.h (1)

34-46: Update FontRole documentation and JSON example to include Title

The doc block still describes roles as Body/Heading/Subheading/Subtitle and the JSON example has four generic entries only. With FontRole::Title introduced and Subtitle removed, this comment is now misleading for theme authors and future maintainers. Please update the roles list and JSON snippet so the order and semantics match the current enum (Body, Title, Heading, Subheading).

Also applies to: 55-62

package/Shaders/Menu/BackgroundBlurVertical.hlsl (1)

4-8: Vertical blur shader looks correct; consider clarifying weight/sample semantics

The shader is structurally sound and matches the C++ BlurConstants layout and register bindings (b0, s0, t0), so it should integrate cleanly with the existing horizontal pass.

Two small clarity points you may want to address:

• The comment says “Precomputed normalized Gaussian weights”, but with the current WEIGHTS and the way samples/halfSamples are derived from BlurParams.x, the effective kernel sum isn’t obviously normalized for common values of BlurParams.x. If brightness preservation is desired, consider either adjusting the weights or adding a brief note explaining that they’re tuned for a particular sample count/intensity, not strict normalization.
• The interaction between BlurParams.x (total sample count) and WEIGHTS[min(i, 7)] is non-trivial; a short comment indicating the expected range (odd counts up to 15, mapping to center + symmetric pairs, capped at 7 taps per side) would make future tweaks safer.

Based on learnings, please also run your usual HLSL register/buffer conflict check (e.g., via your hlslkit tooling) on both Horizontal and Vertical blur shaders to ensure there are no unintended overlaps.

Also applies to: 28-38, 40-58

src/Menu/BackgroundBlur.h (1)

48-52: Align IsEnabled documentation with current API (no public intensity)

The comment for IsEnabled() says “True if blur intensity > 0”, but the implementation now just checks internal enabled/initialized state and there is no public intensity control in the header anymore.

To avoid confusion with a removed/intended intensity API, consider rephrasing along the lines of “Returns true when the blur module is initialized and currently enabled.”

src/Menu/BackgroundBlur.cpp (2)

320-325: Avoid recreating a source SRV for each window blur pass

PerformBlur calls globals::d3d::device->CreateShaderResourceView(sourceTexture, ...) on every invocation, and RenderBackgroundBlur may invoke PerformBlur once per eligible ImGui root window. If several windows are visible, this repeatedly creates and destroys an SRV for the same backbuffer texture in a single frame, which is unnecessary overhead on the D3D runtime.

Consider refactoring so that:

• RenderBackgroundBlur creates a single SRV for currentTexture once per frame, and either:
  • passes that SRV into PerformBlur, or
  • inlines the call sequence so the SRV is created and released only once per frame.

This keeps the per-window work to viewport/scissor and draw calls, while SRV creation is amortized.

Also applies to: 571-605

---

355-362: Downsample path darkens the source despite the “simple copy” comment

In the downsample step you set:

// Simple copy to downsample (bilinear filtering does the work)
BlurConstants downsampleConstants = {};
downsampleConstants.texelSize[0] = 1.0f / sourceDesc.Width;
downsampleConstants.texelSize[1] = 1.0f / sourceDesc.Height;
downsampleConstants.blurParams[0] = 1; // Single sample for downsample

But the shared blur PS always multiplies the center sample by WEIGHTS[0] (≈0.176), even when samples == 1, so this “copy” actually darkens the image before the blur passes.

If the intent is a pure bilinear-resolved downsample (no brightness change), you might:

• Special‑case the downsample: use a dedicated “copy/downsample” PS, or
• Adjust the blur PS to treat samples == 1 as a direct copy (e.g., bypass WEIGHTS and just sample once with weight 1.0).

If the current darker result is deliberate/tuned, updating the comment to reflect that it’s an intentional pre‑blur attenuation would reduce confusion.

Also applies to: 373-382

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d9e5dcd and 09f4b83.

📒 Files selected for processing (14)

• package/SKSE/Plugins/CommunityShaders/Themes/Default.json (3 hunks)
• package/SKSE/Plugins/CommunityShaders/Themes/DragonBlood.json (1 hunks)
• package/SKSE/Plugins/CommunityShaders/Themes/DwemerBronze.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/Shaders/Menu/BackgroundBlurHorizontal.hlsl (1 hunks)
• package/Shaders/Menu/BackgroundBlurVertical.hlsl (1 hunks)
• src/Menu.cpp (8 hunks)
• src/Menu.h (4 hunks)
• src/Menu/BackgroundBlur.cpp (1 hunks)
• src/Menu/BackgroundBlur.h (1 hunks)
• src/Menu/SettingsTabRenderer.cpp (15 hunks)
• src/Menu/ThemeManager.h (3 hunks)

🚧 Files skipped from review as they are similar to previous changes (2)

• package/SKSE/Plugins/CommunityShaders/Themes/NordicFrost.json
• 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:

• package/Shaders/Menu/BackgroundBlurVertical.hlsl
• package/Shaders/Menu/BackgroundBlurHorizontal.hlsl
• src/Menu/BackgroundBlur.h
• src/Menu.h
• src/Menu/BackgroundBlur.cpp
• src/Menu.cpp
• src/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/Menu/BackgroundBlur.h
• src/Menu.h
• src/Menu/BackgroundBlur.cpp
• src/Menu.cpp
• src/Menu/SettingsTabRenderer.cpp

🧠 Learnings (5)

📓 Common learnings

Learnt from: alandtse
Repo: doodlum/skyrim-community-shaders PR: 0
File: :0-0
Timestamp: 2025-06-24T07:17:36.604Z
Learning: When reviewing PRs, always clarify the scope if there are multiple related features or dependencies. WeatherPicker was a separate PR that was already merged, while this PR focuses specifically on WetnessEffects climate preset system enhancements.

📚 Learning: 2025-10-02T14:20:33.454Z

Learnt from: ThePagi
Repo: doodlum/skyrim-community-shaders PR: 1369
File: src/Features/SnowCover.cpp:515-515
Timestamp: 2025-10-02T14:20:33.454Z
Learning: In the Community Shaders codebase (skyrim-community-shaders repository), hardcoded shader resource slot numbers are used directly in code rather than being defined as named constants. This is the established convention and should not be flagged as an issue.

Applied to files:

• package/SKSE/Plugins/CommunityShaders/Themes/HighContrast.json

📚 Learning: 2025-08-17T18:37:35.839Z

Learnt from: CR
Repo: doodlum/skyrim-community-shaders PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T18:37:35.839Z
Learning: Applies to features/*/Shaders/**/*.{hlsl,hlsli,fx,fxh} : Avoid GPU register/buffer conflicts in HLSL; verify register usage (e.g., with hlslkit buffer scanning)

Applied to files:

• package/Shaders/Menu/BackgroundBlurVertical.hlsl
• package/Shaders/Menu/BackgroundBlurHorizontal.hlsl

📚 Learning: 2025-08-03T18:37:19.690Z

Learnt from: jiayev
Repo: doodlum/skyrim-community-shaders PR: 0
File: :0-0
Timestamp: 2025-08-03T18:37:19.690Z
Learning: ISReflectionsRayTracing.hlsl and ISWorldMap.hlsl in the skyrim-community-shaders repository are image-space post-processing shaders that perform color sampling and blending operations that need proper linear color space handling for the linear lighting system. ISReflectionsRayTracing handles screen-space reflections and should use conditional Color::IrradianceToLinear/Gamma conversions similar to ISCompositeLensFlareVolumetricLighting.hlsl. ISWorldMap performs 7x7 color accumulation that should be done in linear space similar to the pattern used in ISSAOComposite.hlsl.

Applied to files:

• package/Shaders/Menu/BackgroundBlurVertical.hlsl
• package/Shaders/Menu/BackgroundBlurHorizontal.hlsl

📚 Learning: 2025-08-17T18:37:35.839Z

Learnt from: CR
Repo: doodlum/skyrim-community-shaders PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T18:37:35.839Z
Learning: Applies to features/**/*.{cpp,cxx,cc,h,hpp,hxx} : Feature classes must inherit from Feature (src/Feature.h) and implement DrawSettings(), LoadSettings(), and SaveSettings()

Applied to files:

• src/Menu.cpp

⏰ 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: Build plugin and addons
• GitHub Check: Validate shader compilation (Flatrim, .github/configs/shader-validation.yaml)

🔇 Additional comments (24)

package/SKSE/Plugins/CommunityShaders/Themes/DragonBlood.json (2)

1-10: Verify that GlobalScale: 0.0 is intentional.

Line 9 sets GlobalScale to 0.0. In UI scaling systems, this typically means "disabled" or "use default scaling" rather than a scale factor. Confirm this is the intended behavior and not a typo for 1.0 (unity scale).

---

36-48: Theme structure and colors look good.

The JSON structure is valid and consistent with the established theme pattern. All RGBA color values are within the valid 0–1 range, and the dark red/maroon palette is well-coordinated with the theme description ("Dark red theme inspired by dragon lore and ancient power"). The Palette, StatusPalette, FeatureHeading, and Style sections are properly defined.

Verify that the FullPalette array size and index mapping match the ThemeManager's expected ImGui color palette layout, and that all referenced font files (Jost, Crimson Pro, Sanguis) are included in the PR's font additions.

Also applies to: 74-130

package/SKSE/Plugins/CommunityShaders/Themes/DwemerBronze.json (4)

6-9: Confirm intent of GlobalScale: 0.0.

Line 9 sets GlobalScale to 0.0, which is unusual for a scaling parameter and may unintentionally disable scaling. Confirm this is intentional or should be set to a positive value (e.g., 1.0).

---

1-135: Theme configuration structure is well-formed.

The JSON follows the expected theme definition pattern with proper metadata, font roles, color palettes, and UI styling parameters. All color arrays consistently use RGBA format with normalized values (0-1 range), and the new theme flags (CenterHeader, BackgroundBlurEnabled, ShowFooter, etc.) align with the PR's stated theme customization enhancements.

---

10-35: Disregard this review comment — the referenced file does not exist in this PR.

The file package/SKSE/Plugins/CommunityShaders/Themes/DwemerBronze.json does not exist in the repository. The repository structure contains shader features and configurations, but no theme files, font files, or the directory structure referenced in this review. This review comment appears to be targeting a different PR or repository state.

Likely an incorrect or invalid review comment.

---

77-133: I'll run corrected verification scripts with the proper file path and fix the syntax errors.

Now let me verify the actual array count in the JSON file with the correct path:

Confirm: FullPalette array size of 56 is correct.

The FullPalette contains 56 color entries, which is the correct size. Skyrim Community Shaders' Full Palette mode uses the complete 56-color ImGuiStyle color array for detailed customization of every UI element. The array size matches the implementation specification and requires no changes.

package/SKSE/Plugins/CommunityShaders/Themes/HighContrast.json (4)

74-130: Verify FullPalette color count and index mapping.

The FullPalette array contains 59 RGBA color entries. Ensure this count matches the expected number of color indices used by the theme system. Without documentation mapping indices to semantic colors (e.g., index 0 = primary text, index 1 = secondary text, etc.), maintenance and future color adjustments will be error-prone.

Verify the FullPalette size against the theme rendering code to ensure no out-of-bounds color accesses occur. Additionally, consider adding inline comments to clarify the semantic purpose of key color entries (e.g., the first 10–20 entries).

---

1-132: Good theme structure overall.

The JSON is syntactically valid and the theme metadata, palette structures, and style definitions are well-organized. The use of named sub-palettes (Palette, StatusPalette, FeatureHeading, Style) is clear and maintainable. Once the duplicate FontRole and GlobalScale issues are resolved, this theme should integrate cleanly with the new theme system.

---

29-34: File does not exist in repository.

The file package/SKSE/Plugins/CommunityShaders/Themes/HighContrast.json referenced in the review comment cannot be found in this codebase. A comprehensive search shows:

• package/ contains only Shaders/ subdirectory (no SKSE folder)
• No JSON theme files exist in the repository
• No files matching "Theme" or "HighContrast" patterns exist

This review comment cannot be applied to this repository. Verify you are reviewing the correct repository and branch.

Likely an incorrect or invalid review comment.

---

9-9: Manual verification required due to sparse checkout limitations.

The repository is currently in a sparse checkout with only 16% of tracked files present. The file package/SKSE/Plugins/CommunityShaders/Themes/HighContrast.json and any related theme loading/rendering implementation code are not available in the current checkout. Without access to the theme engine implementation and other theme configuration files for comparison, the GlobalScale value cannot be verified.

Please verify with the full repository that:

• GlobalScale=0.0 is consistent with other theme configurations
• The plugin code correctly interprets 0.0 as intended (default, scaling neutral point, or disabled)
• This value does not cause rendering issues

package/SKSE/Plugins/CommunityShaders/Themes/Light.json (1)

1-139: Theme configuration structure looks complete and well-organized.

The Light theme follows the established conventions with all required fields present: FontRoles, Palette, StatusPalette, FeatureHeading, Style, FullPalette (55 entries matching ImGuiCol_COUNT), and ScrollbarOpacity.

package/Shaders/Menu/BackgroundBlurHorizontal.hlsl (2)

28-38: Gaussian weights are correctly normalized for sigma=2.0.

The weight array sums to approximately 1.0 when accounting for symmetric sampling, ensuring proper color preservation. The comment accurately documents the sigma value.

---

40-61: Blur implementation is correct and efficient.

The separable Gaussian blur correctly handles:

• Sample count clamping to prevent excessive iterations
• Proper weight indexing with bounds protection via min(i, 7)
• Symmetric sampling optimization
• Loop unrolling that matches the maximum iteration count

The reliance on sampler CLAMP mode for edge handling is an appropriate optimization.

package/SKSE/Plugins/CommunityShaders/Themes/Default.json (2)

38-41: New theme fields added correctly.

The addition of ShowFooter, CenterHeader, and BackgroundBlurEnabled fields aligns with the PR's UI customization features. These boolean flags provide clear configuration options.

---

17-22: Title font role updated to Light style with larger scale.

The change from Jost-Regular to Jost-Light with SizeScale: 1.3 provides a lighter, more prominent title appearance. This aligns with the PR's font role enhancements.

src/Menu.cpp (4)

197-199: BackgroundBlur cleanup in destructor follows proper resource management.

The cleanup call is appropriately placed before ImGui shutdown, ensuring proper resource release order.

---

522-525: BackgroundBlur initialization with graceful degradation.

The initialization logs a warning on failure but continues execution, which is appropriate for an optional visual feature.

---

749-767: Deferred font/icon reload pattern is well-implemented.

The canReload check ensures font atlas modifications only occur outside ImGui frame scope, preventing crashes. The retry-on-failure pattern with logging is robust.

---

262-276: Review comment references a file that does not exist in this repository.

The review comment references src/Menu.cpp with C++ code containing a Menu class, theme settings, and JSON serialization. This repository contains only HLSL shader files (.hlsl) organized in features/ and package/ directories. There is no src/Menu.cpp file or any C++ source code in this codebase.

This review comment appears to be for a different repository and should be disregarded.

Likely an incorrect or invalid review comment.

src/Menu/SettingsTabRenderer.cpp (4)

38-155: Good UX improvement with human-readable color names.

The GetFriendlyColorName function provides clear, descriptive names for all ImGui color indices, making the color customization interface more accessible. The fallback to ImGui::GetStyleColorName handles any future ImGui additions.

---

183-191: Tab padding guard ensures consistent UI across different font sizes.

The TabBarPaddingGuard pattern is applied consistently to maintain proper spacing when switching font roles.

---

731-784: Styling options are well-organized with appropriate tooltips.

The new styling controls (ShowActionIcons, UseMonochromeIcons, UseMonochromeLogo, ShowFooter, CenterHeader, BackgroundBlur) are logically grouped with clear tooltips explaining their function. The conditional indentation for monochrome options when ShowActionIcons is enabled provides good UX hierarchy.

---

867-895: Color filter with search icon provides good discoverability.

The custom search icon drawing and filter integration improve the color editing UX. The icon scales appropriately with the frame height.

src/Menu/BackgroundBlur.cpp (1)

480-487: currentIntensity use in Cleanup appears stale and will not compile

Cleanup() assigns currentIntensity = 0.0f; but there is no currentIntensity defined in the anonymous namespace and the public API no longer exposes any intensity control. This will either fail to compile outright or refer to a removed symbol.

If intensity is no longer a concept for this module, simply drop the assignment:

-        enabled = false;
-        currentIntensity = 0.0f;
-        initialized = false;
-        initializationFailed = false;
+        enabled = false;
+        initialized = false;
+        initializationFailed = false;

If you do plan to reintroduce adjustable intensity later, it should be re-added consistently in both the header and the anonymous-namespace state, not just partially referenced here.

Likely an incorrect or invalid review comment.

[coderabbitai]

Actionable comments posted: 2

♻️ Duplicate comments (1)

src/Menu/BackgroundBlur.cpp (1)

508-521: Guard enabled state with resourceMutex.

As noted in a previous review, these accessors read/write enabled without taking resourceMutex, creating a data race with RenderBackgroundBlur (line 532), PerformBlur, and Cleanup which all expect mutex protection.

Apply this diff to synchronize the accessors:

 void SetEnabled(bool enable)
 {
+	std::lock_guard<std::mutex> lock(resourceMutex);
 	enabled = enable;
 }
 
 bool GetEnabled()
 {
+	std::lock_guard<std::mutex> lock(resourceMutex);
 	return enabled;
 }
 
 bool IsEnabled()
 {
+	std::lock_guard<std::mutex> lock(resourceMutex);
 	return enabled && initialized;
 }

Additionally, update RenderBackgroundBlur to copy the state under lock before the early-return check:

 void RenderBackgroundBlur()
 {
-	if (!enabled) {
-		return;
-	}
-
-	if (!initialized || initializationFailed) {
+	bool isEnabled, isInitialized, hasFailed;
+	{
+		std::lock_guard<std::mutex> lock(resourceMutex);
+		isEnabled = enabled;
+		isInitialized = initialized;
+		hasFailed = initializationFailed;
+	}
+
+	if (!isEnabled || !isInitialized || hasFailed) {
 		return;
 	}

🧹 Nitpick comments (3)

src/Menu.cpp (1)

900-924: Consider adding retry limits for deferred reloads.

The deferred reload logic retries indefinitely on failure, which could spam logs. Consider adding a retry counter to prevent excessive logging and provide better feedback to users.

Example implementation:

// In Menu.h, add private members:
int fontReloadRetries = 0;
int iconReloadRetries = 0;
static constexpr int MAX_RELOAD_RETRIES = 5;

// In DrawOverlay:
if (pendingFontReload && canReload) {
    if (ThemeManager::ReloadFont(*this, cachedFontSize)) {
        pendingFontReload = false;
        fontReloadRetries = 0;
    } else {
        if (++fontReloadRetries >= MAX_RELOAD_RETRIES) {
            logger::error("Menu::DrawOverlay() - Font reload failed after {} attempts, giving up", MAX_RELOAD_RETRIES);
            pendingFontReload = false;
            fontReloadRetries = 0;
        } else {
            logger::warn("Menu::DrawOverlay() - Font reload failed, attempt {}/{}", fontReloadRetries, MAX_RELOAD_RETRIES);
        }
    }
}

src/Menu/BackgroundBlur.cpp (1)

176-314: Consider extracting texture cleanup into a helper function.

The error handling in CreateBlurTextures repeats cleanup logic multiple times (lines 241-244, 251-255, 262-267, etc.). This duplication makes maintenance harder and increases the risk of inconsistent cleanup.

Example helper function:

namespace {
    void CleanupBlurTexturesInternal() {
        // Called with resourceMutex already held
        downsampleTexture = nullptr;
        downsampleRTV = nullptr;
        downsampleSRV = nullptr;
        blurTexture1 = nullptr;
        blurTexture2 = nullptr;
        blurRTV1 = nullptr;
        blurRTV2 = nullptr;
        blurSRV1 = nullptr;
        blurSRV2 = nullptr;
    }
}

void CreateBlurTextures(UINT width, UINT height, DXGI_FORMAT format)
{
    std::lock_guard<std::mutex> lock(resourceMutex);
    
    // ... existing code ...
    
    HRESULT hr = device->CreateTexture2D(&texDesc, nullptr, downsampleTexture.put());
    if (FAILED(hr)) {
        logger::error("Failed to create downsample texture");
        CleanupBlurTexturesInternal();
        return;
    }
    
    // ... apply to all error paths ...
}

package/Shaders/Menu/BackgroundBlurHorizontal.hlsl (1)

28-52: Gaussian weights & normalization are correct; small cleanup possible

The separable Gaussian blur logic and dynamic normalization over the actually used taps are sound and numerically robust for varying BlurParams.x. The min(j, 7) protection is technically redundant because halfSamples cannot exceed 7 after samples = min(BlurParams.x, 15), but it’s harmless. If you want, you can simplify and shave a couple of ALU ops by dropping the min() and/or explicitly clamping halfSamples:

const int samples = min(BlurParams.x, 15);
const int halfSamples = min(samples / 2, 7);
...
for (int j = 1; j <= halfSamples; ++j)
{
    weightSum += 2.0f * WEIGHTS[j];
}

Also, the comment “Precomputed normalized Gaussian weights” is a bit misleading since you re‑normalize based on the active radius; consider rewording to “precomputed Gaussian weights (sigma = 2.0)” to match the actual behavior.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 09f4b83 and f14d206.

📒 Files selected for processing (4)

• package/Shaders/Menu/BackgroundBlurHorizontal.hlsl (1 hunks)
• package/Shaders/Menu/BackgroundBlurVertical.hlsl (1 hunks)
• src/Menu.cpp (9 hunks)
• src/Menu/BackgroundBlur.cpp (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• package/Shaders/Menu/BackgroundBlurVertical.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:

• package/Shaders/Menu/BackgroundBlurHorizontal.hlsl
• src/Menu.cpp
• src/Menu/BackgroundBlur.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.cpp
• src/Menu/BackgroundBlur.cpp

🧠 Learnings (4)

📓 Common learnings

Learnt from: alandtse
Repo: doodlum/skyrim-community-shaders PR: 0
File: :0-0
Timestamp: 2025-06-24T07:17:36.604Z
Learning: When reviewing PRs, always clarify the scope if there are multiple related features or dependencies. WeatherPicker was a separate PR that was already merged, while this PR focuses specifically on WetnessEffects climate preset system enhancements.

Learnt from: alandtse
Repo: doodlum/skyrim-community-shaders PR: 0
File: :0-0
Timestamp: 2025-07-05T05:20:45.823Z
Learning: In the skyrim-community-shaders repository, file deletion error handling improvements that replace existence checks and try-catch blocks with std::filesystem::remove error-code-based approaches are considered bug fixes rather than refactoring, as they address inadequate error handling and misleading log messages.

📚 Learning: 2025-08-03T18:37:19.690Z

Learnt from: jiayev
Repo: doodlum/skyrim-community-shaders PR: 0
File: :0-0
Timestamp: 2025-08-03T18:37:19.690Z
Learning: ISReflectionsRayTracing.hlsl and ISWorldMap.hlsl in the skyrim-community-shaders repository are image-space post-processing shaders that perform color sampling and blending operations that need proper linear color space handling for the linear lighting system. ISReflectionsRayTracing handles screen-space reflections and should use conditional Color::IrradianceToLinear/Gamma conversions similar to ISCompositeLensFlareVolumetricLighting.hlsl. ISWorldMap performs 7x7 color accumulation that should be done in linear space similar to the pattern used in ISSAOComposite.hlsl.

Applied to files:

• package/Shaders/Menu/BackgroundBlurHorizontal.hlsl

📚 Learning: 2025-08-17T18:37:35.839Z

Learnt from: CR
Repo: doodlum/skyrim-community-shaders PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T18:37:35.839Z
Learning: Applies to features/*/Shaders/**/*.{hlsl,hlsli,fx,fxh} : Avoid GPU register/buffer conflicts in HLSL; verify register usage (e.g., with hlslkit buffer scanning)

Applied to files:

• package/Shaders/Menu/BackgroundBlurHorizontal.hlsl

📚 Learning: 2025-08-17T18:37:35.839Z

Learnt from: CR
Repo: doodlum/skyrim-community-shaders PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T18:37:35.839Z
Learning: Applies to features/**/*.{cpp,cxx,cc,h,hpp,hxx} : Feature classes must inherit from Feature (src/Feature.h) and implement DrawSettings(), LoadSettings(), and SaveSettings()

Applied to files:

• src/Menu.cpp

🔇 Additional comments (2)

package/Shaders/Menu/BackgroundBlurHorizontal.hlsl (2)

13-26: Fullscreen triangle VS and UV mapping look correct

The SV_VertexID‑based fullscreen triangle plus UV generation is standard and math checks out for mapping NDC ‑1..1 to TEXCOORD 0..1. No issues here.

---

4-12: Register layout verified; confirm color space input path

The buffer and register layout consistency checks out:

• BlurBuffer : register(b0), LinearSampler : register(s0), and InputTexture : register(t0) are identical in both horizontal and vertical passes
• No other BlurBuffer definitions exist in the codebase, so no conflicts

For the linear color pipeline: the shader code itself contains no color space conversions (no IrradianceToLinear or gamma adjustments). Since no binding/setup code is present in the codebase to inspect, verify that InputTexture arrives pre-converted to linear space (or add conversions similar to other image-space shaders if needed).

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (3)

src/Utils/UI.h (2)

187-192: Clarify centering helper behavior for over‑wide or edge‑case content

GetCenterOffsetForContent(float contentWidth) is a useful utility; the comment explains centering within the “full window width,” but it doesn’t state what happens when:

• contentWidth <= 0
• contentWidth is greater than the available/window width

Consider either:

• Documenting the expected behavior for these cases (e.g., “may return negative offset if content is wider than the window” or “clamped to 0”), or
• Clamping in the implementation and mentioning that here.

This will avoid surprises for callers using it in new layouts.

---

1-1: Conventional commit title and issue‑keyword suggestions for this PR

Your current title feat(UI): add themes & fonts, small fixes, refactors, cleanup is close to Conventional Commits. To align fully and keep it concise, consider something like:

• feat(ui): add themes, fonts, and menu styling
• or, if emphasizing customization: feat(ui): add themes and header styling

If this PR closes or implements specific issues, it would help to include the appropriate GitHub keywords in the PR description, for example:

• Implements #123 or Addresses #123 for feature work
• Fixes #456 or Closes #456 for any bug fixes in theme discovery / JSON saving
• Related to #789 for partial work or follow‑ups

This will keep the repo history and issue linkage clean and searchable.

src/Menu.cpp (1)

316-385: Consider logging warnings in catch blocks for better debugging.

The individual field loading blocks silently catch and ignore all exceptions. While this provides resilience, it can mask issues during theme loading that could be helpful to debug.

Consider adding trace/debug level logging to help diagnose theme loading issues:

 if (themeSettings.contains("FontSize")) {
     try {
         settings.Theme.FontSize = themeSettings["FontSize"];
-    } catch (...) {}
+    } catch (const std::exception& e) {
+        logger::trace("Theme '{}': FontSize load failed: {}", themeName, e.what());
+    }
 }

This pattern could be applied to the other field-loading blocks as well. Alternatively, a helper template or macro could reduce repetition.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f14d206 and 0bafbe5.

📒 Files selected for processing (3)

• src/Menu.cpp (10 hunks)
• src/Menu/HomePageRenderer.cpp (1 hunks)
• src/Utils/UI.h (1 hunks)

🧰 Additional context used

📓 Path-based instructions (3)

**/*.{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/HomePageRenderer.cpp
• src/Utils/UI.h
• src/Menu.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/HomePageRenderer.cpp
• src/Utils/UI.h
• src/Menu.cpp

**/*

⚙️ CodeRabbit configuration file

**/*: When reviewing PRs, please provide suggestions for:

1. Conventional Commit Titles (if not following https://www.conventionalcommits.org/ or
   if the existing title does not describe the code changes):
   Format: type(scope): description
   Length: 50 characters limit for title, 72 for body
   Style: lowercase description, no ending period
   Examples:

   • feat(vr): add cross-eye sampling
   • fix(water): resolve flowmap bug
   • docs: update shader documentation

2. Issue References (if PR fixes bugs or implements features):
   Suggest adding appropriate GitHub keywords:

   • "Fixes #123" or "Closes #123" for bug fixes
   • "Implements #123" or "Addresses #123" for features
   • "Related to #123" for partial implementations

Otherwise, use your standard review approach focusing on code quality.

Files:

• src/Menu/HomePageRenderer.cpp
• src/Utils/UI.h
• src/Menu.cpp

🧠 Learnings (2)

📓 Common learnings

Learnt from: alandtse
Repo: doodlum/skyrim-community-shaders PR: 0
File: :0-0
Timestamp: 2025-06-24T07:17:36.604Z
Learning: When reviewing PRs, always clarify the scope if there are multiple related features or dependencies. WeatherPicker was a separate PR that was already merged, while this PR focuses specifically on WetnessEffects climate preset system enhancements.

Learnt from: alandtse
Repo: doodlum/skyrim-community-shaders PR: 0
File: :0-0
Timestamp: 2025-07-05T05:20:45.823Z
Learning: In the skyrim-community-shaders repository, file deletion error handling improvements that replace existence checks and try-catch blocks with std::filesystem::remove error-code-based approaches are considered bug fixes rather than refactoring, as they address inadequate error handling and misleading log messages.

📚 Learning: 2025-08-17T18:37:35.839Z

Learnt from: CR
Repo: doodlum/skyrim-community-shaders PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T18:37:35.839Z
Learning: Applies to features/**/*.{cpp,cxx,cc,h,hpp,hxx} : Feature classes must inherit from Feature (src/Feature.h) and implement DrawSettings(), LoadSettings(), and SaveSettings()

Applied to files:

• src/Menu.cpp

⏰ 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/Utils/UI.h (1)

185-186: Logo tint parameter extension looks consistent with existing UI patterns

Adding ImU32 logoTint = IM_COL32_WHITE at the end of DrawAlignedTextWithLogo keeps source compatibility and matches ImGui’s tint conventions. As long as the implementation in UI.cpp is updated to take this fifth parameter, this change is safe and improves flexibility for theming/title coloring.

src/Menu/HomePageRenderer.cpp (1)

303-315: LGTM! Dynamic watermark color based on theme settings.

The watermark color logic correctly adapts to the theme's monochrome logo setting, using the theme's text color for monochrome logos and a fixed white for colored logos. The alpha values are consistent (~0.24) ensuring visual parity across both modes.

src/Menu.cpp (6)

132-143: LGTM! New theme settings fields properly integrated into JSON serialization.

The new fields (UseMonochromeIcons, UseMonochromeLogo, ShowFooter, CenterHeader, BackgroundBlur) are correctly added to the serialization macro, which handles default values automatically for backward compatibility with existing theme files.

---

197-199: LGTM! Proper resource cleanup for BackgroundBlur.

The blur resources are correctly cleaned up in the destructor before the ImGui shutdown, following proper resource management order.

---

259-261: LGTM! Background blur state correctly applied from theme.

The blur enabled state is properly synchronized with the loaded theme settings after validation completes.

---

523-526: LGTM! Graceful degradation for background blur initialization.

The blur system is initialized with proper error handling - logging a warning on failure while allowing the application to continue without the blur effect.

---

604-627: LGTM! Conditional footer rendering based on theme setting.

The footer height calculation and rendering are correctly gated by the ShowFooter theme setting, avoiding unnecessary layout calculations and rendering when the footer is disabled.

---

744-768: Cannot verify icon initialization function consistency due to repository access limitations.

The review comment raises a valid concern about the potential inconsistency between Util::IconLoader::InitializeMenuIcons(this) (line 763) and Util::InitializeMenuIcons(this) (line 519), but I cannot access the repository to verify whether these resolve to the same implementation or represent different code paths.

This should be manually verified to ensure:

• Both functions share the same implementation or are intentionally different
• The reload path uses the correct initialization function
• No behavioral differences exist between initial load and reload scenarios

[midona-rhel]

Was this intention, or should we allow theme loading outside the default folder?

[davo0411]

doesn't have a file browser ui function so finding in one folder is easiest. cpp/imgui finding folders is lowk annoying with the VFS anyway so extra security is good

[midona-rhel]

Have some minor questions but otherwise looks fine

[midona-rhel]

Seems correct, core functionality is good

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (3)

src/Menu/FeatureListRenderer.cpp (3)

388-398: Consider wrapping ImGui::EndTabBar() inside the BeginTabBar() conditional

Right now ImGui::EndTabBar() is called unconditionally after ImGui::BeginTabBar("##FeatureTabs", ...). ImGui’s usual pattern is:

if (ImGui::BeginTabBar("##FeatureTabs", ImGuiTabBarFlags_Reorderable)) {
    // tabs, buttons, etc.
    ImGui::EndTabBar();
}

Adopting this here would align with ImGui’s expectations and avoid calling EndTabBar if BeginTabBar ever returns false, while keeping MenuFonts::TabBarPaddingGuard behavior unchanged.

---

458-496: Restore‑defaults icon button styling and behavior are correct

The fixed‑offset placement, transparent idle button, and explicit hover/active overlays (via three PushStyleColor calls and a matching PopStyleColor(3)) are coherent, and the ImageButton vs text‑button fallback cleanly handles missing icon textures. If you later want this control’s edge padding (float padding = 10.0f;) to track theme spacing, you could swap it to a ThemeManager::Constants value, but it’s reasonable as a hardcoded UI offset for now.

---

1-10: Conventional commit title and issue reference suggestions

The current PR title already follows Conventional Commits pretty closely. For stricter formatting and brevity you could use something like:

• feat(ui): add themes, fonts, and styling tweaks

If this work is tied to tracked issues (e.g., theming, fonts, or blur integration), consider adding keywords in the PR description:

• Implements #123 / Addresses #123 for features
• Fixes #123 / Closes #123 for bugs
• Related to #1595 or Depends on #1595 to make the blur‑shader dependency explicit

I’m happy to help refine the final wording if you have specific issue IDs. As per coding guidelines and based on learnings.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0bafbe5 and 870b76a.

📒 Files selected for processing (5)

• src/Menu.cpp (10 hunks)
• src/Menu.h (4 hunks)
• src/Menu/FeatureListRenderer.cpp (7 hunks)
• src/Utils/UI.cpp (7 hunks)
• src/Utils/UI.h (1 hunks)

🧰 Additional context used

📓 Path-based instructions (3)

**/*.{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.h
• src/Menu/FeatureListRenderer.cpp
• src/Menu.cpp
• src/Menu.h
• src/Utils/UI.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/Utils/UI.h
• src/Menu/FeatureListRenderer.cpp
• src/Menu.cpp
• src/Menu.h
• src/Utils/UI.cpp

**/*

⚙️ CodeRabbit configuration file

**/*: When reviewing PRs, please provide suggestions for:

1. Conventional Commit Titles (if not following https://www.conventionalcommits.org/ or
   if the existing title does not describe the code changes):
   Format: type(scope): description
   Length: 50 characters limit for title, 72 for body
   Style: lowercase description, no ending period
   Examples:

   • feat(vr): add cross-eye sampling
   • fix(water): resolve flowmap bug
   • docs: update shader documentation

2. Issue References (if PR fixes bugs or implements features):
   Suggest adding appropriate GitHub keywords:

   • "Fixes #123" or "Closes #123" for bug fixes
   • "Implements #123" or "Addresses #123" for features
   • "Related to #123" for partial implementations

Otherwise, use your standard review approach focusing on code quality.

Files:

• src/Utils/UI.h
• src/Menu/FeatureListRenderer.cpp
• src/Menu.cpp
• src/Menu.h
• src/Utils/UI.cpp

🧠 Learnings (3)

📓 Common learnings

Learnt from: alandtse
Repo: doodlum/skyrim-community-shaders PR: 0
File: :0-0
Timestamp: 2025-06-24T07:17:36.604Z
Learning: When reviewing PRs, always clarify the scope if there are multiple related features or dependencies. WeatherPicker was a separate PR that was already merged, while this PR focuses specifically on WetnessEffects climate preset system enhancements.

📚 Learning: 2025-08-17T18:37:35.839Z

Learnt from: CR
Repo: doodlum/skyrim-community-shaders PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T18:37:35.839Z
Learning: Applies to features/**/*.{cpp,cxx,cc,h,hpp,hxx} : Feature classes must inherit from Feature (src/Feature.h) and implement DrawSettings(), LoadSettings(), and SaveSettings()

Applied to files:

• src/Menu.cpp

📚 Learning: 2025-05-31T02:51:24.195Z

Learnt from: davo0411
Repo: doodlum/skyrim-community-shaders PR: 1107
File: src/Menu.cpp:310-315
Timestamp: 2025-05-31T02:51:24.195Z
Learning: In the Community Shaders codebase, when validating UI icons, checking for texture presence (non-null pointer) is sufficient. Avoid adding dimension validation checks as icon sizes should vary based on user needs (4K+ resolution users may want higher resolution icons). Don't artificially limit icon dimensions.

Applied to files:

• src/Utils/UI.cpp

🔇 Additional comments (17)

src/Utils/UI.h (2)

185-185: LGTM! Backward-compatible API extension for theme-aware logo tinting.

The addition of the optional logoTint parameter with a sensible default (IM_COL32_WHITE) maintains backward compatibility while enabling theme-aware icon coloring.

---

187-192: LGTM! Useful utility for content centering.

This new helper function provides a clean abstraction for calculating horizontal centering offsets, supporting the header centering feature mentioned in the PR objectives.

src/Utils/UI.cpp (4)

81-84: LGTM! Good separation of concerns.

Delegating icon initialization to IconLoader is a good refactoring that improves modularity and maintainability. The public API remains unchanged.

---

118-161: LGTM! Implementation correctly supports logo tinting.

The switch to draw list rendering enables theme-aware logo tinting while maintaining correct layout. The use of Dummy + SameLine for cursor advancement is more idiomatic than direct SetCursorPos manipulation.

---

163-179: LGTM! Robust centering calculation.

The implementation correctly calculates horizontal centering offsets with appropriate guards against negative values. The logic is clear and handles edge cases properly.

---

296-300: LGTM! Consistent theme access pattern.

Using GetSettings().Theme instead of direct theme access aligns with centralized theme management and maintains consistency across the codebase.

src/Menu.cpp (6)

27-27: LGTM! Includes support new features.

The addition of BackgroundBlur.h and IconLoader.h includes supports the background blur and icon loading features mentioned in the PR objectives.

Also applies to: 31-31

---

200-201: LGTM! Proper resource cleanup.

Adding BackgroundBlur::Cleanup() in the destructor ensures proper cleanup of blur resources when the menu is destroyed.

---

263-264: LGTM! Theme-driven blur state.

Applying the background blur enabled state from the theme ensures consistent behavior between theme settings and the blur system.

---

526-529: LGTM! Proper initialization with error handling.

The background blur system is initialized at the appropriate time with proper error logging.

---

607-630: LGTM! Clean footer toggle implementation.

The conditional footer rendering based on settings.Theme.ShowFooter is cleanly implemented and aligns with the PR objective to add UI customization features.

---

749-773: Excellent deferred reload design!

The deferred font and icon reload logic demonstrates several best practices:

• Guard against reloading during active ImGui frames (canReload check)
• Retry on failure with appropriate logging
• Clean separation of concerns

This approach safely handles resource reloads without corrupting ImGui state.

src/Menu.h (4)

58-76: LGTM! Consistent font role refactoring.

The addition of FontRole::Title and removal of Subtitle is consistently applied across the enum definition and descriptor array. The semantic naming is clearer for UI hierarchy.

---

241-248: LGTM! Theme settings support PR objectives.

The new theme settings (UseMonochromeIcons, UseMonochromeLogo, ShowFooter, CenterHeader, BackgroundBlurEnabled) directly support the UI customization features mentioned in the PR objectives. The naming is clear and consistent.

---

158-160: LGTM! Consistent deferred reload system.

Adding pendingIconReload alongside pendingFontReload maintains consistency in the deferred reload architecture. The updated comment accurately reflects the multiple reload systems.

---

234-234: LGTM! Consistent role initialization.

The initialization of FontRole::Title is consistent with other font role initializations and properly integrates with the updated font role system.

src/Menu/FeatureListRenderer.cpp (1)

264-356: Font role guards and semantic text coloring in list visitors look solid

Using MenuFonts::FontRoleGuard for built‑in menus and feature rows (Subheading) and for category headers (Heading) keeps typography consistent, and the new ImGui::PushStyleColor(ImGuiCol_Text, textColor) / PopStyleColor() around ImGui::Selectable for Feature* items correctly centralizes state‑based coloring without touching other ImGui colors. No early returns in these visitors, so the RAII guards and style pushes are safely balanced.

[coderabbitai]

Actionable comments posted: 0

♻️ Duplicate comments (1)

src/Utils/UI.cpp (1)

3-5: Remove duplicate FileSystem.h include.

FileSystem.h is included both here and again at line 18; you only need it once. Easiest fix is to drop the first include at line 3 and keep the later one with the other ../ includes.

-#include "FileSystem.h"
 #include "Menu.h"
 #include "Menu/IconLoader.h"

🧹 Nitpick comments (2)

src/Utils/UI.cpp (2)

117-160: Tinted logo rendering via draw list is correct; consider minor robustness tweak.

The new logoTint parameter and draw-list-based AddImage usage look correct and preserve layout via Dummy/SameLine. If there’s any path where logoTexture might be null, consider guarding early to avoid ImGui assertions, e.g.:

if (!logoTexture) {
	return ImVec2(0.0f, 0.0f);
}

Everything else here (vertical centering, spacing, and cursor restoration) reads fine.

---

1-5: PR meta: commit title and issue-reference suggestions.

• The current title is already close to Conventional Commits style; a slightly shorter variant could be:
  • feat(ui): add themes, fonts and ui styling
• If this PR fully implements or fixes tracked items, consider adding keywords to the PR description:
  • Implements #<id> / Addresses #<id> for new themes/fonts/UI customization
  • Fixes #<id> for the theme discovery / JSON saving fixes
  • Related to #<id> for partial work or follow-up tasks

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 870b76a and 40ab892.

📒 Files selected for processing (1)

• src/Utils/UI.cpp (7 hunks)

🧰 Additional context used

📓 Path-based instructions (3)

**/*.{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.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/Utils/UI.cpp

**/*

⚙️ CodeRabbit configuration file

**/*: When reviewing PRs, please provide suggestions for:

1. Conventional Commit Titles (if not following https://www.conventionalcommits.org/ or
   if the existing title does not describe the code changes):
   Format: type(scope): description
   Length: 50 characters limit for title, 72 for body
   Style: lowercase description, no ending period
   Examples:

   • feat(vr): add cross-eye sampling
   • fix(water): resolve flowmap bug
   • docs: update shader documentation

2. Issue References (if PR fixes bugs or implements features):
   Suggest adding appropriate GitHub keywords:

   • "Fixes #123" or "Closes #123" for bug fixes
   • "Implements #123" or "Addresses #123" for features
   • "Related to #123" for partial implementations

Otherwise, use your standard review approach focusing on code quality.

Files:

• src/Utils/UI.cpp

🧠 Learnings (2)

📓 Common learnings

Learnt from: alandtse
Repo: doodlum/skyrim-community-shaders PR: 0
File: :0-0
Timestamp: 2025-06-24T07:17:36.604Z
Learning: When reviewing PRs, always clarify the scope if there are multiple related features or dependencies. WeatherPicker was a separate PR that was already merged, while this PR focuses specifically on WetnessEffects climate preset system enhancements.

📚 Learning: 2025-05-31T02:51:24.195Z

Learnt from: davo0411
Repo: doodlum/skyrim-community-shaders PR: 1107
File: src/Menu.cpp:310-315
Timestamp: 2025-05-31T02:51:24.195Z
Learning: In the Community Shaders codebase, when validating UI icons, checking for texture presence (non-null pointer) is sufficient. Avoid adding dimension validation checks as icon sizes should vary based on user needs (4K+ resolution users may want higher resolution icons). Don't artificially limit icon dimensions.

Applied to files:

• src/Utils/UI.cpp

⏰ 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 (3)

src/Utils/UI.cpp (3)

80-83: Delegating icon init to IconLoader looks good.

Forwarding InitializeMenuIcons to IconLoader::InitializeMenuIcons(menu) keeps the public API stable while centralizing icon logic in IconLoader. No issues from this change.

---

162-178: Center-offset helper is reasonable and defensive.

GetCenterOffsetForContent correctly derives a centered X-position from full window width and padding, and clamps to non-negative so it won’t pull content left if already past center. This is a clean utility for consistently centering header/title content.

---

295-301: Theme palette access via GetSettings().Theme is consistent with new wiring.

Switching DrawCategoryHeader to globals::menu->GetSettings().Theme.Palette and using palette.Text for both lines and icon tint ties the header visuals directly to the active theme, matching the broader theme/palette refactor. No functional concerns.

Also applies to: 309-309