Skip to content

Commit

Permalink
LibGfx: Change PNGLoader to always return premultiplied color data
Browse files Browse the repository at this point in the history
Although the PNG format inherently works with unpremultiplied color
data, we are seeing issues with Skia trying to blend unpremultiplied
colors and producing the typical erroneous darker color bands.

Our LibWeb tests actually had a couple of screenshot tests that exposed
these graphical glitches; see the big smiley faces in the CSS
backgrounds tests for example. The failing tests are now updated to
accommodate PNGLoader's new behavior.

Chromium seems to apply the same behavior; i.e. it actively decodes PNGs
to a premultiplied bitmap instead, citing the same shortcoming in Skia.

Fixes LadybirdBrowser#3691.
  • Loading branch information
gmta committed Feb 26, 2025
1 parent f2916cb commit dd308bd
Show file tree
Hide file tree
Showing 10 changed files with 56 additions and 0 deletions.
22 changes: 22 additions & 0 deletions Libraries/LibGfx/Bitmap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -245,4 +245,26 @@ bool Bitmap::visually_equals(Bitmap const& other) const
return true;
}

void Bitmap::set_alpha_type_destructive(AlphaType alpha_type)
{
if (alpha_type == m_alpha_type)
return;

if (m_alpha_type == AlphaType::Unpremultiplied) {
for (auto y = 0; y < height(); ++y) {
for (auto x = 0; x < width(); ++x)
set_pixel(x, y, get_pixel(x, y).to_premultiplied());
}
} else if (m_alpha_type == AlphaType::Premultiplied) {
for (auto y = 0; y < height(); ++y) {
for (auto x = 0; x < width(); ++x)
set_pixel(x, y, get_pixel(x, y).to_unpremultiplied());
}
} else {
VERIFY_NOT_REACHED();
}

m_alpha_type = alpha_type;
}

}
1 change: 1 addition & 0 deletions Libraries/LibGfx/Bitmap.h
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ class Bitmap : public RefCounted<Bitmap> {
[[nodiscard]] bool visually_equals(Bitmap const&) const;

[[nodiscard]] AlphaType alpha_type() const { return m_alpha_type; }
void set_alpha_type_destructive(AlphaType);

private:
Bitmap(BitmapFormat, AlphaType, IntSize, BackingStore const&);
Expand Down
9 changes: 9 additions & 0 deletions Libraries/LibGfx/Color.h
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,15 @@ class Color {
return color_with_alpha;
}

constexpr Color to_premultiplied() const
{
return Color(
red() * alpha() / 255,
green() * alpha() / 255,
blue() * alpha() / 255,
alpha());
}

constexpr Color to_unpremultiplied() const
{
if (alpha() == 0 || alpha() == 255)
Expand Down
6 changes: 6 additions & 0 deletions Libraries/LibGfx/ImageFormats/PNGLoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,12 @@ ErrorOr<size_t> PNGLoadingContext::read_frames(png_structp png_ptr, png_infop in
row_pointers[i] = frame_bitmap->scanline_u8(i);

png_read_image(png_ptr, row_pointers.data());

// Convert the frame to premultiplied alpha. Apart from the fact that it's theoretically a bit faster to blend
// premultiplied pixels, Skia eventually runs into issues when trying to blend unpremultiplied pixels when
// scaling up and using linear interpolation.
frame_bitmap->set_alpha_type_destructive(AlphaType::Premultiplied);

return frame_bitmap;
};

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<style>
* {
margin: 0;
}
body {
background-color: cyan;
}
</style>
<!-- To rebase:
1. Open image-unpremultiplied-data.html in Ladybird
2. Resize the window just above the width of the largest element
3. Right click > "Take Full Screenshot"
4. Update the image below:
-->
<img src="../images/image-unpremultiplied-data-ref.png">
Binary file modified Tests/LibWeb/Screenshot/images/border-radius-ref.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified Tests/LibWeb/Screenshot/images/canvas-filters.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified Tests/LibWeb/Screenshot/images/css-backgrounds-ref.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions Tests/LibWeb/Screenshot/input/image-unpremultiplied-data.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<!DOCTYPE html>
<link rel="match" href="../expected/image-unpremultiplied-data-ref.html" />
<img style="width: 200px; height: 200px" src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAADIAAAAyCAYAAAAeP4ixAAAAxUlEQVRoge2Z0Q6EIAwEwf//Z+5pE0P0BCxlLTvvKOOSqm1KQghhRSmpzL5HtrzYyIZzttmDyUUsnvhboVeLZxyZUaGhRS5nvlOoW8RDAvTIdIl4SoBWmWaRFRKgRaZJZKUEeJI5vDYym8dEGNIA/1LZIxGmNMBdKvETYUwDXKUSJhGJsCERNi6rFnPFAnXlUvllQyJsSIQNfcazoV9dNtQOYmOvTiMI0fsFIbrxIMR85MznJ1Y1n58h1qyc6rrA8EIVYnd+9SZLMlMCtbAAAAAASUVORK5CYII=">

0 comments on commit dd308bd

Please sign in to comment.