refactor: 窗口录屏 - #1234
Open
glyvut wants to merge 1 commit into
Open
Conversation
|
Skipping CI for Draft Pull Request. |
Reviewer's GuideForeign toplevel image capture now renders the full Qt Quick surface-item subtree, including subsurfaces, into a dedicated DPR-aware offscreen buffer. Capture scheduling, constraints, full-frame damage, and buffer locking are coordinated across afterRendering, renderEnd, and copy_frame to improve reliability and lifetime safety. Sequence diagram for foreign toplevel offscreen capturesequenceDiagram
participant Client
participant CaptureSource
participant Output
participant Renderer
participant Buffer
Client->>CaptureSource: start()
CaptureSource->>Output: wlr_output_update_needs_frame()
Output-->>CaptureSource: afterRendering
CaptureSource->>CaptureSource: computePixelSize()
CaptureSource->>Output: renderItemToBuffer(renderer, surfaceItem, pixelSize, dpr, format)
Output->>Renderer: beginRender()
Renderer->>Renderer: renderTransientItem(surfaceItem)
Renderer->>Buffer: render full item subtree
Renderer-->>Output: lastBuffer()
Output-->>CaptureSource: renderedBuffer
CaptureSource->>Buffer: wlr_buffer_lock()
Output-->>CaptureSource: renderEnd
CaptureSource-->>Client: frame event with full damage
Client->>CaptureSource: copy_frame()
CaptureSource->>Buffer: wlr_ext_image_copy_capture_frame_v1_copy_buffer()
CaptureSource->>Buffer: wlr_buffer_unlock()
Flow diagram for capture buffer lifetime and resize handlingflowchart TD
A[start capture] --> B[afterRendering]
B --> C[Compute pixel size using item size and DPR]
C --> D[renderItemToBuffer]
D --> E[Lock rendered buffer]
E --> F[renderEnd]
F --> G[Emit frame event with full buffer damage]
G --> H[copy_frame]
H --> I{Destination size matches?}
I -- No --> J[updateConstraints]
J --> K{Size matches after update?}
K -- No --> L[Fail frame and unlock buffer]
K -- Yes --> M[Copy rendered buffer]
I -- Yes --> M
M --> N[Unlock buffer]
N --> O[Frame complete]
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
glyvut
force-pushed
the
refactor/rec
branch
2 times, most recently
from
August 5, 2026 05:15
a57222c to
6f63415
Compare
|
TAG Bot New tag: 0.8.18 |
|
TAG Bot New tag: 0.9.0 |
glyvut
force-pushed
the
refactor/rec
branch
3 times, most recently
from
August 25, 2026 06:42
3f028a8 to
f99681b
Compare
glyvut
marked this pull request as ready for review
August 25, 2026 06:50
![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="waylib/src/server/qtquick/private/wbufferrenderer.cpp" line_range="853-857" />
<code_context>
+ transientRenderer->setProjectionMatrix(projectionMatrix);
+ transientRenderer->setProjectionMatrixWithNativeNDC(projectionMatrixWithNativeNDC);
+
+ auto *textureRT = static_cast<QRhiTextureRenderTarget *>(state.sgRenderTarget.rt);
+ textureRT->setFlags(textureRT->flags() & ~QRhiTextureRenderTarget::PreserveColorContents);
+
+ rc->renderNextFrame(transientRenderer);
+ wd->rhi->finish();
+
+ if (auto *drc = qobject_cast<QSGDefaultRenderContext *>(state.context)) {
</code_context>
<issue_to_address>
**issue (bug_risk):** `renderTransientItem` unconditionally casts `state.sgRenderTarget.rt` to `QRhiTextureRenderTarget` and dereferences `wd->rhi`, but `beginRender` explicitly supports non-RHI paint-device targets where both are null. Software rendering therefore crashes during offscreen capture.
**Triggers:** When the output uses the software or another non-RHI render backend.
**Suggested fix:** Guard the RHI-specific render-target flag update and `finish()` call, or provide the corresponding paint-device rendering path.
</issue_to_address>
### Comment 2
<location path="waylib/src/server/qtquick/private/wbufferrenderer.cpp" line_range="866-874" />
<code_context>
+
+ wlr_damage_ring_add_whole(m_damageRing.get());
+
+ itemNode->setMatrix(savedMatrix);
+ tempRoot->removeChildNode(itemNode);
+ if (savedParent)
+ savedParent->appendChildNode(itemNode);
+
+ transientRenderer->setRootNode(nullptr);
+ delete transientRenderer;
+ delete tempRoot;
+}
+
QSGRenderer *WBufferRenderer::ensureRenderer(int sourceIndex, QSGRenderContext *rc)
</code_context>
<issue_to_address>
**issue (bug_risk):** After rendering, the captured item node is restored with `savedParent->appendChildNode(itemNode)`, which moves it to the end of its parent's child list instead of restoring its original sibling position. Repeated captures therefore permanently change scene-graph stacking order and can make overlapping windows or items render above or below their original siblings.
**Triggers:** When the captured item has siblings under the same scene-graph parent and their visuals overlap.
**Suggested fix:** Save the original sibling position and reinsert the node at that position rather than appending it.
</issue_to_address>
### Comment 3
<location path="waylib/src/server/utils/wextimagecapturesourcev1impl.cpp" line_range="310-312" />
<code_context>
m_renderEndConnection = QMetaObject::Connection();
}
+
+ if (m_renderedBuffer) {
+ wlr_buffer_unlock(m_renderedBuffer);
+ }
+ m_renderedBuffer = nullptr;
}
</code_context>
<issue_to_address>
**issue (bug_risk):** `doOffscreenRender` unlocks and replaces `m_renderedBuffer` on every output frame, even when the previous frame event has not yet been serviced by `copy_frame`. A delayed copy then reads the newest frame instead of the buffer associated with its frame event, so the client receives mismatched frame contents and damage metadata.
**Triggers:** When the client delays `copy_frame` long enough for another output render to run before the copy request is handled.
**Suggested fix:** Keep a locked buffer per outstanding frame, keyed by the frame event, and release it only after that frame's `copy_frame` completes or is cancelled.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
glyvut
marked this pull request as draft
August 25, 2026 07:02
glyvut
force-pushed
the
refactor/rec
branch
4 times, most recently
from
August 26, 2026 06:59
2a9927d to
28189ba
Compare
glyvut
marked this pull request as ready for review
August 26, 2026 07:02
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="waylib/src/server/qtquick/private/wbufferrenderer.cpp" line_range="864-867" />
<code_context>
+ if (wd->rhi)
+ wd->rhi->finish();
+
+ if (auto *drc = qobject_cast<QSGDefaultRenderContext *>(state.context)) {
+ QRhiResourceUpdateBatch *batch = wd->rhi->nextResourceUpdateBatch();
+ drc->currentFrameCommandBuffer()->resourceUpdate(batch);
+ }
+
</code_context>
<issue_to_address>
**issue (bug_risk):** The software-rendering path dereferences `wd->rhi` while flushing the resource update batch, even though the surrounding comment says software rendering has no RHI. A software-rendered capture therefore crashes in `doOffscreenRender()` after the offscreen render completes.
**Triggers:** When the output uses the Qt Quick software renderer and `QSGDefaultRenderContext` is active.
**Suggested fix:** Guard the resource-update-batch block with `wd->rhi` as well as `drc`, or use the appropriate software-renderer synchronization path.
```suggestion
if (wd->rhi) {
if (auto *drc = qobject_cast<QSGDefaultRenderContext *>(state.context)) {
QRhiResourceUpdateBatch *batch = wd->rhi->nextResourceUpdateBatch();
drc->currentFrameCommandBuffer()->resourceUpdate(batch);
}
}
```
</issue_to_address>
### Comment 2
<location path="waylib/src/server/utils/wextimagecapturesourcev1impl.cpp" line_range="429-438" />
<code_context>
- // Use wlroots image copy function with validated buffers
bool success = wlr_ext_image_copy_capture_frame_v1_copy_buffer(dst_frame, src, renderer);
qCDebug(lcWlImageCapture) << "Copy result:" << success;
+ // Unlock regardless of success/failure.
</code_context>
<issue_to_address>
**issue (bug_risk):** When `wlr_ext_image_copy_capture_frame_v1_copy_buffer` returns false, the new code only logs the failure and never calls `wlr_ext_image_copy_capture_frame_v1_fail`. The client receives neither a successful ready event nor a failure completion, leaving the copy-capture frame unfinished and potentially stalling the capture session.
**Triggers:** When copying the rendered buffer fails, for example because of an unsupported format or renderer error.
**Suggested fix:** Call `wlr_ext_image_copy_capture_frame_v1_fail` in the `!success` branch, using the appropriate failure reason.
</issue_to_address>
### Comment 3
<location path="waylib/src/server/utils/wextimagecapturesourcev1impl.cpp" line_range="190-221" />
<code_context>
// which means multiple render listeners for the same surface. Consider implementing
// a manager to share render events among multiple capture sources.
- if (!m_surfaceContent) {
- qCWarning(lcWlImageCapture) << "No surface content available for capture";
- return;
- }
-
- // Get render window
</code_context>
<issue_to_address>
**issue (bug_risk):** `start()` sets `m_capturing` to true before validating that a render window exists, then returns without resetting it when `renderWindow()` is null. The source is left in a capturing state without render connections or a renderer, and a later start/request path cannot reliably recover because the source now appears already active.
**Triggers:** When the surface item is detached from its window or its render window is unavailable during start.
**Suggested fix:** Validate `renderWindow()` before setting `m_capturing`, or reset `m_capturing` before returning on startup failure.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
glyvut
force-pushed
the
refactor/rec
branch
3 times, most recently
from
August 27, 2026 11:10
b40e07d to
412cfc8
Compare
|
TAG Bot New tag: 0.9.1 |
zzxyb
requested changes
Aug 28, 2026
Rework WExtImageCaptureSourceV1Impl to capture the entire surface item subtree (content and subsurfaces) by offscreen rendering, instead of copying only the surface content texture. 重构 ext-image-capture 的窗口截图，将完整窗口（含子表面）离屏渲染到缓冲，替代原有的表面纹理拷贝。 - Add WBufferRenderer::renderTransientItem to render an item subtree by temporarily re-parenting under a transient root node. - Add WOutputRenderWindow::renderItemToBuffer for offscreen item rendering. - Render offscreen buffer in afterRendering, emit frame in renderEnd, and copy the locked buffer in copy_frame. Log: ext-image-capture 窗口截图改为离屏渲染完整窗口 PMS: BUG-374941, BUG-372395 Influence: 窗口截图改为渲染完整窗口（含子表面）,不再局限于表面纹理拷贝。
zzxyb
approved these changes
Sep 2, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: glyvut, zzxyb 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
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.
Summary by Sourcery
Refactor window recording to capture surface items through an isolated offscreen rendering path.
New Features:
Bug Fixes:
Enhancements: