Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: deepin-wm 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 GuideAdds server-side ext-background-effect-v1 blur support by vendoring a wlroots-style implementation, generating and wrapping the Wayland protocol, integrating its manager into Treeland, and updating build/kernel interfaces; the PR reports successful compilation of all 1668 targets. Sequence diagram for the background blur protocol flowsequenceDiagram
participant Client
participant Manager as WBackgroundEffectManagerV1
participant Surface as wl_surface
participant Effect as ext_background_effect_surface_v1
participant Helper
participant Wrapper as SurfaceWrapper
Client->>Manager: get_background_effect(id, surface)
Manager-->>Client: capabilities(blur)
Client->>Effect: set_blur_region(region)
Client->>Surface: commit()
Surface->>Effect: apply pending blur state
Helper->>Effect: wlr_ext_background_effect_v1_get_surface_state(surface)
Helper->>Wrapper: setBlur(hasBlur)
File-Level Changes
Possibly linked issues
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}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/seat/helper.cpp" line_range="1707" />
<code_context>
+ auto updateBackgroundBlur = [wrapper, wlrSurface] {
+ const auto *state = wlr_ext_background_effect_v1_get_surface_state(wlrSurface);
+ bool hasBlur = state && pixman_region32_not_empty(&state->blur_region);
+ wrapper->setBlur(hasBlur);
+ };
+ updateBackgroundBlur();
</code_context>
<issue_to_address>
**issue (broader_impact):** The new integration unconditionally calls `wrapper->setBlur(false)` when a surface has no ext-background-effect blur region, overwriting the existing blur state set by `Personalization::backgroundTypeChanged`. Existing windows that use the personalization blur path therefore lose their blur immediately when the wrapper is added, and again on every surface commit.
**Triggers:** When an XDG toplevel or layer surface uses the existing personalization blur feature but does not use ext-background-effect-v1.
**Suggested fix:** Combine the protocol state with the existing blur source, or only update the wrapper's blur state from this callback when the protocol actually owns the surface's blur state.
</issue_to_address>| auto updateBackgroundBlur = [wrapper, wlrSurface] { | ||
| const auto *state = wlr_ext_background_effect_v1_get_surface_state(wlrSurface); | ||
| bool hasBlur = state && pixman_region32_not_empty(&state->blur_region); | ||
| wrapper->setBlur(hasBlur); |
There was a problem hiding this comment.
issue (broader_impact): The new integration unconditionally calls wrapper->setBlur(false) when a surface has no ext-background-effect blur region, overwriting the existing blur state set by Personalization::backgroundTypeChanged. Existing windows that use the personalization blur path therefore lose their blur immediately when the wrapper is added, and again on every surface commit.
Triggers: When an XDG toplevel or layer surface uses the existing personalization blur feature but does not use ext-background-effect-v1.
Suggested fix: Combine the protocol state with the existing blur source, or only update the wrapper's blur state from this callback when the protocol actually owns the surface's blur state.
wineee
marked this pull request as draft
September 16, 2026 08:18
wineee
force-pushed
the
agent/developer/dddab10db858
branch
from
September 17, 2026 02:57
6eae61d to
b8aaa92
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved production integration, lifecycle, build reproducibility, and example issues must be addressed.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds end-to-end ext-background-effect-v1 blur support to Treeland.
Changes:
- Adds wlroots protocol implementation and build generation.
- Integrates the Waylib manager with Treeland surface blur state.
- Adds a blur client example and replaces the background example.
File summaries
| File | Reviewed change |
|---|---|
waylib/src/server/wlroots_extra/wlr_ext_background_effect_v1.h |
Native protocol declarations |
waylib/src/server/wlroots_extra/wlr_ext_background_effect_v1.c |
Native protocol implementation |
waylib/src/server/protocols/wbackgroundeffectmanagerv1.h |
Waylib manager interface |
waylib/src/server/protocols/wbackgroundeffectmanagerv1.cpp |
Manager implementation |
waylib/src/server/protocols/WBackgroundEffectManagerV1 |
Public forwarding header |
waylib/src/server/kernel/wlr_fwd.h |
Forward declarations |
waylib/src/server/kernel/wlr_all.h |
wlroots header integration |
waylib/src/server/CMakeLists.txt |
Protocol generation and build integration |
src/seat/helper.h |
Manager member declaration |
src/seat/helper.cpp |
Protocol registration and blur integration |
examples/test_window_blur/main.cpp |
Blur client example |
examples/test_window_blur/CMakeLists.txt |
Blur example build target |
examples/test_window_bg/main.cpp |
Replaced background example |
examples/CMakeLists.txt |
Example registration changes |
Review details
Suppressed comments (3)
examples/test_window_blur/main.cpp:135
createEffectSurface()gives up when the manager is not active, but no signal retries it later. Qt registry activation is asynchronous and the other client examples wait foractiveChanged; if activation arrives aftershowEvent/SurfaceCreated, this demo never createsm_effectand can never blur. ConnectactiveChangedand retry once the window surface exists, or defer window creation until the manager is ready.
当 manager 尚未 active 时，createEffectSurface() 会直接放弃，但没有信号在之后重试。Qt registry 的激活是异步的，其他客户端示例都会等待 activeChanged；如果激活发生在 showEvent/SurfaceCreated 之后，此示例将永远不会创建 m_effect，也无法启用模糊。请连接 activeChanged 并在 surface 就绪后重试，或延迟创建窗口直到 manager 准备完成。
if (!m_manager->blurAvailable()) {
qWarning() << "ext-background-effect-v1 blur is not supported by the compositor";
return;
examples/test_window_blur/main.cpp:73
m_manageris allocated withnewbut is never deleted; the destructor only documentsm_effectcleanup. This leaks the client extension object and its manager proxy whenever the example window is destroyed. Destroy the effect and manager explicitly in the window destructor (the window is created afterQApplication, so the display is still valid).
m_manager 通过 new 分配后从未释放，析构函数只处理了 m_effect 的说明。示例窗口销毁时会泄漏 client extension 对象及其 manager proxy。请在窗口析构函数中显式销毁 effect 和 manager（该窗口在 QApplication 之后创建，因此此时 display 仍然有效）。
~BlurWindow() override
{
// m_effect is destroyed in eventFilter() on SurfaceAboutToBeDestroyed,
// which Qt delivers before the underlying wl_surface goes away.
}
waylib/src/server/wlroots_extra/wlr_ext_background_effect_v1.c:135
- This protocol error is exposed to clients, but “a ext_background_effect_surface_v1 object” is grammatically incorrect. Use “an” so the diagnostic is clear. 该协议错误消息会返回给客户端，当前 “a ext_background_effect_surface_v1 object” 语法不正确，请改为 “an”。
"The wl_surface object already has a ext_background_effect_surface_v1 object");
- Files reviewed: 14/14 changed files
- Comments generated: 10
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+1707
to
+1709
| const auto *state = wlr_ext_background_effect_v1_get_surface_state(wlrSurface); | ||
| bool hasBlur = state && pixman_region32_not_empty(&state->blur_region); | ||
| wrapper->setBlur(hasBlur); |
| auto *wlrSurface = wrapper->surface()->handle(); | ||
| auto updateBackgroundBlur = [wrapper, wlrSurface] { | ||
| const auto *state = wlr_ext_background_effect_v1_get_surface_state(wlrSurface); | ||
| bool hasBlur = state && pixman_region32_not_empty(&state->blur_region); |
| wrapper->setBlur(hasBlur); | ||
| }; | ||
| updateBackgroundBlur(); | ||
| connect(wrapper->surface(), &WSurface::commit, this, updateBackgroundBlur); |
| wrapper->setBlur(hasBlur); | ||
| }; | ||
| updateBackgroundBlur(); | ||
| connect(wrapper->surface(), &WSurface::commit, this, updateBackgroundBlur); |
Comment on lines
+1707
to
+1709
| const auto *state = wlr_ext_background_effect_v1_get_surface_state(wlrSurface); | ||
| bool hasBlur = state && pixman_region32_not_empty(&state->blur_region); | ||
| wrapper->setBlur(hasBlur); |
wineee
force-pushed
the
agent/developer/dddab10db858
branch
2 times, most recently
from
September 20, 2026 07:52
2859f51 to
ef6ffd6
Compare
Add ext-background-effect-v1 (blur) protocol server-side support: - Vendored wlroots C implementation in waylib/src/server/wlroots_extra/ based on wlroots MR 5304 (wlr_surface_synced + wlr_addon pattern) - Qt wrapper WBackgroundEffectManagerV1 in waylib/src/server/protocols/ - Integration in Helper: register manager, hook blur state into SurfaceWrapper via setBlur() on surface commit - Delete the obsolete test_window_bg example and add test_window_blur PMS: TASK-395091
Complete the ext-background-effect-v1 data path from protocol state to the actual blur rendering: - waylib: add WBackgroundEffectManagerV1::surfaceBlurRegion(WSurface *) returning the committed blur region as a QRegion (pixman conversion via the existing WTools::fromPixmanRegion) - SurfaceWrapper: add QRegion blurRegion property (+ QML-friendly blurRegionRects); blur() is now true when either the personalization path or a non-empty protocol region requests it; drop syncBackgroundEffectBlur which reached into the wlroots C API and degraded the region to a bool - Helper: sync blur region from the waylib API on surface commit - QML: Blur effect supports the protocol region; the blurred layer is re-drawn clipped to each region rect, and rects touching a window edge inherit the window corner radius (region ∩ rounded shape) - tests: end-to-end protocol test (capabilities, double-buffered region set/keep/NULL/re-enable/destroy, background_effect_exists) and a server-side functional test - waylib: serve an inert xdg_output instead of asserting when get_xdg_output races an output that is not yet registered in the output layout (aborts the whole compositor otherwise) PMS: TASK-395091
deepin-wm
force-pushed
the
agent/developer/dddab10db858
branch
from
September 24, 2026 04:53
ef6ffd6 to
bd102e1
Compare
The SPDX Year Range Checker requires modified files to carry the current year in their copyright range.
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
Add ext-background-effect-v1 (blur) protocol server-side support for treeland.
Changes
Vendored wlroots C implementation (waylib/src/server/wlroots_extra/)
Protocol code generation
Qt wrapper (waylib/src/server/protocols/)
Build integration
Treeland integration
Build verification
All 1668 targets compiled successfully.
Summary by Sourcery
Enable ext-background-effect-v1 blur regions throughout the compositor, surface rendering pipeline, examples, and protocol tests.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation:
Tests:
Chores: