Skip to content

fix: fix window positioning with multi-screen and scaling#730

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
xionglinlin:master
Mar 4, 2026
Merged

fix: fix window positioning with multi-screen and scaling#730
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
xionglinlin:master

Conversation

@xionglinlin
Copy link
Copy Markdown
Contributor

  1. Changed moveToCenter() to use queued invocation of moveToCenterByRect()
  2. This addresses timing issues when moving windows with multi-screen setups and scaling > 1
  3. The queued connection ensures the geometry calculation happens after the window system has processed previous operations
  4. Prevents incorrect window positioning due to race conditions in geometry updates

Log: Fixed window positioning issues in multi-screen environments with display scaling

Influence:

  1. Test window centering on multi-monitor setups with different scaling factors
  2. Verify dialog positioning when moving between screens with different DPI settings
  3. Test window movement operations during screen configuration changes
  4. Verify that windows appear correctly centered on their parent windows
  5. Test with various display scaling values (100%, 125%, 150%, 200%)

fix: 修复多屏幕和缩放时的窗口定位问题

  1. 将 moveToCenter() 改为使用队列调用 moveToCenterByRect()
  2. 解决了多屏幕设置和缩放大于1时窗口移动的时序问题
  3. 队列连接确保几何计算在窗口系统处理完先前操作后执行
  4. 防止由于几何更新中的竞争条件导致的窗口定位错误

Log: 修复了多屏幕环境下显示缩放时的窗口定位问题

Influence:

  1. 在多显示器设置中使用不同缩放因子测试窗口居中功能
  2. 验证在不同DPI设置的屏幕间移动时的对话框定位
  3. 测试屏幕配置更改期间的窗口移动操作
  4. 验证窗口是否在其父窗口上正确居中显示
  5. 使用不同的显示缩放值进行测试(100%、125%、150%、200%)

PMS: BUG-318977

D_DC(DAbstractDialog);

moveToCenterByRect(d->getParentGeometry());
QMetaObject::invokeMethod(this, "moveToCenterByRect", Qt::QueuedConnection, Q_ARG(QRect, d->getParentGeometry()));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

copyright时间改一下,

1. Changed moveToCenter() to use queued invocation of
moveToCenterByRect()
2. This addresses timing issues when moving windows with multi-screen
setups and scaling > 1
3. The queued connection ensures the geometry calculation happens after
the window system has processed previous operations
4. Prevents incorrect window positioning due to race conditions in
geometry updates

Log: Fixed window positioning issues in multi-screen environments with
display scaling

Influence:
1. Test window centering on multi-monitor setups with different scaling
factors
2. Verify dialog positioning when moving between screens with different
DPI settings
3. Test window movement operations during screen configuration changes
4. Verify that windows appear correctly centered on their parent windows
5. Test with various display scaling values (100%, 125%, 150%, 200%)

fix: 修复多屏幕和缩放时的窗口定位问题

1. 将 moveToCenter() 改为使用队列调用 moveToCenterByRect()
2. 解决了多屏幕设置和缩放大于1时窗口移动的时序问题
3. 队列连接确保几何计算在窗口系统处理完先前操作后执行
4. 防止由于几何更新中的竞争条件导致的窗口定位错误

Log: 修复了多屏幕环境下显示缩放时的窗口定位问题

Influence:
1. 在多显示器设置中使用不同缩放因子测试窗口居中功能
2. 验证在不同DPI设置的屏幕间移动时的对话框定位
3. 测试屏幕配置更改期间的窗口移动操作
4. 验证窗口是否在其父窗口上正确居中显示
5. 使用不同的显示缩放值进行测试(100%、125%、150%、200%)

PMS: BUG-318977
@deepin-ci-robot
Copy link
Copy Markdown
Contributor

deepin pr auto review

这段代码的修改主要是将原本的同步调用 moveToCenterByRect 改为了通过 QMetaObject::invokeMethod 的异步调用。

以下是对这段代码的详细审查意见,包括语法逻辑、代码质量、代码性能和代码安全方面的分析:

1. 语法逻辑

  • 修改点:将 moveToCenterByRect(d->getParentGeometry()) 改为 QMetaObject::invokeMethod(..., Qt::QueuedConnection, ...)
  • 分析
    • 从语法上看,QMetaObject::invokeMethod 的使用是正确的。它指定了接收对象 this,方法名,连接方式 Qt::QueuedConnection 以及参数类型 Q_ARG(QRect, ...)
    • 潜在风险moveToCenterByRect 必须是一个 slots 或者标记为 Q_INVOKABLE 的函数,否则 invokeMethod 将无法找到该方法并导致调用失败。如果原函数只是一个普通的 private/protected 函数,需要配合此修改添加相应的宏标记。
    • 逻辑变化:原本是立即执行居中计算和移动,现在是将调用放入事件队列中,等到当前函数栈返回后、主事件循环处理该事件时才执行。这意味着 moveToCenter 函数返回时,窗口可能还没有移动。

2. 代码质量

  • 意图推测:开发者引入异步调用通常是为了解决布局闪烁问题,或者是为了确保在父窗口几何信息完全更新后再进行子窗口的居中计算(例如在窗口 showresize 事件链中)。
  • 可读性:使用 invokeMethod 略显晦涩,不如直接调用直观。如果是为了延迟执行,也可以考虑使用 QTimer::singleShot(0, this, [this]{ moveToCenterByRect(...); });(C++11 风格),这在处理捕获上下文时可能更灵活。
  • 参数捕获:代码中 d->getParentGeometry() 是在 moveToCenter 被调用时立即求值的。如果父窗口的几何信息在异步调用真正执行前发生了变化,窗口将基于旧的几何信息居中。这可能是预期的(基于调用时刻的状态),也可能是一个 Bug(如果期望的是基于最新状态)。

3. 代码性能

  • 开销:同步调用直接压栈执行,开销极小。异步调用涉及构造 QEvent,放入事件队列,以及后续的函数分发,开销略大于同步调用。
  • 影响:对于 UI 操作(如移动窗口),这种微小的性能差异通常可以忽略不计。相反,通过异步调用避免了可能的重绘或布局计算的阻塞,反而可能提升用户感知的流畅度。

4. 代码安全

  • 对象生命周期:使用了 Qt::QueuedConnection。如果在槽函数执行之前,this 对象(即 DAbstractDialog 实例)被销毁了,程序可能会崩溃,或者 Qt 会尝试处理已发送给已销毁对象的事件(取决于 Qt 版本和具体实现,通常 Qt 会在对象析构时清除未处理的事件)。
    • 改进建议:如果在异步执行期间对象极有可能被销毁,需要确保逻辑上的安全性。不过对于对话框对象,通常由用户关闭,生命周期相对可控。
  • 版权年份:Diff 中包含版权年份的更新(2015 - 2026)。这属于元数据修改,不影响代码逻辑,但建议确认 2026 是否为预期的年份(通常更新至当前年份或未来不久的年份)。

改进建议

  1. 函数声明检查
    请确保 moveToCenterByRect 在头文件中的声明如下之一,否则异步调用无效:

    private slots:
        void moveToCenterByRect(const QRect &rect);
    
    // 或者
    
    protected:
        Q_INVOKABLE void moveToCenterByRect(const QRect &rect);
  2. 使用 Lambda 表达式(C++11 推荐)
    如果项目允许使用 C++11,使用 Lambda 表达式配合 QTimer::singleShot 通常比 invokeMethod 更安全且易于维护,因为它是类型安全的,编译器会检查参数类型,不需要字符串硬编码方法名。

    修改建议代码

    void DAbstractDialog::moveToCenter()
    {
        D_DC(DAbstractDialog);
        
        // 获取当前几何信息并捕获
        QRect parentGeo = d->getParentGeometry();
        
        // 延迟 0 毫秒执行,等同于 QueuedConnection,但类型更安全
        QTimer::singleShot(0, this, [this, parentGeo]() {
            moveToCenterByRect(parentGeo);
        });
    }

    优点

    • 编译期类型检查,避免字符串 "moveToCenterByRect" 拼写错误。
    • 不需要强制要求函数是 slotQ_INVOKABLE,只要在当前上下文可访问即可。
    • 明确捕获了 parentGeo,代码意图更清晰。
  3. 关于参数捕获的再思考
    如果业务逻辑要求窗口始终基于父窗口最新的几何信息居中(例如父窗口正在动画调整大小),那么在 invokeMethod 中提前计算 d->getParentGeometry() 可能是不合适的。

    • 如果是这样,异步调用的函数内部应该重新获取父窗口几何,而不是通过参数传递。
    • 如果是基于当前时刻快照居中,则现有逻辑是正确的。

总结

该修改主要是为了解决时序问题(如布局未完成计算就居中)或闪烁问题。

  • 安全性:中等,注意对象生命周期。
  • 性能:可接受。
  • 建议:优先考虑使用 C++11 Lambda 的方式替代 invokeMethod 以获得更好的类型安全和可读性,同时务必检查目标函数的声明是否支持元对象调用。

@deepin-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, xionglinlin

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

@18202781743
Copy link
Copy Markdown
Contributor

/forcemerge

@deepin-bot
Copy link
Copy Markdown
Contributor

deepin-bot bot commented Mar 4, 2026

This pr force merged! (status: blocked)

@deepin-bot deepin-bot bot merged commit 693bdd3 into linuxdeepin:master Mar 4, 2026
18 of 21 checks passed
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.

3 participants