chore: update window icon geometry via new taskmanager interface #1282
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
目前,更新窗口所在任务栏区域的调用仍在依赖 ItemModel::instance() 模型, 此提交转而将相关实现补充到新的 taskmanager interface 中,并移除对即将 废弃的接口的调用. Log:
deepin pr auto review这段代码主要涉及窗口图标几何形状更新的功能改进，我将从以下几个方面进行分析：
改进建议：
void X11WindowMonitor::requestUpdateWindowIconGeometry(const QModelIndex &index, const QRect &geometry, QObject *delegate) const
{
if (!index.isValid() || geometry.isEmpty() || !delegate) {
return;
}
xcb_window_t winId = index.data(TaskManager::WinIdRole).toUInt();
auto pos = qobject_cast<QWindow *>(delegate)->position() + geometry.topLeft();
X11->setWindowIconGemeotry(winId, QRect(pos.x(), pos.y(), geometry.width(), geometry.height()));
}
void DockGroupModel::requestUpdateWindowIconGeometry(const QModelIndex &index, const QRect &geometry, QObject *delegate) const
{
if (!index.isValid()) {
return;
}
auto interface = dynamic_cast<AbstractTaskManagerInterface *>(sourceModel());
if (nullptr == interface)
return;
auto indexRowCount = RoleGroupModel::rowCount(index);
for (int i = 0; i < indexRowCount; i++) {
auto cIndex = RoleGroupModel::index(i, 0, index);
callInterfaceMethod(cIndex, &AbstractTaskManagerInterface::requestUpdateWindowIconGeometry, geometry, delegate);
}
}
onTriggered: {
var pos = icon.mapToItem(null, 0, 0)
var rect = Qt.rect(pos.x, pos.y, icon.width, icon.height)
if (rect.width > 0 && rect.height > 0) {
taskmanager.Applet.requestUpdateWindowIconGeometry(root.modelIndex, rect, Panel.rootObject)
}
}
void X11WindowMonitor::requestUpdateWindowIconGeometry(const QModelIndex &index, const QRect &geometry, QObject *delegate) const
{
if (!index.isValid() || geometry.isEmpty() || !delegate) {
qWarning() << "Invalid parameters in requestUpdateWindowIconGeometry";
return;
}
xcb_window_t winId = index.data(TaskManager::WinIdRole).toUInt();
auto pos = qobject_cast<QWindow *>(delegate)->position() + geometry.topLeft();
X11->setWindowIconGemeotry(winId, QRect(pos.x(), pos.y(), geometry.width(), geometry.height()));
qDebug() << "Updated window icon geometry for window:" << winId
<< "Geometry:" << QRect(pos.x(), pos.y(), geometry.width(), geometry.height());
}这些改进可以提高代码的健壮性、可维护性和可调试性，同时保持原有功能的完整性。 |
Reviewer's GuideMigrates window icon geometry updates from the deprecated ItemModel interface to the new TaskManager interface by renaming methods, removing legacy calls, and implementing the propagation logic across models and monitors. Class diagram for updated window icon geometry propagationclassDiagram
class AbstractTaskManagerInterface {
+requestUpdateWindowIconGeometry(index: QModelIndex, geometry: QRect, delegate: QObject*)
}
class AbstractWindowMonitor {
+requestUpdateWindowIconGeometry(index: QModelIndex, geometry: QRect, delegate: QObject*)
}
class X11WindowMonitor {
+requestUpdateWindowIconGeometry(index: QModelIndex, geometry: QRect, delegate: QObject*)
}
class DockGlobalElementModel {
+requestUpdateWindowIconGeometry(index: QModelIndex, geometry: QRect, delegate: QObject*)
}
class DockGroupModel {
+requestUpdateWindowIconGeometry(index: QModelIndex, geometry: QRect, delegate: QObject*)
}
class TaskManager {
+requestUpdateWindowIconGeometry(index: QModelIndex, geometry: QRect, delegate: QObject*)
}
AbstractTaskManagerInterface <|-- AbstractWindowMonitor
AbstractWindowMonitor <|-- X11WindowMonitor
AbstractTaskManagerInterface <|-- DockGlobalElementModel
AbstractTaskManagerInterface <|-- DockGroupModel
AbstractTaskManagerInterface <|-- TaskManager
TaskManager --> DockGlobalElementModel: uses
DockGlobalElementModel --> X11WindowMonitor: uses
DockGroupModel --> AbstractTaskManagerInterface: uses
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 and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `panels/dock/taskmanager/x11windowmonitor.cpp:157` </location>
<code_context>
+void X11WindowMonitor::requestUpdateWindowIconGeometry(const QModelIndex &index, const QRect &geometry, QObject *delegate) const
+{
+ xcb_window_t winId = index.data(TaskManager::WinIdRole).toUInt();
+ auto pos = qobject_cast<QWindow *>(delegate)->position() + geometry.topLeft();
+ X11->setWindowIconGemeotry(winId, QRect(pos.x(), pos.y(), geometry.width(), geometry.height()));
+}
</code_context>
<issue_to_address>
**issue (bug_risk):** Potential null pointer dereference with delegate casting.
Add a null check or type verification for delegate before casting to QWindow to prevent possible crashes.
</issue_to_address>
### Comment 2
<location> `panels/dock/taskmanager/x11windowmonitor.cpp:158` </location>
<code_context>
+{
+ xcb_window_t winId = index.data(TaskManager::WinIdRole).toUInt();
+ auto pos = qobject_cast<QWindow *>(delegate)->position() + geometry.topLeft();
+ X11->setWindowIconGemeotry(winId, QRect(pos.x(), pos.y(), geometry.width(), geometry.height()));
+}
+
</code_context>
<issue_to_address>
**issue (typo):** Typo in method name 'setWindowIconGemeotry'.
Rename the method to 'setWindowIconGeometry' for clarity and consistency.
Suggested implementation:
```cpp
X11->setWindowIconGeometry(winId, QRect(pos.x(), pos.y(), geometry.width(), geometry.height()));
```
You will also need to:
1. Rename the method definition from `setWindowIconGemeotry` to `setWindowIconGeometry` in the X11 class implementation file.
2. Update the declaration in the corresponding header file for the X11 class.
3. Search for any other usages of `setWindowIconGemeotry` throughout the codebase and update them to `setWindowIconGeometry`.
</issue_to_address>
### Comment 3
<location> `panels/dock/taskmanager/dockgroupmodel.cpp:137-139` </location>
<code_context>
- Q_UNUSED(index)
- Q_UNUSED(geometry)
- Q_UNUSED(delegate)
+ auto interface = dynamic_cast<AbstractTaskManagerInterface *>(sourceModel());
+ if (nullptr == interface)
+ return;
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Silent return on failed interface cast may hide errors.
Consider adding logging or error handling when the cast fails to improve debuggability.
```suggestion
auto interface = dynamic_cast<AbstractTaskManagerInterface *>(sourceModel());
if (nullptr == interface) {
qWarning() << "DockGroupModel::requestUpdateWindowIconGeometry: sourceModel is not an AbstractTaskManagerInterface";
return;
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| void X11WindowMonitor::requestUpdateWindowIconGeometry(const QModelIndex &index, const QRect &geometry, QObject *delegate) const | ||
| { | ||
| xcb_window_t winId = index.data(TaskManager::WinIdRole).toUInt(); | ||
| auto pos = qobject_cast<QWindow *>(delegate)->position() + geometry.topLeft(); |
There was a problem hiding this comment.
issue (bug_risk): Potential null pointer dereference with delegate casting.
Add a null check or type verification for delegate before casting to QWindow to prevent possible crashes.
Comment on lines
+137
to
+139
| auto interface = dynamic_cast<AbstractTaskManagerInterface *>(sourceModel()); | ||
| if (nullptr == interface) | ||
| return; |
There was a problem hiding this comment.
suggestion (bug_risk): Silent return on failed interface cast may hide errors.
Consider adding logging or error handling when the cast fails to improve debuggability.
Suggested change
| auto interface = dynamic_cast<AbstractTaskManagerInterface *>(sourceModel()); | |
| if (nullptr == interface) | |
| return; | |
| auto interface = dynamic_cast<AbstractTaskManagerInterface *>(sourceModel()); | |
| if (nullptr == interface) { | |
| qWarning() << "DockGroupModel::requestUpdateWindowIconGeometry: sourceModel is not an AbstractTaskManagerInterface"; | |
| return; | |
| } |
tsic404
approved these changes
Sep 28, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: BLumia, tsic404 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.
目前,更新窗口所在任务栏区域的调用仍在依赖 ItemModel::instance() 模型, 此提交转而将相关实现补充到新的 taskmanager interface 中,并移除对即将废弃的接口的调用.
Summary by Sourcery
Migrate window icon geometry updates to the new AbstractTaskManagerInterface, rename related methods to requestUpdateWindowIconGeometry, implement them in models and monitors, and remove the deprecated ItemModel-based API.
Enhancements: