Skip to content
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

Indexed images appear dithered in previews #25436

Closed
hatkidchan opened this issue Nov 25, 2022 · 29 comments · Fixed by desktop-app/patches#119
Closed

Indexed images appear dithered in previews #25436

hatkidchan opened this issue Nov 25, 2022 · 29 comments · Fixed by desktop-app/patches#119

Comments

@hatkidchan
Copy link

Steps to reproduce

  1. Have a perfect, non-dithered image without any gradients with plain background and some text
  2. Send it in some chat and look at a modal window before uploading

Expected behaviour

Image should look crispy clear

Actual behaviour

Image looks noisy, almost like it's dithered. Sent image itself is untouched, only previews are suffering.

image
image

Operating system

Arch Linux, i3

Version of Telegram Desktop

4.3.1-2

Installation source

Other (unofficial) source

Logs

No response

@hatkidchan hatkidchan added the bug label Nov 25, 2022
@ilya-fedin
Copy link
Contributor

I believe such strong compression exists for a reason

@hatkidchan
Copy link
Author

It's not a compression issue, image itself doesn't look dithered or noisy, that problem happens only in previews. So it's more like a visual glitch than anything else

@ilya-fedin
Copy link
Contributor

The preview is jpeg compressed IIRC

@hatkidchan
Copy link
Author

Is there any reason why images are compressed in previews?

@ilya-fedin
Copy link
Contributor

Yeah, I believe there is, I don't know which one exactly though

@zvova7890
Copy link

zvova7890 commented Nov 28, 2022

hatkidchan, Have you interface scale? Can you try to build telegram with qt5 and check?

qt6/X11 with UI scale 1.5x by KDE settings all images looks like they upscaled from 4x4 image

qt5 telegram-desktop-qt5
qt6 telegram-desktop-qt6

And all chat/groups icons, all emoji, all pictures looks like that. I see that bug at least a year, maybe more.

@ilya-fedin
Copy link
Contributor

It's sadly a KDE bug and AFAIK no one still reported it to them. They tell scale factor to Qt in a wrong way that overrides the High-DPI scale factor rounding policy tdesktop sets to Qt. After that override the scaling in tdesktop just can't work properly.

@ilya-fedin
Copy link
Contributor

See #25126, it has the link to first discussion

@ilya-fedin
Copy link
Contributor

ilya-fedin commented Dec 31, 2022

@hatkidchan there was some work related to color spaces support, do you see any changes? Note that you should use an official build in order to see the difference as all changes were in Qt patches.

@hatkidchan
Copy link
Author

image
Still happens on latest release from https://github.com/telegramdesktop/tdesktop/releases/tag/v4.5.0

@ilya-fedin
Copy link
Contributor

Honestly saying I don't see anything abnormal on the screenshot. Some comparison would be nice.

@hatkidchan
Copy link
Author

Looks like this happens when there's a lot of colors on the image
image image

Maybe it's the problem with image having more than 256 colors and Telegram rendering them as indexed palette for some reason?

@ilya-fedin
Copy link
Contributor

I meant comparison with how it should look like

@hatkidchan
Copy link
Author

I don't think I could replicate that in Telegram. Though, I just noticed that it only happens when image is quite large (I'm upscaling them 2x)..

I think I found the problem. It happens when Telegram tries to scale down image with indexed palette. When converting same image to rgb24 instead of pal8, it looks like it should:

image

Though, I don't really get the reason of that problem: there's definitely less than 256 colors on that image, and scaling indexed should not resolve it appearing dithered. Maybe there's a problem somewhere in png/pal8->jpeg/rgb24 conversion? I tried converting it myself to jpeg using imagemagick's convert and it's not noisy as on previous screenshots

@ilya-fedin
Copy link
Contributor

I looked at the code generating the preview and there's no conversation in jpeg. It only uses Qt's QImage::scaledToWidth and then writes with white color on transparent pixels if the image has alpha channel

auto scaled = preview.scaledToWidth(
toWidth,
Qt::SmoothTransformation);

https://github.com/desktop-app/lib_ui/blob/3fea35ff19b52da94fa3ed17695f00b79bfb1917/ui/image/image_prepare.cpp#L1180-L1199

@ilya-fedin
Copy link
Contributor

I don't think I could replicate that in Telegram. Though, I just noticed that it only happens when image is quite large (I'm upscaling them 2x)..

I think I found the problem. It happens when Telegram tries to scale down image with indexed palette. When converting same image to rgb24 instead of pal8, it looks like it should:

Both those findings seem to be right. Since Qt 6.0, it doesn't seem to follow the SmoothTransformation hint if scaling more than 2x down and the format is not one of whitelisted: qt/qtbase@b0b4fcd

@ilya-fedin
Copy link
Contributor

You can try to revert the commit on your system Qt package and install tdesktop via package manager or I can provide a build with the Qt commit reverted for test. What do you want?

@hatkidchan
Copy link
Author

It's not that significant issue, just a small annoyance. I think it'd be better for me to change title of that issue to more meaningful and close this, since it's a problem in third-party library

@hatkidchan hatkidchan changed the title Images appear dithered in previews Indexed images appear dithered in previews Dec 31, 2022
@hatkidchan
Copy link
Author

Though, I just noticed that it also happens in sent images as well, are they being scaled down before sending using the same function as for preview? It doesn't happen when I send the same image using bot, so it may be the case here..

@ilya-fedin
Copy link
Contributor

Yeah, compressed images should be affected as well

@ilya-fedin
Copy link
Contributor

I've created an Indexed image in GIMP and can confirm the issue. Can also confirm it doesn't happen with Qt 5.

@hatkidchan
Copy link
Author

hatkidchan commented Dec 31, 2022

Maybe it'd be better to convert image from indexed to rgb24 or any other supported format and then scale it down? I can't imagine any other way of fixing that within app itself without making changes in Qt6

@ilya-fedin
Copy link
Contributor

Will see what approach @john-preston will choose. Converting everything to RGB or patching Qt.

@ilya-fedin
Copy link
Contributor

I don't see any note in Qt documentation that SmoothTransformation doesn't even apply, wonder whether it could be reported as a Qt bug...
https://doc.qt.io/Qt-6/qimage.html#scaled
https://doc.qt.io/Qt-6/qt.html#TransformationMode-enum

@ilya-fedin
Copy link
Contributor

Looking more at the code, it seems the image I test with fits the requirements for smoothScaled (its size is more than 1<<20), so apparently it's caused by Qt converting it back to Indexed8 format.

@ilya-fedin
Copy link
Contributor

I can confirm patching Qt to remove convertToFormat(format()) helps, so apparently Qt doesn't support indexed format good and the conversion is lossy

@ilya-fedin
Copy link
Contributor

@ilya-fedin
Copy link
Contributor

There's sadly too much candidates for pre-converting to RGB format and I likely miss some of them, there are also may appear new places that may forget converting. Since it would be too easy to regress, I think patching Qt is the best way forward. If they will reject to fix this or request to invest too much time (like requiring a minimum reproducible example), I will leave fixing this in 3rd party builds for 3rd party redistributors (or any other interested party) if they need such a fix (i.e. to pick up the bug report or tdesktop Qt patchset).

@hatkidchan
Copy link
Author

It's more than enough already for such a small issue. It is an annoyance, for sure, but this should be done on Qt side, since it was working flawlessly in older versions. I'm gonna check from the to time on that bug report in At in hope they fix it eventually, but for the time being I guess I'm just gonna convert images from indexed palette myself, no big deal

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants