LadybirdBrowser / ladybird

Truly independent web browser
https://ladybird.org
BSD 2-Clause "Simplified" License
21.92k stars 970 forks source link

CSS property `image-rendering` is not handled correctly #1170

Closed simonkrauter closed 2 months ago

simonkrauter commented 2 months ago

Looks like image-rendering: crisp-edges and image-rendering: pixelated are swapped.

Test page in Ladybird:

Test page in Chrome:

Test page in Firefox:

AtkinsSJ commented 2 months ago

Please read the spec if you haven't already. Our existing behaviour looks correct to me. https://www.w3.org/TR/2023/CRD-css-images-3-20231218/#the-image-rendering

simonkrauter commented 2 months ago

@AtkinsSJ Thanks for the spec link.

From the spec:

crisp-edges: The image is scaled in a way that preserves contrast and edges, and which avoids smoothing colors or introducing blur to the image in the process.

pixelated: The image is scaled in a way that preserves the pixelation of the original as much as possible, but allows minor smoothing as necessary to avoid distorting the image when the target size is not a clean multiple of the original.

It's interesting, that Chrome and Firefox behave different. I have added a screenshot of Firefox above.

My conclusion is that Ladybird behavior for crisp-edges is correct, but for pixelated it should be changed, to match the spec, Firefox and Chrome. I will update the PR accordingly.

See also nico's comment on the PR: https://github.com/LadybirdBrowser/ladybird/pull/1171#discussion_r1729358821

gmta commented 2 months ago

~The way I read those, is that we need nearest neighbour interpolation for pixelated and smooth pixels for crisp-edges. I.e. what Firefox is doing seems right.~

Pixelated -> smooth pixels Crisp-edges -> nearest neighbour

AtkinsSJ commented 2 months ago

Crisp edges also says

The image may be scaled using nearest neighbor or any other UA-chosen algorithm that does not blur edges or blend colors from the source image.

It specifically doesn't want any blending.

Pixelated however, allows smoothing when the image isn't scaled to an exact multiple of the original size, so that we don't get the artifacts that nearest-neighbour gives. I can't remember what our smooth pixels algorithm does but it's clearly not doing that so swapping it for nearest neighbour is fine imo. Or maybe it just needs fixing.

gmta commented 2 months ago

@AtkinsSJ you are right, I got them mixed up. Our smooth pixels scaling algo should blend the edges of (only) scaled up pixels in an anti-aliasing fashion, so we're blending at most one pixel high or wide in the target image. If that doesn't work correctly, we should fix it.

simonkrauter commented 2 months ago

I understand that Skia is doing the scaling now.

simonkrauter commented 2 months ago

For the purpose of better understanding, this is the current translation from Gfx::ScalingMode to SkFilterMode:

static SkSamplingOptions to_skia_sampling_options(Gfx::ScalingMode scaling_mode)
{
    switch (scaling_mode) {
    case Gfx::ScalingMode::NearestNeighbor:
        return SkSamplingOptions(SkFilterMode::kNearest);
    case Gfx::ScalingMode::BilinearBlend:
    case Gfx::ScalingMode::SmoothPixels:
        return SkSamplingOptions(SkFilterMode::kLinear);
    case Gfx::ScalingMode::BoxSampling:
        return SkSamplingOptions(SkCubicResampler::Mitchell());
    default:
        VERIFY_NOT_REACHED();
    }
}
gmta commented 2 months ago

Hm, afaics Skia only supports nearest neighbour, nothing similar to smooth pixels to be found. Oh well, NN is definitively an definitive over linear sampling.