fix: Don't upscale images and test that image resolution isn't changed unnecessarily #7769

Hocuri · 2026-01-23T22:18:18Z

This adds a test for #7760.

Also, it fixes another bug which I uncovered with the test: If the resolution was already lower than the max resolution, then the image was upscaled to match the max resolution.

cc @72374

src/blob.rs

Co-authored-by: 72374 <250991390+72374@users.noreply.github.com>

iequidoo · 2026-01-24T10:45:25Z

src/blob.rs

                        }

-                        img_wh = img_wh * 2 / 3;
+                        target_wh = target_wh * 2 / 3;


I'd also remove this line from here and add

if !encoded.is_empty() { target_wh = target_wh * 2 / 3; }

to the beginning of the loop so that no extra iteration for avatars is done. It's a small optimization, but may make the code clearer, otherwise one may wonder why we recode avatars to the same size again.

Not sure, this may easily lead to another issue where we unnecessarily reduce the target resolution.

I anyways find the code block above hard to understand:

let do_scale = exceeds_max_bytes || is_avatar && (exceeds_wh || exif.is_some() && { if mem::take(&mut add_white_bg) { self::add_white_bg(&mut img); } encoded_img_exceeds_bytes( context, &img, ofmt.clone(), max_bytes, &mut encoded, )? });

so... if the avatar does not exceed the max_bytes or width/height, but it has some exif, then we directly do an iteration of adding white background and recoding, and check the resulting bytes size. If the image is too big now (note that it wasn't too big before), then do_scale is true and we reduce the resolution.

So, I guess that the code block is there to make really sure that the avatar file can't be too big in the end

Which is good, of course, it's just hard to understand, and I am wary of adding code that depends on is behavior.

Shouldn't the || exif.is_some() && { there be || exif.is_some() || {, so that it is checked if any of the three is true?
The way i understand it, avatar-images that are not larger than max_bytes and max_wh, but include exif-data, would only be re-encoded, if those also do not fit within the size after being encoded with a white background.
Usually the purpose of specifically re-encoding images with exif-data is, to remove metadata (though it can also be removed from the image-file without re-encoding the image).

[...] || exif.is_some() || { [...]

If it's an avatar w/o Exif, it should only be recoded if it exceeds any limits. White background is also not needed when not recoding an avatar, this is a w/a for transparent avatars recoded to JPEG, otherwise the image crate adds a black one.

That would still be the same after the change. I opened a PR with a short explanation for the change there: #7772

Which is good, of course, it's just hard to understand, and I am wary of adding code that depends on is behavior.

Finally i think that it's fine to move the target_wh reduction above, if it's under !encoded.is_empty(), nothing will depend on the complex logic above: it is plainly "if we've already tried to recode and still inside the loop, we need to try a smaller size". Anyway, this is minor

Seems correct.
A comment could make it more understandable why that is done; then it would be fine to have this optimisation, i think.

Something like this, before the beginning of the loop, as it only needs to be checked once:

if do_scale { if is_avatar && !encoded.is_empty() { // If this happens, the image was already encoded at the original resolution, // for the file-size-check for an avatar with an added white background, // and did not fit within the file-size-limit after that. // Skip encoding it again at the original resolution. target_wh = target_wh * 2 / 3; } loop { …

Which is similar to what you suggested in the previous PR, i think.

Actually, not having this can result in quality-reduction, because, when the file-size-check happens, it does not only check the file-size, it also changes the image that will be used, which, depending on the encoder, can result in encoding the image again with jpeg-quality 75 (or not, if the encoder does detect that it is the same format and quality, and skips re-encoding).

src/blob.rs

72374 · 2026-01-24T13:40:03Z

src/blob.rs

+    /// If `!is_avatar`, then if `max_bytes` is exceeded, reduces the image to `max_wh` and proceeds
+    /// with the result (even if `max_bytes` is still exceeded).
+    ///
+    /// If `is_avatar`, the resolution will be reduced in a loop until the image fits `max_bytes`.


This seems to be an outdated explanation.

Suggested change

/// If `!is_avatar`, then if `max_bytes` is exceeded, reduces the image to `max_wh` and proceeds

/// with the result (even if `max_bytes` is still exceeded).

///

/// If `is_avatar`, the resolution will be reduced in a loop until the image fits `max_bytes`.

/// If `max_bytes` is exceeded,

/// the image will be re-encoded to `max_wh` or the original resolution, whichever is smaller.

/// If it then is still larger than `max_bytes` it will be reduced in resolution,

/// until it fits within `max_bytes`.

///

/// If `is_avatar`, then images with a higher resolution than `max_wh`,

/// and images which include exif-data, will also be re-encoded.

/// If it then is still larger than `max_bytes` it will be reduced in resolution, until it fits within `max_bytes`.

This is only true for avatars, though. If not is_avatar, then we'll break out of the loop after the first iteration.

If is_avatar, then images with a higher resolution than max_wh, and images which include exif-data, will also be re-encoded.

All images will be re-encoded if there is exif, not only avatars:

if do_scale || exif.is_some() { [...] if encoded.is_empty() { if mem::take(&mut add_white_bg) { self::add_white_bg(&mut img); } encode_img(&img, ofmt, &mut encoded)?; }

This is only true for avatars, though. If not is_avatar, then we'll break out of the loop after the first iteration.

Indeed, i overlooked the && is avatar after if encoded_img_exceeds_bytes.

I guess i better wait with further suggestions until i can get Delta Chat to build with a local core (i tried that for multiple hours, without success) and verify the changes i want to suggest; my incorrect suggestions were probably rather distracting. ^^'
(I also forgot i could have tried a few things with the unmodified version.)

But anyway, thank you (you and the others working on Delta Chat and/or Chatmail) for making and improving Delta Chat and Chatmail.
It works well for me and my family-members so far (aside from my issues with the limited image-quality),
and the thing with multiple relays for a more peer-to-peer-like setup is finally something that is both independent and reliable enough, that i can comfortably recommend a messenger to friends and family.

i tried that for multiple hours, without success

What's the problem? And, are you trying to build DC Desktop, DC Android, or the REPL?

BTW, running the Rust tests locally should be just ~~cargo test~~ cargo nextest run, and that way you can already somewhat verify changes by writing a Rust test, as I did in the PR here.

But anyway, thank you (you and the others working on Delta Chat and/or Chatmail) for making and improving Delta Chat and Chatmail.

It's a pleasure 😊

BTW, running the Rust tests locally should be just cargo test

It is actually cargo nextest run, with cargo test some tests fail randomly because they are running in the same process.

What's the problem? And, are you trying to build DC Desktop, DC Android, or the REPL?

DC Desktop.

After setting it up according to the documentation in README.md and UPDATE_CORE.md like this:

git clone --recurse-submodules https://github.com/chatmail/core git clone --recurse-submodules https://github.com/deltachat/deltachat-desktop cd deltachat-desktop/ sudo npm i -g pnpm pnpm install pnpm -w build:electron python ./bin/link_core/build_and_link_local_core.py ../core pnpm -w dev:electron --allow-unsafe-core-replacement

When trying to start it, the following error happens instead (paths shortened at …) :

Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/home/…/Delta_Chat/core/deltachat-rpc-server/npm-package/node_modules/@deltachat/jsonrpc-client/dist/deltachat.js' imported from /home/…/Delta_Chat/core/deltachat-rpc-server/npm-package/index.js

The "dist"-folder does not exist.

Could you open an issue in https://github.com/deltachat/deltachat-desktop ?
It looks like documentation bug, there is some build step for the core missing.

I was able to build and start Delta Chat Desktop, after doing git checkout v1.58.2 && pnpm install && git checkout main && pnpm install. 1.58.2 is the last version before the upgrade to React 19.

A dependency is unavailable since then. There is an issue-report about forking it: deltachat/deltachat-desktop#5935

I opened an issue-report there: deltachat/deltachat-desktop#5971

Co-authored-by: 72374 <250991390+72374@users.noreply.github.com>

Hocuri added 2 commits January 23, 2026 23:00

test: Test that image resolution isn't changed unnecessarily

10e295c

Check that the image resolution isn't changed in a second call

e4e6d3c

Hocuri requested review from iequidoo and link2xt January 23, 2026 22:18

Clarify wording

b392227

72374 reviewed Jan 23, 2026

View reviewed changes

src/blob.rs Outdated Show resolved Hide resolved

iequidoo mentioned this pull request Jan 24, 2026

fix: Do not additionally reduce the resolution of images that fit into the resolution-limit and are larger than the file-size-limit #7760

Merged

Hocuri and others added 2 commits January 24, 2026 11:15

Update src/blob.rs

f74cece

Co-authored-by: 72374 <250991390+72374@users.noreply.github.com>

Rename img_wh -> max_wh and target_wh

0bedc68

iequidoo approved these changes Jan 24, 2026 •

edited

Loading

View reviewed changes

clippy, small wording change

0731bd8

iequidoo changed the title ~~test: Test that image resolution isn't changed unnecessarily~~ fix: Don't upscale images and test that image resolution isn't changed unnecessarily Jan 24, 2026

72374 reviewed Jan 24, 2026

View reviewed changes

src/blob.rs Outdated Show resolved Hide resolved

72374 reviewed Jan 24, 2026

View reviewed changes

Update src/blob.rs

0bf3ec5

Co-authored-by: 72374 <250991390+72374@users.noreply.github.com>

Hocuri enabled auto-merge (squash) January 25, 2026 17:53

Hocuri merged commit f4ee01e into main Jan 25, 2026
30 checks passed

Hocuri deleted the hoc/test-no-unnecessary-resolution-change branch January 25, 2026 17:58

-    /// If `!is_avatar`, then if `max_bytes` is exceeded, reduces the image to `max_wh` and proceeds
-    /// with the result (even if `max_bytes` is still exceeded).
-    ///
-    /// If `is_avatar`, the resolution will be reduced in a loop until the image fits `max_bytes`.
+    /// If `max_bytes` is exceeded,
+    /// the image will be re-encoded to `max_wh` or the original resolution, whichever is smaller.
+    /// If it then is still larger than `max_bytes` it will be reduced in resolution,
+    /// until it fits within `max_bytes`.
+    ///
+    /// If `is_avatar`, then images with a higher resolution than `max_wh`,
+    /// and images which include exif-data, will also be re-encoded.

Uh oh!

fix: Don't upscale images and test that image resolution isn't changed unnecessarily #7769

fix: Don't upscale images and test that image resolution isn't changed unnecessarily #7769

Conversation

Hocuri commented Jan 23, 2026

Uh oh!

Uh oh!

Uh oh!

iequidoo Jan 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

72374 Jan 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iequidoo Jan 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

72374 Jan 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

72374 Jan 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hocuri Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

72374 Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

iequidoo Jan 24, 2026 •

edited

Loading

72374 Jan 24, 2026 •

edited

Loading

iequidoo Jan 24, 2026 •

edited

Loading

72374 Jan 25, 2026 •

edited

Loading

72374 Jan 25, 2026 •

edited

Loading

Hocuri Jan 26, 2026 •

edited

Loading

72374 Jan 26, 2026 •

edited

Loading