fix: upload shm textures via Qt in vulkan mode - #1131
Closed
Groveer wants to merge 1 commit into
Closed
Conversation
1. Add WRenderHelper::makeTextureFromShm to read shm pixels via begin_data_ptr_access and upload to a Qt-owned QSGPlainTexture 2. Intercept shm buffers in WSGTextureProvider::setBuffer under vulkan mode so Qt uploads textures itself instead of borrowing wlroots' device-local VkImage which lacks cross-stream sync 3. Deep-copy shm pixels before end_data_ptr_access since QSGPlainTexture uploads lazily during the render cycle 4. Manage shmTexture lifecycle with deferred cleanup matching the existing rhiTexture pattern; dmabuf path is unchanged Log: Fixed shm client window rendering glitches in vulkan mode by letting Qt upload shm textures directly instead of borrowing wlroots' vulkan texture Influence: 1. Test shm client windows (XWayland, software-rendered apps) under vulkan rendering mode for correct display 2. Verify dmabuf client windows are unaffected under vulkan mode 3. Test content updates on the same shm buffer to ensure texture refreshes correctly 4. Verify no texture leaks when switching shm buffers or destroying surfaces 5. Test gles2/software rendering to confirm no regression fix: vulkan 模式下 shm 纹理改由 Qt 上传 1. 新增 WRenderHelper::makeTextureFromShm，通过 begin_data_ptr_access 读取 shm 像素并上传到 Qt 自有的 QSGPlainTexture 2. 在 WSGTextureProvider::setBuffer 中于 vulkan 模式下拦截 shm buffer， 由 Qt 自行上传纹理而非借用 wlroots 的 device-local VkImage（缺少 跨 command stream 同步） 3. 在 end_data_ptr_access 之前深拷贝 shm 像素，因 QSGPlainTexture 在 渲染周期惰性上传 4. shmTexture 生命周期管理与现有 rhiTexture 延迟清理模式一致；dmabuf 路径保持不变 Log: 修复 vulkan 模式下 shm 客户端窗口渲染异常，由 Qt 直接上传 shm 纹理而非借用 wlroots 的 vulkan 纹理 Influence: 1. 在 vulkan 渲染模式下测试 shm 客户端窗口（XWayland、软件渲染应用） 显示是否正常 2. 验证 vulkan 模式下 dmabuf 客户端窗口未受影响 3. 测试同一 shm buffer 的内容更新确保纹理正确刷新 4. 验证切换 shm buffer 或销毁 surface 时无纹理泄漏 5. 测试 gles2/软件渲染路径确认无回归
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Groveer 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 |
Groveer
marked this pull request as draft
July 13, 2026 07:43
Reviewer's GuideIn Vulkan mode, shm client window buffers are now uploaded into Qt-owned textures directly from shared memory instead of borrowing wlroots VkImages, with supporting helpers and lifecycle management added to WRenderHelper and WSGTextureProvider. Sequence diagram for shm texture upload in Vulkan modesequenceDiagram
actor ShmClient
participant WSGTextureProvider
participant WRenderHelper
participant QSGPlainTexture as shmTexture
ShmClient->>WSGTextureProvider: setBuffer(qw_buffer)
WSGTextureProvider->>WSGTextureProvider: WRenderHelper::getGraphicsApi()
alt Vulkan_api_and_shm_buffer
WSGTextureProvider->>WSGTextureProvider: createPlainTexture()
WSGTextureProvider->>WRenderHelper: makeTextureFromShm(qw_buffer, shmTexture)
WRenderHelper->>WRenderHelper: begin_data_ptr_access(...)
WRenderHelper->>WRenderHelper: WTools::toImageFormat(format)
WRenderHelper->>QSGPlainTexture: setImage(QImage)
WRenderHelper->>QSGPlainTexture: setTextureSize(QSize)
WRenderHelper->>QSGPlainTexture: setHasAlphaChannel(bool)
WRenderHelper->>WRenderHelper: end_data_ptr_access()
WSGTextureProvider->>WSGTextureProvider: cleanTexture()
WSGTextureProvider->>WSGTextureProvider: store shmTexture, hasShmImage = true
WSGTextureProvider-->>ShmClient: textureChanged()
else Non_Vulkan_or_non_shm
WSGTextureProvider->>WSGTextureProvider: cleanTexture()
WSGTextureProvider->>WSGTextureProvider: use existing makeTexture path
WSGTextureProvider-->>ShmClient: textureChanged()
end
ShmClient->>WSGTextureProvider: texture()
WSGTextureProvider-->>ShmClient: shmTexture (if hasShmImage)
File-Level Changes
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, and left some high level feedback:
- TextureCleanupJob is now used for both rhiTexture and shmTexture but still defined locally inside cleanTexture(); consider hoisting it to a reusable helper (e.g., a small private class or lambda) to avoid repeated class definitions and make future extensions easier.
- The hasShmImage flag duplicates shmTexture’s null/non-null state; you could simplify the state machine by deriving this from shmTexture instead of maintaining a separate boolean, reducing the risk of subtle desynchronization bugs.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- TextureCleanupJob is now used for both rhiTexture and shmTexture but still defined locally inside cleanTexture(); consider hoisting it to a reusable helper (e.g., a small private class or lambda) to avoid repeated class definitions and make future extensions easier.
- The hasShmImage flag duplicates shmTexture’s null/non-null state; you could simplify the state machine by deriving this from shmTexture instead of maintaining a separate boolean, reducing the risk of subtle desynchronization bugs.
## Individual Comments
### Comment 1
<location path="waylib/src/server/qtquick/wrenderhelper.cpp" line_range="865-866" />
<code_context>
+ // QSGPlainTexture uploads the QImage lazily during the render cycle, so
+ // the image must own its data instead of referencing the shm mapping
+ // (which is only safely accessible between begin/end_data_ptr_access).
+ image = QImage(static_cast<const uchar *>(data), size.width(), size.height(),
+ static_cast<int>(stride), imageFormat).copy();
+ }
+ buffer->end_data_ptr_access();
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Guard against potential `stride` truncation when casting from `size_t` to `int`.
Here `stride` comes from wlroots as `size_t` and is cast to `int` for the `QImage` constructor without validation. If `stride` ever exceeds `INT_MAX`, this could overflow and produce a malformed image or undefined behavior. Please guard the cast (e.g., `stride <= std::numeric_limits<int>::max()` with an early return) or convert to `qsizetype`, which matches Qt’s internal size type, before passing it to `QImage`.
Suggested implementation:
```cpp
const QSize size(buffer->handle()->width, buffer->handle()->height);
const QImage::Format imageFormat = WTools::toImageFormat(format);
QImage image;
if (imageFormat != QImage::Format_Invalid && data && size.isValid()) {
// Deep-copy the shm pixels while the data pointer is accessible. The
// QSGPlainTexture uploads the QImage lazily during the render cycle, so
// the image must own its data instead of referencing the shm mapping
// (which is only safely accessible between begin/end_data_ptr_access).
// Guard against stride exceeding the range of int expected by QImage.
if (stride > static_cast<size_t>(std::numeric_limits<int>::max())) {
buffer->end_data_ptr_access();
return false;
}
const int bytesPerLine = static_cast<int>(stride);
image = QImage(static_cast<const uchar *>(data),
size.width(),
size.height(),
bytesPerLine,
imageFormat)
.copy();
}
buffer->end_data_ptr_access();
```
If `std::numeric_limits<int>` is not already available in this file, add `#include <limits>` to the include list at the top of `wrenderhelper.cpp`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+865
to
+866
| image = QImage(static_cast<const uchar *>(data), size.width(), size.height(), | ||
| static_cast<int>(stride), imageFormat).copy(); |
There was a problem hiding this comment.
suggestion (bug_risk): Guard against potential stride truncation when casting from size_t to int.
Here stride comes from wlroots as size_t and is cast to int for the QImage constructor without validation. If stride ever exceeds INT_MAX, this could overflow and produce a malformed image or undefined behavior. Please guard the cast (e.g., stride <= std::numeric_limits<int>::max() with an early return) or convert to qsizetype, which matches Qt’s internal size type, before passing it to QImage.
Suggested implementation:
const QSize size(buffer->handle()->width, buffer->handle()->height);
const QImage::Format imageFormat = WTools::toImageFormat(format);
QImage image;
if (imageFormat != QImage::Format_Invalid && data && size.isValid()) {
// Deep-copy the shm pixels while the data pointer is accessible. The
// QSGPlainTexture uploads the QImage lazily during the render cycle, so
// the image must own its data instead of referencing the shm mapping
// (which is only safely accessible between begin/end_data_ptr_access).
// Guard against stride exceeding the range of int expected by QImage.
if (stride > static_cast<size_t>(std::numeric_limits<int>::max())) {
buffer->end_data_ptr_access();
return false;
}
const int bytesPerLine = static_cast<int>(stride);
image = QImage(static_cast<const uchar *>(data),
size.width(),
size.height(),
bytesPerLine,
imageFormat)
.copy();
}
buffer->end_data_ptr_access();
If std::numeric_limits<int> is not already available in this file, add #include <limits> to the include list at the top of wrenderhelper.cpp.
Member
|
通过 #1132 实现了 |
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.
Description
In vulkan rendering mode, shm client window textures currently borrow wlroots' device-local VkImage via
qw_texture::from_buffer/makeTexture→updateVKTexture. That VkImage shares the VkDevice with Qt but runs on a separate command stream without cross-stream synchronization (shm buffers have no dma-buf sync fences), so Qt may sample the image before wlroots finishes uploading, conflict with wlroots' layout transitions, or reference a texture wlroots has already reclaimed.This PR makes shm buffers in vulkan mode bypass wlroots' texture path and instead be uploaded by Qt itself, closing the sync gap. dmabuf client textures are unaffected (dma-buf sync provides cross-stream synchronization) and keep their existing
makeTexture→updateVKTexturepath.Changes
waylib/src/server/qtquick/wrenderhelper.h/wrenderhelper.cpp— NewWRenderHelper::makeTextureFromShm: reads shm pixels viaqw_buffer::begin_data_ptr_access, converts DRM format to QImage format viaWTools::toImageFormat, deep-copies the pixels beforeend_data_ptr_access, and sets the image on a Qt-ownedQSGPlainTexture.waylib/src/server/qtquick/wsgtextureprovider.cpp—setBufferintercepts shm buffers under vulkan mode (guarded byENABLE_VULKAN_RENDER+get_shmcheck) before the wlroots texture path; content-update (buffer == qwBuffer()) re-reads shm pixels;shmTexturelifecycle managed with deferred cleanup matching the existingrhiTexturepattern;texture(),setSmooth(),invalidate(),cleanTexture()all handle the shm path.Testing
Multica Issue WM-78
Summary by Sourcery
Handle shm client window textures in Vulkan rendering mode via Qt-owned textures uploaded from shared memory instead of borrowing wlroots VkImages, improving synchronization and lifecycle management while keeping dmabuf paths unchanged.
New Features:
Bug Fixes:
Enhancements: