fix: unify screen scaling management with ScreenScale plugin#60
Open
fly602 wants to merge 1 commit into
Open
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: fly602 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 |
Reviewer's GuideRefactors XSettings screen scaling to delegate to a new org.deepin.dde.ScreenScale1 D-Bus plugin as the single source of truth, adds the ScreenScale plugin implementation and service wiring, and updates XSettingsManager to synchronize legacy DConfig/XSettings, Plymouth, and cursor size via the new service with fallback logic when the service is unavailable. Sequence diagram for runtime screen scale change via ScreenScale1sequenceDiagram
actor User
participant App
participant XSettingsManager
participant ScreenScale1
participant DConfig_ScreenScale as DConfig_ScreenScale1
participant DConfig_XSettings as DConfig_XSettings
participant Plymouth
User->>App: Change scale in UI
App->>XSettingsManager: setScreenScaleFactors(factors, emitSignal)
XSettingsManager->>XSettingsManager: setSingleScaleFactor(scale, emitSignal)
XSettingsManager->>ScreenScale1: SetScaleFactor(scale)
ScreenScale1->>DConfig_ScreenScale: setValue("scale-factor", scale)
DConfig_ScreenScale-->>ScreenScale1: write OK
ScreenScale1-->>XSettingsManager: ScaleFactorChanged(scale)
Note over ScreenScale1,XSettingsManager: XSettingsManager subscribed to ScaleFactorChanged signal
XSettingsManager->>XSettingsManager: onScaleFactorChanged(scale)
XSettingsManager->>DConfig_XSettings: setValue(dcKeyScaleFactor, scale)
XSettingsManager->>DConfig_XSettings: setValue(dcKeyWindowScale, windowScale)
XSettingsManager->>DConfig_XSettings: setValue(cKeyIndividualScaling, joinedFactors)
XSettingsManager->>DConfig_XSettings: setValue(dcKeyGtkCursorThemeSize, cursorSize)
XSettingsManager->>Plymouth: setScaleFactorForPlymouth(windowScale, false)
XSettingsManager-->>App: scale change persisted (effective after relogin)
alt ScreenScale1 unavailable
XSettingsManager->>XSettingsManager: onScaleFactorChanged(scale) (fallback)
XSettingsManager->>DConfig_XSettings: update legacy keys
XSettingsManager->>Plymouth: setScaleFactorForPlymouth(windowScale, false)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
fly602
force-pushed
the
master
branch
2 times, most recently
from
cb7ca9b to
81adb0e
Compare
yixinshark
reviewed
Apr 15, 2026
| } | ||
|
|
||
| double minScaleFactor = 3.0; | ||
| for (auto &&screen : screens) { |
Contributor
There was a problem hiding this comment.
const auto *screen : screens
| connect(m_config, | ||
| &DTK_CORE_NAMESPACE::DConfig::valueChanged, | ||
| this, | ||
| [this](const QString &key) { |
| , m_config(DTK_CORE_NAMESPACE::DConfig::create( | ||
| "org.deepin.dde.daemon", "org.deepin.dde.ScreenScale1", "", this)) | ||
| { | ||
| if (!m_config) { |
Contributor
There was a problem hiding this comment.
|| !m_config->isValid()
| return; | ||
| } | ||
|
|
||
| if (m_config->isValid()) { |
| return range; | ||
| } | ||
|
|
||
| double ScreenScale::getRecommendedScale() const |
Contributor
Author
There was a problem hiding this comment.
推荐值，可以协助调试或者查看当前的缩放。
yixinshark
reviewed
Apr 15, 2026
| found = true; | ||
| break; | ||
| } | ||
| } |
Contributor
Author
There was a problem hiding this comment.
浮点数精度误差，因此需要qFuzzyCompare比较
| return; | ||
| } | ||
|
|
||
| if (qFuzzyCompare(GetScaleFactor(), factor)) { |
| #include <algorithm> | ||
| #include <cmath> | ||
|
|
||
| namespace { |
Contributor
There was a problem hiding this comment.
建议这里面的函数都改成类的private， 这里写全局的，没有别人调用。
| @@ -0,0 +1,4 @@ | |||
| [D-BUS Service] | |||
Contributor
Author
There was a problem hiding this comment.
session 级dbus，不需要配置安全策略
| "magic": "dsg.config.meta", | ||
| "version": "1.0", | ||
| "contents": { | ||
| "scale-factor": { |
| install(FILES ${CMAKE_CURRENT_SOURCE_DIR}/misc/org.deepin.dde.ScreenScale1.service | ||
| DESTINATION lib/systemd/user/ | ||
| RENAME org.deepin.dde.ScreenScale1.service | ||
| ) |
| install(FILES ${CMAKE_CURRENT_SOURCE_DIR}/misc/plugin-dde-screenscale.json DESTINATION ${CMAKE_INSTALL_FULL_DATADIR}/deepin-service-manager/user) | ||
|
|
||
| # Install dconf config meta | ||
| install(FILES ${CMAKE_CURRENT_SOURCE_DIR}/misc/org.deepin.dde.ScreenScale1.json DESTINATION ${CMAKE_INSTALL_FULL_DATADIR}/dsg/configs/org.deepin.dde.daemon) |
Contributor
There was a problem hiding this comment.
dtk_add_config_meta_files
| @@ -0,0 +1,14 @@ | |||
| [Unit] | |||
Contributor
There was a problem hiding this comment.
不需要这个文件，只需要dbus的service文件
| return 1; | ||
| } | ||
|
|
||
| screenScale = new ScreenScale(); |
Contributor
There was a problem hiding this comment.
std::make_unique<>, 推荐使用只能指针
| "/org/deepin/dde/Greeter1", | ||
| "org.deepin.dde.Greeter1", | ||
| QDBusConnection::systemBus())) | ||
| , m_sysDaemonInterface(new QDBusInterface("com.deepin.daemon.Daemon", |
Contributor
Author
There was a problem hiding this comment.
这个需要，接口名变了而已，dde-daemon中设置plymouth缩放的
fly602
force-pushed
the
master
branch
6 times, most recently
from
4b639da to
7890d93
Compare
deepin pr auto review代码审查报告1. 整体架构与设计优点
改进建议
2. 语法与逻辑问题screenscale.cpp
xsettingsmanager.cpp
3. 代码质量问题
4. 性能问题
5. 安全问题
6. 其他建议
总结整体代码结构清晰，模块化设计良好，但在错误处理、输入验证、性能优化和安全方面还有改进空间。建议优先解决安全和输入验证问题，然后逐步改进代码质量和性能。 |
|
TAG Bot New tag: 1.0.22 |
|
TAG Bot New tag: 1.0.23 |
1. Add new ScreenScale plugin (org.deepin.dde.ScreenScale1) as the single source of truth for screen scale factors 2. Implement GetScreenScaleInfo and SetScaleFactor DBus methods with automatic recommended scale calculation 3. Refactor XSettings to delegate scale management to ScreenScale service via DBus 4. Remove deprecated ScaleFactorHelper, updateFirefoxDPI, and related legacy code 5. Standardize CMake variable naming from BIN_NAME to PLUGIN_NAME across all plugins 6. Update copyright years from 2025 to 2025-2026 7. Use dtk_add_config_meta_files for DConfig installation Log: Added centralized screen scale management service with automatic recommended scale calculation based on monitor physical dimensions Influence: 1. Test GetScreenScaleInfo with various screen configurations (single monitor, multi-monitor, HiDPI) 2. Verify SetScaleFactor validates input range (1.0-3.0) and persists via DConfig 3. Test scale factor synchronization between ScreenScale and XSettings services 4. Verify ScaleFactorChanged signal propagation across services 5. Test recommended scale calculation accuracy for different monitor sizes 6. Verify backward compatibility with existing scale configurations 7. Test plugin loading/unloading via deepin-service-manager 8. Verify DBus permission policies for ScreenScale1 service feat: 添加屏幕缩放插件并重构缩放管理 1. 新增 ScreenScale 插件 (org.deepin.dde.ScreenScale1) 作为屏幕缩放比例 的唯一事实来源 2. 实现 GetScreenScaleInfo 和 SetScaleFactor DBus 方法，支持自动计算推荐 缩放比例 3. 重构 XSettings 插件，通过 DBus 将缩放管理委托给 ScreenScale 服务 4. 移除已废弃的 ScaleFactorHelper、updateFirefoxDPI 及相关遗留代码 5. 统一各插件 CMake 变量命名，将 BIN_NAME 改为 PLUGIN_NAME 6. 更新版权年份从 2025 到 2025-2026 7. 使用 dtk_add_config_meta_files 安装 DConfig 配置 Log: 新增屏幕缩放管理服务，支持根据显示器物理尺寸自动计算推荐缩放比例 Influence: 1. 测试 GetScreenScaleInfo 接口，覆盖单屏、多屏、HiDPI 等场景 2. 验证 SetScaleFactor 对输入范围 (1.0-3.0) 的校验及 DConfig 持久化 3. 测试 ScreenScale 与 XSettings 服务间的缩放比例同步 4. 验证 ScaleFactorChanged 信号在各服务间的传播 5. 测试不同显示器尺寸下推荐缩放比例计算的准确性 6. 验证与现有缩放配置的向后兼容性 7. 测试通过 deepin-service-manager 加载/卸载插件 8. 验证 ScreenScale1 服务的 DBus 权限策略 PMS: BUG-355227 Change-Id: I17fbc039b207e020b74b5183b78463b97ef9eb3e
yixinshark
reviewed
Apr 29, 2026
| 4. 多屏 (笔记本 + 外接显示器): | ||
| [{"widthPx":1920,"heightPx":1080,"widthMm":310,"heightMm":174},{"widthPx":2560,"heightPx":1440,"widthMm":597,"heightMm":336}] | ||
| */ | ||
| QString ScreenScale::GetScreenScaleInfo(const QString &screensJson) |
Contributor
There was a problem hiding this comment.
widthPx
heightPx
physicalWidthMm
physicalHeightMm
建议参数名称
|
TAG Bot New tag: 1.0.24 |
|
TAG Bot New tag: 1.0.25 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refactored the XSettings scaling logic to use the new ScreenScale plugin as the primary source of truth. The XSettingsManager now delegates scale factor updates to the "org.deepin.dde.ScreenScale1" D-Bus service. This change removes redundant scaling calculations and ensures that scaling configurations are synchronized correctly between XSettings and the dedicated scaling service. Added a fallback synchronization logic in onScaleFactorChanged for cases where the ScreenScale service is unavailable.
Influence:
fix: 统一屏幕缩放管理到 ScreenScale 插件
重构了 XSettings 缩放逻辑，将 ScreenScale 插件作为缩放配置的事实来源。
XSettingsManager 现在将缩放因子更新操作委托给
"org.deepin.dde.ScreenScale1" D-Bus 服务。此更改删除了冗余的缩放计算逻 辑，并确保 XSettings 与专用缩放服务之间的配置保持同步。同时，在
ScreenScale 服务不可用时，在 onScaleFactorChanged 中提供了回退同步逻辑。
影响范围：
pms: BUG-355227
Change-Id: I2363239245451fc5079c39e6b76042d42a87b2a4
Summary by Sourcery
Unify screen scaling management around a new ScreenScale D-Bus plugin and make it the single source of truth for XSettings and related components.
New Features:
Bug Fixes:
Enhancements:
Build: