Skip to content

Conversation

@davo0411
Copy link
Collaborator

@davo0411 davo0411 commented Nov 6, 2025

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
  1. Some minor logic fixes:
  • Theme discovery & saving .jsons
  • ImGui::Text streamlining across functions
image image image and more

Summary by CodeRabbit

  • New Features

    • Added five new themes: Dragon Blood, Dwemer Bronze, High Contrast, Light, and Nordic Frost.
    • Added monochrome icon and logo support for enhanced theming flexibility.
    • Introduced header centering and footer visibility toggles.
    • Added background blur rendering capability.
  • Styling & UI Enhancements

    • Updated default theme with refined visual styling, spacing, and typography.
    • Enhanced color palette system with improved status colors and feature heading support.
    • Improved typography hierarchy with refined font role system.

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

davo0411 and others added 30 commits September 22, 2025 22:05
Automated formatting by clang-format, prettier, and other hooks.
See https://pre-commit.ci for details.
Automated formatting by clang-format, prettier, and other hooks.
See https://pre-commit.ci for details.
Automated formatting by clang-format, prettier, and other hooks.
See https://pre-commit.ci for details.
Automated formatting by clang-format, prettier, and other hooks.
See https://pre-commit.ci for details.
Automated formatting by clang-format, prettier, and other hooks.
See https://pre-commit.ci for details.
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.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 6, 2025

Walkthrough

This PR adds SIL Open Font License (OFL) files for multiple font families, introduces five new theme configurations (DragonBlood, DwemerBronze, HighContrast, Light, NordicFrost), updates the default theme with visual styling changes, refactors the menu system to integrate icon loading and background blur functionality, extends font role semantics with a new Title role, and refactors UI rendering paths to use the new icon loader and theme-aware rendering patterns.

Changes

Cohort / File(s) Summary
Font License Files
package/Interface/CommunityShaders/Fonts/IBMPlexMono/OFL.txt, package/Interface/CommunityShaders/Fonts/IBMPlexSans/OFL.txt, package/Interface/CommunityShaders/Fonts/IBMPlexSerif/OFL.txt, package/Interface/CommunityShaders/Fonts/Inter/OFL.txt, package/Interface/CommunityShaders/Fonts/OpenDyslexic/OFL.txt, package/Interface/CommunityShaders/Fonts/Roboto/OFL.txt, package/Interface/CommunityShaders/Fonts/Rubik/OFL.txt, package/Interface/CommunityShaders/Fonts/Sanguis/Sanguis OFL License.txt, package/Interface/CommunityShaders/Fonts/Sovngarde/Sovngarde OFL License.txt
Added SIL Open Font License Version 1.1 text files for each font family, documenting license terms, permissions, conditions, and disclaimers.
Theme Configuration Files
package/SKSE/Plugins/CommunityShaders/Themes/Default.json
Updated default theme with font role changes (Jost-Light with SizeScale 1.3), new boolean toggles (ShowFooter, CenterHeader), replaced BackgroundBlur with BackgroundBlurEnabled, adjusted ScrollbarOpacity values, and modified Style properties (borders, padding, rounding, spacing).
New Theme Definitions
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/NordicFrost.json
Added five new theme configurations with metadata, font roles, UI toggles, color palettes (Palette, StatusPalette, FeatureHeading), style parameters, and FullPalette arrays for comprehensive theme customization.
Menu Core System
src/Menu.h, src/Menu.cpp
Refactored ThemeSettings with new fields (UseMonochromeIcons, UseMonochromeLogo, ShowFooter, CenterHeader, BackgroundBlurEnabled), added FontRole::Title replacing Subtitle, integrated IconLoader and BackgroundBlur lifecycles, implemented deferred reload mechanics (pendingFontReload, pendingIconReload), reworked theme loading with guarded/backup/fallback logic and error handling, and updated rendering paths for conditional footer/header display.
Font Management
src/Menu/Fonts.h, src/Menu/Fonts.cpp
Added new TabBarPaddingGuard RAII class to conditionally scale ImGui FramePadding based on tab-to-body font size ratio, with state restoration on destruction.
Icon Loading
src/Menu/IconLoader.h, src/Menu/IconLoader.cpp
Introduced new Util::IconLoader namespace with InitializeMenuIcons, LoadTextureFromFile, GetIconDefinitions, and LoadThemeSpecificIcons functions to handle PNG-to-D3D11 texture conversion, theme-specific icon overrides, and fallback to colored icons; includes comprehensive logging and GPU synchronization.
Feature List Rendering
src/Menu/FeatureListRenderer.cpp
Replaced contrast-based selectable rendering with standard ImGui calls, introduced FontRoleGuard for font switching, added TabBarPaddingGuard for tab bar padding, and simplified icon tinting logic.
Menu Header Rendering
src/Menu/MenuHeaderRenderer.cpp
Added header centering logic when CenterHeader is enabled, introduced monochrome tint support for logos/icons based on UseMonochromeLogo/UseMonochromeIcons, added null texture checks, adjusted watermark color derivation based on theme settings, and updated Title font usage.
Overlay Rendering
src/Menu/OverlayRenderer.cpp
Integrated BackgroundBlur rendering by calling RenderBackgroundBlur before ImGui draws in FinalizeImGuiFrame.
Settings Tab Rendering
src/Menu/SettingsTabRenderer.cpp
Added TabBarPaddingGuard usage, refactored font role assignments (Subheading→Heading), expanded Styling UI with monochrome/footer/center options and live BackgroundBlur toggle, overhauled Colors tab with search filter and hierarchical palette structure, and added comprehensive logging around theme operations.
Home Page Rendering
src/Menu/HomePageRenderer.cpp
Added dynamic watermark color calculation based on UseMonochromeLogo theme setting, deriving color from Text palette with low alpha in monochrome mode.
Theme Management
src/Menu/ThemeManager.h, src/Menu/ThemeManager.cpp
Updated icon size constants (HEADER_BASE_ICON_MULTIPLIER 1.5f→1.85f, DOCKED_ICON_SIZE_MULTIPLIER 1.25f→1.5f), added MO2 Overwrite path to theme discovery with multi-path searching and per-path error handling, enhanced GPU synchronization in ReloadFont via event queries, improved theme persistence logging, and added BLUR SHADER SYSTEM documentation.
UI Utilities
src/Utils/UI.h, src/Utils/UI.cpp
Removed ColorUtils namespace (contrast/luminance utilities), added GetCenterOffsetForContent helper, updated DrawAlignedTextWithLogo signature to accept logoTint parameter, refactored icon initialization to delegate to IconLoader::InitializeMenuIcons, and updated palette access to use MenuSettings::Theme instead of globals.
File System Utilities
src/Utils/FileSystem.cpp
Refactored GetDataPath to use intermediate variable for clarity and added explicit fallback return in exception handler (std::filesystem::current_path() / "Data").

Sequence Diagram(s)

sequenceDiagram
    participant Init as Menu::Init()
    participant Loader as Theme Loader
    participant Parser as JSON Parser
    participant Guard as Guard/Backup
    participant IconLoad as IconLoader
    participant BlurInit as BackgroundBlur

    Init->>Loader: Load default theme
    activate Loader
    Loader->>Parser: Parse theme JSON
    
    alt Parse Success
        Parser->>Guard: Create backup
        Guard->>Loader: Validate fields
        
        alt Validation OK
            Loader->>Loader: Apply theme settings
            Loader->>IconLoad: InitializeMenuIcons
            IconLoad->>IconLoad: Load base icons (monochrome/colored)
            IconLoad->>IconLoad: Load theme-specific overrides
            IconLoad-->>Loader: Icons loaded
            
            Loader->>BlurInit: Apply BackgroundBlurEnabled
            BlurInit-->>Loader: Blur initialized
            Loader-->>Init: Theme applied ✓
        else Validation Fails
            Guard->>Loader: Restore backup
            Loader-->>Init: Theme reverted
        end
    else Parse Fails
        Parser-->>Loader: Error caught
        Guard->>Loader: Restore backup
        Loader-->>Init: Fallback theme applied
    end
    deactivate Loader
Loading
sequenceDiagram
    participant Render as DrawOverlay()
    participant Deferred as Deferred Reload Check
    participant Font as Font Manager
    participant Icon as IconLoader
    participant Blur as BackgroundBlur
    participant ImGui as ImGui Frame

    Render->>Deferred: Check pendingFontReload?
    alt pendingFontReload = true
        Deferred->>Font: ReloadFont with GPU sync
        Font->>Font: GPU idle wait (event query)
        Font->>Font: Invalidate/recreate device objects
        Font-->>Deferred: Font reloaded
        Deferred->>Deferred: Set pendingFontReload = false
    end
    
    Render->>Deferred: Check pendingIconReload?
    alt pendingIconReload = true
        Deferred->>Icon: InitializeMenuIcons
        Icon->>Icon: Load theme icons with overrides
        Icon-->>Deferred: Icons reloaded
        Deferred->>Deferred: Set pendingIconReload = false
    end
    
    Render->>Blur: RenderBackgroundBlur (pre-draw)
    Blur-->>Render: Blur rendered
    
    Render->>ImGui: Draw ImGui frame
    ImGui-->>Render: Frame complete
Loading
sequenceDiagram
    participant IconInit as InitializeMenuIcons()
    participant GetDef as GetIconDefinitions()
    participant LoadTex as LoadTextureFromFile()
    participant D3D as Direct3D 11
    participant Theme as LoadThemeSpecificIcons()
    
    IconInit->>GetDef: Get all icon definitions
    GetDef-->>IconInit: [Icon definitions + paths]
    
    loop For each icon definition
        IconInit->>LoadTex: Load icon from base path
        activate LoadTex
        LoadTex->>D3D: Load PNG via stb_image
        D3D->>D3D: Create D3D11 texture
        D3D->>D3D: Upload pixel data
        D3D->>D3D: Create SRV + mipmaps
        LoadTex-->>IconInit: SRV ✓
        deactivate LoadTex
        
        alt Load fails & monochrome
            IconInit->>LoadTex: Retry with colored icon
            LoadTex-->>IconInit: Fallback SRV ✓
        end
    end
    
    IconInit->>Theme: Apply theme-specific overrides
    activate Theme
    Theme->>Theme: Check theme directory
    Theme->>LoadTex: Load override for matching icon
    LoadTex-->>Theme: Override SRV
    Theme-->>IconInit: Overrides applied
    deactivate Theme
    
    IconInit-->>IconInit: Return success (any icons loaded)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

  • Menu system integration: The refactoring of Menu.h/cpp with new deferred reload mechanics, FontRole changes (Title replacing Subtitle), ThemeSettings expansion, and lifecycle integration of IconLoader and BackgroundBlur requires careful verification of state management and error handling paths.
  • Icon loader implementation: New IconLoader subsystem with D3D11 texture handling, theme-specific overrides, GPU synchronization, and fallback logic involves multiple error paths and resource management that need scrutiny.
  • Theme configuration changes: Five new theme files plus Default.json modifications require validation that JSON structure is consistent and all new fields (ShowFooter, CenterHeader, BackgroundBlurEnabled, etc.) are properly wired through the system.
  • Multi-renderer refactoring: Changes across FeatureListRenderer, MenuHeaderRenderer, SettingsTabRenderer, and HomePageRenderer involve font role switching, centering logic, monochrome tinting, and conditional rendering that must be cross-verified for consistency.
  • Theme manager enhancements: Multi-path theme discovery, GPU synchronization via event queries, and enhanced error handling require careful review of filesystem traversal and GPU idle waiting logic.
  • UI utility changes: Removal of ColorUtils namespace and refactoring of palette access patterns need verification that all dependent code paths use the new MenuSettings::Theme access pattern correctly.

Areas requiring extra attention:

  • GPU synchronization in ReloadFont (event query loop with timeout) — verify wait conditions and timeout behavior
  • Icon loading fallback flow (monochrome → colored) — ensure all code paths handle null textures safely
  • Deferred reload gating in DrawOverlay — verify ImGui frame safety of font/icon reloads
  • Theme loading guarded/backup/fallback logic — trace all error branches to ensure state consistency
  • SettingsTabRenderer TabBarPaddingGuard usage — verify padding calculations across all tab contexts
  • MenuSettings::Theme palette access migration — scan all renderers for proper access patterns

Possibly related PRs

Suggested reviewers

  • alandtse
  • doodlum

Poem

🐰 Fonts now licensed, themes in bloom,
Icons load where shadows loom.
Titles bold and footers dance,
Blur and monochrome enhance.
Five new themes hop into place,
While GPU waits at deferred's pace! 🎨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 24.24% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the PR's main changes: adding themes and fonts, along with fixes and refactors. While broad in scope, it clearly represents the primary changes across the changeset.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Automated formatting by clang-format, prettier, and other hooks.
See https://pre-commit.ci for details.
@github-actions
Copy link

github-actions bot commented Nov 6, 2025

Using provided base ref: b102274
Using base ref: b102274
Base commit date: 2025-11-19T11:19:34Z (Wednesday, November 19, 2025 11:19 AM)
No actionable suggestions for changed features.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (2)
src/Menu/FeatureListRenderer.cpp (1)

465-466: Consider using theme colors for button hover states.

The hover and active colors are now hardcoded as ImVec4(1.0f, 1.0f, 1.0f, 0.3f) and ImVec4(1.0f, 1.0f, 1.0f, 0.5f). These could be derived from theme palette colors to maintain consistency with user-selected themes.

Apply this diff to use theme-derived colors:

 ImGui::PushStyleColor(ImGuiCol_Button, ImVec4(0.0f, 0.0f, 0.0f, 0.0f));
-ImGui::PushStyleColor(ImGuiCol_ButtonHovered, ImVec4(1.0f, 1.0f, 1.0f, 0.3f));
-ImGui::PushStyleColor(ImGuiCol_ButtonActive, ImVec4(1.0f, 1.0f, 1.0f, 0.5f));
+auto hoverColor = themeSettings.Palette.Text;
+hoverColor.w = 0.3f;
+ImGui::PushStyleColor(ImGuiCol_ButtonHovered, hoverColor);
+auto activeColor = themeSettings.Palette.Text;
+activeColor.w = 0.5f;
+ImGui::PushStyleColor(ImGuiCol_ButtonActive, activeColor);
src/Menu/IconLoader.cpp (1)

216-228: Simplify fallback path construction to reduce error risk.

The current string manipulation for monochrome fallback paths is complex and error-prone, using multiple find, erase, and substr operations that could fail silently if the path structure doesn't match expectations.

Consider a more robust approach using filesystem operations:

 if (basePath.find("Monochrome") != std::string::npos) {
-    std::string fallbackPath = basePath;
-    size_t pos = fallbackPath.find("\\Monochrome");
-    if (pos != std::string::npos) {
-        fallbackPath.erase(pos, 11);  // Remove "\Monochrome"
-    }
-    fallbackPath += iconDef.filename;
-    // Try to extract just the filename from iconDef.filename if it has path
-    size_t lastSlash = iconDef.filename.find_last_of("\\/");
-    if (lastSlash != std::string::npos) {
-        std::string justFilename = iconDef.filename.substr(lastSlash + 1);
-        fallbackPath = fallbackPath.substr(0, fallbackPath.find_last_of("\\/") + 1) + justFilename;
-    }
+    // Build fallback path using filesystem operations
+    auto baseIconPath = std::filesystem::path(basePath).parent_path();
+    auto iconFilename = std::filesystem::path(iconDef.filename).filename();
+    std::string fallbackPath = (baseIconPath / iconFilename).string();
+    
     if (LoadTextureFromFile(device, fallbackPath.c_str(), iconDef.texture, *iconDef.size)) {

This approach:

  • Uses filesystem path operations instead of string manipulation
  • Automatically handles path separators
  • More maintainable and less error-prone
📜 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 3f14125 and d9e5dcd.

⛔ Files ignored due to path filters (55)
  • package/Interface/CommunityShaders/Fonts/CrimsonPro/CrimsonPro-Light.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Fonts/CrimsonPro/CrimsonPro-Regular.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Fonts/CrimsonPro/CrimsonPro-SemiBold.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Fonts/IBMPlexMono/IBMPlexMono-Light.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Fonts/IBMPlexMono/IBMPlexMono-Regular.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Fonts/IBMPlexMono/IBMPlexMono-SemiBold.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Fonts/IBMPlexSans/IBMPlexSans-Light.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Fonts/IBMPlexSans/IBMPlexSans-Regular.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Fonts/IBMPlexSans/IBMPlexSans-SemiBold.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Fonts/IBMPlexSans/IBMPlexSans_Condensed-Light.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Fonts/IBMPlexSans/IBMPlexSans_Condensed-Regular.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Fonts/IBMPlexSans/IBMPlexSans_Condensed-SemiBold.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Fonts/IBMPlexSerif/IBMPlexSerif-Light.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Fonts/IBMPlexSerif/IBMPlexSerif-Regular.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Fonts/IBMPlexSerif/IBMPlexSerif-SemiBold.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Fonts/Inter/Inter_24pt-Light.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Fonts/Inter/Inter_24pt-Regular.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Fonts/Inter/Inter_24pt-SemiBold.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Fonts/OpenDyslexic/OpenDyslexic3-Bold.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Fonts/OpenDyslexic/OpenDyslexic3-Regular.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Fonts/Roboto/Roboto-Bold.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Fonts/Roboto/Roboto-Regular.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Fonts/Roboto/Roboto-SemiBold.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Fonts/Roboto/Roboto-Thin.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Fonts/Roboto/Roboto_Condensed-Light.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Fonts/Roboto/Roboto_Condensed-Regular.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Fonts/Roboto/Roboto_Condensed-SemiBold.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Fonts/RobotoSlab/RobotoSlab-Light.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Fonts/RobotoSlab/RobotoSlab-Regular.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Fonts/RobotoSlab/RobotoSlab-SemiBold.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Fonts/Rubik/Rubik-Light.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Fonts/Rubik/Rubik-Regular.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Fonts/Rubik/Rubik-SemiBold.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Fonts/Sanguis/Sanguis.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Fonts/Sovngarde/SovngardeBold.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Fonts/Sovngarde/SovngardeLight.ttf is excluded by !**/*.ttf
  • package/Interface/CommunityShaders/Icons/Action Icons/Monochrome/clear-cache.png is excluded by !**/*.png
  • package/Interface/CommunityShaders/Icons/Action Icons/Monochrome/load-settings.png is excluded by !**/*.png
  • package/Interface/CommunityShaders/Icons/Action Icons/Monochrome/restore-settings.png is excluded by !**/*.png
  • package/Interface/CommunityShaders/Icons/Action Icons/Monochrome/save-settings.png is excluded by !**/*.png
  • package/Interface/CommunityShaders/Icons/Action Icons/load-settings.png is excluded by !**/*.png
  • package/Interface/CommunityShaders/Icons/Action Icons/restore-settings.png is excluded by !**/*.png
  • package/Interface/CommunityShaders/Icons/Action Icons/save-settings.png is excluded by !**/*.png
  • package/Interface/CommunityShaders/Icons/Categories/display.png is excluded by !**/*.png
  • package/Interface/CommunityShaders/Icons/Community Shaders Logo/Monochrome/cs-logo.png is excluded by !**/*.png
  • package/SKSE/Plugins/CommunityShaders/Themes/DragonBlood/discord.png is excluded by !**/*.png
  • package/SKSE/Plugins/CommunityShaders/Themes/DwemerBronze/discord.png is excluded by !**/*.png
  • package/SKSE/Plugins/CommunityShaders/Themes/HighContrast/discord.png is excluded by !**/*.png
  • package/SKSE/Plugins/CommunityShaders/Themes/Light/clear-cache.png is excluded by !**/*.png
  • package/SKSE/Plugins/CommunityShaders/Themes/Light/cs-logo.png is excluded by !**/*.png
  • package/SKSE/Plugins/CommunityShaders/Themes/Light/discord.png is excluded by !**/*.png
  • package/SKSE/Plugins/CommunityShaders/Themes/Light/load-settings.png is excluded by !**/*.png
  • package/SKSE/Plugins/CommunityShaders/Themes/Light/restore-settings.png is excluded by !**/*.png
  • package/SKSE/Plugins/CommunityShaders/Themes/Light/save-settings.png is excluded by !**/*.png
  • package/SKSE/Plugins/CommunityShaders/Themes/NordicFrost/discord.png is excluded by !**/*.png
📒 Files selected for processing (35)
  • 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/OpenDyslexic/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/Sanguis/Sanguis OFL License.txt (1 hunks)
  • package/Interface/CommunityShaders/Fonts/Sovngarde/Sovngarde OFL License.txt (1 hunks)
  • 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)
  • src/Features/PerformanceOverlay.cpp (4 hunks)
  • src/Features/PerformanceOverlay.h (1 hunks)
  • src/Menu.cpp (10 hunks)
  • src/Menu.h (4 hunks)
  • src/Menu/BackgroundBlur.cpp (1 hunks)
  • src/Menu/BackgroundBlur.h (1 hunks)
  • src/Menu/FeatureListRenderer.cpp (6 hunks)
  • src/Menu/Fonts.cpp (1 hunks)
  • src/Menu/Fonts.h (1 hunks)
  • src/Menu/HomePageRenderer.cpp (1 hunks)
  • src/Menu/IconLoader.cpp (1 hunks)
  • src/Menu/IconLoader.h (1 hunks)
  • src/Menu/MenuHeaderRenderer.cpp (6 hunks)
  • src/Menu/OverlayRenderer.cpp (2 hunks)
  • src/Menu/SettingsTabRenderer.cpp (15 hunks)
  • src/Menu/ThemeManager.cpp (6 hunks)
  • src/Menu/ThemeManager.h (3 hunks)
  • src/Utils/FileSystem.cpp (1 hunks)
  • src/Utils/UI.cpp (7 hunks)
  • src/Utils/UI.h (1 hunks)
🧰 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/FileSystem.cpp
  • src/Menu/OverlayRenderer.cpp
  • src/Menu/Fonts.h
  • src/Menu/IconLoader.h
  • src/Features/PerformanceOverlay.h
  • src/Menu/MenuHeaderRenderer.cpp
  • src/Utils/UI.cpp
  • src/Features/PerformanceOverlay.cpp
  • src/Menu/ThemeManager.cpp
  • src/Menu/HomePageRenderer.cpp
  • src/Menu/FeatureListRenderer.cpp
  • src/Menu/SettingsTabRenderer.cpp
  • src/Menu/IconLoader.cpp
  • src/Menu/BackgroundBlur.cpp
  • src/Menu/ThemeManager.h
  • src/Menu.cpp
  • src/Menu/Fonts.cpp
  • src/Utils/UI.h
  • src/Menu/BackgroundBlur.h
  • src/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/Utils/FileSystem.cpp
  • src/Menu/OverlayRenderer.cpp
  • src/Menu/Fonts.h
  • src/Menu/IconLoader.h
  • src/Features/PerformanceOverlay.h
  • src/Menu/MenuHeaderRenderer.cpp
  • src/Utils/UI.cpp
  • src/Features/PerformanceOverlay.cpp
  • src/Menu/ThemeManager.cpp
  • src/Menu/HomePageRenderer.cpp
  • src/Menu/FeatureListRenderer.cpp
  • src/Menu/SettingsTabRenderer.cpp
  • src/Menu/IconLoader.cpp
  • src/Menu/BackgroundBlur.cpp
  • src/Menu/ThemeManager.h
  • src/Menu.cpp
  • src/Menu/Fonts.cpp
  • src/Utils/UI.h
  • src/Menu/BackgroundBlur.h
  • src/Menu.h
🧠 Learnings (9)
📓 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
  • src/Menu/IconLoader.cpp
📚 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
  • src/Menu/ThemeManager.h
📚 Learning: 2025-06-17T05:40:22.785Z
Learnt from: alandtse
Repo: doodlum/skyrim-community-shaders PR: 577
File: features/Wetness Effects/Shaders/WetnessEffects/WetnessEffects.hlsli:57-61
Timestamp: 2025-06-17T05:40:22.785Z
Learning: Default parameter values are supported in the HLSL compiler used by the skyrim-community-shaders project, contrary to standard HLSL (FXC/DXC) limitations.

Applied to files:

  • src/Menu/ThemeManager.h
📚 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:

  • src/Menu/ThemeManager.h
📚 Learning: 2025-06-08T11:25:14.536Z
Learnt from: alandtse
Repo: doodlum/skyrim-community-shaders PR: 577
File: src/Features/WetnessEffects.h:36-36
Timestamp: 2025-06-08T11:25:14.536Z
Learning: In the skyrim-community-shaders project, boolean flags in C++ structs that interface with HLSL shaders use `uint` type instead of `bool` for compatibility reasons. This ensures consistent 4-byte size, proper 16-byte alignment in constant buffers, and cross-platform compatibility when passing data between C++ and HLSL shaders.

Applied to files:

  • src/Menu/ThemeManager.h
📚 Learning: 2025-06-08T11:25:14.536Z
Learnt from: alandtse
Repo: doodlum/skyrim-community-shaders PR: 577
File: src/Features/WetnessEffects.h:36-36
Timestamp: 2025-06-08T11:25:14.536Z
Learning: In the skyrim-community-shaders project, boolean flags in C++ structs that interface with HLSL shaders use `uint` type instead of `bool` for compatibility reasons. This ensures consistent size, alignment, and cross-platform compatibility when passing data to shader constant buffers.

Applied to files:

  • src/Menu/ThemeManager.h
📚 Learning: 2025-07-01T18:01:07.079Z
Learnt from: alandtse
Repo: doodlum/skyrim-community-shaders PR: 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/Menu/ThemeManager.h
📚 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
🧬 Code graph analysis (18)
src/Menu/OverlayRenderer.cpp (1)
src/Menu/BackgroundBlur.cpp (2)
  • RenderBackgroundBlur (636-731)
  • RenderBackgroundBlur (636-636)
src/Menu/Fonts.h (1)
src/Menu/Fonts.cpp (2)
  • TabBarPaddingGuard (144-165)
  • TabBarPaddingGuard (167-173)
src/Menu/IconLoader.h (4)
src/Menu.cpp (1)
  • Menu (178-206)
src/Menu.h (1)
  • Menu (23-153)
src/Utils/UI.h (1)
  • Util (36-382)
src/Utils/UI.cpp (2)
  • InitializeMenuIcons (81-84)
  • InitializeMenuIcons (81-81)
src/Features/PerformanceOverlay.h (1)
src/Features/PerformanceOverlay.cpp (2)
  • DrawABTestSection (1140-1292)
  • DrawABTestSection (1140-1140)
src/Menu/MenuHeaderRenderer.cpp (1)
src/Utils/UI.cpp (4)
  • GetCenterOffsetForContent (163-179)
  • GetCenterOffsetForContent (163-163)
  • DrawAlignedTextWithLogo (118-161)
  • DrawAlignedTextWithLogo (118-118)
src/Utils/UI.cpp (2)
src/Menu/IconLoader.cpp (2)
  • InitializeMenuIcons (161-246)
  • InitializeMenuIcons (161-161)
src/Menu/FeatureListRenderer.cpp (4)
  • menu (264-282)
  • menu (264-264)
  • menu (358-364)
  • menu (358-358)
src/Features/PerformanceOverlay.cpp (3)
src/Utils/UI.cpp (2)
  • HoverTooltipWrapper (38-45)
  • HoverTooltipWrapper (47-53)
src/Utils/UI.h (1)
  • HoverTooltipWrapper (57-66)
src/Utils/FileSystem.cpp (4)
  • GetDataPath (12-32)
  • GetDataPath (12-12)
  • LoadJsonDiff (204-249)
  • LoadJsonDiff (204-204)
src/Menu/ThemeManager.cpp (1)
src/Utils/FileSystem.cpp (2)
  • GetDataPath (12-32)
  • GetDataPath (12-12)
src/Menu/HomePageRenderer.cpp (1)
src/Menu/FeatureListRenderer.cpp (4)
  • menu (264-282)
  • menu (264-264)
  • menu (358-364)
  • menu (358-358)
src/Menu/FeatureListRenderer.cpp (1)
src/Utils/UI.cpp (2)
  • DrawCategoryHeader (230-350)
  • DrawCategoryHeader (230-230)
src/Menu/SettingsTabRenderer.cpp (3)
src/Menu/Fonts.cpp (2)
  • BeginTabItemWithFont (175-183)
  • BeginTabItemWithFont (175-175)
src/Utils/UI.cpp (2)
  • HoverTooltipWrapper (38-45)
  • HoverTooltipWrapper (47-53)
src/Menu/BackgroundBlur.cpp (2)
  • SetIntensity (613-617)
  • SetIntensity (613-613)
src/Menu/IconLoader.cpp (1)
src/Utils/FileSystem.cpp (4)
  • GetThemesPath (79-82)
  • GetThemesPath (79-79)
  • GetIconsPath (54-57)
  • GetIconsPath (54-54)
src/Menu/ThemeManager.h (3)
src/Menu/ThemeManager.cpp (4)
  • ReloadFont (189-448)
  • ReloadFont (189-189)
  • ForceApplyDefaultTheme (141-187)
  • ForceApplyDefaultTheme (141-141)
src/Menu.cpp (1)
  • Menu (178-206)
src/Menu.h (1)
  • Menu (23-153)
src/Menu.cpp (4)
src/Menu/BackgroundBlur.h (1)
  • BackgroundBlur (9-70)
src/Menu/BackgroundBlur.cpp (6)
  • Cleanup (587-611)
  • Cleanup (587-587)
  • SetIntensity (613-617)
  • SetIntensity (613-613)
  • Initialize (237-349)
  • Initialize (237-237)
src/Menu/Fonts.cpp (4)
  • NormalizeFontRoles (90-116)
  • NormalizeFontRoles (90-90)
  • ValidateFont (605-660)
  • ValidateFont (605-605)
src/Menu/IconLoader.cpp (2)
  • InitializeMenuIcons (161-246)
  • InitializeMenuIcons (161-161)
src/Menu/Fonts.cpp (1)
src/Menu/Fonts.h (1)
  • TabBarPaddingGuard (63-75)
src/Utils/UI.h (1)
src/Utils/UI.cpp (4)
  • DrawAlignedTextWithLogo (118-161)
  • DrawAlignedTextWithLogo (118-118)
  • GetCenterOffsetForContent (163-179)
  • GetCenterOffsetForContent (163-163)
src/Menu/BackgroundBlur.h (1)
src/Menu/BackgroundBlur.cpp (18)
  • Initialize (237-349)
  • Initialize (237-237)
  • RenderBackgroundBlur (636-731)
  • RenderBackgroundBlur (636-636)
  • CreateBlurTextures (351-440)
  • CreateBlurTextures (351-351)
  • PerformBlur (442-585)
  • PerformBlur (442-442)
  • Cleanup (587-611)
  • Cleanup (587-587)
  • SetIntensity (613-617)
  • SetIntensity (613-613)
  • GetIntensity (619-622)
  • GetIntensity (619-619)
  • IsEnabled (624-627)
  • IsEnabled (624-624)
  • GetTextureDimensions (629-634)
  • GetTextureDimensions (629-629)
src/Menu.h (1)
src/Menu/BackgroundBlur.h (1)
  • BackgroundBlur (9-70)
🪛 Clang (14.0.6)
src/Menu/IconLoader.h

[error] 4-4: unknown type name 'class'

(clang-diagnostic-error)


[error] 6-6: unknown type name 'namespace'

(clang-diagnostic-error)


[error] 6-6: expected ';' after top level declarator

(clang-diagnostic-error)

src/Menu/IconLoader.cpp

[error] 1-1: 'PCH.h' file not found

(clang-diagnostic-error)

src/Menu/BackgroundBlur.h

[error] 3-3: 'd3d11.h' file not found

(clang-diagnostic-error)

⏰ 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 (15)
package/Interface/CommunityShaders/Fonts/Inter/OFL.txt (1)

1-93: Font licensing documentation properly documented.

All eight font license files contain properly formatted SIL Open Font License Version 1.1 text with accurate copyright attributions for each respective font author. The inclusion of these licenses ensures proper compliance for distributing fonts under OFL terms and aligns with the PR objective of adding new fonts with open licensing.

package/Interface/CommunityShaders/Fonts/IBMPlexSans/OFL.txt (1)

1-93: License documentation properly added.

The SIL Open Font License v1.1 is the standard and correct license for IBM Plex fonts. The file placement in the font directory and complete license text ensure proper legal compliance for distributing the IBM Plex Sans font assets.

src/Utils/FileSystem.cpp (1)

12-32: LGTM! Error handling improvements.

The explicit return statement in the catch block (line 30) makes the fallback behavior clearer and more maintainable. The intermediate variable on line 25 is unnecessary but harmless and may aid debugging. The function has appropriate error handling with proper fallback to current_path(), which aligns with the robust resource loading patterns introduced in this PR.

src/Utils/UI.h (1)

185-192: Logo tint and centering helper line up with the new theming goals.

The optional tint keeps existing calls intact while letting monochrome themes reuse the same assets, and the centering helper will make header alignment consistent.

src/Menu/OverlayRenderer.cpp (1)

2-227: Background blur hook is in the right spot.

Calling BackgroundBlur::RenderBackgroundBlur() immediately after ImGui::Render() captures the finalized window list while staying ahead of ImGui_ImplDX11_RenderDrawData, so the blur can target the correct back buffer content without disrupting the draw submission.

src/Menu/HomePageRenderer.cpp (1)

301-309: Theme-aware watermark tint looks solid.

Pulling the alpha-tweaked text color when UseMonochromeLogo is enabled keeps the watermark subtle and consistent with the active palette; the white fallback covers the non-monochrome case cleanly.

src/Menu/IconLoader.h (1)

1-11: Lean header keeps the loader interface tidy.

The forward declarations plus the nested namespace export just what the UI layer needs without pulling heavy D3D headers into every include site.

src/Features/PerformanceOverlay.h (1)

166-166: Simplified A/B section signature matches the new internal state handling.

With collapsible control now internal to the widget, removing the extra parameter keeps the API cleaner for callers while preserving behavior.

src/Menu/ThemeManager.cpp (1)

459-529: Multi-path theme discovery implementation looks solid.

The refactored discovery properly handles:

  • Multiple search paths (base themes + MO2 overwrite)
  • Per-path error handling without aborting the entire discovery
  • File size validation to prevent loading malicious/corrupted files
  • Early exit when MAX_THEMES is reached
  • Clear logging at each step
src/Menu/BackgroundBlur.h (1)

1-70: LGTM!

The BackgroundBlur API header is well-structured with:

  • Clear function declarations with doxygen documentation
  • Proper separation of concerns (init/cleanup, rendering, state management)
  • Forward-declared ImVec2 to minimize dependencies
  • Thread-safe resource management implied by the design
src/Features/PerformanceOverlay.cpp (2)

1140-1141: Function signature change is consistent with implementation.

The DrawABTestSection signature was simplified by removing the showCollapsibleSections parameter and managing UI state internally with a local static variable (line 1186). The single call site at line 394 has been updated correctly.


167-213: Improved settings UI organization with semantic sections.

The refactored DrawSettings now uses clear semantic sections ("Display Options" and "Appearance") with consistent layout and better grouping of related controls. The conditional rendering for frame generation graphs is more intuitive.

src/Menu/SettingsTabRenderer.cpp (3)

38-155: Comprehensive color name mapping for user-friendly UI.

The GetFriendlyColorName function provides clear, descriptive names for ImGui colors, covering all common color indices and falling back to ImGui's internal names for any missing entries. This significantly improves the UX of the color customization interface.


746-761: Good practice: Deferring icon reload to avoid rendering with released textures.

Setting pendingIconReload = true instead of immediately reloading ensures that textures aren't released while still referenced in the current frame's draw data. This prevents potential crashes or visual artifacts.


867-896: Well-implemented search icon with custom rendering.

The custom search icon (magnifying glass) is rendered using ImDrawList with proper positioning and styling. The implementation correctly:

  • Adjusts frame padding to make space for the icon
  • Centers the icon vertically within the input field
  • Uses theme-derived color with reduced opacity for subtle appearance

@github-actions
Copy link

github-actions bot commented Nov 6, 2025

✅ A pre-release build is available for this PR:
Download

@davo0411 davo0411 changed the title feat(UI): new themes & fonts, misc fixes & features feat(UI): add themes & fonts, small fixes & features Nov 6, 2025
@davo0411 davo0411 changed the title feat(UI): add themes & fonts, small fixes & features feat(UI): add themes & fonts, small fixes, refactors, cleanup Nov 6, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

pre-commit-ci bot and others added 3 commits November 25, 2025 10:13
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

davo0411 and others added 3 commits November 26, 2025 19:16
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant