Incorrect color bleed when applying processors to images with alpha

[vpenades]

Prerequisites

• [x] I have written a descriptive issue title
• [x] I have verified that I am running the latest version of ImageSharp
• [x] I have verified if the problem exist in both DEBUG and RELEASE mode
• [x] I have searched open and closed issues to ensure it has not already been reported

Description

When dealing with images that containt transparent pixels, some filters don't work correctly. Essentially, all processors that take the surrounding of a given pixel as input (the kernel) and return a single pixel color as output.

Usually, in images with transparent pixels, the fully transparent pixels have a black color,

The problem is that the kernels of these processors indiscriminately take all the pixel colors and handle each channel separately, without taking into account if the pixels are transparent, and should be discarded.

I am aware that handling alpha in such important filters as Resize and Blur might be difficult, and if handling transparency correctly, might end slowing down considerably.

Solution1: to make all these processors handle transparency correctly, by weighting in the transparency amount into the kernel solution. For example, if a BoxBlur kernel peeks 25 pixels, and all but 2 pixels are fully transparent, only the colors of these two non transparent pixels should be accounted for in the final solution.

Solution2: before applying any of these filters, do some sort of preprocessing that would "bleed" the non transparent colors to the transparent surrounding. Even if all these pixels remain transparent, when aplying processors like blur or resize, the end result will have much better looking borders.

Here's an example of how the transparent pixels look like when they're all made opaque. This particular solution is typical for texture mipmaps.

Steps to Reproduce

Case 1: Blur Filters 1- Create a new 256x256 image. 2- Draw a 32x32 white rectangle at the center of the image. 3- Apply a GaussianBlur.

Result: the blur around the white rectangle has a gradient to Black.

Expected: all the visible pixels should be white.

Case 2: Resize Filters 1- Create a new 256x256 image. 2- Draw some thin lines, text, or whatever that has a high frequency detail. 3- Downsize the image to 250x250.

Result: the resize filter has peeked a bit of the surrounding of the text, which is transparent black, that has bleeded to the final image. Expected: all the pixels should be white.

System Configuration

• ImageSharp version: 1.0.0 beta 2
• Other ImageSharp packages and versions:
• Environment (Operating system, version and so on):
• .NET Framework version:
• Additional information:

[JimBobSquarePants]

Hi @vpendes,

Thanks for all the detail. Can you do me a favour and add something that is missing though? Reference output of equivalent operations created with another library or two?

Without such we have no indicator of what the correct output should be, only your expectation.

Cheers

James

[vpenades]

Here's the examples:

OriginalTransparentWhite and OriginalTransparentBlack are, from the user's perspective, essentially the same.

OriginalTransparentWhite: transparent pixels are white colored. OriginalTransparentBlack: transparent pixels are black colored. GaussianRadiusOne: Applying GaussianBlur filter of radius 1 in Photoshop (same result both images) DownsizedOnePixel: Resized from 256x256 to 255x255 in photoshop (same result both images)

In imagesharp, you will get "bleed to black" if you perform these operations in the OriginalTransparentBlack image.

The rule of thumb is that, for any operation, if the pixel is fully transparent, it's color should be irrelevant for any subsequent operation, and that not only includes blending, but also blurs, resizes, etc.

I can figure supporting this is expensive... my guess is that IPixel could have a fast bool IsFullyTransparent property to check if an image has any fully transparent pixel, and choose between the current paths and the "transparent pixel aware" paths.

TransparentReferenceExamples.zip

Btw, you could add OriginalTransparentWhite and OriginalTransparentBlack to the unit tests... any filter processing these two images with the same parameters, should produce exactly the same final image.

[JimBobSquarePants]

Thanks for the images. I appreciate it.

The rule of thumb is that, for any operation, if the pixel is fully transparent, it's color should be irrelevant for any subsequent operation, and that not only includes blending, but also blurs, resizes, etc.

I would argue that should not actually be a hard rule. You can lose pixel information that could be important further down the line.

I can figure supporting this is expensive... my guess is that IPixel could have a fast bool IsFullyTransparent property to check if an image has any fully transparent pixel, and choose between the current paths and the "transparent pixel aware" paths.

We'd have to check the W property on the expended Vector4 itself as anything else would require double unpacking which would be computationally infeasable.

What I think we are looking at here as a solution is premultiplication.

You can see the theory here http://entropymine.com/imageworsener/resizealpha/

We'd need to create fast operations for performing premultiplication perations and the reverse on rows of images and single pixels to correctly handle the alpha component.

We've actually taken that into consideration when performing affine transforms in our new Affine transform methods PR.

LibVips went through similar issues.

https://github.com/lovell/sharp/pull/213 https://github.com/jcupitt/libvips/issues/291

[JimBobSquarePants]

Interestingly LibVips has added Premultiply and Unpremultiply methods to the image itself. That definitely the simplest solution! (Though maybe not the most performant)

https://github.com/jcupitt/libvips/issues/291#issuecomment-100172537

@antonfirsov @tocsoft @dlemstra Would love to have your thoughts here.

[vpenades]

@JimBobSquarePants I feel like the Premultiply/Unpremultiply trick would degrade the color precission if applied to 8 bit component pixels... it's something that can only be done at full Vector4 precission.

[JimBobSquarePants]

@vpenades Good point. We'll have to go with the first approach of working directly on the Vector4 within the algorithms.

[vpenades]

@JimBobSquarePants Also, I feel like the algorythms for BoxBlur and GaussianBlur would have to take a different approach than premultiply/unpremultiply.

In the case of blur, you actually blur only the visible pixels.

[JimBobSquarePants]

@vpenades I disagree. If you look at the linked issues it's blur they specifically mention.

[vpenades]

@JimBobSquarePants A well, yes, they also mention Blur.

Anyway, the premultiply trick is something used for realtime rendering, because it's indended to be a "final effect", and the loss of quality/accuracy is acceptable.

For proper image processing, I feel like the main processing path should be accurate and strict on the results... after than it's fine to add faster paths on special cases (like fully opaque images)

[vpenades]

@JimBobSquarePants After reviewing the code, I think the color bleeding issue could be fixed here:

ResamplingWeightedProcessor.Weights.cs L94

By changing this line: result += v * weight; to: if (v.W >0) result += v * weight;

Similarly, here:

Convolution2PassProcessor.cs L117

From: destination += kernel[fy, fx] * currentColor; To: if (currentColor.W >0) destination += kernel[fy, fx] * currentColor;

There's probably more places like this one that suffer of the same problem.

Also, not sure, but maybe this solution is faster/simpler than doing an alpha premultiply/postmultiply step.

[JimBobSquarePants]

@vpenades Some pixels formats, e.g Short4 operate in the vector range -37267 to 37267so we can't do that. Premultiplication and reversal is the answer, I just need to get round to it.

[saucecontrol]

@JimBobSquarePants Not sure if you've got this covered, but hopefully this helps...

@vpenades Your proposed solution essentially treats all transparent pixels as being black. That is, if currentColor is 0, the result of multiplying by the kernel weight is also 0, so it doesn't matter whether you add it to the destination accumulator or not. Of course, if there's random color data in there, treating it as black is better than nothing, but it doesn't solve your stated problem in the issue.

Essentially, the problem is that if you skip a pixel, that pixel's weight in the kernel is never accounted for. In order to produce accurate output colors, you must normalize the kernel weights such that they total to 1. In the case of a blur, skipping a pixel results in a total applied weight less than 1 and has the effect of darkening any pixels that would have been partially influenced by the transparent pixel. Again, it's the same result as if the pixel were black.

The solution is to keep track of any weights that were discarded so that you know how much total weight factored into the destination pixel's final value. That value can then be re-scaled to account for the different weight factor. I do this in my non-SIMD pipeline if you want to see how it works. (Apologies for the non-human-friendly code -- it's T4 generated with loop unrolling and such)

It also works out that if you're using a premultiplied format, the value of the alpha channel post-convolution will be affected in the same way as the color channels. The result being that when you unpremultiply, the value of the color channels is scaled back up by the same factor, restoring the proper color. Essentially, the alpha channel acts as a proxy for the applied kernel weight, and normalization is applied as the premultiplication is reversed. Bit of magic, really... but it works a treat.

So premultiplication does work, regardless of the kernel type, but it's not the only way to solve the problem.

[saucecontrol]

Oops, I didn't see you had replied while I was typing all that out. But man, I love it when writing something out makes me see something I hadn't before. As I was sitting down to dinner this evening, it occurred to me I wasn't handling partially-transparent pixels correctly, because they need to have their weight reduced proportionally as well. I'll be pushing some updated code to reflect that, so at least I helped myself if I didn't help you :)

The other thing I didn't mention (although this might be obvious to you) is that if you eliminate pixels that have negative weight in the kernel, the output will end up brighter. The same happens if you clamp premultiplied pixel values before reversing the premultiplication. If a pixel ends up brighter than it should be, it should also have an alpha value >1, so it would normalize when you unpremultiply it. If you clamp the alpha before that, though, the pixel is left overly bright. Very few apps/libraries get that right. The light line artifacts I describe here in the System.Drawing output are caused by that premature clamping, even after you work around its other issues.

[vpenades]

@JimBobSquarePants what about defining a threshold value for every pixel format, that tells at which point the pixel becomes fully transparent? It could even be disabled by passing a negative infinity.

@saucecontrol the RGB elements of a fully transparent pixel should be treated as irrelevant, not as black

For simplicity, let's assume two images of 2x1 pixel, and we want to reduce them to 1x1 pixel:

Image 1: 255,255,255,255 - 0,0,0,0 Image 2: 255,255,255,255 - 255,0,0,0

From the point of view of a user viewing both images, the two images are visually equal, so all operations applied to these two images should be the same. In the example I've provided, both images should shrink to a single, 255,255,255,127 , white, half transparent pixel. The RGB part of the second pixel is irrelevant in this result.

[saucecontrol]

@vpenades That's correct. I was pointing out that the code changes you suggested have the same result as treating the pixel as black -- or reading an actual black pixel.

Imagine a simple averaging filter applied to that image. The kernel would be defined as [0.5, 0.5].

So you multiply the first image by the kernel and you get [127, 127, 127, 127] + [0, 0, 0, 0] = [127, 127, 127, 127]. If you simply skip over the second pixel you get [127, 127, 127, 127] + [skipped] = [127, 127, 127, 127].

Point is, if you don't adjust the weights to account for the fact that pixel was undefined, it's the same as if you blended a black pixel in.

As @JimBobSquarePants pointed out, premultiplication is the silver bullet solution here. I was simply trying to explain the alternatives and why they work.

Edit: Just to finish the scenario out so it's extra clear...

If you assume the input image is premultiplied, then when you reverse that, you end up with the correct [255, 255, 255, 127], assuming you were carrying enough precision.

If you don't want to premultiply the input and use the extra memory, you can get the same result by re-normalizing the kernel weights. In this case, you would save off the 0.5 weight you skipped for the second pixel and use that to adjust the result back up to a full weight of 1. Again, you get the correct result of [255, 255, 255, 127].

[vpenades]

I took the liberty to prepare this image as a test case:

At first sight it looks like Sistine Chapel painting. @JimBobSquarePants I would suggest to add it to the test images.

I've only tried with Resize and GaussianBlur, most probably, all convolutional filters are affected, but it's possible other filters might look weird too.

So far, Resize and GaussianBlur go Kaboom! with this image.

[JimBobSquarePants]

@saucecontrol Many thanks for your input here, I had not covered the weight normalization requirements and they are vital to producing the correct output. What I'll look at is per-row (or per-pixel if I can't do row) premultiplication and reversal within the algorithms themselves to that I'm always working in floating point and do not lose precision. A couple of extension methods to Vector4 should make that easier. Your blog posts btw are simply brilliant!

@vpenades Thanks for the test image, I'll use it to test against and will most likely add it to the repo. This task is now my priority since everyone else is working on memory management. Hopefully I can get a fixed build out in the next couple of days.

[JimBobSquarePants]

@vpenades Here's the result of blurring your test image using premultiplication (dead easy to add to the source since they all use convolution base classes) Is this the output you would expect? If so I can fix resize also.

[JimBobSquarePants]

Haha! @vpenades you're a genius! 🤣

This should be a standard test image for all libraries.

[saucecontrol]

Oh man, that image is great :D

@vpenades Do you mind if I steal that for my test image suite as well?

@JimBobSquarePants I'm happy if I was able to help, although it seems you had everything under control with your premultiplication plan. And thanks for the kind words re: the blog. I keep thinking of things to write about and can never find the time. I'm sure you can relate :)

[JimBobSquarePants]

@saucecontrol I'm terrible for blogging. I know I should do more but I find it so difficult to string enough of what is going on in my head together to be meaningful.

[vpenades]

@JimBobSquarePants that image was the best way I could think to make the problem apparent, sometimes it's not easy to explain with words 😄

@saucecontrol Sure you can use it. 👍

[JimBobSquarePants]

Aaaaaand it's in, thanks for the suggestion and excellent testing image.