fix: buffer assignment error with clang build#739
Open
zorowk wants to merge 1 commit intolinuxdeepin:masterfrom
Open
fix: buffer assignment error with clang build#739zorowk wants to merge 1 commit intolinuxdeepin:masterfrom
zorowk wants to merge 1 commit intolinuxdeepin:masterfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: zorowk 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 guide (collapsed on small PRs)Reviewer's GuideAdjusts the WBufferItem QML-exposed buffer property to use QObject* instead of qw_buffer* and performs casting/validation inside C++, fixing Clang/QML type assignment issues and removing the now-unneeded qw_buffer* meta-type registration. Sequence diagram for buffer assignment from QML to WBufferItemsequenceDiagram
actor User
participant QML as QML_Engine
participant BufferItem as WBufferItem
participant BufferObj as qw_buffer
participant TextureProvider as WBufferItemTextureProvider
User->>QML: Open_PrelaunchSplash
QML->>BufferObj: Create_qw_buffer
QML->>BufferItem: set_property_buffer(BufferObj_as_QObject)
BufferItem->>BufferItem: setBuffer(QObject_buffer)
BufferItem->>BufferItem: ibuffer = qobject_cast_qw_buffer(QObject_buffer)
alt ibuffer_is_null
BufferItem-->>BufferItem: return_without_changes
else ibuffer_is_valid
BufferItem->>BufferItem: h = ibuffer.handle()
alt invalid_handle_or_size
BufferItem-->>BufferItem: log_warning_and_return
else valid_handle
BufferItem->>BufferObj: ibuffer.lock()
BufferItem->>BufferItem: d.buffer.reset(ibuffer)
BufferItem->>TextureProvider: setBuffer(d.buffer.get())
end
end
Updated class diagram for WBufferItem buffer propertyclassDiagram
class WBufferItem {
+WBufferItem(parent QQuickItem)
+~WBufferItem()
+QObject buffer() const
+void setBuffer(buffer QObject)
+bool isTextureProvider() const
+QSGTextureProvider textureProvider() const
}
class qw_buffer {
+void lock()
+Handle handle() const
}
class WBufferItemPrivate {
+std_unique_ptr~qw_buffer~ buffer
+WBufferItemTextureProvider* textureProvider
}
class WBufferItemTextureProvider {
+void setBuffer(buffer qw_buffer)
}
WBufferItem --> WBufferItemPrivate : uses
WBufferItemPrivate --> qw_buffer : owns
WBufferItemPrivate --> WBufferItemTextureProvider : uses
WBufferItemTextureProvider --> qw_buffer : uses
class QQuickItem
WBufferItem --|> QQuickItem
class QObject
WBufferItem --> QObject : exposes_buffer_as_QObject
qw_buffer --|> QObject
class Handle {
+int width
+int height
}
qw_buffer --> Handle : returns_handle
File-Level Changes
Assessment against linked issues
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, and left some high level feedback:
- When casting the
QObject*back toqw_buffer*insetBuffer, consider logging or otherwise handling the case whereqobject_castfails so that accidental assignment of an incompatible QObject does not silently clear the current buffer. - Now that the QML property is typed as
QObject*, it may be worth documenting or enforcing (e.g., via a runtime check) that onlyqw_bufferinstances are expected, to avoid misuse from QML or future C++ callers.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- When casting the `QObject*` back to `qw_buffer*` in `setBuffer`, consider logging or otherwise handling the case where `qobject_cast` fails so that accidental assignment of an incompatible QObject does not silently clear the current buffer.
- Now that the QML property is typed as `QObject*`, it may be worth documenting or enforcing (e.g., via a runtime check) that only `qw_buffer` instances are expected, to avoid misuse from QML or future C++ callers.
## Individual Comments
### Comment 1
<location> `waylib/src/server/qtquick/wbufferitem.cpp:102-111` </location>
<code_context>
W_D(WBufferItem);
- if (d->buffer.get() == buffer)
+ QW_NAMESPACE::qw_buffer *ibuffer = qobject_cast<QW_NAMESPACE::qw_buffer*>(buffer);
+ if (d->buffer.get() == ibuffer)
return;
</code_context>
<issue_to_address>
**issue (bug_risk):** Passing a non-`qw_buffer` QObject clears the current buffer silently, which may be unintended.
With the `QObject*` API, passing a non-`qw_buffer` object makes `ibuffer` null, so `d->buffer.reset(ibuffer)` drops the existing buffer while effectively ignoring the new argument. This is surprising for callers and hard to debug. Consider either returning early when `buffer` is non-null but `ibuffer` is null (keeping the existing buffer) and/or logging a warning in that case so the behavior is explicit and valid buffers aren’t discarded due to a type mismatch.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
wineee
previously approved these changes
Feb 10, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a clang/QML runtime type assignment issue that prevented PrelaunchSplash from rendering correctly by changing how qw_buffer is exposed to QML through Waylib’s WBufferItem.
Changes:
- Expose
WBufferItem::bufferto QML asQObject*instead ofQW_NAMESPACE::qw_buffer*, and cast back internally in C++. - Update
WBufferItemgetter/setter implementations to work withQObject*while still operating onqw_buffer. - Remove the explicit
qRegisterMetaType<QW_NAMESPACE::qw_buffer*>("qw_buffer*")call from application startup.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
waylib/src/server/qtquick/wbufferitem.h |
Changes the QML-facing buffer property type to QObject* to avoid QML assignment failures. |
waylib/src/server/qtquick/wbufferitem.cpp |
Implements the QObject* setter/getter and casts to qw_buffer* internally for rendering/locking. |
src/main.cpp |
Removes explicit qw_buffer* metatype registration during startup. |
Clang rejects assigning qw_buffer to a Q_PROPERTY of type qw_buffer* in QML, causing runtime errors like: Unable to assign qw_buffer to qw_buffer* Change the buffer property to QObject* and cast back to qw_buffer in C++, allowing QML to pass the buffer through bindings correctly. This fixes incorrect rendering when opening PrelaunchSplash. Issue: Fixes linuxdeepin#737 Log: fixes incorrect icon rendering when opening PrelaunchSplash. Influence: Launching the application for the first time
zccrs
requested changes
Feb 10, 2026
| QGuiApplication::setQuitOnLastWindowClosed(false); | ||
|
|
||
| QGuiApplication app(argc, argv); | ||
| qRegisterMetaType<QW_NAMESPACE::qw_buffer*>("qw_buffer*"); |
Contributor
Author
There was a problem hiding this comment.
我实验了一下，不行 qw_buffer和qw_buffer*都试过不行
modified qwlroots/src/types/qwbuffer.h
@@ -57,3 +57,4 @@ public:
};
QW_END_NAMESPACE
+Q_DECLARE_METATYPE(QW_NAMESPACE::qw_buffer) **qw_buffer和qw_buffer*都试过不行**
modified src/main.cpp
@@ -29,7 +29,7 @@ int main(int argc, char *argv[])
QGuiApplication::setQuitOnLastWindowClosed(false);
QGuiApplication app(argc, argv);
- qRegisterMetaType<QW_NAMESPACE::qw_buffer*>("qw_buffer*");
+ //qRegisterMetaType<QW_NAMESPACE::qw_buffer*>("qw_buffer*");
app.setOrganizationName("deepin");
app.setApplicationName("treeland");
modified src/modules/prelaunch-splash/prelaunchsplash.h
@@ -22,7 +22,7 @@ QW_BEGIN_NAMESPACE
class qw_display;
class qw_buffer;
QW_END_NAMESPACE
-Q_DECLARE_OPAQUE_POINTER(QW_NAMESPACE::qw_buffer*)
+//Q_DECLARE_OPAQUE_POINTER(QW_NAMESPACE::qw_buffer*) **这里不注释会和Q_DECLARE_METATYPE产生冲突**
class PrelaunchSplashPrivate;
struct wl_global;
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
4 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.
Clang rejects assigning qw_buffer to a Q_PROPERTY of type qw_buffer* in QML, causing runtime errors like:
Unable to assign qw_buffer to qw_buffer*
Change the buffer property to QObject* and cast back to qw_buffer in C++, allowing QML to pass the buffer through bindings correctly.
This fixes incorrect rendering when opening PrelaunchSplash.
Issue: Fixes #737
Log: fixes incorrect icon rendering when opening PrelaunchSplash.
Influence: Launching the application for the first time
Summary by Sourcery
Adjust buffer property exposure for WBufferItem to resolve QML buffer assignment issues with clang builds.
Bug Fixes:
Enhancements: