fix: avoid use DQuickDrag under wayland session #1353
Merged
Conversation
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
DQuickDrag 目前不支持 wayland 会话,直接使用会导致一些问题.暂时在 wayland 会话下规避使用. PMS: BUG-339309 Log:
deepin pr auto review我来对这段代码进行审查分析：
第一处修改（dock/package/main.qml）： function onWidthChanged() {
if (dock.useColumnLayout) {
Panel.notifyDockPositionChanged(dock.width, 0)
}
}
function onHeightChanged() {
if (!dock.useColumnLayout) {
Panel.notifyDockPositionChanged(0, dock.height)
}
}建议改进：
第二处修改（ActionLegacyTrayPluginDelegate.qml）： DQuickDrag.active: Drag.active && Qt.platform.pluginName === "xcb"这个修改很好，增加了平台判断，避免了在非 X11 平台上的潜在问题。 if (Qt.platform.pluginName !== "xcb") {
root.grabToImage(function(result) {
root.Drag.imageSource = result.url;
})
}建议改进：
这些修改总体上是合理的，主要解决了跨平台的兼容性问题，但在性能和错误处理方面还有改进空间。 |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR gates DQuickDrag usage to X11 (xcb) sessions and provides fallback drag imagery for Wayland, while also updating dock position notifications to include explicit dimensions depending on orientation. 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 there - I've reviewed your changes - here's some feedback:
- The
grabToImagelogic for non-xcbplatforms is duplicated in multiple places; consider extracting it into a small helper function (e.g.,updateDragImageIfNeeded()) to keep behavior consistent and easier to maintain. - Calling
grabToImageon everyonWidthChangedfor non-xcbplatforms may be expensive during continuous resize; consider guarding it with additional conditions (e.g., only when dragging or visible) or throttling to reduce potential performance impact. - The platform check
Qt.platform.pluginName === "xcb"is used in several places; consider centralizing this into a reusable property or helper (e.g.,isXcbPlatform) to avoid scattering string literals and to simplify future platform handling changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `grabToImage` logic for non-`xcb` platforms is duplicated in multiple places; consider extracting it into a small helper function (e.g., `updateDragImageIfNeeded()`) to keep behavior consistent and easier to maintain.
- Calling `grabToImage` on every `onWidthChanged` for non-`xcb` platforms may be expensive during continuous resize; consider guarding it with additional conditions (e.g., only when dragging or visible) or throttling to reduce potential performance impact.
- The platform check `Qt.platform.pluginName === "xcb"` is used in several places; consider centralizing this into a reusable property or helper (e.g., `isXcbPlatform`) to avoid scattering string literals and to simplify future platform handling changes.
## Individual Comments
### Comment 1
<location> `panels/dock/tray/package/ActionLegacyTrayPluginDelegate.qml:175-179` </location>
<code_context>
}
+ if (Qt.platform.pluginName !== "xcb") {
+ root.grabToImage(function(result) {
+ root.Drag.imageSource = result.url;
+ })
+ }
</code_context>
<issue_to_address>
**suggestion:** Consider guarding against grabToImage failures before using result.url.
grabToImage can fail or return an empty/invalid result. Using result.url unconditionally risks subtle issues (e.g., broken or stale drag thumbnail, platform-specific quirks). Please validate result and result.url (or result.image) before assigning to root.Drag.imageSource, and consider keeping the previous image or using a fallback when the grab fails.
```suggestion
if (Qt.platform.pluginName !== "xcb") {
root.grabToImage(function(result) {
// Guard against grabToImage failures or empty results before updating the drag image.
var hasValidImage = result && result.image && result.image.width > 0 && result.image.height > 0
var hasValidUrl = result && result.url && result.url !== ""
if (hasValidImage) {
root.Drag.imageSource = result.image
} else if (hasValidUrl) {
root.Drag.imageSource = result.url
}
// If neither image nor URL is valid, keep the existing imageSource as a fallback.
})
}
```
</issue_to_address>
### Comment 2
<location> `panels/dock/tray/package/ActionLegacyTrayPluginDelegate.qml:190-196` </location>
<code_context>
Panel.contextDragging = true
}
+ onWidthChanged: {
+ if (Qt.platform.pluginName !== "xcb") {
+ root.grabToImage(function(result) {
+ root.Drag.imageSource = result.url;
+ })
</code_context>
<issue_to_address>
**suggestion (performance):** Re-evaluate triggering grabToImage on every width change for non-xcb platforms.
This handler will invoke `grabToImage` on every width change for non-xcb platforms, which can be expensive (e.g., during animations, layout changes, or DPI/scale updates) and may cause jank if multiple items update. Consider only doing this while a drag is active, or debounce/coalesce width changes to avoid repeated grabs during rapid size updates.
```suggestion
onWidthChanged: {
// Avoid expensive grabToImage calls on every width change:
// only update the drag image while a drag is actually active.
if (Qt.platform.pluginName !== "xcb" && Drag.active) {
root.grabToImage(function(result) {
root.Drag.imageSource = result.url;
})
}
}
```
</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
+190
to
+196
| onWidthChanged: { | ||
| if (Qt.platform.pluginName !== "xcb") { | ||
| root.grabToImage(function(result) { | ||
| root.Drag.imageSource = result.url; | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
suggestion (performance): Re-evaluate triggering grabToImage on every width change for non-xcb platforms.
This handler will invoke grabToImage on every width change for non-xcb platforms, which can be expensive (e.g., during animations, layout changes, or DPI/scale updates) and may cause jank if multiple items update. Consider only doing this while a drag is active, or debounce/coalesce width changes to avoid repeated grabs during rapid size updates.
Suggested change
| onWidthChanged: { | |
| if (Qt.platform.pluginName !== "xcb") { | |
| root.grabToImage(function(result) { | |
| root.Drag.imageSource = result.url; | |
| }) | |
| } | |
| } | |
| onWidthChanged: { | |
| // Avoid expensive grabToImage calls on every width change: | |
| // only update the drag image while a drag is actually active. | |
| if (Qt.platform.pluginName !== "xcb" && Drag.active) { | |
| root.grabToImage(function(result) { | |
| root.Drag.imageSource = result.url; | |
| }) | |
| } | |
| } |
wjyrich
approved these changes
Nov 24, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: BLumia, wjyrich 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 |
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.
DQuickDrag 目前不支持 wayland 会话,直接使用会导致一些问题.暂时在
wayland 会话下规避使用.
Summary by Sourcery
Fix dock tray drag behavior under non-X11 sessions and adjust dock position notification to accept explicit dimensions.
Bug Fixes: