feat(UI): reset window positions after resolution change #2067
Conversation
|
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)
📝 WalkthroughWalkthroughRegisters an ImGui "CommunityShaders" settings handler to persist ImGui::GetIO().DisplaySize across sessions, tracks runtime display size changes, and triggers conditional layout resets when resolution changes are detected. Changes
Sequence DiagramsequenceDiagram
participant AppInit as App Init
participant Settings as ImGuiSettingsHandler
participant Overlay as OverlayRenderer::InitializeImGuiFrame
participant Menu as Menu::DrawSettings
participant ImGui as ImGui
AppInit->>Settings: Register "CommunityShaders"
Settings->>Settings: ReadOpenFn / ReadLineFn -> set lastDisplaySize
Overlay->>Overlay: Cache displayW, displayH
Overlay->>Overlay: Compare with menu.lastDisplaySize
alt Display size changed
Overlay->>Overlay: Log change
Overlay->>Menu: set resetLayout = true
Overlay->>Overlay: Update lastDisplaySize
else No change
Overlay->>Overlay: Update lastDisplaySize
end
Menu->>Menu: Check resetLayout
alt resetLayout == true
Menu->>ImGui: Set position/size with ImGuiCond_Always
Menu->>Menu: resetLayout = false
else
Menu->>ImGui: Set position/size with ImGuiCond_FirstUseEver
end
Settings->>Settings: WriteAllFn -> serialize current DisplaySize
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
|
No actionable suggestions for changed features. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 587-601: The ImGuiSettingsHandler instance is not
value-initialized; change the declaration of ImGuiSettingsHandler handler to use
value-initialization (e.g., ImGuiSettingsHandler handler{} ) so all fields
(including optional callbacks like ClearAllFn/ReadInitFn/ApplyAllFn) are
zero-initialized before assigning ReadOpenFn, ReadLineFn and WriteAllFn and
pushing to ImGui::GetCurrentContext()->SettingsHandlers.
In `@src/Menu/OverlayRenderer.cpp`:
- Around line 228-232: The code should not set
EditorWindow::GetSingleton()->resetLayout while the editor is closed; instead,
move or gate that assignment so it only occurs when the editor is actually
rendering this frame (i.e., after calling UpdateOpenState()). Concretely: stop
directly writing EditorWindow::GetSingleton()->resetLayout = true where
menu.lastDisplaySize triggers a layout reset; instead, propagate
menu.resetLayout and after calling EditorWindow::UpdateOpenState() check the
editor window instance (e.g., auto editorWindow = EditorWindow::GetSingleton())
and only set editorWindow->resetLayout = true when editorWindow->open is true
(one-shot behavior preserved by leaving menu.resetLayout cleared elsewhere).
🪄 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: b1069cda-0139-4058-9395-209ff1a63c17
📒 Files selected for processing (3)
src/Menu.cppsrc/Menu.hsrc/Menu/OverlayRenderer.cpp
|
✅ A pre-release build is available for this PR: |
…haders#2067) (cherry picked from commit 79d4389)
Resets the window positions in the UI when detecting a change in game resolution.
Summary by CodeRabbit
New Features
Improvements