Skip to content

fix(wallpapercache): use QImageReader scaled decoding to avoid OOM on…#65

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
mhduiy:alloc
May 7, 2026
Merged

fix(wallpapercache): use QImageReader scaled decoding to avoid OOM on…#65
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
mhduiy:alloc

Conversation

@mhduiy

@mhduiy mhduiy commented May 7, 2026

Copy link
Copy Markdown
Contributor

… large images

  1. Replace QImage::load() with QImageReader in pixmix effect processing, cap decoded resolution at 8K (7680px) to prevent excessive memory usage
  2. Use QImageReader::setScaledSize() for scaleImage thread so decoding outputs directly at target size instead of full-size then downscale
  3. Use QImageReader::imageFormat() static method for format detection to avoid instantiating a full reader when only the format is needed
  4. Add proper error handling with canRead/isEmpty checks and error strings

Log: Optimize wallpaper image decoding to use scaled reading and prevent OOM on large images

fix(wallpapercache): 使用 QImageReader 缩放解码避免大图内存溢出

  1. 将 pixmix 特效处理中的 QImage::load() 替换为 QImageReader, 限制解码分辨率上限为 8K (7680px),防止内存占用过大
  2. 缩放线程使用 QImageReader::setScaledSize() 在解码时直接输出目标尺寸, 避免先全尺寸加载再缩放带来的额外内存开销
  3. 格式检测改用 QImageReader::imageFormat() 静态方法, 避免实例化完整 reader 造成不必要的图片加载
  4. 增加完善的错误检查,包含 canRead/isEmpty 校验及错误信息输出

Log: 优化壁纸图片解码流程,采用缩放读取方式防止超大图片导致内存溢出
PMS: BUG-359341
Change-Id: I4ce0c8d32b51e7036a3457490b318f540b51f0f4

Summary by Sourcery

Optimize wallpaper image decoding to use scaled reads and avoid excessive memory usage or OOM with large images.

Bug Fixes:

  • Prevent out-of-memory conditions when processing very large wallpaper images by capping decode resolution and using scaled decoding.

Enhancements:

  • Use QImageReader with scaled decoding in the scaling thread to decode images directly at target size instead of loading full-resolution images.
  • Limit pixmix effect image decoding to a maximum long side of 8K resolution to reduce memory usage while preserving aspect ratio.
  • Improve image format detection by using QImageReader::imageFormat to avoid unnecessary full image loading.
  • Add more robust error handling and logging for image reading failures, including canRead checks and detailed error messages.

@sourcery-ai

sourcery-ai Bot commented May 7, 2026

Copy link
Copy Markdown

Reviewer's Guide

Refactors wallpaper image loading to use QImageReader with scaled decoding, caps effective decode resolution for pixmix and scaling threads, and switches format detection to QImageReader::imageFormat() to avoid loading full images, improving memory usage and error handling for large wallpapers.

Class diagram for updated wallpaper image processing components

classDiagram
    class ScaleImageThread {
        +ScaleImageThread(parent: QObject*)
        +void executeTask(task: TaskData)
        +QImage scaleImage(task: TaskData)
        +QString cacheImageToDisk(image: QImage&, task: TaskData, md5String: QString)
    }

    class ImageEffectProcessor {
        +ImageEffectProcessor()
        +EffectType effectTypeFromString(type: QString) EffectType
        +QImage processPixmixEffect(imagePath: QString)
    }

    class WallpaperCacheService {
        +WallpaperCacheService(parent: QObject*)
        +QString saveImageFromFd(fd: QDBusUnixFileDescriptor, format: QString)
    }

    class TaskData {
        +QString originalPath
        +QSize targetSize
    }

    ScaleImageThread --> TaskData : uses
    ImageEffectProcessor --> QImageReader : uses
    ScaleImageThread --> QImageReader : uses
    WallpaperCacheService --> QImageReader : uses

    class QImageReader {
        +QImageReader(fileName: QString)
        +bool canRead()
        +QSize size()
        +void setScaledSize(size: QSize)
        +QImage read()
        +QString errorString()
        +static QByteArray imageFormat(fileName: QString)
    }
Loading

File-Level Changes

Change Details Files
Use QImageReader-based scaled decoding in the background scaling thread to avoid loading full-size images into memory.
  • Replace QImage::load() with QImageReader in ScaleImageThread::scaleImage
  • Add canRead and size() checks, returning early on failure or missing size
  • Compute a decode size based on task.targetSize with KeepAspectRatioByExpanding and set it via setScaledSize() before decoding
  • Read the image via reader.read() and improve failure logging with errorString()
  • Remove a separate QImage::scaled step and reuse the decoded image for cropping to the requested target rect
src/plugin-qt/wallpapercache/scaleimagethread.cpp
Limit pixmix effect decoding resolution and improve robustness of pixmix image loading.
  • Introduce PIXMIX_MAX_DIM (7680) as an 8K long-side cap for pixmix processing
  • Replace QImage::load() with QImageReader in processPixmixEffect and guard with canRead and size checks
  • If the original image exceeds the 8K cap, compute a reduced decode size with KeepAspectRatio and setScaledSize() before reading
  • Decode via reader.read(), logging detailed errors including reader.errorString() on failure
  • Fix minor formatting of the QColorMatrix::apply call arguments
src/plugin-qt/wallpapercache/imageeffectprocessor.cpp
Optimize image format detection when saving images by avoiding full reader instantiation and enforcing error handling on unknown formats.
  • Replace QImageReader instance-based format() usage with static QImageReader::imageFormat(originPath)
  • Treat empty format results as an error: log a warning, delete the temporary file, and abort with an empty QString
  • Derive the destination file path using the detected format via QString::fromLatin1 and rename the original file accordingly
src/plugin-qt/wallpapercache/wallpapercacheservice.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="src/plugin-qt/wallpapercache/scaleimagethread.cpp" line_range="169-170" />
<code_context>
-                             size.height()));
-
-    return image;
+    const QSize &size = task.targetSize;
+    return image.copy(QRect((image.width() - size.width()) / 2,
+                            (image.height() - size.height()) / 2,
+                            size.width(),
</code_context>
<issue_to_address>
**issue (bug_risk):** Guard against crop rect going out of bounds if decoded size is smaller than `task.targetSize`.

Because some decoders may ignore `setScaledSize`, apply rounding differently, or not honor the scaling hint, `image.size()` can occasionally be smaller than `task.targetSize`. In that case, the center offsets `(image.width() - size.width()) / 2` (and height) become negative and the crop `QRect` can fall partially or fully outside the image, leading to undefined behavior or a null image. Please clamp the crop rect to `image.size()` (e.g., via min/max on width/height and offsets), or fall back to a non-centered crop when `image.width() < size.width()` or `image.height() < size.height()`.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread src/plugin-qt/wallpapercache/scaleimagethread.cpp
… large images

1. Replace QImage::load() with QImageReader in pixmix effect processing,
   cap decoded resolution at 8K (7680px) to prevent excessive memory usage
2. Use QImageReader::setScaledSize() for scaleImage thread so decoding
   outputs directly at target size instead of full-size then downscale
3. Use QImageReader::imageFormat() static method for format detection to
   avoid instantiating a full reader when only the format is needed
4. Add proper error handling with canRead/isEmpty checks and error strings

Log: Optimize wallpaper image decoding to use scaled reading and prevent OOM on large images

fix(wallpapercache): 使用 QImageReader 缩放解码避免大图内存溢出

1. 将 pixmix 特效处理中的 QImage::load() 替换为 QImageReader,
   限制解码分辨率上限为 8K (7680px),防止内存占用过大
2. 缩放线程使用 QImageReader::setScaledSize() 在解码时直接输出目标尺寸,
   避免先全尺寸加载再缩放带来的额外内存开销
3. 格式检测改用 QImageReader::imageFormat() 静态方法,
   避免实例化完整 reader 造成不必要的图片加载
4. 增加完善的错误检查,包含 canRead/isEmpty 校验及错误信息输出

Log: 优化壁纸图片解码流程,采用缩放读取方式防止超大图片导致内存溢出
PMS: BUG-359341
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

这份代码 diff 主要对图像加载和处理逻辑进行了优化,特别是在处理大图像和性能方面做了改进。以下是对代码的详细审查,包括语法逻辑、代码质量、代码性能和代码安全方面的改进意见。

1. 语法逻辑与代码质量

优点:

  • 资源管理优化:将 QImage::load() 替换为 QImageReader。这是一个很好的改进。QImage::load() 会将整个图像解码到内存中,而 QImageReader 允许在解码前获取元数据(如尺寸)并设置解码尺寸,从而有效控制内存使用。
  • 错误处理增强:增加了 reader.canRead()reader.errorString() 的调用,使得错误日志更加详细,便于排查问题。
  • 常量定义:引入了 PIXMIX_MAX_DIM 宏定义,比硬编码的数字更具可读性和可维护性。
  • 逻辑修正:在 scaleImage 函数中,移除了 image.scaled() 调用,改为直接使用 reader.setScaledSize() 进行解码时缩放,避免了先解码大图再缩放的二次操作。

改进建议:

  1. wallpapercacheservice.cpp 中的类型转换

    • 代码:QString::fromLatin1(format)
    • 意见:虽然 QImageReader::imageFormat 返回的是 QByteArray,通常图像格式(如 "png", "jpg")都是 ASCII 字符,使用 fromLatin1 是安全的。但为了代码更通用和现代,建议使用 QString::fromUtf8QString::fromStdString(format.toStdString()),尽管在图像格式这个场景下差异不大。
    • 代码:QString destinationPath = originPath + QString(".%1").arg(QString::fromLatin1(format));
    • 建议:可以简化为 QString destinationPath = originPath + '.' + QString::fromLatin1(format);,可读性稍好。
  2. imageeffectprocessor.cpp 中的逻辑检查

    • 代码:if (!originalSize.isEmpty() && ...)
    • 意见:QSize::isEmpty() 当宽或高 <= 0 时返回 true。如果 reader.size() 返回无效尺寸(例如某些损坏的图片或格式不支持),后续逻辑会跳过缩放设置。这是合理的防御性编程。

2. 代码性能

优点:

  • 降低内存峰值:通过 reader.setScaledSize(),图像在解码阶段就被缩小了,而不是先解码成巨大的位图再缩小。这对于处理 8K 甚至更大的图片至关重要,能有效防止 OOM(Out of Memory)崩溃。
  • 避免不必要的拷贝:在 scaleimagethread.cpp 中,原代码先 scaledcopy。新代码直接解码到目标尺寸(或接近目标尺寸),然后只进行一次 copy 操作(裁剪到中心),减少了像素数据的拷贝和 CPU 消耗。
  • 快速检测格式:在 wallpapercacheservice.cpp 中,使用静态方法 QImageReader::imageFormat(originPath) 代替实例化 QImageReader 并调用 format(),虽然性能差异微小,但逻辑上更轻量。

改进建议:

  1. scaleimagethread.cpp 中的缩放逻辑

    • 代码:decodeSize.scale(task.targetSize, Qt::KeepAspectRatioByExpanding);
    • 分析:Qt::KeepAspectRatioByExpanding 会确保解码后的图像至少覆盖 targetSize,这意味着解码后的图像尺寸可能比 targetSize 大。
    • 潜在问题:如果 task.targetSize 非常小(例如缩略图),而原图是 8K,即使设置了 PIXMIX_MAX_DIM,如果 task.targetSize 极小,解码过程仍然可能分配较多内存(尽管比原图小)。
    • 建议:虽然当前逻辑是为了配合后续的 copy(中心裁剪),但建议确认 task.targetSize 的合理范围。如果 task.targetSize 可能非常大,这里也应该像 imageeffectprocessor.cpp 那样加一个最大尺寸限制(例如 PIXMIX_MAX_DIM),以防止极端情况下的内存分配。
  2. wallpapercacheservice.cpp 中的文件操作

    • 代码:QFile::remove(originPath);format.isEmpty() 时被调用。
    • 意见:这是正确的清理操作,防止残留无效文件。

3. 代码安全

优点:

  • 防止 DoS(拒绝服务):通过限制最大解码尺寸 (PIXMIX_MAX_DIM),防止了恶意构造的超大分辨率图片导致应用程序消耗过多内存甚至崩溃。
  • 空指针/空对象检查:增加了对 image.isNull() 的检查,防止后续操作空对象导致崩溃。

改进建议:

  1. 路径安全

    • 虽然代码中使用了 QString 处理路径,但未看到对路径遍历(如 ../../../etc/passwd)的检查。如果 imagePathtask.originalPath 来自不可信的外部输入(如网络下载、用户输入),建议在加载前验证路径是否在预期的允许目录内。
  2. 整数溢出风险

    • 代码:decodeSize.scale(PIXMIX_MAX_DIM, PIXMIX_MAX_DIM, Qt::KeepAspectRatio);
    • 分析:PIXMIX_MAX_DIM 定义为 7680。QSize 内部使用 int。计算过程中涉及宽高比乘除,理论上存在整数溢出的风险,但在 7680 这个数值范围内,int (通常是 32 位) 足够表示宽高达 2,147,483,647 的图像,因此这里是安全的。如果未来提高此限制,需注意 int 溢出问题。
  3. scaleimagethread.cpp 中的逻辑漏洞

    • 代码:
      if (image.width() < size.width() || image.height() < size.height()) {
          return image;
      }
    • 分析:这段代码的意图是:如果解码后的图像(经过 KeepAspectRatioByExpanding 缩放后)竟然比目标尺寸还小(理论上不应该发生,除非原图比目标小),则直接返回原图。
    • 安全/逻辑隐患:如果原图比 targetSize 小,decodeSize.scale(..., Qt::KeepAspectRatioByExpanding) 会将 decodeSize 设置为 targetSize(放大)。QImageReader 会尝试将小图解码并放大到 targetSize。如果 targetSize 很大(例如 8K),而原图很小(例如 10x10),QImageReader 可能会生成一个巨大的、充满锯齿的图像,消耗大量内存。
    • 建议:应该增加一个检查,如果 originalSize 小于 targetSize,不要进行放大解码,而是直接加载原图或进行较小比例的处理,或者直接返回原图(取决于业务需求是否允许图片小于目标尺寸)。如果必须放大,应限制放大的最大尺寸。

总结

这份代码 diff 质量很高,主要解决了处理大图时的性能和内存问题。

  • 核心改进:使用 QImageReader 替代 QImage::load 并结合 setScaledSize,极大地优化了内存使用和性能。
  • 建议重点
    1. ScaleImageThread::scaleImage 中,增加对 targetSize 的上限检查,防止因目标尺寸过大而消耗过多内存。
    2. 考虑原图小于目标尺寸时的处理逻辑,避免对小图进行不必要的过度放大解码。
    3. 在涉及外部输入路径时,增加路径合法性校验。

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mhduiy, yixinshark

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mhduiy

mhduiy commented May 7, 2026

Copy link
Copy Markdown
Contributor Author

/forcemerge

@deepin-bot

deepin-bot Bot commented May 7, 2026

Copy link
Copy Markdown

This pr force merged! (status: blocked)

@deepin-bot deepin-bot Bot merged commit 8fb420b into linuxdeepin:master May 7, 2026
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants