refactor: bgi启动时恢复上一次窗口的位置和大小 #3383 - #3401
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Walkthrough新增窗口位置、尺寸和状态配置。设置页面提供启用开关。主窗口在启动时恢复配置,并在关闭时保存窗口状态。恢复位置会限制在显示器工作区范围内。 Changes窗口状态恢复
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CommonSettingsPage
participant CommonConfig
participant MainWindow
participant VirtualScreen
CommonSettingsPage->>CommonConfig: 更新 RestoreWindowPositionAndSize
MainWindow->>CommonConfig: 启动时读取窗口配置
MainWindow->>VirtualScreen: 获取显示器工作区
MainWindow->>MainWindow: 恢复尺寸、位置和窗口状态
MainWindow->>CommonConfig: 关闭时保存窗口状态
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@BetterGenshinImpact/View/MainWindow.xaml.cs`:
- Around line 274-283: Update ClampWindowToScreen to identify the monitor
containing or nearest to the saved coordinates, then clamp the window against
that monitor’s working-area bounds instead of SystemParameters.VirtualScreen*.
Use the project’s Screen/MonitorInfo handling and normalize coordinate units for
mixed-DPI displays before applying Left, Top, Width, and Height, while
preserving manual startup positioning.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 203d3cf5-29a8-419f-bce6-254e3d09789b
📒 Files selected for processing (3)
BetterGenshinImpact/Core/Config/CommonConfig.csBetterGenshinImpact/View/MainWindow.xaml.csBetterGenshinImpact/View/Pages/CommonSettingsPage.xaml
Greptile Summary此 PR 新增保存和恢复主窗口位置、尺寸及最大化状态的配置与设置入口,并尝试将恢复位置限制在实际显示器工作区内。
Confidence Score: 4/5当前不建议合并,混合 DPI 显示器间恢复窗口时仍可能选择错误显示器并产生不可操作的位置。 窗口恢复逻辑将当前窗口的 DPI 同时用于保存坐标和目标显示器工作区换算;当两台显示器缩放比例不同时,两侧坐标不在同一单位体系,先前报告的恢复错误仍然存在。 Files Needing Attention: BetterGenshinImpact/View/MainWindow.xaml.cs
|
| Filename | Overview |
|---|---|
| BetterGenshinImpact/View/MainWindow.xaml.cs | 实现窗口状态保存与恢复,并修复托盘状态和显示器间隙处理;混合 DPI 恢复仍使用错误的单一比例。 |
| BetterGenshinImpact/Core/Config/CommonConfig.cs | 新增窗口恢复开关及位置、尺寸和状态的持久化字段。 |
| BetterGenshinImpact/View/Pages/CommonSettingsPage.xaml | 新增窗口位置与尺寸恢复设置开关。 |
Prompt To Fix All With AI
### Issue 1
BetterGenshinImpact/View/MainWindow.xaml.cs:304-312
**混合 DPI 换算仍错配**
当上次保存窗口所在显示器与初始化窗口所在显示器的缩放比例不同时,这里使用当前窗口的单一 DPI 同时换算保存的 DIP 坐标和目标显示器的设备像素边界,导致 `Screen.FromPoint` 选中错误显示器或计算出错误位置,部分布局下标题栏仍会落在不可操作区域。
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (3): Last reviewed commit: "fix: 考虑显示器的DPI" | Re-trigger Greptile
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 270fdb6922
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| var screen = System.Windows.Forms.Screen.FromPoint(new System.Drawing.Point(testX, testY)); | ||
| var work = screen.WorkingArea; | ||
| Left = Math.Max(work.Left, Math.Min(left, work.Right - Width)); | ||
| Top = Math.Max(work.Top, Math.Min(top, work.Bottom - 30)); |
There was a problem hiding this comment.
严重程度:P2;问题位置:ClampWindowToScreen;问题原因:这里把 WPF Window 的 left、top、Width 这些设备无关像素(DIP)直接传给 System.Windows.Forms.Screen.FromPoint 并和 WorkingArea 的物理像素比较,而 MainWindow 已调用 InitializeDpiAwareness() 启用每显示器 DPI;在 125%/150% 缩放或混合 DPI 多显示器环境下会用错误单位选屏和钳制坐标。可能造成的影响:用户启用“启动时恢复上次窗口位置和大小”后,窗口可能被恢复到错误显示器或明显偏移,甚至被钳到不可见区域,导致新功能失效。推荐修复方案:在调用 WinForms/Win32 屏幕 API 前后使用 VisualTreeHelper.GetDpi(this)、PointToScreen 或 HwndSource.CompositionTarget 做 DIP↔物理像素转换,或全程使用同一坐标单位。
Useful? React with 👍 / 👎.
|
|
||
| protected override void OnClosed(EventArgs e) | ||
| { | ||
| SaveWindowPosition(); |
There was a problem hiding this comment.
严重程度:P2;问题位置:OnClosed;问题原因:我检查了 ApplicationHostService.HandleActivationAsync,--instance childSession 进程同样会创建 MainWindow,并且 ChildSessionProcessLauncher.CreateBetterGiStartInfo 使用同一 AppContext.BaseDirectory 启动,所以 ConfigService 仍写同一个 User/config.json;这里所有 MainWindow 关闭时都保存 CommonConfig 窗口坐标,没有限制为 InstanceBootstrap.Current.Context.IsRoot 或区分实例。可能造成的影响:启用桌面分身后,子实例退出会覆盖主实例的窗口位置,下一次主程序启动恢复到分身/RDP 会话的坐标或大小。推荐修复方案:只在主实例保存/恢复主窗口位置,或按实例类型/session 单独存储窗口 bounds。
AGENTS.md reference: AGENTS.md:L102-L102
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 93331bae66
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
issue #3383
Summary by CodeRabbit