Skip to content

fix: use window center for shortcuts popup position - #702

Draft
pppanghu77 wants to merge 1 commit into
linuxdeepin:masterfrom
pppanghu77:agent/agent/35fa779880ba
Draft

pppanghu77 wants to merge 1 commit into
linuxdeepin:masterfrom
pppanghu77:agent/agent/35fa779880ba

Conversation

@pppanghu77

@pppanghu77 pppanghu77 commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Root Cause Analysis

In Window::displayShortcuts() (src/widgets/window.cpp), the shortcuts popup position was first correctly calculated as the window's geometric center (lines 2574–2575), but then overwritten by screen->geometry().center() at line 2591 (introduced by commit 5d0074a2). In split-screen mode the full-screen center falls exactly on the split boundary, causing deepin-shortcut-viewer to center the popup on the boundary — producing partial occlusion and a visible reposition animation by the compositor. See analysis-report.md §6 for the root-cause conclusion.

Fix Approach

Remove the pos = screen->geometry().center() override line, keeping the screen-selection logic (QGuiApplication::screenAt(QCursor::pos()) etc.) for multi-monitor support while using the window center as the positioning coordinate. This aligns with the deepin-shortcut-viewer --help documentation, which states -p should be "your window's central position". See analysis-report.md §10 for the fix recommendation.

Change Safety Assessment

Code Safety

  • Risk Level: Low
  • The target line was introduced by feature commit 5d0074a2 (not a historical bug fix); removing it does not revert any prior bug fix. Screen-selection logic is preserved, only the position override is deleted.
  • displayShortcuts() is a private slot invoked only by the Qt signal system; no external callers are affected.

Business Impact Scope

  • Shortcuts help popup (Ctrl+Shift+?) — the popup now centers on the editor window instead of the full screen center.
  • Affected scenarios: single-screen, split-screen (left/right halves), and multi-monitor setups where the popup should appear on the screen under the cursor.

Verification Suggestion

  • Regression test priorities: verify popup positioning in split-screen mode (no occlusion, no animation), single-screen mode (normal centering), and multi-monitor mode (popup appears on cursor screen).

根因分析

Window::displayShortcuts()(src/widgets/window.cpp)在第 2574–2575 行正确计算了窗口几何中心坐标,但第 2591 行用 screen->geometry().center() 覆盖了该值(由 commit 5d0074a2 引入)。在分屏模式下,整屏中心恰好位于分屏分界线上,导致 deepin-shortcut-viewer 以分界线为中心定位浮窗,产生遮挡和合成器重定位动画。详见 analysis-report.md 第六节。

修复方案

删除 pos = screen->geometry().center() 覆盖行,保留屏幕选择逻辑(QGuiApplication::screenAt(QCursor::pos()) 等)用于多显示器场景确定目标屏幕,定位坐标使用窗口中心。这与 deepin-shortcut-viewer --help 文档一致(-p 参数为 "your window's central position")。详见 analysis-report.md 第十节。

改动安全评估

代码安全评估

  • 风险等级: 低风险
  • 目标行由 feature commit 5d0074a2 引入(非历史 bug 修复),删除不撤销任何已有 bug 修复。屏幕选择逻辑保留,仅删除位置覆盖行。
  • displayShortcuts() 为私有槽函数,仅由 Qt 信号系统调用,无外部调用者受影响。

业务影响范围

  • 快捷键帮助浮窗(Ctrl+Shift+?)— 浮窗现以编辑器窗口中心定位,而非整屏中心。
  • 受影响场景:单屏模式、分屏模式(左右半屏)、多显示器场景(浮窗出现在光标所在屏幕)。

验证建议

  • 回归测试重点:分屏模式下浮窗定位(无遮挡、无动画)、单屏模式正常居中、多显示器场景浮窗出现在光标所在屏幕。

Summary by Sourcery

Bug Fixes:

  • Fix shortcuts popup positioning so it remains centered on the application window instead of the full screen, preventing split-screen boundary occlusion and repositioning animation.

@pppanghu77
pppanghu77 marked this pull request as draft October 29, 2026 19:07
@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: pppanghu77

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sourcery-ai

sourcery-ai Bot commented Oct 29, 2026

Copy link
Copy Markdown
Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Removes an incorrect screen-center overwrite in Window::displayShortcuts(), allowing the shortcuts popup to use the window’s geometric center while preserving screen selection for multi-monitor scenarios.

Sequence diagram for shortcuts popup positioning

sequenceDiagram
    actor User
    participant Window
    participant QtScreen
    participant ShortcutViewer

    User->>Window: displayShortcuts()
    Window->>QtScreen: screenAt(QCursor::pos())
    QtScreen-->>Window: screen
    Window->>Window: calculate window geometry center
    Window->>ShortcutViewer: launch with window center position
    ShortcutViewer-->>User: show shortcuts popup centered on window
Loading

File-Level Changes

Change Details Files
Preserve the window-center coordinate when positioning the shortcuts popup.
  • Removed the screen-geometry center override.
  • Kept screen selection and diagnostic logging intact for multi-monitor support.
src/widgets/window.cpp
Align shortcuts popup behavior with the intended positioning contract across display layouts.
  • Positions the popup relative to the editor window rather than the full screen.
  • Avoids split-screen boundary placement while retaining cursor-screen selection.
src/widgets/window.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

1. Root cause: displayShortcuts() calculated window center at
   line 2574-2575 but then overwrote it with screen center via
   screen->geometry().center() at line 2593, causing the popup
   to center on the full screen instead of the window
2. Fix: remove the pos = screen->geometry().center() override
   line, keeping the screen selection logic for multi-monitor
   support while using window center as the position
3. Impact: shortcuts popup now centers on the editor window
   instead of screen center; fixes occlusion in split-screen
   mode where screen center falls on the split boundary

Log: Fix shortcuts popup occlusion in split-screen mode

Influence:
1. Test shortcuts popup position in single-screen mode
2. Test shortcuts popup position in split-screen mode
3. Test shortcuts popup position in multi-monitor mode

fix: 修复分屏模式下快捷键帮助浮窗遮挡问题

1. 根因:displayShortcuts() 在第 2574-2575 行计算了窗口中心
   坐标,但第 2593 行用 screen->geometry().center() 覆盖为
   屏幕中心,导致浮窗以整屏中心定位而非窗口中心
2. 方案:删除 pos = screen->geometry().center() 覆盖行,保留
   屏幕选择逻辑用于多显示器场景,定位坐标使用窗口中心
3. 影响:快捷键帮助浮窗以编辑器窗口中心定位,修复分屏模式
   下浮窗定位于分屏分界线导致的遮挡问题

Log: 修复分屏模式下快捷键帮助浮窗遮挡问题

Influence:
1. 测试单屏模式下快捷键帮助浮窗定位
2. 测试分屏模式下快捷键帮助浮窗定位
3. 测试多显示器场景下快捷键帮助浮窗定位

Bug: #189255
@pppanghu77
pppanghu77 force-pushed the agent/agent/35fa779880ba branch from 5d9825b to 51114ac Compare October 29, 2026 20:32
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.

2 participants