[cairo] cairo pattern/glitz patches

[Kristian Høgsberg]

David Reveman wrote:
...
> 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. 

Do you want gradients as a user visible object distinct from pattern? 
That is, cairo_gradient_t?

> 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) {
>     ...
>     ...
> }

These two cases are probably not the best examples, since matrix and 
extend would be in the common part of the pattern struct:

	struct cairo_pattern_t {
		cairo_matrix_t matrix;
		cairo_pattern_extend_t extend;
		...
		union {
			...
		} u;
	};

and you could access them as pattern->matrix and pattern->extend.  In 
fact reading your patch again, there's a lot of these changes

	-    pattern.matrix = user_to_image;
	-    pattern.filter = surface->filter;
	+    pattern.any.matrix = user_to_image;
	+    pattern.any.filter = surface->filter;

which sort of defeats the purpose of the change.  Or maybe I'm reading 
it wrong and the idea to get a more "typesafe" implementation where you 
pass more precise pointer types (cairo_radial_gradient_pattern_t instead 
of cairo_pattern_t) around, rather than to save typing?  If that's the 
case, shouldn't we use a construction more like cairo_surface_t?  For 
example:

	struct _cairo_pattern {
		cairo_pattern_type_t type;
		unsigned int        ref_count;
		double              alpha;
		cairo_extend_t      extend;
		cairo_filter_t      filter;
		cairo_matrix_t      matrix;
	};

	struct _cairo_solid_pattern {
		cairo_pattern_t	base;
		double red, green, blue;
	};

which is much like your patch, except without the preprocessor hack and 
without changing the public visible struct _cairo_pattern to a union.

...

>>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.

I guess I wasn't too clear there.  Instead of

	pattern->type = CAIRO_PATTERN_SOLID;
	_cairo_pattern_init (pattern);
	pattern->solid.red   = red;
	pattern->solid.green = green;
	pattern->solid.blue  = blue;

I'm suggesting

	_cairo_pattern_init (pattern, CAIRO_PATTERN_SOLID);
	pattern->solid.red   = red;
	pattern->solid.green = green;
	pattern->solid.blue  = blue;

It just seems logical to me that _cairo_pattern_init() should initialize 
all the common fields, of which 'type' is one.  Anyway, I'm splitting 
hairs now.

cheers,
Kristian