[cairo] more pattern rewrite stuff...

[Kristian Høgsberg]

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

What I mean is, why not just pass a cairo_pattern_t instead of 
cairo_surface_attributes_t, since the pattern will contain all the 
information needed.

Another point is that these attributes aren't necessarily surface 
attributes, that's just an artifact^Wimplementation detail of the 
render/pixman based backends.  For the PDF backend there are no surfaces 
involved in solid color or gradient patterns.

Anyway, I guess this will look all different when you get it merged with 
current CVS.

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

Yeah, but why don't you just change the alpha value in the pattern 
instead of overriding it by this new alpha argument?

> 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;
> };

Yeah, that looks pretty good.

cheers,
Kristian