-
Notifications
You must be signed in to change notification settings - Fork 30
fix: skip scale factor retrieval on Wayland temporarily #186
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
1. Modified scale factor retrieval logic to only query XSettings DBus interface when running under X11 session 2. Added session type check using XDG_SESSION_TYPE environment variable 3. Added TODO comment explaining that XSettings is not available on Wayland during early startup 4. This change prevents failed DBus calls and potential startup delays on Wayland sessions 5. The scale factor is temporarily defaulted to 1.0 on Wayland, affecting only double-click distance calculation Log: Fixed Wayland session startup issue by skipping XSettings scale factor query Influence: 1. Test application startup on both X11 and Wayland sessions 2. Verify double-click behavior works correctly with default scale factor on Wayland 3. Ensure no DBus errors appear during Wayland session initialization 4. Confirm X11 sessions still properly retrieve scale factor from XSettings 5. Test that QT_QPA_PLATFORM environment variable is correctly set based on session type fix: Wayland 下暂时跳过缩放因子获取 1. 修改缩放因子获取逻辑,仅在 X11 会话下查询 XSettings DBus 接口 2. 添加使用 XDG_SESSION_TYPE 环境变量的会话类型检查 3. 添加 TODO 注释说明 Wayland 在早期启动阶段无法使用 XSettings 4. 此更改防止 Wayland 会话上的 DBus 调用失败和潜在的启动延迟 5. Wayland 上的缩放因子暂时默认为 1.0,仅影响双击距离计算 Log: 修复 Wayland 会话启动问题,跳过 XSettings 缩放因子查询 Influence: 1. 测试在 X11 和 Wayland 会话上的应用程序启动 2. 验证 Wayland 上使用默认缩放因子的双击行为正常工作 3. 确保 Wayland 会话初始化期间不出现 DBus 错误 4. 确认 X11 会话仍能正确从 XSettings 获取缩放因子 5. 测试 QT_QPA_PLATFORM 环境变量根据会话类型正确设置
deepin pr auto review这段代码主要是在会话启动时设置环境变量,特别是针对 X11 和 Wayland 会话类型的不同处理。以下是对该 1. 语法逻辑
2. 代码质量
3. 代码性能
4. 代码安全
改进建议基于以上分析,建议对代码进行如下微调: void EnvironmentsManager::createGeneralEnvironments()
{
// 1. 定义常量以提高可读性(建议放在类头文件中)
// static const char* kSessionTypeX11 = "x11";
// static const char* kSessionTypeWayland = "wayland";
// 2. 提前获取环境变量
QByteArray sessionType = qgetenv("XDG_SESSION_TYPE");
double scaleFactor = 1.0;
// 3. 仅在 X11 下尝试获取缩放因子
if (sessionType == "x11") { // 建议使用常量 kSessionTypeX11
QDBusInterface dbusInterface("org.deepin.dde.XSettings1", "/org/deepin/dde/XSettings1",
"org.deepin.dde.XSettings1", QDBusConnection::sessionBus());
if (dbusInterface.isValid()) {
QDBusReply<double> scaleFactorReply = dbusInterface.call("GetScaleFactor");
if (scaleFactorReply.isValid()) {
scaleFactor = scaleFactorReply.value();
}
}
} else if (sessionType == "wayland") { // 建议使用常量 kSessionTypeWayland
// 4. 添加日志记录,便于调试
qWarning() << "Running under Wayland, scale factor detection is skipped, using default 1.0. "
<< "This might affect double-click distance settings.";
// TODO: (保留原有注释)
// wayland环境下在这里通过xsettings获取是不合理的,此时xsettings服务还没有启动...
}
auto envs = QProcessEnvironment::systemEnvironment();
// ... 后续代码保持不变 ...
}总结: |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRestricts scale factor retrieval via XSettings D-Bus to X11 sessions only, defaults to scale factor 1.0 on Wayland, and keeps QT_QPA_PLATFORM selection based on session type while documenting current Wayland limitations. Sequence diagram for session-based scale factor retrieval in createGeneralEnvironmentssequenceDiagram
participant EnvironmentsManager
participant SystemEnv as SystemEnvironment
participant XSettingsService
participant QProcessEnv as QProcessEnvironment
EnvironmentsManager->>SystemEnv: qgetenv(XDG_SESSION_TYPE)
SystemEnv-->>EnvironmentsManager: sessionType
alt X11 session
EnvironmentsManager->>XSettingsService: DBus call GetScaleFactor
XSettingsService-->>EnvironmentsManager: scaleFactor (double)
EnvironmentsManager->>EnvironmentsManager: set scaleFactor from reply
else Wayland or other session
EnvironmentsManager->>EnvironmentsManager: keep default scaleFactor = 1.0
end
EnvironmentsManager->>QProcessEnv: QProcessEnvironment::systemEnvironment
QProcessEnv-->>EnvironmentsManager: envs
EnvironmentsManager->>EnvironmentsManager: insert DDE related envs into m_envMap
EnvironmentsManager->>EnvironmentsManager: insert QT_DBL_CLICK_DIST = 15 * scaleFactor
alt X11 session
EnvironmentsManager->>EnvironmentsManager: insert QT_QPA_PLATFORM = dxcb and xcb
else Wayland session
EnvironmentsManager->>EnvironmentsManager: insert QT_QPA_PLATFORM = wayland
else other session
EnvironmentsManager->>EnvironmentsManager: do not set QT_QPA_PLATFORM
end
Flow diagram for session-type dependent environment and scale factor handlingflowchart TD
A["Start createGeneralEnvironments"] --> B["Read XDG_SESSION_TYPE into sessionType"]
B --> C["Set scaleFactor = 1.0"]
C --> D{sessionType == x11?}
D -->|Yes| E["Create QDBusInterface to org.deepin.dde.XSettings1"]
E --> F{dbusInterface isValid?}
F -->|Yes| G["Call GetScaleFactor via DBus"]
G --> H{scaleFactorReply isValid?}
H -->|Yes| I["Set scaleFactor = scaleFactorReply.value"]
H -->|No| J["Keep scaleFactor = 1.0"]
F -->|No| J
D -->|No| K["Skip XSettings DBus call for Wayland or other sessions (TODO for Wayland handling)"]
I --> L["Load system environment via QProcessEnvironment::systemEnvironment"]
J --> L
K --> L
L --> M["Insert common envs into m_envMap including XDG_CURRENT_DESKTOP = DDE"]
M --> N["Insert QT_DBL_CLICK_DIST = 15 * scaleFactor"]
N --> O{sessionType == x11?}
O -->|Yes| P["Set QT_QPA_PLATFORM = dxcb;xcb"]
O -->|No| Q{sessionType == wayland?}
Q -->|Yes| R["Set QT_QPA_PLATFORM = wayland"]
Q -->|No| S["Do not set QT_QPA_PLATFORM"]
P --> T["End createGeneralEnvironments"]
R --> T
S --> T
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've left some high level feedback:
- The
sessionTypechecks are now used in two separate places in this method; consider factoring the environment lookup and session-type handling into a small helper or enum to avoid string literals and keep the logic centralized. - In the new
elsebranch for non-x11sessions, you implicitly fall back to a scale factor of 1.0; consider making this default explicit via a named constant or comment near the initialization so the behavior is clear even without reading the TODO.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `sessionType` checks are now used in two separate places in this method; consider factoring the environment lookup and session-type handling into a small helper or enum to avoid string literals and keep the logic centralized.
- In the new `else` branch for non-`x11` sessions, you implicitly fall back to a scale factor of 1.0; consider making this default explicit via a named constant or comment near the initialization so the behavior is clear even without reading the TODO.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, mhduiy 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 |
Log: Fixed Wayland session startup issue by skipping XSettings scale factor query
Influence:
fix: Wayland 下暂时跳过缩放因子获取
Log: 修复 Wayland 会话启动问题,跳过 XSettings 缩放因子查询
Influence:
Summary by Sourcery
Guard XSettings-based scale factor retrieval behind an X11 session check to avoid Wayland startup issues while keeping existing behavior for X11.
Bug Fixes:
Enhancements: