The easiest case to look at here is drawing the points with semi-transparent green in the cells effect, and having the cell color scheme use the palette with the primary color set to fully opaque green

The result of this should be fully opaque green.
Going through the math for the over operation with premultiplied alpha, using floats for simplicity:

A = (0, 0.5, 0, 0.5)
B = (0, 1.0, 0, 1.0)
A over B = (0, 1.0, 0, 1.0)
since the resulting color channels are 0.5 + 1.0 * (1.0 - 0.5)

(https://en.wikipedia.org/wiki/Alpha_compositing#Straight_versus_premultiplied)

But the result has a darker green - the eyedropper tool produces (0, 191, 0, 255) which is about 0.75 for the green channel, so that could maybe be produced by doing an extra multiplication by the alpha if the code is assuming unpremultiplied alpha

ColorBgra.Lerp, using the alpha of the destination color, produces the expected result, and same for setting the layer blend mode which does the blend operation through Cairo

@cameronwhite I tried to implement and thoroughly explain the logic of the "normal" blend op (see new Blending file) and it seems to work well, and now I'm using it in the effects.

I don't know if we should try to match the blending behavior of Cairo at this stage. There are many things that go into it that we would need to investigate, like whether or not Cairo does gamma correction.

Perhaps we should look at these algorithms as completely independent at first, and later on we could allow for different settings.

The commits after this message break down the rounding logic in different ways, and we could choose which commits to discard depending on the desired style.

👍 The implementation looks reasonable to me

A couple thoughts:

• Just using 128 for the rounding seems good enough to me (the original code was using 0x80 too)
• I think this should be replacing the existing NormalBlendOp - I don't really want to have multiple versions of the blend modes in the codebase, and existing uses of the blend op should also benefit from being fixed

@cameronwhite I just replaced the body of NormalBlendOp.ApplyStatic with the implementation that was in that static class.

A few notes:

• I removed the redundant overrides. They are almost exactly the same code as the original virtual methods, the only difference being that the overrides call ApplyStatic. I think the JIT compiler can optimize that out and de-virtualize and/or inline these calls.
• I removed the child class NormalBlendOpWithOpacity and the method CreateWithOpacity. It wasn't being used and if we want the blend ops to offer that in the future we could re-implement it in a cleaner way. I think there is no need to create a new class for that...for every blend op, no less. I have to think about this, but I think it doesn't need to be implemented separately for every blend op, but it can be implemented once and then composed.
• OutlineObjectEffect was using this blend op, so its outputs changed and I had to update the tests.
• The gradient tool is using this blend op, so that's something to audit, too
• This was not implemented by Jonathan Pobst, so I think we could safely remove the license header? That is in a separate commit, so we can decide on that

In 64d9f1c I made some optimizations similar to those in #1703, but for the files involved in this pull request. ...and for some reason the output is not the same, but I will check that in a different pull request.

• I think the JIT compiler can optimize that out and de-virtualize and/or inline these calls. - this seems to be the case in the benchmarks I ran. I'll put together a PR which adds blend op benchmarks to the PintaBenchmarks project
• The PDN license header can be removed here since it's been entirely rewritten from scratch
• I think I've gone through the gradient tool and otherwise fixed the premultiplied alpha issues in its own code, so the blend ops were just the remaining case

This looks good to merge, but it would be great to also add a test for the blend op first

I just created a test setup for the blend ops, and test cases for the normal blend op @cameronwhite.

There are two different commits:

• The first one (cf38fdb) includes cases where at least one color channel is greater than the alpha, which makes the color invalid in premultiplied alpha
• In the second one, the blend op throws an exception if an invalid color is passed to it, and adds a test that checks that verifies that the exception is thrown

I don't know if any existing tool uses invalid color values. I don't want to break stuff that is working, but perhaps if it's using invalid colors it should be failing loudly.

Thanks! I was thinking it would be good to have some visual tests for these too, something like https://www.pinta-project.com/user-guide/edit/#apply-blend-mode where it's easier to see what effect is produced

I'm very uneasy about validating colors here for a couple reasons:

• If we do want to check for unexpected color values, deep inside an inner loop of specific algorithms isn't really a good place. That means we'd have to add it everywhere for complete coverage, and that'll just add performance overhead and touch a lot of code. Somewhere more general like the SimpleHistoryItem would capture most modifications to a layer that Pinta makes
• Since still have known issues with premultiplied alpha, this could cause a lot of random crashes for users as soon as they have a transparent layer, and I really don't want to deal with an influx of bug reports about Pinta being unstable. At most, this should be a warning in the console, and probably only turned on for developers builds for testing purposes

@cameronwhite yeah, that's understandable. I just reverted it. In any case, the inputs with invalid colors are clearly separated in the code, so they should be easy to remove in the future if we are absolutely sure.

And performance is another very good point. I'm not sure that the JIT could do much there, so maybe the validation code belongs elsewhere and the blending methods should assume that the inputs are valid.

I am now going to create some visual tests.

@cameronwhite I just added visual tests. The images used were generated from scratch so that we don't have to deal with any licensing or copyright issues. Unfortunately I didn't find information about the ducky.png picture used by the W3C specification documents

import numpy as np
from PIL import Image

TARGET_SIZE = 250
SCALE_FACTOR = 4
SUPER_SIZE = TARGET_SIZE * SCALE_FACTOR

def get_arr_a(y, x, ss):
    arr = np.stack([255*(ss-x)/ss, 255*(x+y)/(2*ss), 255*y/ss, 255*(ss-y)/ss], -1)
    cx, cy, r = ss//2, ss//2, ss//4
    arr[(x-cx)**2 + (y-cy)**2 < r**2] = [255, 255, 0, 200]
    return arr

def get_arr_b(y, x, ss):
    cx, cy, arr = ss//2, ss//2, np.zeros((ss, ss, 4), 'u1')
    arr[..., 3] = 255
    arr[..., 2] = 255 * (1 - np.clip(np.hypot(x-cx, y-cy) / (ss*2**-0.5), 0, 1))
    bw = ss//8
    arr[:,ss//4-bw:ss//4] = [255,0,0,180]
    arr[:,ss//2-bw:ss//2] = [0,255,0,180]
    arr[:,3*ss//4-bw:3*ss//4] = [0,0,255,180]
    tsize = ss//16
    in_zone = (x > cx) & (y > cy)
    is_black = ((x // tsize + y // tsize) % 2).astype(bool)
    arr[in_zone & is_black], arr[in_zone & ~is_black] = [0,0,0,255], 255
    return arr

content_factories = {'a': get_arr_a, 'b': get_arr_b}
for name, generator_func in content_factories.items():
    y, x = np.mgrid[0:SUPER_SIZE, 0:SUPER_SIZE]
    arr = generator_func(y, x, SUPER_SIZE)
    img = Image.fromarray(np.clip(arr, 0, 255).astype('u1'), 'RGBA').resize((TARGET_SIZE, TARGET_SIZE), Image.Resampling.LANCZOS)
    img.save(filename := f"visual_{name}.png")
    print(f"Saved '{filename}'")

The best part is that I generated the blended output image with Pinta layers (not through the re-written blend op). So it appears that the blend op works the same as Cairo's!

And as a sanity check to verify that the tests are actually testing the image, I altered the blended version in 3af4819, and reverted it afterwards.

Thanks! Those tests look good, and generating the output with the regular layer blending is great to make sure they match as closely as possible