Fix garbled colors when scaling indexed-color images
Fixes bug where indexed-color or monochrome-palette images (eg. from pngquant) would render with garbled colors or B&W noise when zoomed.
Test images: gwenview-indexed.zip
In this PR, I converted images from indexed colors to (A)RGB32 even when not zooming. This prevents RasterImageItem::applyDisplayTransform() -> updateDisplayTransform()
from printing error "Gwenview cannot apply color profile on QImage::... images" depending on zoom level.
-
image.scaled()
can return various formats. Unscaled images returnQImage::Format_Mono=1
orQImage::Format_Indexed8=3
(unsupported), while scaled images without/with alpha returnQImage::Format_RGB32=4
(ok) orFormat_ARGB32_Premultiplied=6
(unsupported). (Qt doesn't appear to outputQImage::Format_ARGB32=5
from indexed images?) - Somehow zooming into a no-alpha image (like
solid-g-fs8.png
) withQImage::Format_RGB32=4
"creates" transparency, returningFormat_ARGB32_Premultiplied=6
instead.-
When we scale an opaque image and it produces premultiplied alpha, we convert to QImage::Format_RGB32=4
without alpha. I hope this doesn't break any invariants.
-
- Sidenote: if I remove the call to
image.convertTo(...)
and open an unsupported image, "cannot apply color profile" is only printed on first failure, becausemApplyDisplayTransform
is set to false and not attempted again.
I verified that wide-gamut images (Webkit-logo-P3.png) are still clipped to SDR gamut. I verified (using a screenshot and eyedropper, and switching between RGB/indexed images) that wide-gamut indexed images (gradient-indexed.png
) are now properly clipped rather than being desaturated, but I don't think this type of image is common (I think true-color replaced indexed color before colorspace tagging became mainstream?).
Unfortunately I do not have a wide-gamut display to verify that Gwenview shows indexed images as sRGB on wide-gamut displays, but my changes follow existing codepaths that do this (RasterImageItem::updateDisplayTransform()
→ profile = Cms::Profile::getSRgbProfile()
).
-
Should I convert opaque indexed images to QImage::Format_RGB888
? This saves 25% of memory, but is inconsistent with how Qt/Gwenview loads opaque PNG/GIF files toQImage::Format_RGB32
(with a dummy FF alpha). -
Is testing for image.colorTable()
the best way to detect indexed color and convert to (A)RGB32 (becauseimage.convertTo(originalImageFormat)
will break colors orRasterImageItem::updateDisplayTransform()
does not support the format)?- I could explicitly test for every format supported by
RasterImageItem::updateDisplayTransform()
, but the list inRasterImageItem::paint()
could go out of sync with the switch inRasterImageItem::updateDisplayTransform()
.
- I could explicitly test for every format supported by
- Hot take: Should we run LittleCMS transforms directly on premultiplied alpha (so we don't spend time converting formats)? Brightness-invariant transforms will be unaffected, but this introduces errors to gamma or nonlinear operations. (Are they still smaller than how Gwenview blends translucent images onto the background in gamma space rather than linear light? What even is correct alpha blending...)
The existing code fails to handle images which are already in premultiplied alpha before scaling.
- PNGs are never premultiplied-alpha (so this does not happen), but I found that some (animated?) GIFs are loaded as
Format_ARGB32_Premultiplied=6
, which trips the "cannot apply color profile" error. (Oddly Qt treats GIFs as non-indexed, possibly because each frame of an animation has its own index and can overlay on previous frames?) - GIMP also outputs TIFF files with premultiplied alpha (
Webkit-logo-P3.tiff
isQImage::Format_RGBA64_Premultiplied=27
, and without this MR prints "cannot apply color profile" and looks different fromWebkit-logo-P3.png
).- GIMP can export 10/12 bit HEIF/AVIFs, but even without an alpha channel, Qt loads them as
QImage::Format_RGBX64=25
rather thanQImage::Format_BGR30
(even though the latter was added earlier?)
- GIMP can export 10/12 bit HEIF/AVIFs, but even without an alpha channel, Qt loads them as
I fixed this bug by renaming QImage::Format originalImageFormat
to target/outputImageFormat
(removing bool isIndexedColor
from my first commit), and find-and-replace Format_ARGB32_Premultiplied
with Format_ARGB32
, and Format_RGBA64_Premultiplied
with RGBA64.
-
What other formats should we convert? I've converted all outputs supported by updateDisplayTransform()
, but there are other formats, premultiplied and not (like <8-bit, >8-bit, or floating-point) we can convert into a supported one.- Hmm should I convert all formats except ones Little CMS supports, or just the ones I've found when loading real image files?
- I'm unsure about the code formatting around
switch (outputImageFormat)
. clang-format wouldn't allow one-liners, thedefault
is there to avoid warnings and the semicolon is ugly butbreak
adds another line, and if/else would be less lines of code (but writing outelse if (outputImageFormat == ...
produces longer lines).
Bug introduced by commit 33d6203c.
BUG: 459627