-
Notifications
You must be signed in to change notification settings - Fork 55
feat: enhance notification icon handling #1186
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: yixinshark 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 GuideThis PR refactors notification icon handling by eliminating temporary files in favor of in-memory data URLs (with JPEG compression), adding automatic scaling for oversized icons, and simplifying the fallback logic. Class diagram for updated notification icon handlingclassDiagram
class BubbleItem {
+static QString imagePathOfNotification(const QVariantMap &hints, const QString &appName)
}
BubbleItem : imagePathOfNotification() now returns data URL string instead of file path
BubbleItem : imagePathOfNotification() scales images >16x16
BubbleItem : imagePathOfNotification() compresses to JPEG
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @yixinshark - 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/notification/bubble/bubbleitem.cpp:155` </location>
<code_context>
+ QByteArray ba;
+ QBuffer buffer(&ba);
+ buffer.open(QIODevice::WriteOnly);
+ // 使用JPEG格式和适当压缩率减少内存占用
+ img.save(&buffer, "JPEG", 85);
+ QString dataUrl = QString("data:image/jpeg;base64,%1").arg(QString::fromLatin1(ba.toBase64()));
+ return dataUrl;
</code_context>
<issue_to_address>
Consider using PNG for icons to avoid lossy compression artifacts.
JPEG may cause visible artifacts and lacks transparency support, which can reduce icon quality. PNG offers lossless compression and alpha channel support, making it more suitable for icons.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
// 使用JPEG格式和适当压缩率减少内存占用
img.save(&buffer, "JPEG", 85);
QString dataUrl = QString("data:image/jpeg;base64,%1").arg(QString::fromLatin1(ba.toBase64()));
=======
// 使用PNG格式以获得无损压缩和透明度支持,适合图标
img.save(&buffer, "PNG");
QString dataUrl = QString("data:image/png;base64,%1").arg(QString::fromLatin1(ba.toBase64()));
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| DGUI_USE_NAMESPACE; | ||
| auto icon = DIconTheme::findQIcon(appName, DIconTheme::findQIcon("application-x-desktop")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Consider using PNG for icons to avoid lossy compression artifacts.
JPEG may cause visible artifacts and lacks transparency support, which can reduce icon quality. PNG offers lossless compression and alpha channel support, making it more suitable for icons.
| // 使用JPEG格式和适当压缩率减少内存占用 | |
| img.save(&buffer, "JPEG", 85); | |
| QString dataUrl = QString("data:image/jpeg;base64,%1").arg(QString::fromLatin1(ba.toBase64())); | |
| // 使用PNG格式以获得无损压缩和透明度支持,适合图标 | |
| img.save(&buffer, "PNG"); | |
| QString dataUrl = QString("data:image/png;base64,%1").arg(QString::fromLatin1(ba.toBase64())); |
| buffer.open(QIODevice::WriteOnly); | ||
| // 使用JPEG格式和适当压缩率减少内存占用 | ||
| img.save(&buffer, "JPEG", 85); | ||
| QString dataUrl = QString("data:image/jpeg;base64,%1").arg(QString::fromLatin1(ba.toBase64())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
decodeImageFromBase64函数里在它前面加上前缀了,应该不需要再处理吧,另外img可能不是base64的图标吧,
update changelog to 2.0.3
Updated the image processing for notification icons to avoid temporary file usage by converting images to data URL format. Added scaling for icons larger than 16x16 pixels and implemented JPEG compression to reduce memory usage. This change improves performance and reliability in displaying notification icons. Log: as title
|
TAG Bot TAG: 2.0.3 |
deepin pr auto review代码审查反馈:
以上是对代码审查的反馈,希望能够对您的项目有所帮助。 |
Updated the image processing for notification icons to avoid temporary file usage by converting images to data URL format. Added scaling for icons larger than 16x16 pixels and implemented JPEG compression to reduce memory usage. This change improves performance and reliability in displaying notification icons.
Log: as title
Summary by Sourcery
Enhance notification icon handling by replacing temporary files with data URL encoding, scaling oversized icons, and applying JPEG compression to improve performance and reliability
Enhancements: