Repository navigation
feat(personalization): implement new protocol split for treeland-protocols 0.6.0 - #1384
Merged
zccrs merged 1 commit intoSep 17, 2026
Merged
Conversation
|
Skipping CI for Draft Pull Request. |
Reviewer's GuideThis PR migrates personalization appearance functionality to the treeland-protocols 0.6.0 split by adding read-only appearance and privileged appearance-manager server globals, wiring them to DConfig with push updates and value conversions, while preserving the legacy personalization protocol and updating splash rendering, examples, build registration, and smoke coverage. Sequence diagram for appearance push updatessequenceDiagram
participant Client as AppearanceClient
participant Appearance as AppearanceInterfaceV1
participant Config as TreelandUserConfig
Client->>Appearance: bind_resource
Appearance->>Config: read appearance values
Appearance-->>Client: push all current values
Config-->>Appearance: appearanceChanged signal
Appearance-->>Client: push changed value
Sequence diagram for privileged appearance updatessequenceDiagram
participant Control as PrivilegedClient
participant Manager as AppearanceManagerInterfaceV1
participant Config as TreelandUserConfig
participant Appearance as AppearanceInterfaceV1
participant Clients as BoundAppearanceClients
participant Helper as Helper
Control->>Manager: set_color_scheme(scheme)
Manager->>Config: setWindowColorScheme(scheme)
Manager->>Helper: syncPaletteTypeWithWindowColorScheme(scheme)
Config-->>Appearance: windowColorSchemeChanged
Appearance-->>Clients: send_color_scheme(scheme)
State diagram for appearance configuration replacementstateDiagram-v2
[*] --> ConfigConnected
ConfigConnected --> ValuesPushed: bind_resource
ValuesPushed --> ValuesPushed: appearance setting changed
ValuesPushed --> ConfigReplaced: Helper.configChanged
ConfigReplaced --> ConfigConnected: setupConfigConnections
ConfigConnected --> ValuesPushed: sendAll to bound clients
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
TAG Bot New tag: 0.10.0 |
deepin-wm
force-pushed
the
agent/git-commit/8caa5a57575b
branch
2 times, most recently
from
September 15, 2026 13:21
759191e to
c87cf8d
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved initialization, protocol-scope, conversion, packaging, build, and test-coverage issues remain.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR adapts personalization for treeland-protocols 0.6.0 by adding appearance protocols, retaining the legacy interface, and updating build and test integration.
Changes:
- Adds appearance and appearance-manager protocol servers.
- Shares decoration types and preserves legacy protocol support.
- Replaces legacy protocol coverage with appearance smoke tests.
File summaries
| File | Summary |
|---|---|
tests/test_protocol_personalization/main.cpp |
Removes the legacy unit test. |
tests/test_protocol_personalization/CMakeLists.txt |
Removes the legacy test target. |
tests/test_protocol_appearance/main.cpp |
Adds limited appearance smoke tests. |
tests/test_protocol_appearance/CMakeLists.txt |
Adds the appearance test target. |
tests/protocols/treeland-personalization-manager-v1/treeland-personalization-manager-v1.h |
Removes legacy client declarations. |
tests/protocols/treeland-personalization-manager-v1/treeland-personalization-manager-v1.c |
Removes legacy protocol client coverage. |
tests/protocols/treeland-personalization-manager-v1/setup.cpp |
Removes the legacy fixture. |
tests/protocols/treeland-personalization-manager-v1/README.md |
Removes legacy test documentation. |
tests/protocols/treeland-personalization-manager-v1/CMakeLists.txt |
Removes the legacy protocol target. |
tests/protocols/treeland-personalization-desktop-v1/treeland-personalization-desktop-v1.h |
Removes desktop test declarations. |
tests/protocols/treeland-personalization-desktop-v1/treeland-personalization-desktop-v1.c |
Removes the desktop protocol client. |
tests/protocols/treeland-personalization-desktop-v1/setup.cpp |
Removes the desktop fixture. |
tests/protocols/treeland-personalization-desktop-v1/CMakeLists.txt |
Removes the desktop protocol target. |
tests/protocols/CMakeLists.txt |
Unregisters legacy protocol tests. |
tests/CMakeLists.txt |
Registers the appearance test. |
src/seat/helper.h |
Declares appearance interfaces. |
src/seat/helper.cpp |
Registers the new appearance globals. |
src/modules/personalization/personalizationmanagerinterfacev1.h |
Uses shared decoration types. |
src/modules/personalization/personalizationmanagerinterfacev1.cpp |
Retains and documents the legacy protocol. |
src/modules/personalization/CMakeLists.txt |
Keeps legacy protocol generation; decoration protocol remains unimplemented. |
src/modules/CMakeLists.txt |
Adds the appearance module. |
src/modules/appearance/CMakeLists.txt |
Adds protocol generation and build integration; a 0.6.0 minimum is needed. |
src/modules/appearance/appearancemanagerinterfacev1.h |
Declares appearance-manager operations. |
src/modules/appearance/appearancemanagerinterfacev1.cpp |
Implements setters; initialization gating and fixed-point conversion need correction. |
src/modules/appearance/appearanceinterfacev1.h |
Declares the read-only appearance interface. |
src/modules/appearance/appearanceinterfacev1.cpp |
Implements appearance pushes and config replacement handling. |
src/common/windowdecorations.h |
Defines shared decoration structures. |
src/CMakeLists.txt |
Includes the shared header in the build. |
Review details
Suppressed comments (3)
src/modules/appearance/appearanceinterfacev1.cpp:123
- EN: This
configChangedlambda now performs reconnection plus a resource iteration and resend, which is beyond the repository's “avoid complex lambdas” convention. Extract it into a named member slot/helper so the config-replacement lifecycle is independently testable and easier to maintain.
中文：这个configChangedlambda 同时负责重连、遍历资源和重新发送，已经超出仓库“避免复杂 lambda”的约定。请提取为具名成员槽函数/辅助函数，使配置替换生命周期更易测试和维护。
connect(helper, &Helper::configChanged, this, [this] {
setupConfigConnections();
// Push the new config values to all already-bound clients.
for (const auto &resource : d->server->resourceMap())
sendAll(resource->handle);
});
src/modules/appearance/appearancemanagerinterfacev1.cpp:150
wl_fixed_thas 8 fractional bits, so a common value such as0.4is received as0.3984375; multiplying and truncating here stores39instead of the requested40. A manager set followed by an appearance push therefore drifts for ordinary percentages. Round (and apply the protocol's range validation) before converting to the integer DConfig value.
中文：wl_fixed_t 使用 8 位小数精度，因此常见的 0.4 会以 0.3984375 接收；这里相乘后直接截断会把请求的 40 写成 39。这会导致 manager 设置后再由 appearance 推送时发生偏差。转换为 DConfig 整数前应四舍五入，并执行协议规定的范围校验。
const int value = static_cast<int>(wl_fixed_to_double(opacity) * 100.0);
tests/CMakeLists.txt:7
- Replacing the old test target leaves
tests/protocols/INDEX.mdstale: its personalization-manager row still links to a deleted README, and no appearance/decor protocol coverage is indexed. Update the protocol index when removing and adding protocol tests so the documented inventory matches CMake.
中文：替换旧测试目标后，tests/protocols/INDEX.md 已过时：其中 personalization-manager 行仍链接到已删除的 README，也没有记录 appearance/decor 协议覆盖。删除和新增协议测试时请同步更新索引，使文档清单与 CMake 保持一致。
add_subdirectory(test_protocol_appearance)
- Files reviewed: 28/28 changed files
- Comments generated: 6
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
deepin-wm
force-pushed
the
agent/git-commit/8caa5a57575b
branch
2 times, most recently
from
September 16, 2026 03:36
9e09ff9 to
a300d44
Compare
Member
|
Thanks for the review! All three points have been addressed in the follow-up code:
Note: the decoration protocol ( |
wineee
force-pushed
the
agent/git-commit/8caa5a57575b
branch
from
September 16, 2026 06:18
a300d44 to
c0e0ba4
Compare
deepin-wm
force-pushed
the
agent/git-commit/8caa5a57575b
branch
2 times, most recently
from
September 16, 2026 07:01
9f1a6f8 to
37cd09c
Compare
wineee
force-pushed
the
agent/git-commit/8caa5a57575b
branch
2 times, most recently
from
September 16, 2026 08:39
02d0f3b to
bb8bf70
Compare
Implement treeland_appearance_unstable_v1 and treeland_appearance_manager_unstable_v1 protocols for treeland-protocols 0.6.0. Move appearance code to src/modules/appearance/. Freeze old personalization module with DEPRECATED comments. Remove personalization tests, add appearance tests (8 tests). 为 treeland-protocols 0.6.0 实现 appearance 和 appearance-manager 两个新协议，将代码迁移到 src/modules/appearance/ 目录。冻结旧 personalization 模块并添加 DEPRECATED 注释。删除 personalization 测试， 新增 appearance 单元测试（8 项全部通过）。 Log: 实现新协议拆分，适配 treeland-protocols 0.6.0 Influence: 新增 2 个 appearance 协议实现，旧 personalization 协议冻结保留，不影响现有功能。 PMS: TASK-395091
wineee
force-pushed
the
agent/git-commit/8caa5a57575b
branch
from
September 16, 2026 09:42
bb8bf70 to
b2067b3
Compare
wineee
marked this pull request as ready for review
September 16, 2026 09:45
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/modules/personalization/CMakeLists.txt" line_range="8" />
<code_context>
+# has been split into separate protocols in treeland-protocols 0.6.0:
+# - treeland-appearance-unstable-v1 → src/modules/appearance/
+# - treeland-appearance-manager-unstable-v1 → src/modules/appearance/
+# - treeland-decoration-unstable-v1 → (not yet implemented)
+#
+# No new features will be added here. New code should use the new modules.
</code_context>
<issue_to_address>
**issue (broader_impact):** The advertised `treeland-decoration-unstable-v1` server implementation is absent: the build file explicitly records it as “not yet implemented” and never generates, compiles, or registers a decoration global. Clients requiring the new decoration protocol cannot bind it.
**Suggested fix:** Add the decoration protocol generation and `DecorationManagerInterfaceV1` implementation, then attach it from `Helper::init()`.
</issue_to_address>
### Comment 2
<location path="src/modules/appearance/appearanceinterfacev1.cpp" line_range="144-150" />
<code_context>
+ // once per-user resource tracking is available (e.g. track resources per
+ // client credentials / security context and push only to those).
+ //
+ // Current behavior: push to ALL bound clients unconditionally. If the new
+ // config hasn't finished async initialization, getters return DConfig
+ // default values, which is still better than stale values from the old
+ // config; later change signals will push actual values.
+ setupConfigConnections();
+ for (const auto &resource : d->server->resourceMap())
+ sendAll(resource->handle);
+}
+
</code_context>
<issue_to_address>
**🚨 issue (security):** The appearance globals never validate the requesting Wayland client's identity. Any local client can bind the supposedly privileged manager and invoke `set_*` requests to modify the current user's DConfig, while the read-only interface broadcasts the new user's values to every already-bound client after a user switch.
**Triggers:** When an untrusted local client binds the new globals, or when the active user changes while clients from the previous user remain connected.
**Suggested fix:** Restrict the manager global/bindings to authorized clients and track resources by user/security context before sending appearance values or accepting mutations.
</issue_to_address>
### Comment 3
<location path="src/modules/appearance/appearanceinterfacev1.cpp" line_range="164-167" />
<code_context>
+ if (!config)
+ return;
+
+ // Settings change signals. Initialization completion is not watched:
+ // bind_resource() and onConfigChanged() push values unconditionally, and
+ // uninitialized getters return DConfig defaults.
+ d->configConnections.append(
+ connect(config, &TreelandUserConfig::cursorThemeNameChanged, this, [this] {
+ const auto name = Helper::instance()->config()->cursorThemeName();
</code_context>
<issue_to_address>
**issue (bug_risk):** Clients that bind before `TreelandUserConfig::configInitializeSucceed` receive DConfig default values, but the interface does not connect to the initialization-success signal and therefore never re-pushes the actual settings when asynchronous initialization completes. The same stale-default state occurs after a user-config replacement that is still initializing.
**Triggers:** When a client binds during asynchronous DConfig initialization or immediately after a user switch.
**Suggested fix:** Connect each active config's `configInitializeSucceed` signal and call `sendAll` for all bound resources after initialization succeeds.
</issue_to_address>
zccrs
approved these changes
Sep 17, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: deepin-wm, zccrs 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 |
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
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.
Summary
Adapt the personalization module to treeland-protocols 0.6.0 breaking changes by implementing 3 new protocol server implementations while keeping the old
personalization-manager-v1for backward compatibility during the transition period.New protocol implementations
treeland_decoration_unstable_v1→decorationmanagerinterfacev1.{h,cpp}get_decoration_contexttreeland_appearance_unstable_v1→appearanceinterfacev1.{h,cpp}treeland_appearance_manager_unstable_v1→appearancemanagerinterfacev1.{h,cpp}set_*requests writing to DConfigHelper::syncPaletteTypeWithWindowThemeType()for color scheme changesShared types
ShadowandBorderstructs extracted tosrc/common/windowdecorations.h, shared by old and new protocol implementations.Key design decisions
PersonalizationManagerInterfaceV1kept alongside new protocols; old code has zero changes (only struct extraction).get_*requests removed; values pushed on bind and on change.Files changed
src/common/windowdecorations.hsrc/modules/personalization/decorationmanagerinterfacev1.{h,cpp}src/modules/personalization/appearanceinterfacev1.{h,cpp}src/modules/personalization/appearancemanagerinterfacev1.{h,cpp}src/modules/personalization/CMakeLists.txtsrc/modules/personalization/personalizationmanagerinterfacev1.hsrc/seat/helper.{h,cpp}src/CMakeLists.txttests/test_protocol_personalization/main.cppMultica issue: WM-444
Summary by Sourcery
Adopt treeland-protocols 0.6.0 appearance interfaces while retaining the legacy personalization protocol for compatibility.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation:
Tests:
Chores: