[cairo] Image surface pixel twiddling API

[Jeff Muizelaar]

On Mon, Mar 05, 2007 at 02:20:21PM -0600, T Rowley wrote:
> In implementing SVG filters in mozilla, we have the need to access and 
> modify individual pixels of an image.  Our existing code relied on a 
> particular byte layout, which broke when tested on a big-endian cpu for 
> reasons that were obvious after taking a close read of cairo.h.
> 
> We would like an API to obtain the particular channel of a pixel.  In 
> our experiments we tried two different types of API.  The first was a 
> set of macros that returned a particular channel from pixel word [1], 
> the second were channel offset macros that could be used to index into a 
> byte pointer.  For our use, the first approach of word accessors made 
> for somewhat messier source code and generated code that was slightly 
> slower for x86 though faster on PPC [3].

This is probably because x86 is better at byte addressing/operations.  If
it's faster on PPC it will probably be faster on ARM which I imagine is a
platform Mozilla will care about more in the future.

> For that reason we're more in favor of the channel offsets, but would
> like to hear feedback from others about what seems best.

Personally, I think the word accessors are a 'cleaner' API. However, the
offsets might be easier to use when writing byte oriented code. They might
also be useful if the inkscape people want to convert their pixel munging
routines to the same format as cairo's.

Well looking into this, I noticed that mozilla's macros for 255 division
do not do any rounding, which is sort of wrong. e.g (1 * 254) is 0 with
mozilla's code and should be 1.

To fix this at no extra cost:

#define FAST_DIVIDE_BY_255(target,v)               \
  PR_BEGIN_MACRO                                   \
    unsigned tmp_ = v;                             \
    target = ((tmp_ << 8) + tmp_ + 255) >> 16;     \
  PR_END_MACRO

should become:

#define FAST_DIVIDE_BY_255(target,v)               \
  PR_BEGIN_MACRO                                   \
    unsigned tmp_ = v + 128;                       \
    target = ((tmp_ >> 8) + tmp_) >> 8;            \
  PR_END_MACRO

and

#define GFX_DIVIDE_BY_255(v)  \
     (((((unsigned)(v)) << 8) + ((unsigned)(v)) + 255) >> 16

becomes:

#define GFX_DIVIDE_BY_255(v)  \
     ((((unsigned)(v) + 128) >> 8) + ((unsigned)(v) + 128)) >> 8

In this case the compiler's common subexpression elimination should take
care of combining the two '+ 128' so that it is only performed once.

If anyone is interested, the derivation of the correct formula is in a
paper by Jim Blinn called "Three wrongs make a right".

-Jeff