[cairo] Blend modes take 3

[Soeren Sandmann]

Hi Benjamin,

> > Maybe this one could be done by simply using the blending with white
> > and using the Difference blend mode? Is there a way that Subtract can
> > be made more regular?
> >
> Uh yeah, INVERT is of course just difference to white, so a simple
> push/pop_group() and using the result as mask with a white difference
> should take care of this.

So far as pixman and Render are concerned, it seems like the INVERT
operator can be trivially implemented with (white IN src) DIFFERENCE
dest. If so, there doesn't seem to be a good reason for pixman to
export the INVERT operator. Whether cairo should is another matter -
personally, I wouldn't add it, but I'll let other people answer that
question.

> I always stopped thinking about the blend modes when I couldn't come
> up with a SUBTRACT replacement.
> 
> This leaves the question if we should add these blend modes for
> supporting Flash and do tricks/fallbacks in the backends or if we
> should tell Flash implementors to do tricks to simulate these blend
> modes. I don't have a strong opinion there, but I guess I would leave
> them in as they are already implemented in my code.

It does bother me that SUBTRACT is so odd in several ways. I would
prefer to not add it if there were some way to avoid it, but I guess
the answer depends on what hacks exactly would be required to
implement it in swfdec.

If it would be really expensive, say involving reading pixels back
from the X server, then I'd say add it, but if there is a fairly
simple workaround, then maybe we can leave it out and revisit the
question later.


Comments on the code

I agree with most people that just adding the blend modes as new
operators is probably the better way to go, so here are comments on
the code.

Overall, it looks good. I checked almost all the math and everything
looks correct to me.

Here are the things that I think should be fixed before merging:

> +       rca += sca * ida + dca * isa;
> +       rca = DivOne (rca);

This fragment is duplicated in almost all the FbBlendOp macros. The
only case where it isn't, is in the Exclusion mode, but that mode
could easily be written in a similar way to Screen. For all the
others, the fragment could be moved into the FbBlendLoop macro.

> +#define FbBlendOp(rca)
> +    do {
> +       if (2 * dca  < da)
> +           rca = 2 * sca * dca;
> +       else
> +           rca = sada - 2 * (da - dca) * (sa - sca);
> +       rca += sca * ida + dca * isa;
> +       rca = DivOne (rca);
> +    } while (0);

I dislike the fact that these macros don't have names that indicate
what they do. With the fragment above moved out, the
FbBlendLoop macro could be made a FbBlendFunc macro that took the name
of a blend mode as parameter, and the FbBlendMode macros could be
turned into inline functions. Ie., something like

#define FbBlendFuncU(name)               \
        static FASTCALL void           \
        fbCombine ## name ## yU (comp4_t *dest, \
                                 const comp4_t *src, \
                                int width)  \
        ...
        
with operators defined as inline functions like this:

        static inline comp4_t
        Overlay (comp4_t sca, comp4_t sa, comp4_t dca, comp4_t da, comp4_t sada)
        {
            if (2 * dca < da)
                return 2 * sca * dca;
            else
                return sada - 2 * (da - dca) * (sa - sca);
        }

All the blend loops could then be defined by simply

        FbBlendFuncU(Overlay);
        FbBlendFuncU(Hardlight);
        ...

Also, I think we need component-alpha versions of the blend modes. As
you noted on IRC, this should be a mostly a matter of writing an
FbBlendFuncC macro.

> +#define FbBlendOp(rca)
> +    do {
> +       if (2 * sca < sa) {
> +           if (da == 0)
> +               rca = dca * sa;
> +           else
> +               rca = dca * sa - dca * (da - dca) * (sa - 2 * sca) / da;           
> +       } else if (da == 0) {
> +           rca = 0;
> +       } else if (4 * dca <= da) {
> +           int dc = dca * MASK / da;
> +           rca = dca * sa + (int) (2 * sca - sa) * (int) dca * ((16 *dc - 12 * MASK) * dc + 3 * MASK * MASK) / (MASK * MASK); 
> +        } else {
> +           rca = dca * sa + ((comp4_t) (sqrt (dca * da) - dca)) * (2 * sca - sa);
> +       }
> +       rca += sca * ida + dca * isa;
> +       rca = DivOne (rca);
> +    } while (0);

Could we just use floating point here? The integer math above is quite
incomprehensible. Ie,. just doing

        float scaf = (float)sca * (1.0 / MASK);
        ...;

and converting back before returning:

        return (comp4_t)rcaf * MASK + 0.5;

GCC doesn't convert a division with 255 into a multiplication with the
inverse unless you compile with -ffast-math, so we'll need to write it
that way explicitly. It may be worth considering using floating point
for some of the other more complicated blend modes.

I'll look at the separable blend modes later, but from a quick look it
seems like it could benefit from using floating point in various
places too.



Soren