fix(ui): save resolution based font in setting#2171
Merged
alandtse merged 4 commits intoApr 21, 2026
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdded a persistent Changes
Sequence Diagram(s)sequenceDiagram
participant UI as "SettingsTabRenderer (UI)"
participant Menu as "Menu (Settings)"
participant Theme as "ThemeManager"
participant Display as "Display / VR Config"
UI->>Menu: Toggle UseResolutionFont
Menu-->>UI: Persisted UseResolutionFont
UI->>Theme: Request ResolveFontSize(menu)
Theme->>Menu: Read settings (UseResolutionFont, Theme.FontSize)
alt UseResolutionFont == false
Theme->>Theme: Use settings.Theme.FontSize (round/validate/ clamp)
else UseResolutionFont == true
Theme->>Display: Query overlay/screen height
Display-->>Theme: height
Theme->>Theme: Compute dynamic font size (ratio, fallback, clamp)
end
Theme-->>UI: Effective font size
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Menu/SettingsTabRenderer.cpp (1)
855-870:⚠️ Potential issue | 🟡 MinorClarify that this checkbox is saved globally.
UseResolutionFontis now a user setting, but the Fonts tab still opens with theme-save guidance. A user could save the theme and expect this checkbox to persist, even though only the global settings save writes it.Consider adding a short note near this checkbox, or splitting the save guidance so global/user settings are distinguished from theme settings.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Menu/SettingsTabRenderer.cpp` around lines 855 - 870, The "UseResolutionFont" checkbox is a global user setting but the Fonts tab still shows theme-save guidance, which is misleading; update the UI near the checkbox (in SettingsTabRenderer::Render... where UseResolutionFont is used) to add a short inline note or tooltip clarifying that UseResolutionFont is saved to the global user settings and not to the current theme, and/or adjust the existing theme save guidance (e.g., RenderSaveInfoText or the Fonts subheading) to distinguish theme-scoped saves from global/user-scoped settings so users know this checkbox will persist globally rather than with a saved theme.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Menu.cpp`:
- Around line 431-435: The ApplyResolutionFontOverride() mutates
settings.Theme.FontSize based on settings.UseResolutionFont which pollutes theme
state and causes incorrect resolution and theme persistence; instead stop
writing to settings.Theme.FontSize and implement the override in the
font-resolution path: update ThemeManager::ResolveFontSize(...) to accept the
user toggle/optional fixed size (e.g., pass settings.UseResolutionFont and an
optional settings.UserFontSize) and, when the toggle is enabled, return the
computed resolution-based size without modifying settings.Theme, or if you
prefer a stored user value add a separate settings field (e.g., UserFontSize)
and ensure SaveTheme() only persists theme fields, not the user toggle/value.
---
Outside diff comments:
In `@src/Menu/SettingsTabRenderer.cpp`:
- Around line 855-870: The "UseResolutionFont" checkbox is a global user setting
but the Fonts tab still shows theme-save guidance, which is misleading; update
the UI near the checkbox (in SettingsTabRenderer::Render... where
UseResolutionFont is used) to add a short inline note or tooltip clarifying that
UseResolutionFont is saved to the global user settings and not to the current
theme, and/or adjust the existing theme save guidance (e.g., RenderSaveInfoText
or the Fonts subheading) to distinguish theme-scoped saves from
global/user-scoped settings so users know this checkbox will persist globally
rather than with a saved theme.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c2fc9b67-fdf9-4802-946e-94a5887ded74
📒 Files selected for processing (3)
src/Menu.cppsrc/Menu.hsrc/Menu/SettingsTabRenderer.cpp
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/Menu.h (1)
414-414: Comment wording is slightly stale.The trailing
(FontSize=0)hint implies runtime still keys offFontSize==0, butThemeManager::ResolveFontSizenow dispatches onUseResolutionFontdirectly. Consider updating the comment to describe what this flag truly controls (runtime dispatch + theme-file zeroing for persistence), so future readers don't look for aFontSize==0check that's no longer there.📝 Suggested wording
- bool UseResolutionFont = true; // When true, font size scales with screen resolution (FontSize=0) + bool UseResolutionFont = true; // When true, ThemeManager::ResolveFontSize derives font size from the current screen resolution; the theme's FontSize is ignored.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Menu.h` at line 414, Update the comment for the boolean UseResolutionFont to accurately describe its behavior: explain that when true ThemeManager::ResolveFontSize will choose a resolution-scaled font size at runtime and when persisted to theme files the font size is written as zero for backward-compatible representation; remove the outdated "(FontSize=0)" implication that code checks FontSize==0 and instead state that UseResolutionFont directly controls runtime dispatch and causes theme-file zeroing for persistence.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/Menu.h`:
- Line 414: Update the comment for the boolean UseResolutionFont to accurately
describe its behavior: explain that when true ThemeManager::ResolveFontSize will
choose a resolution-scaled font size at runtime and when persisted to theme
files the font size is written as zero for backward-compatible representation;
remove the outdated "(FontSize=0)" implication that code checks FontSize==0 and
instead state that UseResolutionFont directly controls runtime dispatch and
causes theme-file zeroing for persistence.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e9610787-54ba-4af1-ba53-d46ccac7e5e1
📒 Files selected for processing (4)
src/Menu.cppsrc/Menu.hsrc/Menu/SettingsTabRenderer.cppsrc/Menu/ThemeManager.cpp
✅ Files skipped from review due to trivial changes (1)
- src/Menu.cpp
soda3000
suggested changes
Apr 21, 2026
soda3000
approved these changes
Apr 21, 2026
alandtse
approved these changes
Apr 21, 2026
alandtse
changed the title
ParticleTroned
pushed a commit
to ParticleTroned/skyrim-community-shaders
that referenced
this pull request
May 2, 2026
(cherry picked from commit 4e1ea45)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2166
this PR changes the saving logic for the resolution based font setting. Now it will save in the usersettings, and when it is toggled on, it will apply the fontsize = 0.0f setting on theme load. This way the toggle persists logically, while themes don't get affected by it when saved.
Summary by CodeRabbit
New Features
Improvements