【Experimental】Enable Vulkan Renderer#1032
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: LFRon 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 |
|
Hi @LFRon. Thanks for your PR. I'm waiting for a linuxdeepin member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Reviewer's GuideIntegrates wlroots' Vulkan renderer with Qt Quick by allowing a Vulkan wlroots backend to coexist with a GL-based Qt RHI via dmabuf/EGL interop, adds Vulkan-specific image/sync handling, and refactors texture/cleanup paths to support mixed backends and stable animations. 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 5 issues, and left some high level feedback:
- The new Vulkan/GL interop path allocates and destroys a command pool, command buffer and semaphore on every call to transitionVkImageToGeneral(), and uses vkQueueWaitIdle(), which can severely stall the GPU; consider caching a per-device command pool and reusing a command buffer plus using fences or a lighter-weight wait to reduce overhead and avoid full-queue stalls.
- The code relies on non-public wlroots EGL symbols (wlr_egl_create_with_drm_fd / wlr_egl_destroy) declared manually; to make this more robust, consider guarding these calls with runtime checks and providing a clear fallback/log path when they are not available, or centralizing the ABI-dependent logic behind a helper abstraction so it is easier to adjust for different wlroots versions.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new Vulkan/GL interop path allocates and destroys a command pool, command buffer and semaphore on every call to transitionVkImageToGeneral(), and uses vkQueueWaitIdle(), which can severely stall the GPU; consider caching a per-device command pool and reusing a command buffer plus using fences or a lighter-weight wait to reduce overhead and avoid full-queue stalls.
- The code relies on non-public wlroots EGL symbols (wlr_egl_create_with_drm_fd / wlr_egl_destroy) declared manually; to make this more robust, consider guarding these calls with runtime checks and providing a clear fallback/log path when they are not available, or centralizing the ABI-dependent logic behind a helper abstraction so it is easier to adjust for different wlroots versions.
## Individual Comments
### Comment 1
<location path="waylib/src/server/qtquick/wrenderhelper.cpp" line_range="1004-1013" />
<code_context>
+ for (int i = 0; i < dmabuf.n_planes; ++i) {
</code_context>
<issue_to_address>
**issue (bug_risk):** Avoid reusing and double-closing the same sync_file FD across planes
DMA_BUF_IOCTL_IMPORT_SYNC_FILE closes the given sync_file FD. In this loop, the same syncFileFd is passed to every plane, then close(syncFileFd) is called again. After the first ioctl the FD is already closed, so later iterations see EBADF and the final close() is a double-close. Please either import once (e.g., plane 0 only) and use that for the buffer, or dup() per plane and drop the final close(), only closing in the error path before any ioctl calls.
</issue_to_address>
### Comment 2
<location path="waylib/src/server/qtquick/wrenderhelper.cpp" line_range="69-72" />
<code_context>
~BufferData() {
resetWindowRenderTarget();
+#ifdef ENABLE_VULKAN_RENDER
+ destroyEglDmabufTexture();
+#endif
}
</code_context>
<issue_to_address>
**issue (bug_risk):** Guard GL resource destruction on a current context in BufferData destructor
`destroyEglDmabufTexture()` calls `glDeleteTextures`, but `BufferData` can be destroyed when no OpenGL context is current (e.g. outside a render pass). `releaseNativeTexture()` avoids this by checking `QOpenGLContext::currentContext()` first.
Please either call `releaseNativeTexture` (or a similar helper) from `~BufferData` or add a `QOpenGLContext::currentContext()` guard inside `destroyEglDmabufTexture()` to prevent GL calls without a current context and the resulting undefined behaviour.
</issue_to_address>
### Comment 3
<location path="waylib/src/server/qtquick/wrenderhelper.cpp" line_range="925-934" />
<code_context>
+ beginInfo.flags = VK_COMMAND_BUFFER_USAGE_ONE_TIME_SUBMIT_BIT;
+ vkBeginCommandBuffer(cb, &beginInfo);
+
+ // NOTE: No layout barrier is recorded here. The previous approach
+ // (transition COLOR_ATTACHMENT_OPTIMAL -> GENERAL + setNativeLayout) caused
+ // InvalidImageLayout validation errors (13.txt) because Qt RHI does not
+ // update usageState.layout after its render pass (finalLayout stays
+ // COLOR_ATTACHMENT_OPTIMAL but the tracked value goes stale), and
+ // preserveColorContents mode expects COLOR_ATTACHMENT_OPTIMAL as
+ // initialLayout. KMS scanout reads the dmabuf's physical memory directly
+ // and does not care about the Vulkan image layout, so leaving the image in
+ // COLOR_ATTACHMENT_OPTIMAL is acceptable. This command buffer is empty and
+ // serves only as a submit载体 to signal the semaphore for sync_file export.
+
+ vkEndCommandBuffer(cb);
+
+ // Create a binary semaphore exportable as a sync_file (VK_KHR_external_
</code_context>
<issue_to_address>
**nitpick:** Align function naming and comments with the fact that no layout transition is performed
Given this function now records no barriers and uses the command buffer only to signal a semaphore, its name and header comment are misleading. Please either rename it to reflect its role as a render completion/sync signalling helper, or update the existing comments to explicitly state that no layout transition or ownership transfer occurs and why this is valid in this pipeline.
</issue_to_address>
### Comment 4
<location path="waylib/src/server/qtquick/wrenderhelper.h" line_range="59" />
<code_context>
static QSGRendererInterface::GraphicsApi probe(QW_NAMESPACE::qw_backend *testBackend, const QList<QSGRendererInterface::GraphicsApi> &apiList);
- static bool makeTexture(QRhi *rhi, QW_NAMESPACE::qw_texture *handle, QSGPlainTexture *texture);
+ struct NativeTextureCleanup {
+ enum class Type {
+ None,
</code_context>
<issue_to_address>
**issue (complexity):** Consider hiding the new low‑level EGL/Vulkan structures and functions behind private or detail helpers while keeping a simple public `makeTexture`/scanout API on WRenderHelper.
You can keep the new functionality while hiding most of the low‑level surface area from `WRenderHelper`’s public API.
### 1. Hide `NativeTextureCleanup` behind a private/detail type
Right now `NativeTextureCleanup` is a public nested struct, so every user of `WRenderHelper` sees EGL/VK details. You can keep it, but make it an implementation detail:
```cpp
// in the header
class WRenderHelper : public QObject
{
Q_OBJECT
public:
// keep the simple API
static bool makeTexture(QRhi *rhi,
QW_NAMESPACE::qw_texture *handle,
QSGPlainTexture *texture);
// optional: a focused, higher-level API if callers really need cleanup
static void releaseImportedTexture(QRhiTexture *texture);
struct TextureEntry {
wlr_buffer *buffer;
QW_NAMESPACE::qw_texture *texture;
QRhiTexture *rhiTexture;
};
// ...
private:
struct NativeTextureCleanup {
enum class Type {
None,
OpenGLTexture,
};
Type type = Type::None;
quint64 texture = 0;
void *eglImage = nullptr;
void *eglDisplay = nullptr;
};
static void releaseNativeTexture(NativeTextureCleanup *cleanup);
};
```
```cpp
// in the cpp
bool WRenderHelper::makeTexture(QRhi *rhi,
qw_texture *handle,
QSGPlainTexture *texture)
{
NativeTextureCleanup cleanup;
return makeTextureImpl(rhi, handle, texture, nullptr, &cleanup);
}
// internal implementation, not declared in the header
static bool makeTextureImpl(QRhi *rhi,
qw_texture *handle,
QSGPlainTexture *texture,
qw_buffer *buffer,
WRenderHelper::NativeTextureCleanup *nativeCleanup)
{
// existing logic...
}
```
This keeps the low‑level `NativeTextureCleanup` visible only inside the implementation, while preserving the behavior. If some special caller really needs to hook into cleanup, you can give them a higher‑level API (`releaseImportedTexture`) instead of the raw struct.
### 2. Split `makeTexture` into a public high‑level API + internal low‑level variant
The expanded `makeTexture` signature can be kept internal. Publicly, keep or reintroduce the simple form:
```cpp
// header
class WRenderHelper : public QObject
{
public:
// high-level API
static bool makeTexture(QRhi *rhi,
QW_NAMESPACE::qw_texture *handle,
QSGPlainTexture *texture);
private:
static bool makeTextureImpl(QRhi *rhi,
QW_NAMESPACE::qw_texture *handle,
QSGPlainTexture *texture,
QW_NAMESPACE::qw_buffer *buffer,
NativeTextureCleanup *nativeCleanup);
};
```
```cpp
// cpp
bool WRenderHelper::makeTexture(QRhi *rhi,
qw_texture *handle,
QSGPlainTexture *texture)
{
return makeTextureImpl(rhi, handle, texture, /*buffer*/ nullptr,
/*nativeCleanup*/ nullptr);
}
```
Internally (or in a private helper), you can still call `makeTextureImpl` with `buffer` and `nativeCleanup` when you need the advanced path, without exposing those extra responsibilities to all users.
### 3. Move `transitionVkImageToGeneral` to a backend/detail helper
`transitionVkImageToGeneral` is very backend‑specific. You can keep the logic but tuck it into a Vulkan‑specific helper and only expose higher‑level operations on `WRenderHelper`:
```cpp
// header
namespace detail {
class WVulkanInterop
{
public:
static void transitionImageToGeneral(QRhi *rhi,
QRhiTexture *texture,
QW_NAMESPACE::qw_buffer *buffer);
};
}
class WRenderHelper : public QObject
{
public:
// high-level API if you need a public entry point
static void prepareExternalScanout(QRhi *rhi,
QRhiTexture *texture,
QW_NAMESPACE::qw_buffer *buffer);
};
```
```cpp
// cpp
void detail::WVulkanInterop::transitionImageToGeneral(QRhi *rhi,
QRhiTexture *texture,
qw_buffer *buffer)
{
// current implementation of transitionVkImageToGeneral
}
void WRenderHelper::prepareExternalScanout(QRhi *rhi,
QRhiTexture *texture,
qw_buffer *buffer)
{
detail::WVulkanInterop::transitionImageToGeneral(rhi, texture, buffer);
}
```
Callers who don’t care about Vulkan specifics can stick to `prepareExternalScanout`, while the heavy Vulkan semantics live in a clearly named backend utility class. This also avoids forcing every `WRenderHelper` user to read Vulkan‑specific comments in the main header.
These small restructurings keep all current behavior, but reduce what the main public header exposes, so most callers don’t have to understand EGL/Vulkan/implicit sync details.
</issue_to_address>
### Comment 5
<location path="waylib/src/server/qtquick/woutputrenderwindow.cpp" line_range="1399" />
<code_context>
} else
#endif
if (rhiSupport->rhiBackend() == QRhi::OpenGLES2) {
- Q_ASSERT(wlr_renderer_is_gles2(m_renderer->handle()));
- auto egl = wlr_gles2_renderer_get_egl(m_renderer->handle());
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting backend-specific initialization into helper functions and wrapping the independent EGL pointer in an RAII helper to simplify initRCWithRhi and resource management.
You can reduce the added complexity by:
1. **Isolating backend‑specific paths into helpers**
2. **Moving `m_independentEgl` lifetime management into an RAII helper**
Below are focused examples that keep all functionality intact.
---
### 1. Extract GLES2 / Vulkan+GL logic into helpers
Right now, `initRCWithRhi()` branches inline between:
- GLES2 wlroots renderer → adopt EGL from wlroots
- Vulkan wlroots renderer + GL RHI → create independent EGL
You can hide this distinction in a helper, so the top‑level reads linearly:
```cpp
bool WOutputRenderWindowPrivate::initRCWithRhi()
{
// ...
#ifdef ENABLE_VULKAN_RENDER
if (rhiSupport->rhiBackend() == QRhi::Vulkan) {
if (!initVulkanRhiFromWlrootsRenderer())
return false;
} else
#endif
if (rhiSupport->rhiBackend() == QRhi::OpenGLES2) {
if (!initGlesRhiFromWlrootsRenderer())
return false;
} else {
return false;
}
// offscreen surface + QRhi creation remains unchanged
}
```
Then move your detailed logic into these two helpers:
```cpp
bool WOutputRenderWindowPrivate::initGlesRhiFromWlrootsRenderer()
{
auto r = m_renderer->handle();
if (wlr_renderer_is_gles2(r)) {
// current "GL wlroots renderer" path
auto egl = wlr_gles2_renderer_get_egl(r);
auto display = wlr_egl_get_display(egl);
auto context = wlr_egl_get_context(egl);
glContext = new QW::OpenGLContext(display, context, rc());
if (!glContext->create())
return false;
q->setGraphicsDevice(QQuickGraphicsDevice::fromOpenGLContext(glContext));
return true;
}
// current "Vulkan wlroots renderer with GL Qt RHI" path
return initIndependentEglForVulkanRenderer();
}
```
```cpp
bool WOutputRenderWindowPrivate::initIndependentEglForVulkanRenderer()
{
int drm_fd = wlr_renderer_get_drm_fd(m_renderer->handle());
if (drm_fd < 0) {
qCWarning(lcWlRenderer) << "Vulkan+GL: wlr_renderer_get_drm_fd failed";
return false;
}
IndependentEglGuard eglGuard(wlr_egl_create_with_drm_fd(drm_fd));
if (!eglGuard.egl) {
qCWarning(lcWlRenderer) << "Vulkan+GL: wlr_egl_create_with_drm_fd failed";
return false;
}
EGLDisplay display = wlr_egl_get_display(eglGuard.egl);
EGLContext context = wlr_egl_get_context(eglGuard.egl);
glContext = new QW::OpenGLContext(display, context, rc());
if (!glContext->create()) {
qCWarning(lcWlRenderer) << "Vulkan+GL: failed to create QW::OpenGLContext";
return false;
}
q->setGraphicsDevice(QQuickGraphicsDevice::fromOpenGLContext(glContext));
m_independentEgl = eglGuard.release(); // hand off ownership
qCInfo(lcWlRenderer) << "Vulkan wlroots renderer with GL Qt RHI: using independent EGL context for dmabuf import";
return true;
}
```
You can similarly extract the Vulkan path if desired:
```cpp
bool WOutputRenderWindowPrivate::initVulkanRhiFromWlrootsRenderer()
{
// move the existing Vulkan logic here (including phdev/dev checks & logging)
}
```
This keeps `initRCWithRhi()` as a short dispatcher, which matches the reviewer’s suggestion and makes future backend additions easier.
---
### 2. Wrap `m_independentEgl` in a small RAII helper
Instead of manually destroying `m_independentEgl` in the destructor, you can encapsulate the lifetime in a tiny helper. This removes stateful cleanup logic from the window class and makes ownership clearer:
```cpp
struct IndependentEglContext
{
wlr_egl *egl = nullptr;
~IndependentEglContext()
{
if (egl)
wlr_egl_destroy(egl);
}
void reset(wlr_egl *newEgl = nullptr)
{
if (egl && egl != newEgl)
wlr_egl_destroy(egl);
egl = newEgl;
}
wlr_egl *release()
{
wlr_egl *tmp = egl;
egl = nullptr;
return tmp;
}
};
```
Member:
```cpp
IndependentEglContext m_independentEgl;
```
Usage in your Vulkan+GL path becomes:
```cpp
wlr_egl *egl = wlr_egl_create_with_drm_fd(drm_fd);
if (!egl) {
qCWarning(lcWlRenderer) << "Vulkan+GL: wlr_egl_create_with_drm_fd failed";
return false;
}
m_independentEgl.reset(egl);
// ... create QW::OpenGLContext from egl ...
```
And the class destructor no longer needs to mention `wlr_egl_destroy` explicitly:
```cpp
~WOutputRenderWindowPrivate() {
qDeleteAll(layers);
// m_independentEgl cleans itself up
}
```
These two small refactorings keep your new feature exactly as‑is but localize backend‑specific complexity and make the lifecycle of the independent EGL context easier to reason about.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| for (int i = 0; i < dmabuf.n_planes; ++i) { | ||
| struct dma_buf_import_sync_file data = {}; | ||
| data.flags = DMA_BUF_SYNC_WRITE; | ||
| data.fd = syncFileFd; | ||
| if (ioctl(dmabuf.fd[i], DMA_BUF_IOCTL_IMPORT_SYNC_FILE, &data) != 0) { | ||
| qCWarning(lcWlRenderHelper) << "Vulkan: DMA_BUF_IOCTL_IMPORT_SYNC_FILE failed on plane" << i | ||
| << "errno=" << errno << "- KMS implicit sync may not wait"; | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
issue (bug_risk): Avoid reusing and double-closing the same sync_file FD across planes
DMA_BUF_IOCTL_IMPORT_SYNC_FILE closes the given sync_file FD. In this loop, the same syncFileFd is passed to every plane, then close(syncFileFd) is called again. After the first ioctl the FD is already closed, so later iterations see EBADF and the final close() is a double-close. Please either import once (e.g., plane 0 only) and use that for the buffer, or dup() per plane and drop the final close(), only closing in the error path before any ioctl calls.
| static QSGRendererInterface::GraphicsApi probe(QW_NAMESPACE::qw_backend *testBackend, const QList<QSGRendererInterface::GraphicsApi> &apiList); | ||
|
|
||
| static bool makeTexture(QRhi *rhi, QW_NAMESPACE::qw_texture *handle, QSGPlainTexture *texture); | ||
| struct NativeTextureCleanup { |
There was a problem hiding this comment.
issue (complexity): Consider hiding the new low‑level EGL/Vulkan structures and functions behind private or detail helpers while keeping a simple public makeTexture/scanout API on WRenderHelper.
You can keep the new functionality while hiding most of the low‑level surface area from WRenderHelper’s public API.
1. Hide NativeTextureCleanup behind a private/detail type
Right now NativeTextureCleanup is a public nested struct, so every user of WRenderHelper sees EGL/VK details. You can keep it, but make it an implementation detail:
// in the header
class WRenderHelper : public QObject
{
Q_OBJECT
public:
// keep the simple API
static bool makeTexture(QRhi *rhi,
QW_NAMESPACE::qw_texture *handle,
QSGPlainTexture *texture);
// optional: a focused, higher-level API if callers really need cleanup
static void releaseImportedTexture(QRhiTexture *texture);
struct TextureEntry {
wlr_buffer *buffer;
QW_NAMESPACE::qw_texture *texture;
QRhiTexture *rhiTexture;
};
// ...
private:
struct NativeTextureCleanup {
enum class Type {
None,
OpenGLTexture,
};
Type type = Type::None;
quint64 texture = 0;
void *eglImage = nullptr;
void *eglDisplay = nullptr;
};
static void releaseNativeTexture(NativeTextureCleanup *cleanup);
};// in the cpp
bool WRenderHelper::makeTexture(QRhi *rhi,
qw_texture *handle,
QSGPlainTexture *texture)
{
NativeTextureCleanup cleanup;
return makeTextureImpl(rhi, handle, texture, nullptr, &cleanup);
}
// internal implementation, not declared in the header
static bool makeTextureImpl(QRhi *rhi,
qw_texture *handle,
QSGPlainTexture *texture,
qw_buffer *buffer,
WRenderHelper::NativeTextureCleanup *nativeCleanup)
{
// existing logic...
}This keeps the low‑level NativeTextureCleanup visible only inside the implementation, while preserving the behavior. If some special caller really needs to hook into cleanup, you can give them a higher‑level API (releaseImportedTexture) instead of the raw struct.
2. Split makeTexture into a public high‑level API + internal low‑level variant
The expanded makeTexture signature can be kept internal. Publicly, keep or reintroduce the simple form:
// header
class WRenderHelper : public QObject
{
public:
// high-level API
static bool makeTexture(QRhi *rhi,
QW_NAMESPACE::qw_texture *handle,
QSGPlainTexture *texture);
private:
static bool makeTextureImpl(QRhi *rhi,
QW_NAMESPACE::qw_texture *handle,
QSGPlainTexture *texture,
QW_NAMESPACE::qw_buffer *buffer,
NativeTextureCleanup *nativeCleanup);
};// cpp
bool WRenderHelper::makeTexture(QRhi *rhi,
qw_texture *handle,
QSGPlainTexture *texture)
{
return makeTextureImpl(rhi, handle, texture, /*buffer*/ nullptr,
/*nativeCleanup*/ nullptr);
}Internally (or in a private helper), you can still call makeTextureImpl with buffer and nativeCleanup when you need the advanced path, without exposing those extra responsibilities to all users.
3. Move transitionVkImageToGeneral to a backend/detail helper
transitionVkImageToGeneral is very backend‑specific. You can keep the logic but tuck it into a Vulkan‑specific helper and only expose higher‑level operations on WRenderHelper:
// header
namespace detail {
class WVulkanInterop
{
public:
static void transitionImageToGeneral(QRhi *rhi,
QRhiTexture *texture,
QW_NAMESPACE::qw_buffer *buffer);
};
}
class WRenderHelper : public QObject
{
public:
// high-level API if you need a public entry point
static void prepareExternalScanout(QRhi *rhi,
QRhiTexture *texture,
QW_NAMESPACE::qw_buffer *buffer);
};// cpp
void detail::WVulkanInterop::transitionImageToGeneral(QRhi *rhi,
QRhiTexture *texture,
qw_buffer *buffer)
{
// current implementation of transitionVkImageToGeneral
}
void WRenderHelper::prepareExternalScanout(QRhi *rhi,
QRhiTexture *texture,
qw_buffer *buffer)
{
detail::WVulkanInterop::transitionImageToGeneral(rhi, texture, buffer);
}Callers who don’t care about Vulkan specifics can stick to prepareExternalScanout, while the heavy Vulkan semantics live in a clearly named backend utility class. This also avoids forcing every WRenderHelper user to read Vulkan‑specific comments in the main header.
These small restructurings keep all current behavior, but reduce what the main public header exposes, so most callers don’t have to understand EGL/Vulkan/implicit sync details.
| q->setGraphicsDevice(gd); | ||
| } else | ||
| #endif | ||
| if (rhiSupport->rhiBackend() == QRhi::OpenGLES2) { |
There was a problem hiding this comment.
issue (complexity): Consider extracting backend-specific initialization into helper functions and wrapping the independent EGL pointer in an RAII helper to simplify initRCWithRhi and resource management.
You can reduce the added complexity by:
- Isolating backend‑specific paths into helpers
- Moving
m_independentEgllifetime management into an RAII helper
Below are focused examples that keep all functionality intact.
1. Extract GLES2 / Vulkan+GL logic into helpers
Right now, initRCWithRhi() branches inline between:
- GLES2 wlroots renderer → adopt EGL from wlroots
- Vulkan wlroots renderer + GL RHI → create independent EGL
You can hide this distinction in a helper, so the top‑level reads linearly:
bool WOutputRenderWindowPrivate::initRCWithRhi()
{
// ...
#ifdef ENABLE_VULKAN_RENDER
if (rhiSupport->rhiBackend() == QRhi::Vulkan) {
if (!initVulkanRhiFromWlrootsRenderer())
return false;
} else
#endif
if (rhiSupport->rhiBackend() == QRhi::OpenGLES2) {
if (!initGlesRhiFromWlrootsRenderer())
return false;
} else {
return false;
}
// offscreen surface + QRhi creation remains unchanged
}Then move your detailed logic into these two helpers:
bool WOutputRenderWindowPrivate::initGlesRhiFromWlrootsRenderer()
{
auto r = m_renderer->handle();
if (wlr_renderer_is_gles2(r)) {
// current "GL wlroots renderer" path
auto egl = wlr_gles2_renderer_get_egl(r);
auto display = wlr_egl_get_display(egl);
auto context = wlr_egl_get_context(egl);
glContext = new QW::OpenGLContext(display, context, rc());
if (!glContext->create())
return false;
q->setGraphicsDevice(QQuickGraphicsDevice::fromOpenGLContext(glContext));
return true;
}
// current "Vulkan wlroots renderer with GL Qt RHI" path
return initIndependentEglForVulkanRenderer();
}bool WOutputRenderWindowPrivate::initIndependentEglForVulkanRenderer()
{
int drm_fd = wlr_renderer_get_drm_fd(m_renderer->handle());
if (drm_fd < 0) {
qCWarning(lcWlRenderer) << "Vulkan+GL: wlr_renderer_get_drm_fd failed";
return false;
}
IndependentEglGuard eglGuard(wlr_egl_create_with_drm_fd(drm_fd));
if (!eglGuard.egl) {
qCWarning(lcWlRenderer) << "Vulkan+GL: wlr_egl_create_with_drm_fd failed";
return false;
}
EGLDisplay display = wlr_egl_get_display(eglGuard.egl);
EGLContext context = wlr_egl_get_context(eglGuard.egl);
glContext = new QW::OpenGLContext(display, context, rc());
if (!glContext->create()) {
qCWarning(lcWlRenderer) << "Vulkan+GL: failed to create QW::OpenGLContext";
return false;
}
q->setGraphicsDevice(QQuickGraphicsDevice::fromOpenGLContext(glContext));
m_independentEgl = eglGuard.release(); // hand off ownership
qCInfo(lcWlRenderer) << "Vulkan wlroots renderer with GL Qt RHI: using independent EGL context for dmabuf import";
return true;
}You can similarly extract the Vulkan path if desired:
bool WOutputRenderWindowPrivate::initVulkanRhiFromWlrootsRenderer()
{
// move the existing Vulkan logic here (including phdev/dev checks & logging)
}This keeps initRCWithRhi() as a short dispatcher, which matches the reviewer’s suggestion and makes future backend additions easier.
2. Wrap m_independentEgl in a small RAII helper
Instead of manually destroying m_independentEgl in the destructor, you can encapsulate the lifetime in a tiny helper. This removes stateful cleanup logic from the window class and makes ownership clearer:
struct IndependentEglContext
{
wlr_egl *egl = nullptr;
~IndependentEglContext()
{
if (egl)
wlr_egl_destroy(egl);
}
void reset(wlr_egl *newEgl = nullptr)
{
if (egl && egl != newEgl)
wlr_egl_destroy(egl);
egl = newEgl;
}
wlr_egl *release()
{
wlr_egl *tmp = egl;
egl = nullptr;
return tmp;
}
};Member:
IndependentEglContext m_independentEgl;Usage in your Vulkan+GL path becomes:
wlr_egl *egl = wlr_egl_create_with_drm_fd(drm_fd);
if (!egl) {
qCWarning(lcWlRenderer) << "Vulkan+GL: wlr_egl_create_with_drm_fd failed";
return false;
}
m_independentEgl.reset(egl);
// ... create QW::OpenGLContext from egl ...And the class destructor no longer needs to mention wlr_egl_destroy explicitly:
~WOutputRenderWindowPrivate() {
qDeleteAll(layers);
// m_independentEgl cleans itself up
}These two small refactorings keep your new feature exactly as‑is but localize backend‑specific complexity and make the lifecycle of the independent EGL context easier to reason about.
|
TAG Bot New tag: 0.8.12 |
LFRon
force-pushed
the
feat-and-refactor/enable-vulkan-renderer
branch
2 times, most recently
from
f33c0ac to
974a92a
Compare
zccrs
left a comment
zccrs
left a comment
There was a problem hiding this comment.
这个改动的核心思想是啥？不至于有这么多地方需要对vulkan、gles2做判断区分呀，原本不就支持vulkan渲染吗，核心链路应该是通的（最近没验证过），不应该出现这么多改动。
| #if WLR_HAVE_VULKAN_RENDERER | ||
| // Access the wlroots-adopted Vulkan device handles. Used by compositors | ||
| // (e.g. waylib) that adopt the wlroots VkDevice into Qt RHI. | ||
| QW_FUNC_MEMBER(vk_renderer, get_instance, VkInstance) |
There was a problem hiding this comment.
不应该加到这个class里，要有应该是个新的 vk_renderer 类型
| QW_FUNC_STATIC(texture, from_dmabuf, qw_texture *, wlr_renderer *renderer, wlr_dmabuf_attributes *attribs) | ||
| QW_FUNC_STATIC(texture, from_buffer, qw_texture *, wlr_renderer *renderer, wlr_buffer *buffer) | ||
| #if WLR_HAVE_VULKAN_RENDERER | ||
| QW_FUNC_MEMBER(vk_texture, get_image_attribs, void, wlr_vk_image_attribs *attribs) |
| m_windowAnimation = m_engine->createNewAnimation(this, container(), direction); | ||
| m_windowAnimation->setProperty("enableBlur", m_blur); | ||
| if (m_type == Type::SplashScreen && isVulkanRendererBackend()) | ||
| m_windowAnimation->setProperty("liveSource", false); |
There was a problem hiding this comment.
这是为啥？为什么vulkan backend时liveSource需要是false？
| }); | ||
| // QQuickStyle::setStyle("Material"); | ||
|
|
||
| QGuiApplication::setAttribute(Qt::AA_UseOpenGLES); |
There was a problem hiding this comment.
这行应该可以直接删掉，WRenderHelper::setupRendererBackend(); 会找到合适的backend
There was a problem hiding this comment.
这里的设置不要紧，只是告诉Qt如果用opengl，应该用gles版本，不会影响treeland和QtQuick的渲染引擎。
| auto dev = wlr_vk_renderer_get_device(m_renderer->handle()); | ||
| auto queue_family = wlr_vk_renderer_get_queue_family(m_renderer->handle()); | ||
| if (Q_UNLIKELY(!phdev || !dev)) { | ||
| qCWarning(lcWlRenderer) << "Vulkan: wlroots renderer exposed null VkPhysicalDevice/VkDevice, cannot adopt into Qt RHI"; |
| bool ok = this->glContext->create(); | ||
| if (!ok) | ||
| return false; | ||
| if (wlr_renderer_is_gles2(m_renderer->handle())) { |
There was a problem hiding this comment.
为什么把这里的断言改成了if判断？不应该出现 rhiSupport->rhiBackend() == QRhi::OpenGLES2 时，wlroots的backend不是gles2的情况，如果存在，那就是其他地方的代码出现了逻辑性错误。
| return false; | ||
|
|
||
| q->setGraphicsDevice(QQuickGraphicsDevice::fromOpenGLContext(this->glContext)); | ||
| } else { |
There was a problem hiding this comment.
不要单独处理这种else的情况，项目的设计要求就是wlroots的render backend和QRhi的backend必须一致，不应该出现不一致的情况，不支持这种用法。没看出来做这种情况的兼容有啥意义。
| } | ||
|
|
||
| #ifdef ENABLE_VULKAN_RENDER | ||
| if (isVulkanRenderer()) { |
我让GPT跟GLM都试过, Qt的RHI Vulkan的生命周期管理等一系列行为与wlroots的Vulkan有区别, 要改动的幅度实在是太大了必须要人力介入改, 所以我才刻意避开了, 并把这个PR标注为了实验性仅供参考 |
LFRon
force-pushed
the
feat-and-refactor/enable-vulkan-renderer
branch
2 times, most recently
from
bd7354e to
a3652d2
Compare
直接WLR_RENDERER=vulkan就是用vulkan渲染了，无需做改动，你那边有啥问题呀 |
|
LFRon
force-pushed
the
feat-and-refactor/enable-vulkan-renderer
branch
from
a3652d2 to
0b5e38a
Compare
|
TAG Bot New tag: 0.8.13 |
LFRon
force-pushed
the
feat-and-refactor/enable-vulkan-renderer
branch
from
0b5e38a to
e91ea02
Compare
|
这个我说的是现状, 因为QML RHI走Vulkan需要自己手动管理生命周期显式同步等太麻烦了, 而且wlroots用Vulkan合成/渲染并不影响对应的应用该走OpenGL走OpenGL, wlroots的Vk渲染只是走最后一步, 这个类似于安卓HWUI Vulkan |
LFRon
force-pushed
the
feat-and-refactor/enable-vulkan-renderer
branch
from
e91ea02 to
684b9c6
Compare
这里的设置只对wlroots和Treeland生效，不会影响其他程序，wayland client程序该是opengl还是opengl。 |
对, 我这么试着做了, 然后出了一大堆问题, 包括但不局限于光标丢失/圆角丢失/只有图标, 我把新的半成品代码放上来了 |
LFRon
force-pushed
the
feat-and-refactor/enable-vulkan-renderer
branch
from
f4e5ab5 to
809d783
Compare
|
我的对话记录: |
LFRon
force-pushed
the
feat-and-refactor/enable-vulkan-renderer
branch
from
809d783 to
fa23670
Compare
对于上屏的理解是对的，上屏用的buffer无论在什么渲染接口下，都是由wlroots提供的。treeland的工作是把这个buffer作为render target给QtQuick使用，QtQuick会把那些qml元素都合成到这个buffer上，合成完成后由Treeland通过wlroots的接口上屏。 vulkan模式下我这里的已知问题只有一个：没有支持高斯模糊效果。其他的应该都ok，包括窗口阴影、圆角之类的。 你遇到的问题可以截个图看看。 |
就是这个分支的代码, 有严重的闪屏等一系列问题 顺带一句,treeland本体跑在OpenGL(GLES2)下, 切QML RHI到vulkan一样会导致dde-shell起不来, 这个好像是Qt自己的问题, 我之前翻到过有人反馈Wayland下vulkan后端的QML速度巨慢 |
|
- add Vulkan RHI output-layer compositor plumbing for treeland canvas - experiment with client dmabuf import paths for Qt Quick Vulkan RHI - add diagnostics for output buffers, surface textures, alpha, and modifiers - track surface opaque regions through texture provider/render helper paths - prefer texture-compatible modifiers for intermediate Vulkan output buffers - document current investigation state in chatgpt-5.5.md This is still a work in progress. The remaining blocker is stabilizing client dmabuf import/sampling semantics inside the Qt Quick Vulkan scene graph.
Drop the experimental Vulkan direct-surface rendering path that tried to bypass the Qt Quick scene graph for client surfaces. The removed path included: - WVulkanSurfaceRenderNode and its custom shaders - per-surface vulkanDirectSurfaceAllowed QML/property plumbing - output-layer direct client surface collection/composition logic - the unsafe wlroots native VkImage wrapper fallback for dmabuf textures Keep client contents represented through the normal Qt Quick SurfaceItemContent path, and let the Vulkan RHI path use QRhiTexture import plus synchronized fallbacks instead of injecting separate client-surface render nodes. This intentionally keeps the Vulkan compositor architecture centered around a single Qt Quick scene render target. The remaining Vulkan runtime issue is in the output intermediate dmabuf image usage contract: the Qt render target is later sampled by the wlroots Vulkan output-layer compositor, so its imported VkImage usage needs to include the real roles used by both sides.
LFRon
force-pushed
the
feat-and-refactor/enable-vulkan-renderer
branch
from
db2626d to
e258631
Compare
|
@zccrs 大佬, 我现在把这个分支的AI写的代码已经改成了几乎贴近treeland实现方式的方案, 但是有大量问题比如Vulkan显式同步等都没解决, 我把我能做到更改的放这了 |
|
我现在有考虑一个方案: client dmabuf -> wlroots Vulkan renderer 合成 |
3 participants
这个PR让treeland内调用的Qt RHI Backend切换到Vulkan, 然后通过wlroots Vulkan后端进行上屏