[cairo] cairo pattern/glitz patches

[David Reveman]

On Mon, 2005-02-21 at 22:53 -0500, Kristian Høgsberg wrote: 
> David Reveman wrote:
> > I've split up my pending pattern updates and glitz backend updates as
> > much as possible. Here's a set of patches. They need to be applied in
> > the following order:
> 
> I've looked through this and I think it looks really good.  These 
> patches implement most of the pending backend API changes that we 
> discussed and I think we should get this into cvs as soon as possible.
>  
> Just a couple of minor nitpicks:
> 
> Should _cairo_*_surface_change_attributes() be called 
> _cairo_*_surface_set_attributes instead?  It's implemented by calling a 
> number of setter functions...

yes, I agree.

> 
> I'm not a big fan of the preprocessor hacks in the cairo_pattern_t 
> definition, I prefer the nested structs union approach:
> 
> 	struct _cairo_pattern {
> 		/* common fields */
> 		cairo_pattern_type_t type;
> 		int field1;
> 		int field2;
> 
> 		/* type specific fields in union */
> 		union {
> 			struct { int foo; } solid;
> 		} u;
> 	};
> 
> even though it adds an extra .u in all the pattern expression.  C sucks 
> sometimes and I think it's better to suck it up than to try to hack 
> around it.

Having the pattern as one big union cleaned up things a lot and I think
it's a more appropriate way to store patterns internally. I'd also like
to change so that PATTERN_LINEAR and PATTERN_RADIAL are gradient types
and not pattern types. 

with the pattern as a union we can write code similar to this: 

cairo_image_surface_t *
get_radial (cairo_radial_gradient_t *radial, cairo_stops_t *stops)
{
    double a, b, c, d, tx, ty;

    cairo_matrix_get_affine (&radial->matrix, &a, &b, &c, &d, &tx, &ty);

    switch (radial->extend) {
    ...
    ...
}

cairo_image_surface_t *
get_gradient (cairo_gradient_t *grad)
{
    cairo_image_surface_t *image;
    cairo_stops_t              *stops;

    stops = get_sorted_gradient_stops_using_alpha (grad, grad->alpha);

    switch (grad->gradient_type) {
    case LINEAR:
        image = get_linear (&grad->linear, stops);
        break;
    case RADIAL:
        image = get_radial (&grad->radial, stops);
        break;
    }
    free (stops); 
    return image;
}

cairo_image_surface_t *
cairo_get_pattern (cairo_pattern_t *pattern)
{
    switch (pattern->type) {
    case GRADIENT:
        return get_gradient (&pattern->gradient);
    ...
    ...
}

> 
> How about passing the pattern type to _cairo_pattern_init()?

As of right now, we always want to init patterns with some type specific
parameters. e.g. _cairo_pattern_init_solid (&pattern, red, green, blue).
So passing a type to _cairo_pattern_init doesn't seem like a good idea.

> 
> As for the pixman patch, my immediate response was that the order or 
> traps and ntraps arguments is reversed compared to the other pixman 
> calls. 

yes, I don't know why I used that order. I'll fix that.

> Looking closer at pixman_composite_trapezoids I came across some 
> odd looking code.  If pixman_composite_trapezoids doesn't need to 
> implement the RENDER behavior where you can pass in a mask format or 
> NULL to determine the compositing behaviour, there's a fair amount of 
> code that can be removed; see my attached alternative pixman patch.

I know, but I think it's ok that pixman_composite_trapezoids behaves
like this. Eventually, we're going to switch to using pixman_add_traps
anyway.  

-David