fix(power): unify screen-off handling in session - #166
Conversation
Reviewer's GuideScreen-off handling is consolidated behind PowerManager::TurnOffScreen, with controller availability checks, power-button wake toggling, and an X11 lock/BlackScreen/DPMS sequence; dde-shortcut-tool now makes only the Power1 D-Bus call, and translation file metadata is normalized. Sequence diagram for unified screen-off handlingsequenceDiagram
participant Shortcut as dde-shortcut-tool
participant Power as PowerManager
participant Screen as ScreenController
participant KWin as KWinBlackScreen
participant DPMS as DisplayPower
Shortcut->>Power: TurnOffScreen()
alt screen controller unavailable
Power-->>Shortcut: ignore request
else screen is off
Power->>Power: TurnOnScreen()
else Wayland session
Power->>Power: doTurnOffScreen()
else X11 session
Power->>Screen: doLock(true)
Power->>Power: SetPrepareSuspend(PS_ButtonClick)
Power->>KWin: wmBlackScreenActive()
opt mask not active
Power->>KWin: setWmBlackScreenActive(true)
end
Power->>DPMS: setDPMSModeOff()
opt mask was enabled
Power->>KWin: setWmBlackScreenActive(false)
end
Power->>Power: SetPrepareSuspend(PS_Finish)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/plugin-qt/power/session/powermanager.cpp" line_range="641-644" />
<code_context>
+ doLock(true);
+ SetPrepareSuspend(PS_ButtonClick);
+
+ const bool needMask = m_screenBlackLock && !m_proxy->wmBlackScreenActive();
+ if (needMask) {
+ m_proxy->setWmBlackScreenActive(true);
+ QThread::msleep(kBlackScreenMaskDelayMs);
+ }
+
</code_context>
<issue_to_address>
**issue (performance):** The session power service synchronously blocks its main thread for 100 ms after requesting the KWin mask, delaying all D-Bus handling, timers, and power-state signals during every X11 screen-off request.
**Triggers:** On X11 when the KWin BlackScreen mask is not already active.
**Suggested fix:** Replace the blocking sleep with an asynchronous timer/state machine that triggers DPMS after the compositor mask has had time to render.
</issue_to_address>
### Comment 2
<location path="src/plugin-qt/power/session/powermanager.cpp" line_range="641-646" />
<code_context>
+ doLock(true);
+ SetPrepareSuspend(PS_ButtonClick);
+
+ const bool needMask = m_screenBlackLock && !m_proxy->wmBlackScreenActive();
+ if (needMask) {
+ m_proxy->setWmBlackScreenActive(true);
+ QThread::msleep(kBlackScreenMaskDelayMs);
+ }
+
+ setDPMSModeOff();
+
</code_context>
<issue_to_address>
**nitpick (bug_risk):** When KWin BlackScreen is unavailable or its setActive call fails, wmBlackScreenActive returns false, needMask remains true, and TurnOffScreen still sleeps for 100 ms before DPMS; the requested mask is never installed and the failure is reduced to a warning.
**Triggers:** On X11 sessions without the KWin BlackScreen D-Bus interface, or when KWin rejects setActive.
**Suggested fix:** Check the KWin interface before enabling the mask and skip the delay when the mask cannot be activated; only wait after a successful setActive call.
</issue_to_address>d3cdb08 to
36d7489
Compare
1. Move the X11 lock-before-DPMS screen-off sequence out of dde-shortcut-tool and into the power session service's TurnOffScreen. 2. Turn the screen back on when the power button is pressed while the screen is already off, and skip the request when no screen controller is available. 3. Delegate dde-shortcut-tool's turn-off-screen action to Power1.TurnOffScreen instead of duplicating the sequence. Log: Screen-off logic now lives in the power session service and the shortcut tool only calls Power1.TurnOffScreen. Influence: Power-key screen-off and wake behave consistently. fix(power): 统一屏幕关闭处理到会话服务 1. 将 dde-shortcut-tool 中「先锁屏再 DPMS 关屏」的 X11 处理移入电源会话服务的 TurnOffScreen。 2. 屏幕已关闭时再次按电源键改为重新点亮;屏幕控制器不可用时忽略请求。 3. dde-shortcut-tool 的关屏动作改为调用 Power1.TurnOffScreen,不再重复实现。 Log: 关屏逻辑收敛到电源会话服务,快捷工具只调用 Power1.TurnOffScreen。 PMS: BUG-375787 Influence: 电源键关屏与唤醒行为保持一致。 Change-Id: Ib0f330915ba419603aabb9039bd1fd8e393470c4
36d7489 to
71e35b1
Compare
deepin pr auto reviewAI 代码审查报告总体评价
漏洞对比统计
详细分析1. 语法逻辑 (25/25分) ✓ 通过评价词: 语法正确,逻辑清晰 分析详情:
2. 代码质量 (23/25分) ✓ 通过评价词: 代码结构清晰,注释完整 分析详情:
潜在问题:
3. 代码性能 (20/20分) ✓ 通过评价词: 性能良好,资源使用合理 分析详情:
4. 代码安全 (30/30分) ✓ 通过存在 0 个安全漏洞 漏洞对比统计:新增漏洞 0 个,减少漏洞 1 个,持平 0 个 分析详情:
代码变更概要
审查结论本次 PR 是一次高质量的重构,将屏幕关闭逻辑从 dde-shortcut-tool 收敛到 PowerManager 会话服务中:
建议: 可考虑清理 本报告由 AI 代码审查工具自动生成 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mhduiy, yixinshark The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/forcemerge |
|
This pr force merged! (status: blocked) |
变更
PowerManager::TurnOffScreen,dde-shortcut-tool 仅调用Power1.TurnOffScreen关联
PMS: BUG-375787
Summary by Sourcery
Centralize screen power transitions in the power session service and delegate shortcut-triggered screen-off requests through Power1.
Bug Fixes:
Enhancements:
Chores: