fix: improve window icon handling and model reset logic#1578
Open
wjyrich wants to merge 1 commit intolinuxdeepin:masterfrom
Open
fix: improve window icon handling and model reset logic#1578wjyrich wants to merge 1 commit intolinuxdeepin:masterfrom
wjyrich wants to merge 1 commit intolinuxdeepin:masterfrom
Conversation
1. Fixed DockCombineModel to respect window split setting for icon display 2. Simplified DockItemModel source model change to use full reset instead of incremental updates 3. Optimized AppItem icon loading to avoid retaining while window is active 4. Added explicit icon update for X11 windows on mapping The changes fix icon display inconsistencies where window-specific icons were incorrectly overridden by application icons when window split is disabled. The model reset simplification fixes edge cases where incremental row updates could cause state corruption. The icon loading optimization improves performance by not retaining loading states for active windows. Log: Fixed window icon display logic and model synchronization Influence: 1. Test dock icon display with multiple windows, verify window-specific icons show correctly 2. Test window split mode toggle, verify icon behavior changes accordingly 3. Test quick window switching, verify no icon loading artifacts 4. Test X11 window mapping, verify icons update immediately 5. Test model source changes, verify no crashes or data corruption 6. Test with high window count, verify performance is not degraded fix: 改进窗口图标处理和模型重置逻辑 1. 修复 DockCombineModel 中图标显示逻辑，遵循窗口分裂设置 2. 简化 DockItemModel 源模型变更时的处理，使用全量重置替代增量更新 3. 优化 AppItem 图标加载，窗口活跃时不再保留加载状态 4. 为 X11 窗口映射时添加显式图标更新 这些修改修复了当窗口分裂功能禁用时，窗口特定图标被应用程序图标错误覆盖的 问题。模型重置的简化修复了增量行更新可能导致的内部状态损坏问题。图标加载 优化通过不在活动窗口中保留加载状态来提高性能。 Log: 修复了窗口图标显示逻辑和模型同步问题 Influence: 1. 测试多窗口 dock 图标显示，验证窗口特定图标正确显示 2. 测试窗口分裂模式切换，验证图标行为相应变更 3. 测试快速窗口切换，验证无图标加载残留 4. 测试 X11 窗口映射，验证图标立即更新 5. 测试模型源变更，验证无崩溃或数据损坏 6. 测试高窗口数量场景，验证性能不下降 PMS: TASK-389031
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRefactors dock task manager models and related window/icon handling to use a full model reset on source changes, respect window-split settings for choosing window vs. app icons, optimize QML icon loading for active windows, and immediately refresh X11 window icons upon mapping. Sequence diagram for X11 window mapping and immediate icon updatesequenceDiagram
participant XServer
participant X11WindowMonitor
participant Window
participant AbstractWindowMonitor
XServer->>X11WindowMonitor: MapNotify(xcb_window)
activate X11WindowMonitor
X11WindowMonitor->>X11WindowMonitor: onWindowMapped(xcb_window)
X11WindowMonitor->>X11WindowMonitor: xcb_change_window_attributes
X11WindowMonitor->>Window: trackWindow(window)
X11WindowMonitor->>Window: updateIcon()
X11WindowMonitor->>AbstractWindowMonitor: windowAdded(window)
deactivate X11WindowMonitor
Class diagram for updated dock task manager models and window/icon handlingclassDiagram
class DockItemModel {
+bool m_isUpdating
+void setSourceModel(QAbstractItemModel *model)
}
class QAbstractProxyModel {
+void setSourceModel(QAbstractItemModel *model)
+int rowCount()
}
DockItemModel --|> QAbstractProxyModel
class DockCombineModel {
+QVariant data(const QModelIndex &index, int role) const
-QHash<int,int> m_roleMaps
+DockCombineModel(QAbstractItemModel *major, QAbstractItemModel *minor, int majorRoles, CombineFunc func, QObject *parent)
}
class RoleCombineModel {
+QVariant data(const QModelIndex &index, int role) const
}
DockCombineModel --|> RoleCombineModel
class TaskManager {
<<enumeration>>
+int IconNameRole
+int WinIconRole
}
DockCombineModel ..> TaskManager : uses roles
class TaskManagerSettings {
+static TaskManagerSettings* instance()
+bool isWindowSplit() const
}
DockCombineModel ..> TaskManagerSettings : queries isWindowSplit
class X11WindowMonitor {
+void onWindowMapped(xcb_window_t xcb_window)
+void trackWindow(Window *window)
}
class AbstractWindowMonitor {
+void windowAdded(QPointer<AbstractWindow> window)
}
X11WindowMonitor --|> AbstractWindowMonitor
class Window {
+void updateIcon()
}
X11WindowMonitor ..> Window : trackWindow
class AppItem {
+bool titleActive
+Image iconImage
}
class Image {
+bool retainWhileLoading
}
AppItem *-- Image : contains iconImage
Flow diagram for DockCombineModel icon selection logicflowchart TD
A(Start) --> B[Request data for IconNameRole]
B --> C[Read winIcon from WinIconRole]
C --> D{isWindowSplit and winIcon not empty}
D -- Yes --> E[Return winIcon]
D -- No --> F[Read icon from IconNameRole]
F --> G{icon empty}
G -- Yes --> H[Set icon to winIcon]
G -- No --> I[Keep app icon]
H --> J[Return icon]
I --> J[Return icon]
E --> K(End)
J --> K(End)
Flow diagram for DockItemModel source model reset logicflowchart TD
A(Start setSourceModel) --> B{Existing sourceModel}
B -- Yes --> C[Disconnect old source model signals]
B -- No --> D[Skip disconnect]
C --> E
D --> E
E[beginResetModel] --> F["QAbstractProxyModel::setSourceModel new model"]
F --> G[endResetModel]
G --> H[Connect rowsInserted, rowsRemoved, rowsMoved, dataChanged signals]
H --> I[Set m_isUpdating to false]
I --> J(End)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review这份代码变更主要涉及任务栏管理器的图标显示逻辑、模型更新机制以及窗口监控优化。以下是对每个变更文件的详细审查和改进建议： 1.
|
![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:
- In DockItemModel::setSourceModel, consider skipping the beginResetModel/endResetModel path when the new source model is identical to the current one to avoid unnecessary full model resets and view churn.
- DockCombineModel::data now depends on TaskManagerSettings::isWindowSplit(), but the model does not emit any dataChanged signals when this setting toggles; consider listening to the setting change and triggering a suitable dataChanged/model reset so views update their icons immediately when the split mode changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In DockItemModel::setSourceModel, consider skipping the beginResetModel/endResetModel path when the new source model is identical to the current one to avoid unnecessary full model resets and view churn.
- DockCombineModel::data now depends on TaskManagerSettings::isWindowSplit(), but the model does not emit any dataChanged signals when this setting toggles; consider listening to the setting change and triggering a suitable dataChanged/model reset so views update their icons immediately when the split mode changes.
## Individual Comments
### Comment 1
<location path="panels/dock/taskmanager/x11windowmonitor.cpp" line_range="189" />
<code_context>
xcb_change_window_attributes(X11->getXcbConnection(), xcb_window, XCB_CW_EVENT_MASK, value_list);
trackWindow(window.get());
+
+ window->updateIcon();
Q_EMIT AbstractWindowMonitor::windowAdded(static_cast<QPointer<AbstractWindow>>(window.get()));
}
</code_context>
<issue_to_address>
**question (bug_risk):** Check for any ordering assumptions between `updateIcon()` and `windowAdded`.
Calling `updateIcon()` before emitting `windowAdded` changes the lifecycle: listeners now see a window whose icon may already be updated, and any signals from `updateIcon()` fire first. Please confirm that no existing `windowAdded` consumers assume they run before icon‑related updates; if they do, consider keeping `updateIcon()` after `windowAdded` or clearly documenting this new ordering.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| xcb_change_window_attributes(X11->getXcbConnection(), xcb_window, XCB_CW_EVENT_MASK, value_list); | ||
| trackWindow(window.get()); | ||
|
|
||
| window->updateIcon(); |
There was a problem hiding this comment.
question (bug_risk): Check for any ordering assumptions between updateIcon() and windowAdded.
Calling updateIcon() before emitting windowAdded changes the lifecycle: listeners now see a window whose icon may already be updated, and any signals from updateIcon() fire first. Please confirm that no existing windowAdded consumers assume they run before icon‑related updates; if they do, consider keeping updateIcon() after windowAdded or clearly documenting this new ordering.
|
TAG Bot New tag: 2.0.39 |
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
2 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.
The changes fix icon display inconsistencies where window-specific icons were incorrectly overridden by application icons when window split is disabled. The model reset simplification fixes edge cases where incremental row updates could cause state corruption. The icon loading optimization improves performance by not retaining loading states for active windows.
Log: Fixed window icon display logic and model synchronization
Influence:
fix: 改进窗口图标处理和模型重置逻辑
这些修改修复了当窗口分裂功能禁用时，窗口特定图标被应用程序图标错误覆盖的
问题。模型重置的简化修复了增量行更新可能导致的内部状态损坏问题。图标加载
优化通过不在活动窗口中保留加载状态来提高性能。
Log: 修复了窗口图标显示逻辑和模型同步问题
Influence:
PMS: TASK-389031
Summary by Sourcery
Improve dock task manager icon handling and model synchronization for better window-specific icon display and stability.
Bug Fixes:
Enhancements: