Colour balance

[Tor Lillqvist <tml@xxxxxx>]

I think it is time to repeat this message, from a year ago... The same
strange-looking code is still present in color_transfer.c. Is it a
bug? Should it be fixed before 1.2? Is my suggestion below a good one?

It seems obvious to me that the current color_transfer_init() function
in color_transfer.c has some cut-and-paste bug in it:

  for (i = 0; i < 256; i++)
    {
      highlights_add[i] =
	shadows_sub[255 - i] = (1.075 - 1 / ((double) i / 16.0 + 1));
      midtones_add[i] = 
	midtones_sub[i] = 0.667 * (1 - SQR (((double) i - 127.0) / 127.0));
      shadows_add[i] = 
	highlights_sub[i] = 0.667 * (1 - SQR (((double) i - 127.0) / 127.0));
    }

Note how the third assignment statement uses the same value as the
second. Can this be the intention, that adjusting shadows up is the
same as adjusting midtones up? (And adjusting highlights down the same
as adjusting midtones down.)

And, what *is* the definition of what the colour balance adjustment
should do? I couldn't find any deeper explanation in my Photoshop
books. Are the sliders you adjust from -100 to 100 percentages (in
that case, percentages of what?), or pixel values?

I would tentatively suggest replacing those lines with the following
code.  This uses Bernstein polynomials as weight curves, with are
scaled with the difference from the min or max channel value (0 or
255) and the current value, depending on if we are subtracting or
adding. (Er, that probably isn't very clearly said. Play with gnuplot
to visualize these curves... Gnuplot rocks.)

  for (i = 0; i < 256; i++)
    {
      /* Let's try using Bernstein polynomials...  At least this gives
       * a certain smoothness to the transfer functions.  This is
       * definitely not what P*shop, uses, however. In P*shop if you
       * apply +100 shadows and -100 highlights, you get back what you
       * started with. Should this be our goal, too? To me, this seems
       * conceptually a bit odd, why should reducing red from
       * highlights be the opposite of adding red to shadows?  Note
       * that it doesn't work in the other order, though, so maybe
       * it's just a coincidence, and not a design goal.  */
 
#define C41 ((4*3*2*1)/(1 * 3*2*1))
#define C42 ((4*3*2*1)/(2*1 * 2*1))
#define C43 ((4*3*2*1)/(3*2*1 * 1))
 
#define B14(u) (C41*u*(1-u)*(1-u)*(1-u))
#define B24(u) (C42*u*u*(1-u)*(1-u))
#define B34(u) (C43*u*u*u*(1-u))
 
      shadows_add[i] = B14((double) i / 255.0)*(255 - i);
      shadows_sub[i] = B14((double) i / 255.0)*i;
      midtones_add[i] = B24((double) i / 255.0)*(255 - i);
      midtones_sub[i] = B24((double) i / 255.0)*i;
      highlights_add[i] = B34((double) i / 255.0)*(255 - i);
      highlights_sub[i] = B34((double) i / 255.0)*i;
  }


And the code that uses these tables, in the function
color_balance_create_lookup_tables() in color_balance.c, would be
modified as follows (treat the slider values as percentages of the
above values, clamp each channel value only once ).

    r_n += cbd->cyan_red[SHADOWS]/100.0 * cyan_red_transfer[SHADOWS][r_n];
    r_n += cbd->cyan_red[MIDTONES]/100.0 * cyan_red_transfer[MIDTONES][r_n];
    r_n += cbd->cyan_red[HIGHLIGHTS]/100.0 * cyan_red_transfer[HIGHLIGHTS][r_n];
    r_n = CLAMP0255 (r_n);
 
    g_n += cbd->magenta_green[SHADOWS]/100.0 * magenta_green_transfer[SHADOWS][g
_n];
    g_n += cbd->magenta_green[MIDTONES]/100.0 * magenta_green_transfer[MIDTONES]
[g_n];
    g_n += cbd->magenta_green[HIGHLIGHTS]/100.0 * magenta_green_transfer[HIGHLIG
HTS][g_n];
    g_n = CLAMP0255 (g_n);
 
    b_n += cbd->yellow_blue[SHADOWS]/100.0 * yellow_blue_transfer[SHADOWS][b_n];
    b_n += cbd->yellow_blue[MIDTONES]/100.0 * yellow_blue_transfer[MIDTONES][b_n
];
    b_n += cbd->yellow_blue[HIGHLIGHTS]/100.0 * yellow_blue_transfer[HIGHLIGHTS]
[b_n];
    b_n = CLAMP0255 (b_n);
 
    cbd->r_lookup[i] = r_n;
    cbd->g_lookup[i] = g_n;
    cbd->b_lookup[i] = b_n;


Comments, please?

--tml