[cairo] cairo pattern/glitz patches
David Reveman
davidr at novell.com
Tue Feb 22 03:37:45 PST 2005
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
More information about the cairo
mailing list