Blend modes take 3

[Soeren Sandmann]

"Benjamin Otte" <otte at gnome.org> writes:

> On Tue, Nov 25, 2008 at 10:10 PM, Soeren Sandmann <sandmann at daimi.au.dk> wrote:
> >    - When mask is 0, there is no reason to read the source.
> >
> >    - There is no reason to read the destination if the inverse
> >      combined src/mask is 0
> >
> Don't compilers take care of that?
> The code looks better to me if the variable assignments all occur in one place.
> 
> >    - The continue is incorrect; it needs to write out 0 in that case,
> >      since the intermediate buffer is not zero-filled.
> >
> Oh, the code is supposed to modify the _source_ parameter, too?
> Someone really needs to document what component alpha does properly so
> I really grok it.
> Fixed.

No, what I meant was that the destination buffer would need to be
modified in that case too, but that was just a thinko on my part.

However, I don't think the new code is correct either. In order to
skip the computation, it needs to ensure that both m and s are 0. So
let's not do this optimization for now. 

Another problem I forgot to mention: fbByteMulAddC() produces a result
in its first argument, not its last.

> >>  if (min < 0) {
> >>    tmp[0] = l + (tmp[0] - l) / 4 * l / (l - min) * 4;
> >>    tmp[1] = l + (tmp[1] - l) / 4 * l / (l - min) * 4;
> >>    tmp[2] = l + (tmp[2] - l) / 4 * l / (l - min) * 4;
> >>  }
> >>  if (max > a) {
> >>    tmp[0] = l + (tmp[0] - l) / 4 * (a - l) / (max - l) * 4;
> >>    tmp[1] = l + (tmp[1] - l) / 4 * (a - l) / (max - l) * 4;
> >>    tmp[2] = l + (tmp[2] - l) / 4 * (a - l) / (max - l) * 4;
> >>  }
> >
> > Where do those 4s come from?
> >
> Overflow protection. The tmp value can range from -COMP2_T_MAX .. 2 *
> COMP2_T_MAX (I think), so I took the shortcut to just divide by a high
> enough number to avoid it. Also, this code will break on 64bit cases
> as I'm using ints there for lack of a signed comp4_t type.
> It's one of the cases I asked about previously on IRC I think as I was
> unsure if this is a case for doubles or how it should best be
> handled.

I would be more comfortable with floating points, yes. In the
'blendmode' branch here:

        git://freedesktop.org/~sandmann/pixman/

I fixed some of the above things and added a long comment about the
linearity of the HSL functions. Here is the comment:

/* For premultiplied colors, we need to know what happens when C is
 * multiplied by a real number. Lum and Sat are linear:
 *
 *    Lum (r * C) = r * Lum (C)          Sat (r * C) = r * Sat (C)
 *
 * If we extend ClipColor with an extra argument a and change
 *
 *        if x >= 1.0
 *
 * into
 *
 *        if x >= a
 *
 * then ClipColor is also linear:
 *
 *    r * ClipColor (C, a) = ClipColor (rC, ra);
 *
 * for positive r.
 *
 * Similarly, we can extend SetLum with an extra argument that is just passed
 * on to ClipColor:
 *
 *   r * SetLum ( C, l, a)
 *
 *   = r * ClipColor ( C + l - Lum (C), a)
 *
 *   = ClipColor ( r * C + r * l - r * Lum (C), r * a)
 *
 *   = SetLum ( r * C, r * l, r * a)
 *
 * Finally, SetSat:
 *
 *    r * SetSat (C, s) = SetSat (x * C, r * s)
 *
 * The above holds for all non-zero x, because they x'es in the fraction for
 * C_mid cancel out. Specifically, it holds for x = r:
 *
 *    r * SetSat (C, s) = SetSat (rC, rs)
 *  
 */

/* So, for the non-separable PDF blend modes, we have (using s, d for non-premultiplied
 * colors, and S, D for premultiplied:
 *
 *   Color:
 *
 *     a_s * a_d * B(s, d)
 *   = a_s * a_d * SetLum (S/a_s, Lum (D/a_d), 1)
 *   = SetLum (S * a_d, a_s * Lum (D), a_s * a_d)
 *
 *
 *   Luminosity:
 *
 *     a_s * a_d * B(s, d)
 *   = a_s * a_d * SetLum (D/a_d, Lum(S/a_s), 1)
 *   = SetLum (a_s * D, a_d * Lum(S), a_s * a_d)
 *
 *
 *   Saturation:
 *
 *     a_s * a_d * B(s, d)
 *   = a_s * a_d * SetLum (SetSat (D/a_d, Sat (S/a_s)), Lum (D/a_d), 1)
 *   = SetLum (a_s * a_d * SetSat (D/a_d, Sat (S/a_s)), a_s * Lum (D), a_s * a_d)
 *   = SetLum (SetSat (a_s * D, a_d * Sat (S), a_s * Lum (D), a_s * a_d))
 *
 *   Hue:
 *
 *     a_s * a_d * B(s, d)
 *   = a_s * a_d * SetLum (SetSat (S/a_s, Sat (D/a_d)), Lum (D/a_d), 1)
 *   = a_s * a_d * SetLum (SetSat (a_d * S, a_s * Sat (D)), a_s * Lum (D), a_s * a_d)
 *
 */

A careful review of both the code in the branch and the math in the
comment would be appreciated.

The things remaining to be done on top of that branch:

        - Review that what I did makes sense

        - Use floating point in the HSL modes

        - Getting rid of component alpha versions of HSL and
          return_if_fail()ing if you to use them with a component
          alpha mask. [1]

        - There is an "#undef Set" which should be "#undef Sat"

Soren


[1] They could be done in ways that would be mathematically consistent
with the other blend modes, but I don't know that it would make any
visual sense. I could be convinced otherwise, though.