-
Notifications
You must be signed in to change notification settings - Fork 92
feat(UI): add themes & fonts, small fixes, refactors, cleanup #1596
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
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.
update to dev
Added collapsible sections for FPS, Draw Calls, VRAM, and A/B Test results in the Performance Overlay. Updated function signatures and logic to support the new collapsible behavior.
WalkthroughThis 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
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
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
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)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
There was a problem hiding this 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)andImVec4(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, andsubstroperations 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
⛔ Files ignored due to path filters (55)
package/Interface/CommunityShaders/Fonts/CrimsonPro/CrimsonPro-Light.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/CrimsonPro/CrimsonPro-Regular.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/CrimsonPro/CrimsonPro-SemiBold.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/IBMPlexMono/IBMPlexMono-Light.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/IBMPlexMono/IBMPlexMono-Regular.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/IBMPlexMono/IBMPlexMono-SemiBold.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/IBMPlexSans/IBMPlexSans-Light.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/IBMPlexSans/IBMPlexSans-Regular.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/IBMPlexSans/IBMPlexSans-SemiBold.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/IBMPlexSans/IBMPlexSans_Condensed-Light.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/IBMPlexSans/IBMPlexSans_Condensed-Regular.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/IBMPlexSans/IBMPlexSans_Condensed-SemiBold.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/IBMPlexSerif/IBMPlexSerif-Light.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/IBMPlexSerif/IBMPlexSerif-Regular.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/IBMPlexSerif/IBMPlexSerif-SemiBold.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Inter/Inter_24pt-Light.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Inter/Inter_24pt-Regular.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Inter/Inter_24pt-SemiBold.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/OpenDyslexic/OpenDyslexic3-Bold.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/OpenDyslexic/OpenDyslexic3-Regular.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Roboto/Roboto-Bold.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Roboto/Roboto-Regular.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Roboto/Roboto-SemiBold.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Roboto/Roboto-Thin.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Roboto/Roboto_Condensed-Light.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Roboto/Roboto_Condensed-Regular.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Roboto/Roboto_Condensed-SemiBold.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/RobotoSlab/RobotoSlab-Light.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/RobotoSlab/RobotoSlab-Regular.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/RobotoSlab/RobotoSlab-SemiBold.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Rubik/Rubik-Light.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Rubik/Rubik-Regular.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Rubik/Rubik-SemiBold.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Sanguis/Sanguis.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Sovngarde/SovngardeBold.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Fonts/Sovngarde/SovngardeLight.ttfis excluded by!**/*.ttfpackage/Interface/CommunityShaders/Icons/Action Icons/Monochrome/clear-cache.pngis excluded by!**/*.pngpackage/Interface/CommunityShaders/Icons/Action Icons/Monochrome/load-settings.pngis excluded by!**/*.pngpackage/Interface/CommunityShaders/Icons/Action Icons/Monochrome/restore-settings.pngis excluded by!**/*.pngpackage/Interface/CommunityShaders/Icons/Action Icons/Monochrome/save-settings.pngis excluded by!**/*.pngpackage/Interface/CommunityShaders/Icons/Action Icons/load-settings.pngis excluded by!**/*.pngpackage/Interface/CommunityShaders/Icons/Action Icons/restore-settings.pngis excluded by!**/*.pngpackage/Interface/CommunityShaders/Icons/Action Icons/save-settings.pngis excluded by!**/*.pngpackage/Interface/CommunityShaders/Icons/Categories/display.pngis excluded by!**/*.pngpackage/Interface/CommunityShaders/Icons/Community Shaders Logo/Monochrome/cs-logo.pngis excluded by!**/*.pngpackage/SKSE/Plugins/CommunityShaders/Themes/DragonBlood/discord.pngis excluded by!**/*.pngpackage/SKSE/Plugins/CommunityShaders/Themes/DwemerBronze/discord.pngis excluded by!**/*.pngpackage/SKSE/Plugins/CommunityShaders/Themes/HighContrast/discord.pngis excluded by!**/*.pngpackage/SKSE/Plugins/CommunityShaders/Themes/Light/clear-cache.pngis excluded by!**/*.pngpackage/SKSE/Plugins/CommunityShaders/Themes/Light/cs-logo.pngis excluded by!**/*.pngpackage/SKSE/Plugins/CommunityShaders/Themes/Light/discord.pngis excluded by!**/*.pngpackage/SKSE/Plugins/CommunityShaders/Themes/Light/load-settings.pngis excluded by!**/*.pngpackage/SKSE/Plugins/CommunityShaders/Themes/Light/restore-settings.pngis excluded by!**/*.pngpackage/SKSE/Plugins/CommunityShaders/Themes/Light/save-settings.pngis excluded by!**/*.pngpackage/SKSE/Plugins/CommunityShaders/Themes/NordicFrost/discord.pngis 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.cppsrc/Menu/OverlayRenderer.cppsrc/Menu/Fonts.hsrc/Menu/IconLoader.hsrc/Features/PerformanceOverlay.hsrc/Menu/MenuHeaderRenderer.cppsrc/Utils/UI.cppsrc/Features/PerformanceOverlay.cppsrc/Menu/ThemeManager.cppsrc/Menu/HomePageRenderer.cppsrc/Menu/FeatureListRenderer.cppsrc/Menu/SettingsTabRenderer.cppsrc/Menu/IconLoader.cppsrc/Menu/BackgroundBlur.cppsrc/Menu/ThemeManager.hsrc/Menu.cppsrc/Menu/Fonts.cppsrc/Utils/UI.hsrc/Menu/BackgroundBlur.hsrc/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.cppsrc/Menu/OverlayRenderer.cppsrc/Menu/Fonts.hsrc/Menu/IconLoader.hsrc/Features/PerformanceOverlay.hsrc/Menu/MenuHeaderRenderer.cppsrc/Utils/UI.cppsrc/Features/PerformanceOverlay.cppsrc/Menu/ThemeManager.cppsrc/Menu/HomePageRenderer.cppsrc/Menu/FeatureListRenderer.cppsrc/Menu/SettingsTabRenderer.cppsrc/Menu/IconLoader.cppsrc/Menu/BackgroundBlur.cppsrc/Menu/ThemeManager.hsrc/Menu.cppsrc/Menu/Fonts.cppsrc/Utils/UI.hsrc/Menu/BackgroundBlur.hsrc/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.cppsrc/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.jsonsrc/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 afterImGui::Render()captures the finalized window list while staying ahead ofImGui_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
UseMonochromeLogois 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
DrawABTestSectionsignature was simplified by removing theshowCollapsibleSectionsparameter 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
DrawSettingsnow 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
GetFriendlyColorNamefunction 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 = trueinstead 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
|
✅ A pre-release build is available for this PR: |
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
…skyrim-community-shaders into separate-menu-settings
There was a problem hiding this 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: InitializecachedFontFilesByRolefor the newTitlerole
cachedFontFilesByRolesets defaults forBody,Heading, andSubheading, butTitleis 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: Guardenabled/ initialization state withresourceMutexto avoid data races
enabled,initialized, andinitializationFailedare mutating in some functions underresourceMutex(e.g.,Initialize,Cleanup) but are read/written without that mutex inSetEnabled,GetEnabled,IsEnabled, and at the top ofRenderBackgroundBlur. In a multi-threaded scenario (e.g., theme/menu toggling blur while the render thread callsRenderBackgroundBlur), this is a data race and technically undefined behaviour.You already have
resourceMutex; recommend:
- Take
resourceMutexinSetEnabled,GetEnabled, andIsEnabledwhen accessingenabled(andinitialized/initializationFailedinIsEnabled).- In
RenderBackgroundBlur, copyenabled,initialized, andinitializationFailedinto local variables under a shortstd::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
StatusPaletteuses 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
memsetto 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 includeTitleThe doc block still describes roles as Body/Heading/Subheading/Subtitle and the JSON example has four generic entries only. With
FontRole::Titleintroduced andSubtitleremoved, 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 semanticsThe shader is structurally sound and matches the C++
BlurConstantslayout 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/halfSamplesare derived fromBlurParams.x, the effective kernel sum isn’t obviously normalized for common values ofBlurParams.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) andWEIGHTS[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: AlignIsEnableddocumentation with current API (no public intensity)The comment for
IsEnabled()says “True if blur intensity > 0”, but the implementation now just checks internalenabled/initializedstate 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
PerformBlurcallsglobals::d3d::device->CreateShaderResourceView(sourceTexture, ...)on every invocation, andRenderBackgroundBlurmay invokePerformBluronce 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:
RenderBackgroundBlurcreates a single SRV forcurrentTextureonce 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” commentIn 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 downsampleBut the shared blur PS always multiplies the center sample by
WEIGHTS[0](≈0.176), even whensamples == 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 == 1as 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
📒 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.hlslpackage/Shaders/Menu/BackgroundBlurHorizontal.hlslsrc/Menu/BackgroundBlur.hsrc/Menu.hsrc/Menu/BackgroundBlur.cppsrc/Menu.cppsrc/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.hsrc/Menu.hsrc/Menu/BackgroundBlur.cppsrc/Menu.cppsrc/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.hlslpackage/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.hlslpackage/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 thatGlobalScale: 0.0is intentional.Line 9 sets
GlobalScaleto 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 for1.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
FullPalettearray 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 ofGlobalScale: 0.0.Line 9 sets
GlobalScaleto 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.jsondoes 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:
FullPalettearray 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
FullPalettearray 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.jsonreferenced in the review comment cannot be found in this codebase. A comprehensive search shows:
package/contains onlyShaders/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.jsonand 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.0is consistent with other theme configurations- The plugin code correctly interprets
0.0as 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, andBackgroundBlurEnabledfields 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-RegulartoJost-LightwithSizeScale: 1.3provides 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
canReloadcheck 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.cppwith C++ code containing aMenuclass, theme settings, and JSON serialization. This repository contains only HLSL shader files (.hlsl) organized infeatures/andpackage/directories. There is nosrc/Menu.cppfile 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
GetFriendlyColorNamefunction provides clear, descriptive names for all ImGui color indices, making the color customization interface more accessible. The fallback toImGui::GetStyleColorNamehandles any future ImGui additions.
183-191: Tab padding guard ensures consistent UI across different font sizes.The
TabBarPaddingGuardpattern 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:currentIntensityuse inCleanupappears stale and will not compile
Cleanup()assignscurrentIntensity = 0.0f;but there is nocurrentIntensitydefined 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.
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
There was a problem hiding this 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: Guardenabledstate withresourceMutex.As noted in a previous review, these accessors read/write
enabledwithout takingresourceMutex, creating a data race withRenderBackgroundBlur(line 532),PerformBlur, andCleanupwhich 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
RenderBackgroundBlurto 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
CreateBlurTexturesrepeats 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 possibleThe separable Gaussian blur logic and dynamic normalization over the actually used taps are sound and numerically robust for varying
BlurParams.x. Themin(j, 7)protection is technically redundant becausehalfSamplescannot exceed 7 aftersamples = min(BlurParams.x, 15), but it’s harmless. If you want, you can simplify and shave a couple of ALU ops by dropping themin()and/or explicitly clampinghalfSamples: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
📒 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.hlslsrc/Menu.cppsrc/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.cppsrc/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 correctThe 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 pathThe buffer and register layout consistency checks out:
BlurBuffer : register(b0),LinearSampler : register(s0), andInputTexture : register(t0)are identical in both horizontal and vertical passes- No other
BlurBufferdefinitions exist in the codebase, so no conflictsFor the linear color pipeline: the shader code itself contains no color space conversions (no
IrradianceToLinearor gamma adjustments). Since no binding/setup code is present in the codebase to inspect, verify thatInputTexturearrives pre-converted to linear space (or add conversions similar to other image-space shaders if needed).
…skyrim-community-shaders into separate-menu-settings
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
There was a problem hiding this 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 <= 0contentWidthis greater than the available/window widthConsider 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 PRYour current title
feat(UI): add themes & fonts, small fixes, refactors, cleanupis 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 stylingIf this PR closes or implements specific issues, it would help to include the appropriate GitHub keywords in the PR description, for example:
Implements #123orAddresses #123for feature workFixes #456orCloses #456for any bug fixes in theme discovery / JSON savingRelated to #789for partial work or follow‑upsThis 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
📒 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.cppsrc/Utils/UI.hsrc/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.cppsrc/Utils/UI.hsrc/Menu.cpp
**/*
⚙️ CodeRabbit configuration file
**/*: When reviewing PRs, please provide suggestions for:
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
Issue References (if PR fixes bugs or implements features):
Suggest adding appropriate GitHub keywords:Otherwise, use your standard review approach focusing on code quality.
Files:
src/Menu/HomePageRenderer.cppsrc/Utils/UI.hsrc/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 patternsAdding
ImU32 logoTint = IM_COL32_WHITEat the end ofDrawAlignedTextWithLogokeeps source compatibility and matches ImGui’s tint conventions. As long as the implementation inUI.cppis 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
ShowFootertheme 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) andUtil::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
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.
Summary by CodeRabbit
New Features
Styling & UI Enhancements
✏️ Tip: You can customize this high-level summary in your review settings.