treeland-snap - #1307
Open
glyvut wants to merge 2 commits into
Open
treeland-snap#1307glyvut wants to merge 2 commits into
glyvut wants to merge 2 commits into
Conversation
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: glyvut 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 GuideThis PR removes the legacy screenshot integration in favor of a compositor-provided snap-target protocol, adds snap-mask-specific layering and window-management exclusions, and ships a multi-output interactive example with optional pre-captured screen backgrounds. Review the protocol lifecycle and single-session semantics, target ordering/hit testing, pidfd ownership, mask-surface state transitions, and the example’s Wayland capture/resource cleanup paths. Sequence diagram for snap-target pointer selectionsequenceDiagram
participant Client as SnapTargetClient
participant Protocol as SnapTargetV1
participant Detector as SnapDetector
participant Cursor as WCursor
participant Surface as WToplevelSurface
Client->>Protocol: start(seat, events)
Protocol->>Detector: collect(renderWindow, excludeSurfaces)
Protocol->>Cursor: positionChanged
Cursor-->>Protocol: onCursorMoved()
Protocol->>Detector: hitTest(cursorPos, snapshot)
Detector-->>Protocol: SnapTarget
alt target has owning process and pidfd requested
Protocol->>Surface: pidFD()
Surface-->>Protocol: pidfd
Protocol-->>Client: sendPidfd(pidfd)
end
Protocol-->>Client: sendSnapRegion(x, y, width, height)
Client->>Protocol: stop()
Protocol->>Protocol: stopSnapping()
Flow diagram for snap-mask surface handlingflowchart TD
TaggedSurface["Surface tagged org.deepin.treeland.snap-mask"] --> Apply["applySnapMask(wrapper)"]
Apply --> State["setSnapMask(true)"]
Apply --> Layer["Add to snapMaskContainer"]
Apply --> Exclude["addSnapMaskSurface(surface)"]
Apply --> WindowRules["Disable animation and window-management views"]
Exclude --> Detect["SnapDetector.collect(renderWindow, excludeSurfaces)"]
Detect --> HitTest["SnapDetector.hitTest(cursorPos, snapshot)"]
State --> Capabilities["Reject resize, maximize, and tiling capabilities"]
SurfaceRemoved["XDG surface removed"] --> Remove["removeSnapMaskSurface(surface)"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
TAG Bot New tag: 0.9.0 |
|
TAG Bot New tag: 0.9.1 |
|
TAG Bot New tag: 0.10.0 |
glyvut
force-pushed
the
s4
branch
6 times, most recently
from
September 17, 2026 03:17
d99b852 to
c02cb85
Compare
glyvut
force-pushed
the
s4
branch
5 times, most recently
from
September 28, 2026 03:06
b590482 to
201787f
Compare
glyvut
marked this pull request as ready for review
September 28, 2026 03:13
![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 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="examples/test_snap_target/screencapture.cpp" line_range="189-190" />
<code_context>
+ static int serial = 0;
+ int framesSent = 0;
+ for (Output *output : std::as_const(m_outputs)) {
+ if (!output->constraintsDone || output->imageFormat == QImage::Format_Invalid)
+ continue;
+
+ const int stride = output->bufferW * 4;
</code_context>
<issue_to_address>
**issue (bug_risk):** When any output does not complete capture constraints or does not advertise a supported shared-memory format, that output is skipped without setting `captureReady` or `captureFailed`. If another output has a frame in flight, the dispatch loop waits forever because it continues treating the skipped output as pending, so the example hangs before showing its UI.
**Triggers:** When background capture is enabled and at least one output cannot be captured while another output is capture-capable.
**Suggested fix:** Track only outputs for which a frame was submitted in the pending loop, or mark skipped outputs as failed before entering the loop.
```suggestion
if (!output->constraintsDone || output->imageFormat == QImage::Format_Invalid) {
output->captureFailed = true;
continue;
}
```
</issue_to_address>
### Comment 2
<location path="examples/test_snap_target/CMakeLists.txt" line_range="52" />
<code_context>
+ Qt6::Core
+ Qt6::Gui
+ Qt6::WaylandClient
+ Qt6::WaylandClientPrivate
+ Qt6::Quick
+)
</code_context>
<issue_to_address>
**issue (bug_risk):** The example links `Qt6::WaylandClientPrivate` unconditionally even though the corresponding package component is found only for Qt versions greater than or equal to 6.10. On supported older Qt versions where that imported target is unavailable, CMake configuration fails with an unknown target error.
**Triggers:** When building with Qt 6 before 6.10.
**Suggested fix:** Link `Qt6::WaylandClientPrivate` only under the same Qt-version condition, or require Qt 6.10 explicitly for this example.
</issue_to_address>
wineee
reviewed
Sep 28, 2026
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved critical protocol resource validation and undefined-slot link issues remain, along with additional correctness fixes.
Review effort: Lite
Findings: 2
Open (4)
What changed in this PR
Adds snap-target Wayland protocol support, snap-mask handling, and a multi-output Qt/QML demonstration client.
Changes:
- Adds cursor-based target detection with optional pidfd reporting.
- Integrates snap-mask layering and interaction restrictions.
- Adds screen capture support and build integration for the demo.
| File | Description |
|---|---|
src/surface/surfacewrapper.h |
Adds snap-mask API. |
src/surface/surfacewrapper.cpp |
Applies snap-mask restrictions. |
src/surface/seatsurfacemanager.cpp |
Blocks snap-mask move/resize operations. |
src/seat/helper.cpp |
Registers the snap-target protocol. |
src/modules/snap-target/snaphandler.h |
Declares protocol session handling. |
src/modules/snap-target/snaphandler.cpp |
Implements sessions and events. |
src/modules/snap-target/snapdetector.h |
Declares target detection types. |
src/modules/snap-target/snapdetector.cpp |
Performs target collection and hit testing. |
src/modules/snap-target/impl/snapv1impl.h |
Declares protocol resource wrappers. |
src/modules/snap-target/impl/snapv1impl.cpp |
Implements protocol requests and events. |
src/modules/snap-target/CMakeLists.txt |
Builds and installs the module. |
src/modules/CMakeLists.txt |
Registers the module. |
src/core/shellhandler.h |
Adds snap-mask container APIs. |
src/core/shellhandler.cpp |
Manages snap-mask lifecycle and placement. |
src/core/rootsurfacecontainer.h |
Defines snap-mask layer ordering. |
examples/test_snap_target/snapproxy.h |
Declares demo protocol classes. |
examples/test_snap_target/snapproxy.cpp |
Implements demo client interaction. |
examples/test_snap_target/screencapture.h |
Declares capture support. |
examples/test_snap_target/screencapture.cpp |
Implements output capture. |
examples/test_snap_target/Main.qml |
Provides the selection UI. |
examples/test_snap_target/main.cpp |
Creates demo windows. |
examples/test_snap_target/CMakeLists.txt |
Builds the demo client. |
examples/CMakeLists.txt |
Registers the example. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
wineee
reviewed
Sep 28, 2026
glyvut
force-pushed
the
s4
branch
2 times, most recently
from
September 28, 2026 09:17
2acdcab to
80d134f
Compare
wineee
reviewed
Sep 28, 2026
| } | ||
| } // namespace | ||
|
|
||
| QList<SnapTarget> SnapDetector::collect(WOutputRenderWindow *renderWindow) |
Contributor
Author
There was a problem hiding this comment.
按绘制顺序收集所有可吸附目标并整理成自顶向下的顺序的列表
wineee
reviewed
Sep 28, 2026
wineee
reviewed
Sep 28, 2026
wineee
reviewed
Sep 28, 2026
wineee
reviewed
Sep 28, 2026
glyvut
force-pushed
the
s4
branch
2 times, most recently
from
September 29, 2026 02:45
15a2a13 to
74dcd29
Compare
Detect snap regions under the cursor and report them to clients, and treat xdg-toplevel-tag surfaces tagged org.deepin.treeland.snap-mask as overlay masks. - Add the snap-target protocol module with detector, handler and v1 impl - Track snap-mask surfaces in a dedicated shell container and surface flag, and expose them through snapMaskContainer() - Keep the existing capture module untouched 新增 snap-target 协议模块：检测光标下的吸附区域并上报给客户端，同时将带有 org.deepin.treeland.snap-mask 标签的 xdg-toplevel-tag 表面作为覆盖遮罩处理。 Log: 新增 treeland-snap-target-unstable-v1 协议模块 PMS: TASK-395745 Influence: 新增快照/吸附目标协议与遮罩表面支持，原有截图模块不变。
Add examples/test_snap_target: a per-output mask window demo that drives the snap-target protocol, with optional ext-image-copy-capture backgrounds. - Create one mask window per output sharing a single snap session - Capture each output with ext-image-copy-capture before mapping masks and use the images as window backgrounds with --background - Drive the client through a SnapProxy/SnapController layer and a QML selection UI 新增 test_snap_target 示例：为每个输出创建遮罩窗口并通过 snap-target 协议客户端 驱动，可选使用 ext-image-copy-capture 抓取每个输出作为窗口背景。 Log: 新增 snap-target 示例客户端 PMS: TASK-395745 Influence: 提供可手动运行的 snap-target 协议演示，便于验证相关协议行为。
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.
前置：linuxdeepin/treeland-protocols#87
Summary by Sourcery
Implement snap-target selection support in the compositor and provide a multi-output demonstration client.
New Features:
Bug Fixes:
Enhancements:
Build: