fix(footer): skip background blur when using default cover image#670
Conversation
|
Hi @Resurgamz. Thanks for your PR. 😃 |
|
CLA Assistant Lite bot: |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates the footer background refresh logic so that the blurred background is only generated when an actual cover image file exists; otherwise the background is cleared, avoiding use of the default built-in cover image. Sequence diagram for updated footer background refresh logicsequenceDiagram
actor User
participant Player
participant FooterWidget
participant FileSystem
participant ForwardWidget as m_forwardWidget
User->>Player: Change active track
Player-->>FooterWidget: activeMeta updated
FooterWidget->>FooterWidget: slotFlushBackground()
FooterWidget->>FileSystem: Check imagesDirPath exists
alt Cover image file does not exist
FooterWidget->>ForwardWidget: setSourceImage(empty QImage)
FooterWidget->>ForwardWidget: update()
FooterWidget-->>FooterWidget: return (no blur background)
else Cover image file exists
FooterWidget->>FileSystem: Load QImage(imagesDirPath)
FooterWidget->>FooterWidget: Compute windowScale and imageWidth
FooterWidget->>FooterWidget: Crop or fill coverImage based on play queue state
FooterWidget->>ForwardWidget: setSourceImage(coverImage)
FooterWidget->>ForwardWidget: update()
end
Class diagram for FooterWidget background handling changesclassDiagram
class FooterWidget {
+void resizeEvent(QResizeEvent* event)
+void slotFlushBackground()
-QWidget* m_forwardWidget
-QAbstractButton* m_btPlayQueue
}
class Player {
+static Player* getInstance()
+Meta getActiveMeta()
}
class Meta {
+QString hash
}
class Global {
+static QString cacheDir()
}
class QImage {
+QImage()
+QImage(QString filePath)
+int width()
+int height()
+void fill(QColor color)
+QImage copy(int x, int y, int width, int height)
}
class QFileInfo {
+QFileInfo(QString filePath)
+bool exists()
}
class ForwardWidget {
+void setSourceImage(QImage image)
+void update()
}
FooterWidget --> ForwardWidget : uses
FooterWidget --> Player : queries activeMeta
FooterWidget --> Global : uses cacheDir
FooterWidget --> QFileInfo : checks cover file
FooterWidget --> QImage : loads and processes cover
Player --> Meta : returns
Meta --> hash : field
FooterWidget --> QAbstractButton : uses m_btPlayQueue to alter background
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
slotFlushBackground, consider also handling the case where the cover file exists butQImage imagesDirPathfails to load (e.g., corrupt file) by falling back to clearing the background instead of proceeding with an invalid image. - When clearing the background for the no-cover case, verify whether
m_forwardWidgetmaintains any cached blur/transition state and, if so, reset that state alongsidesetSourceImage(QImage())to avoid visual artifacts from a previous track.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `slotFlushBackground`, consider also handling the case where the cover file exists but `QImage imagesDirPath` fails to load (e.g., corrupt file) by falling back to clearing the background instead of proceeding with an invalid image.
- When clearing the background for the no-cover case, verify whether `m_forwardWidget` maintains any cached blur/transition state and, if so, reset that state alongside `setSourceImage(QImage())` to avoid visual artifacts from a previous track.
## Individual Comments
### Comment 1
<location path="src/music-player/mainFrame/footerwidget.cpp" line_range="935-942" />
<code_context>
- cover = QImage(Global::cacheDir() + "/images/" + Player::getInstance()->getActiveMeta().hash + ".jpg");
+
+ // 如果没有实际封面图片,清空背景
+ if (!file.exists()) {
+ m_forwardWidget->setSourceImage(QImage());
+ m_forwardWidget->update();
+ return;
}
+ // 加载实际封面图片
+ QImage cover = QImage(imagesDirPath);
double windowScale = (width() * 1.0) / height();
int imageWidth = static_cast<int>(cover.height() * windowScale);
</code_context>
<issue_to_address>
**issue:** Handle the case where the image file exists but fails to load (e.g., corrupt file).
With this change, `cover` is only set from the jpg. If `QImage(imagesDirPath)` fails (e.g. corrupt/unsupported file), `cover` will be null and later dimension-dependent logic may misbehave. Consider checking `cover.isNull()` after loading and, if true, either clear the background (as in the `!file.exists()` branch) or fall back to a default image.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
98803e5 to
10f6042
Compare
|
Hi @Resurgamz. Thanks for your PR. I'm waiting for a linuxdeepin member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
f2649b3 to
d266fa1
Compare
| { | ||
| QImage cover = QImage(":/icons/deepin/builtin/actions/info_cover_142px.svg"); | ||
| // 直接检查实际封面文件是否存在 | ||
| QString imagesDirPath = Global::cacheDir() + "/images/" + Player::getInstance()->getActiveMeta().hash + ".jpg"; |
There was a problem hiding this comment.
缺少对 Global::cacheDir() 空路径的处理,如果 Global::cacheDir() 返回空字符串,路径拼接可能产生意外结果。建议:添加防御性检查
| // 加载实际封面图片 | ||
| QImage cover = QImage(imagesDirPath); | ||
| // 如果图片加载失败(文件损坏或格式不支持),清空背景 | ||
| if (cover.isNull() || cover.width() == 0 || cover.height() == 0) { |
There was a problem hiding this comment.
cover.width() == 0 || cover.height() == 0 会不会过渡检查了
|
|
||
| // 如果没有实际封面图片,清空背景 | ||
| if (!file.exists()) { | ||
| m_forwardWidget->setSourceImage(QImage()); |
When no actual cover exists, clear background instead of using default cover. 当无实际封面时,清空背景而非使用默认封面图片。 Log: 无实际封面时不设置背景 PMS: https://pms.uniontech.com/bug-view-357677.html Influence: 当音乐无封面图片时,底部工具栏不再显示默认封面的模糊背景,界面更加简洁。
d266fa1 to
92b57c6
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: max-lvs, Resurgamz 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 |
When no actual cover exists, clear background instead of using default cover.
当无实际封面时,清空背景而非使用默认封面图片。
Log: 无实际封面时不设置背景
PMS: BUG-357677
Influence: 当音乐无封面图片时,底部工具栏不再显示默认封面的模糊背景,界面更加简洁。
Summary by Sourcery
Bug Fixes: