Feat: improve PageMultiInfo CPU header info display and device genera…#653
Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:develop/eaglefrom May 7, 2026
Conversation
…tion - Update PageMultiInfo to use scroll area container layout for CPU header detail widgets - Add/adjust accessor usage for header info in CPU page - Refactor scroll area content resizing and visibility behavior - Update DeviceGenerator logic and declarations (internal feature updates) - Keep UI behavior consistent with latest header info widget layout Log: add feature for cpu info show. Task: https://pms.uniontech.com/task-view-387697.html
Reviewer's GuideImplements a scrollable CPU header info section in PageMultiInfo and wires it to new CPU header metadata generated in DeviceGenerator, including layout, visibility, and data calculation logic. Sequence diagram for CPU header info generation and displaysequenceDiagram
actor User
participant DeviceGenerator
participant DeviceManager
participant PageMultiInfo
participant HeaderInfoTableWidget
User->>DeviceGenerator: generatorCpuDevice()
DeviceGenerator->>DeviceGenerator: iterate lsCpu
DeviceGenerator->>DeviceManager: addCpuDevice(device)
DeviceGenerator->>DeviceGenerator: calAndSetCpuHeaderInfo(firstProcessorInfo, coreNum, logicalNum)
DeviceGenerator->>DeviceManager: setCpuHeaderInfo(cpuHeaderInfo)
User->>PageMultiInfo: updateInfo(lst)
PageMultiInfo->>PageMultiInfo: dynamic_cast to DeviceCpu
alt lst contains DeviceCpu
PageMultiInfo->>DeviceManager: getCpuHeaderInfo(headerInfo)
PageMultiInfo->>PageMultiInfo: clearLayout(mp_HeaderInfoWidgetLay)
loop for each headerInfoGroup
PageMultiInfo->>HeaderInfoTableWidget: new HeaderInfoTableWidget(mp_HeaderInfoWidget)
PageMultiInfo->>HeaderInfoTableWidget: updateData(group)
PageMultiInfo->>PageMultiInfo: mp_HeaderInfoWidgetLay->addWidget(widget)
end
PageMultiInfo->>PageMultiInfo: configure mp_HeaderInfoWidget
PageMultiInfo->>PageMultiInfo: mp_HeaderInfoWidget->setVisible(true)
else lst does not contain DeviceCpu
PageMultiInfo->>PageMultiInfo: mp_HeaderInfoWidget->setVisible(false)
PageMultiInfo->>PageMultiInfo: mp_HeaderInfoWidget->setMaximumHeight(0)
end
Class diagram for updated CPU header info flowclassDiagram
class PageMultiInfo {
+PageMultiInfo(QWidget *parent)
+~PageMultiInfo()
+void updateInfo(QList~DeviceBaseInfo *~ lst)
-void initWidgets()
-void getTableListInfo(QList~DeviceBaseInfo *~ lst, QList~QStringList~ &deviceList, QList~QStringList~ &menuList)
-void clearLayout(QLayout *layout)
-DLabel *mp_Label
-DScrollArea *mp_HeaderInfoWidget
-DWidget *mp_HeaderInfoContainer
-QVBoxLayout *mp_HeaderInfoWidgetLay
-PageTableHeader *mp_Table
-PageDetail *mp_Detail
-QList~DeviceBaseInfo *~ m_lstDevice
-QList~QStringList~ m_deviceList
-QList~QStringList~ m_menuControlList
}
class DeviceGenerator {
+DeviceGenerator(QObject *parent)
+void generatorCpuDevice()
-void calAndSetCpuHeaderInfo(QMap~QString, QString~ firstProcessorInfo, int coreNum, int logicalNum)
#QStringList m_ListBusID
}
class DeviceManager {
+static DeviceManager *instance()
+void addCpuDevice(DeviceBaseInfo *device)
+void setCpuHeaderInfo(QList~QList~QPair~QString, QString~~~ cpuHeaderInfo)
+void getCpuHeaderInfo(QList~QList~QPair~QString, QString~~~ &cpuHeaderInfo)
}
class DeviceBaseInfo {
}
class DeviceCpu {
}
class HeaderInfoTableWidget {
+HeaderInfoTableWidget(QWidget *parent)
+void updateData(QList~QPair~QString, QString~~ headerInfo)
}
class PageTableHeader {
}
class PageDetail {
}
class QWidget {
}
PageMultiInfo --|> QWidget
DeviceCpu --|> DeviceBaseInfo
PageMultiInfo o--> PageTableHeader : mp_Table
PageMultiInfo o--> PageDetail : mp_Detail
PageMultiInfo o--> DLabel : mp_Label
PageMultiInfo o--> DScrollArea : mp_HeaderInfoWidget
PageMultiInfo o--> DWidget : mp_HeaderInfoContainer
PageMultiInfo o--> QVBoxLayout : mp_HeaderInfoWidgetLay
PageMultiInfo ..> DeviceCpu : uses dynamic_cast
PageMultiInfo ..> HeaderInfoTableWidget : creates
PageMultiInfo ..> DeviceManager : calls getCpuHeaderInfo
DeviceGenerator ..> DeviceManager : addCpuDevice
DeviceGenerator ..> DeviceManager : setCpuHeaderInfo
DeviceGenerator ..> Common : formatTotalCache
DeviceManager ..> DeviceCpu : manages
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review这份代码 diff 主要是为了在设备管理器中为 CPU 设备添加一个新的头部信息展示区域（HeaderInfo），包括型号、架构、缓存等信息。我将从语法逻辑、代码质量、性能和安全四个方面进行审查。 1. 语法逻辑优点：
问题与建议：
2. 代码质量问题与建议：
3. 代码性能问题与建议：
4. 代码安全问题与建议：
总结这段代码整体质量不错，逻辑清晰，且在内存管理（如 主要改进建议：
|
![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 2 issues, and left some high level feedback:
- In PageMultiInfo::updateInfo, consider removing the explicit mp_HeaderInfoWidget->resize() call and let the layout manage the scroll area's size, as manual resizing inside a managed layout can lead to unexpected layout behavior on different window sizes or DPI settings.
- clearLayout() currently assumes the QLayout pointer is non-null and only handles direct widgets; it would be safer to guard against a nullptr and optionally handle nested layouts to avoid potential leaks if the layout ever contains sublayouts in the future.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In PageMultiInfo::updateInfo, consider removing the explicit mp_HeaderInfoWidget->resize() call and let the layout manage the scroll area's size, as manual resizing inside a managed layout can lead to unexpected layout behavior on different window sizes or DPI settings.
- clearLayout() currently assumes the QLayout pointer is non-null and only handles direct widgets; it would be safer to guard against a nullptr and optionally handle nested layouts to avoid potential leaks if the layout ever contains sublayouts in the future.
## Individual Comments
### Comment 1
<location path="deepin-devicemanager/src/GenerateDevice/DeviceGenerator.cpp" line_range="1286-1295" />
<code_context>
+ if (firstProcessorInfo.contains("L1d cache")) {
</code_context>
<issue_to_address>
**issue (bug_risk):** Cache total calculations should guard against coreNum being zero to avoid invalid computations inside formatTotalCache.
In `calAndSetCpuHeaderInfo`, `Common::formatTotalCache` is called for L1/L2 caches with `coreNum` even when it may be zero, unlike the guarded core/thread section. If `formatTotalCache` divides or scales by `coreNum`, this can yield invalid results. Please either guard these calls with `coreNum > 0` or return early when `coreNum <= 0` before computing cache totals.
</issue_to_address>
### Comment 2
<location path="deepin-devicemanager/src/Page/PageMultiInfo.cpp" line_range="68" />
<code_context>
PageMultiInfo::~PageMultiInfo()
{
// 清空指针
</code_context>
<issue_to_address>
**issue (complexity):** Consider simplifying the new CPU header UI integration by relying on Qt parent ownership, extracting CPU-specific helpers, and reducing member state and layout utilities to keep PageMultiInfo easier to read and maintain.
You can keep the feature as-is but reduce complexity and surface area with a few targeted changes.
### 1. Drop manual deletion of `mp_HeaderInfoWidget` / `mp_HeaderInfoContainer`
Qt’s parent ownership already handles these. The explicit `delete` + `nullptr` adds noise and potential for double-delete if parents change.
**Current:**
```cpp
PageMultiInfo::~PageMultiInfo()
{
// 清空指针
if (mp_HeaderInfoWidget) {
delete mp_HeaderInfoWidget;
mp_HeaderInfoWidget = nullptr;
mp_HeaderInfoContainer = nullptr;
}
if (mp_Table) {
delete mp_Table;
mp_Table = nullptr;
}
if (mp_Detail) {
delete mp_Detail;
mp_Detail = nullptr;
}
m_deviceList.clear();
m_menuControlList.clear();
}
```
**Suggestion:**
Let parents clean up, only delete when no parent:
```cpp
PageMultiInfo::~PageMultiInfo()
{
// mp_HeaderInfoWidget / mp_HeaderInfoContainer rely on QObject parent deletion
if (!mp_Table->parent()) {
delete mp_Table;
}
if (!mp_Detail->parent()) {
delete mp_Detail;
}
m_deviceList.clear();
m_menuControlList.clear();
}
```
If `mp_Table` and `mp_Detail` also always have `this` as parent, you can remove their manual deletes too.
---
### 2. Simplify show/hide & size logic for the scroll area
You already set size policy, min/max and visibility in `initWidgets`. Updating height and resizing on every `updateInfo` call is redundant.
**Current in `updateInfo`:**
```cpp
if (cpuInfo) {
...
mp_HeaderInfoWidget->setMaximumHeight(kHeaderInfoDefaultHeight);
mp_HeaderInfoWidget->resize(this->width(), kHeaderInfoDefaultHeight);
mp_HeaderInfoWidget->setVisible(true);
} else {
mp_HeaderInfoWidget->setVisible(false);
mp_HeaderInfoWidget->setMaximumHeight(0);
}
```
**Suggestion:** configure once in `initWidgets`, then just toggle visibility:
```cpp
// initWidgets()
mp_HeaderInfoWidget->setMaximumHeight(kHeaderInfoDefaultHeight);
mp_HeaderInfoWidget->setMinimumHeight(0);
mp_HeaderInfoWidget->setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Preferred);
mp_HeaderInfoWidget->setVisible(false);
```
```cpp
// updateInfo()
if (cpuInfo) {
updateCpuHeaderInfo(headerInfo);
mp_HeaderInfoWidget->setVisible(true);
} else {
clearCpuHeaderInfo();
mp_HeaderInfoWidget->setVisible(false);
}
```
This removes the height juggling and the explicit `resize`.
---
### 3. Extract CPU-specific logic out of `updateInfo`
Right now `updateInfo` handles:
- CPU type detection
- Data retrieval
- Layout management
- Scroll area visibility/size
Splitting that into focused helpers will make `updateInfo` easier to read, without changing behavior.
**Suggestion:**
```cpp
void PageMultiInfo::updateInfo(const QList<DeviceBaseInfo *> &lst)
{
m_lstDevice = lst;
if (lst.isEmpty() || !lst.at(0))
return;
updateCpuHeaderSection(lst.at(0)); // CPU-specific logic extracted
m_deviceList.clear();
m_menuControlList.clear();
getTableListInfo(lst, m_deviceList, m_menuControlList);
mp_Table->setVisible(true);
mp_Table->setFixedHeight(TABLE_HEIGHT);
mp_Table->updateTable(m_deviceList, m_menuControlList);
mp_Detail->updateDetail(lst);
}
```
```cpp
void PageMultiInfo::updateCpuHeaderSection(DeviceBaseInfo *device)
{
auto *cpuInfo = dynamic_cast<DeviceCpu *>(device);
if (!cpuInfo) {
clearCpuHeaderInfo();
mp_HeaderInfoWidget->setVisible(false);
return;
}
QList<QList<QPair<QString, QString>>> headerInfo;
DeviceManager::instance()->getCpuHeaderInfo(headerInfo);
auto *layout = cpuHeaderLayout();
clearLayout(layout);
for (const auto &group : headerInfo) {
auto *headerWidget = new HeaderInfoTableWidget(mp_HeaderInfoWidget);
headerWidget->updateData(group);
layout->addWidget(headerWidget);
}
mp_HeaderInfoWidget->setVisible(true);
}
void PageMultiInfo::clearCpuHeaderInfo()
{
auto *layout = cpuHeaderLayout();
clearLayout(layout);
}
```
This keeps `updateInfo` focused on orchestration and isolates CPU-specific UI behavior.
---
### 4. Avoid `mp_HeaderInfoWidgetLay` as a member if not necessary
You only need the layout when populating/clearing CPU header info. It doesn’t have to be a long-lived member; reducing member state lowers cognitive load.
**Current constructor:**
```cpp
, mp_HeaderInfoWidget(new DScrollArea(this))
, mp_HeaderInfoContainer(new DWidget(mp_HeaderInfoWidget))
, mp_HeaderInfoWidgetLay(new QVBoxLayout(mp_HeaderInfoContainer))
```
**Suggestion:** keep only the widget/container members; get the layout on demand:
```cpp
// header
QVBoxLayout *cpuHeaderLayout() const;
// cpp
QVBoxLayout *PageMultiInfo::cpuHeaderLayout() const
{
return qobject_cast<QVBoxLayout *>(mp_HeaderInfoContainer->layout());
}
```
Then in `updateCpuHeaderSection` / `clearCpuHeaderInfo` use `cpuHeaderLayout()` instead of `mp_HeaderInfoWidgetLay`. You can drop `mp_HeaderInfoWidgetLay` from the class members and constructor initializer list.
---
### 5. Make `clearLayout` clearly scoped and/or truly generic
Right now `clearLayout` is:
- A member method
- Only used for the CPU header layout
- Written as if it were a generic utility, but not recursive
Two options:
**a) Make it clearly local to CPU header**
Rename and keep behaviour explicit:
```cpp
void PageMultiInfo::clearCpuHeaderLayout(QLayout *layout)
{
QLayoutItem *item;
while ((item = layout->takeAt(0)) != nullptr) {
if (QWidget *widget = item->widget()) {
widget->deleteLater();
}
delete item;
}
}
```
Call it only from CPU-specific helpers to avoid suggesting it’s a generic utility.
**b) Or make it generic and recursive if you intend broader reuse:**
```cpp
void clearLayoutRecursive(QLayout *layout)
{
QLayoutItem *item;
while ((item = layout->takeAt(0)) != nullptr) {
if (QWidget *widget = item->widget()) {
widget->deleteLater();
} else if (QLayout *childLayout = item->layout()) {
clearLayoutRecursive(childLayout);
delete childLayout;
}
delete item;
}
}
```
And either keep it `static` in the `.cpp` or in an anonymous namespace if it’s not meant to be part of the class API.
---
These changes keep all behavior but reduce manual lifetime management, member state, and responsibility mixing, which should address most of the “too complex” concerns without undoing the feature.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
max-lvs
approved these changes
May 7, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: GongHeng2017, max-lvs 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 |
Contributor
Author
|
/forcemerge |
Contributor
|
This pr force merged! (status: unstable) |
deepin-bot
Bot
merged commit 674a9cf
into
linuxdeepin:develop/eagle
20 of 22 checks passed
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
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.
…tion
Log: add feature for cpu info show.
Task: https://pms.uniontech.com/task-view-387697.html
Summary by Sourcery
Add scrollable CPU header info display to the multi-info page and generate corresponding header data during CPU device creation.
New Features:
Enhancements: