[cairo] Operator optimization based on source or dest opacity

[Soeren Sandmann]

Hi Antoine,

> >But frankly in both cases, a simpler, and much bigger, speed-up than
> >the operator replacement would be to just write a couple of fast paths
> >that would deal with such simple transformations without the matrix
> >multiplication and the indirect fetch functions.
> 
> Indeed. Is it on any todo list?

Not that I know of. 

> I was actually thinking of something, whenever we're doing bilinear
> filtering with an opaque source, we're forced of going the whole
> compositing way, even though we don't really need compositing. All we
> need is to sample from the dest whenever we're oustide the
> source. That would simply require to have a different fetchFromRegion
> function that returns the dest instead of 0 when ouside the image. I'm
> not sure of a clean way to do it right now, but ideas are welcome.
> 
> I'm attaching the three new patches. Let me know how it looks now.

The first one about moving the composite functions looks good, except
that I'd like to have the functions in the C file marked 'static'.

For the second one, I still have some comments:

> +    if (image->common.repeat == PIXMAN_REPEAT_NONE &&
> +        ((image->common.filter != PIXMAN_FILTER_NEAREST && image->common.filter != PIXMAN_FILTER_FAST) ||
> +        image->common.transform ))
> +        return FALSE;

Can we get this written like this:

        if (image->common.repeat == PIXMAN_REPEAT_NONE)
        {
                if (image->common.filter != PIXMAN_FILTER_NEAREST)
                        return FALSE;
                
                if (image->common.transform)
                        return FALSE;
        }

to make it less convoluted?

In the table you have these:

> +    { PIXMAN_OP_XOR,            PIXMAN_OP_CLEAR,   PIXMAN_OP_OVER,         PIXMAN_OP_OVER_REVERSE },

Shouldn't this be { XOR, CLEAR, OUT, OUT_REVERSE } ?

> +    { PIXMAN_OP_SATURATE,       PIXMAN_OP_ADD, PIXMAN_OP_ADD,          PIXMAN_OP_ADD },

Shouldn't this be { SATURATE, DST, OVER_REVERSE, DST } ?

There are also a number of PIXMAN_OP_NONE that really should be
PIXMAN_OP_DST, I think.

> +const OptimizedOperatorInfo*
> +pixman_operator_can_be_optimized(pixman_op_t op)

> +pixman_op_t
> +pixman_optimize_operator(pixman_op_t op, pixman_image_t *pSrc, pixman_image_t *pMask, pixman_image_t *pDst )

These functions should be static.

> @ -1696,6 +1729,8 @@ pixman_image_composite (pixman_op_t      op,
>      pixman_bool_t      maskAlphaMap = FALSE;
>      pixman_bool_t      dstAlphaMap = pDst->common.alpha_map != NULL;
>      CompositeFunc   func = NULL;
> +    pixman_bool_t      isSourceOpaque = FALSE, isDestOpaque = FALSE;
> +    pixman_bool_t       canOptimize = FALSE;

These variables are not used any more

> +    op = pixman_optimize_operator(op, pSrc, pMask, pDst);

Don't need we need

    if (op == PIXMAN_OP_DST)
        return;

here?

DST is not a particularly useful operator, so people wouldn't normally
use it, but this optimization can generate them.

> @@ -299,6 +299,17 @@ union pixman_image
>      solid_fill_t               solid;
>  };
>  
> +/*
> + * Operator optimizations based on source or destination opacity
> + */
> +typedef struct
> +{
> +    pixman_op_t                        op;
> +    pixman_op_t                        opSrcDstOpaque;
> +    pixman_op_t                        opSrcOpaque;
> +    pixman_op_t                        opDstOpaque;
> +} OptimizedOperatorInfo;

There is no reason to put this in a header file. It is only used in
pixman-pict.c.
 

Soren