[cairo] more pattern rewrite stuff...

[David Reveman]

On Mon, 2005-01-31 at 15:14 -0500, Kristian Høgsberg wrote: 
> David Reveman wrote:
> > ok, I couldn't stop myself from trying to get this right. This patch
> > includes new changes to the pattern system, this time I'm pretty happy
> > with how it turned out. 
> > 
> > It removes backend functions set_filter, set_repeat and set_matrix.
> > Makes the "pattern <-> gstate" and "pattern <-> backend" connections
> > more appropriate and cleans up a lot of stuff. Looking at the diff is
> > probably not good enough.
> 
> We discussed this in IRC a couple of days ago, and general consensus was 
> that filter, repeat and transform should be moved out of the surface and 
> into the pattern.  Owen summarized it here:
> 
>    https://bugs.freedesktop.org/show_bug.cgi?id=2397

ok, I've done the first step of this then. The the repeat, filter and
matrix attributes are still left in the surface only to be picked up by
caior_show_surface. Moving filter attribute into the gstate and removing
the surface matrix should now be simple, but as it's a quite large
change to public API I think it can wait until we get my current patch
merged with Owen's fall-back bits. We could also remove the width and
height parameters from show_surface, at the same time.

> 
> > Futher on, we should make the mask surface passed to surface_composite a
> > pattern as well, 
> 
> I think we should get this done sooner rather than later.  I can take a 
> look at that once you get your patch landed.

good, it shouldn't be to hard once we get the _cairo_pattern_get_surface
stuff working well.

> 
> >  static cairo_int_status_t
> > +_cairo_image_surface_change_attributes (cairo_image_surface_t	   *surface,
> > +					cairo_surface_attributes_t *attributes)
> 
> So, instead of this, we would set the pixman surface attributes from the 
> pattern.

_cairo_image_surface_change_attributes is actually doing this.
cairo_surface_attributes_t is basically the same as cairo_pattern_info_t
in Owen's patch. Hmm, _cairo_pattern_get_surface returns a surface
representing the pattern so I think cairo_surface_attributes_t is more
appropriate.

> 
> > @@ -380,33 +387,60 @@
> >  				unsigned int		width,
> >  				unsigned int		height)
> >  {
> > -    cairo_image_surface_t *dst = abstract_dst;
> > -    cairo_image_surface_t *src;
> > -    cairo_image_surface_t *mask = (cairo_image_surface_t *) generic_mask;
> > -    int x_offset, y_offset;
> > -
> > +    cairo_surface_attributes_t	attributes;
> > +    cairo_image_surface_t	*dst = abstract_dst;
> > +    cairo_image_surface_t	*src;
> > +    cairo_image_surface_t	*mask = (cairo_image_surface_t *) generic_mask;
> > +    cairo_image_surface_t	*mask_alpha = NULL;
> > +    cairo_int_status_t		status;
> > +    double			alpha = pattern->color.alpha;
> > +
> > +    if (generic_mask)
> > +    {
> > +	if (generic_mask->backend != dst->base.backend)
> > +	    return CAIRO_INT_STATUS_UNSUPPORTED;
> > +    }
> > +    else if (pattern->type == CAIRO_PATTERN_SURFACE && alpha != 1.0)
> > +    {
> > +	mask = mask_alpha = (cairo_image_surface_t *)
> > +	    _cairo_surface_create_similar_solid (&dst->base, CAIRO_FORMAT_A8,
> > +						 1, 1, &pattern->color);
> > +	if (!mask)
> > +	    return CAIRO_STATUS_NO_MEMORY;
> > +	
> > +	cairo_surface_set_repeat (&mask->base, 1);
> > +	alpha = 1.0;
> > +    }
> > +    
> >      src = (cairo_image_surface_t *)
> > -	_cairo_pattern_get_surface (pattern, &dst->base,
> > +	_cairo_pattern_get_surface (pattern, alpha, &dst->base,
> >  				    src_x, src_y, width, height,
> > -				    &x_offset, &y_offset);
> 
> Why are you reading out the alpha value from the pattern color only to 
> pass it back to _cairo_pattern_get_surface, which use it as it used to 
> use pattern->color.alpha?  

If no mask surface or eventually mask pattern is passed to composite, we
can use the mask to accelerate overall surface alpha. The alpha
parameter tells _cairo_pattern_get_surface what alpha to use for surface
patterns. Hence, passing alpha=1.0 and _cairo_pattern_get_surface will
not do overall alpha for us. 

> 
> I'll agree that using the solid color alpha 
> for surface patterns is not quite right, but if we're changing it, I 
> think we should rewrite it to something like:
> 
> 	struct _cairo_pattern {
> 		unsigned int ref_count;
> 
> 		cairo_extend_t extend;
> 		cairo_filter_t filter;
> 		cairo_matrix_t matrix;
> 
> 		cairo_pattern_type_t type;
> 		union {
> 			cairo_color_t color;
> 			struct {
> 				cairo_surface_t *source;
> 				double alpha;
> 			} surface;
> 			struct {
> 				cairo_point_double_t point0;
> 				cairo_point_double_t point0;
> 				cairo_color_stop_t *stops;
> 				int num_stops;
> 			} gradient;
> 		} u;
> 	};

The gradient struct will need to have alpha as well. How about this?

struct _cairo_pattern {
	unsigned int ref_count;

	cairo_extend_t extend;
	cairo_filter_t filter;
	cairo_matrix_t matrix;

	cairo_pattern_type_t type;
	union {
		double	alpha;
		struct {
			double	alpha;
			double	red, green, blue;
		} color;
		struct {
			double		alpha;
			cairo_surface_t *source;
		} surface;
		struct {
			double			alpha;
			cairo_point_double_t	point0;
			cairo_point_double_t	point0;
			cairo_color_stop_t	*stops;
			int			num_stops;
		} gradient;
	} u;
};

> 
> Note, we don't need the save_* fields, since a surface doesn't have 
> these attributes with the changes suggested above.

correct, in my latest patch, I had those removed.

-David