Skip to content

fix: prevent pending state failure in window platform interface#396

Merged
18202781743 merged 1 commit into
masterfrom
fix/treeland-pending-state
Jul 9, 2026
Merged

fix: prevent pending state failure in window platform interface#396
18202781743 merged 1 commit into
masterfrom
fix/treeland-pending-state

Conversation

@18202781743

Copy link
Copy Markdown
Contributor

Remove Qt::QueuedConnection and early return to ensure pending operations always execute. The previous approach using stack variable comparisons was unreliable, causing pending state to fail intermittently. By directly invoking applyPending without queued connection and removing the unnecessary early return check, we guarantee consistent pending state application for window feature updates.

改动文件:

  • src/plugins/platform/treeland/dtreelandplatformwindowinterface.cpp
  • src/plugins/platform/treeland/dtreelandplatformwindowinterface.h

@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.

Sorry @18202781743, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@18202781743 18202781743 force-pushed the fix/treeland-pending-state branch from dcb7198 to 3928e82 Compare July 9, 2026 06:48
@18202781743

18202781743 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

目前协议导致无法使用applyPending,noTitlebar需要客户端强制提交,等之后协议添加configure后,再启用,
Qt::QueuedConnection是因为部分场景,栈变量使用场景,导致这种场景延迟无法生效,

BLumia
BLumia previously approved these changes Jul 9, 2026
Remove Qt::QueuedConnection and value comparison to ensure pending
operations always execute synchronously. QueuedConnection fails when
stack variables go out of scope before execution. Removing the early
return prevents state inconsistency when initial values match desired
values. Treeland protocol currently lacks configure support, so all
features must apply immediately.

Log: 修复窗口平台接口中 pending 状态失败问题
@18202781743 18202781743 force-pushed the fix/treeland-pending-state branch from 85f478b to 6862e66 Compare July 9, 2026 07:57
@deepin-ci-robot

Copy link
Copy Markdown
Contributor

deepin pr auto review

★ 总体评分:75分

■ 【总体评价】

代码修改了窗口属性应用调度和特征更新逻辑,但引入了性能退化和潜在重入风险
逻辑基本正确但因性能问题和代码质量下降扣分

■ 【详细分析】

  • 1.语法逻辑(基本正确)✓

scheduleApply 函数移除了 Qt::QueuedConnection 参数,QMetaObject::invokeMethod 将默认使用 Qt::AutoConnection,在同一线程下表现为同步调用。updateFeature 模板移除了值相等判断。代码语法正确,可编译通过。
潜在问题:scheduleApply 同步调用可能破坏原有的防重入逻辑,若 applyPending 重置了 m_applyScheduled,在调用栈中重复调用 scheduleApply 将导致非预期的多次执行;updateFeature 移除短路返回可能改变下游依赖 dirty 状态的逻辑。
建议:如果需要同步执行,应显式使用 Qt::DirectConnection 并评估重入风险;updateFeature 应保留短路逻辑以维护状态机正确性。

  • 2.代码质量(较差)✕

移除 Qt::QueuedConnection 降低了代码意图的明确性,隐式依赖 AutoConnection 行为不利于维护。updateFeature 移除相等判断通常是为了规避某些类型缺少 operator== 的编译错误,这种为了编译通过而牺牲设计逻辑的做法属于不良实践。
潜在问题:代码意图模糊,后续维护者难以理解为何移除显式连接类型和短路优化。
建议:显式声明连接类型;若类型 T 不支持比较,应通过模板约束或 std::equal_to 解决,而非删除优化代码。

  • 3.代码性能(存在性能问题)✕

updateFeature 中移除了 if (member == value) return;,导致每次属性设置都会执行 m_dirty |= flag;。这会使得即使属性值未发生变化,也会在后续触发不必要的 Wayland 协议请求和窗口更新操作。scheduleApply 变为同步调用可能导致在批量设置属性时无法合并事件,增加 applyPending 的执行次数。
潜在问题:高频属性设置场景下,合成器将收到大量冗余更新请求,造成 CPU 和 IPC 资源浪费。
建议:恢复 updateFeature 的值相等检查;恢复 scheduleApply 的异步队列调用以合并事件。

  • 4.代码安全(存在0个安全漏洞)✓

漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个
本次修改不涉及外部输入处理、权限操作或敏感信息管理,未引入安全漏洞。

  • 建议:无需安全修复,保持现有安全水平。

■ 【改进建议代码示例】

diff --git a/src/plugins/platform/treeland/dtreelandplatformwindowinterface.cpp b/src/plugins/platform/treeland/dtreelandplatformwindowinterface.cpp
index 758aedc6..ca45ef54 100644
--- a/src/plugins/platform/treeland/dtreelandplatformwindowinterface.cpp
+++ b/src/plugins/platform/treeland/dtreelandplatformwindowinterface.cpp
@@ -354,7 +354,7 @@ void DTreeLandPlatformWindowHelper::scheduleApply()
     if (m_applyScheduled)
         return;
     m_applyScheduled = true;
-    QMetaObject::invokeMethod(this, &DTreeLandPlatformWindowHelper::applyPending);
+    QMetaObject::invokeMethod(this, &DTreeLandPlatformWindowHelper::applyPending, Qt::QueuedConnection);
 }
 
 void DTreeLandPlatformWindowHelper::applyPending()
diff --git a/src/plugins/platform/treeland/dtreelandplatformwindowinterface.h b/src/plugins/platform/treeland/dtreelandplatformwindowinterface.h
index 552e8fc7..57d7dd18 100644
--- a/src/plugins/platform/treeland/dtreelandplatformwindowinterface.h
+++ b/src/plugins/platform/treeland/dtreelandplatformwindowinterface.h
@@ -63,8 +63,6 @@ private slots:
 
     template<typename T>
     void updateFeature(T &member, const T &value, Feature flag) {
+        if (member == value)
+            return;
         member = value;
         m_initialized |= flag;
         m_dirty |= flag;

@18202781743 18202781743 requested a review from BLumia July 9, 2026 08:01
@deepin-ci-robot

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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 18202781743 merged commit 2e6b7c9 into master Jul 9, 2026
43 of 44 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