[cairo] some fixes and improvements to current cvs version of cairo

Kristian Høgsberg krh at bitplanet.net
Fri Jan 28 12:37:51 PST 2005


David Reveman wrote:
> I ran some test on current cvs version of cairo today and found a number
> of problems. Most problems where simple or obvious to fix, I've attached
> my current diff. It includes a lot of important fixes to the pattern
> system, some clipping fixes, some improvements to the glitz backend and
> an up to date version of the xcb backend.
> 
> I'll probably not be able to write changelog entries and commit this
> until next week, but you're very welcome to pick fixes from my diff and
> commit them if you like. Most of them are obvious fixes so you should be
> able to fill out changelog entries yourself. 

> Index: src/cairo_gstate.c
> ===================================================================
> RCS file: /cvs/cairo/cairo/src/cairo_gstate.c,v
> retrieving revision 1.77
> diff -u -r1.77 cairo_gstate.c
> --- src/cairo_gstate.c	28 Jan 2005 01:21:13 -0000	1.77
> +++ src/cairo_gstate.c	28 Jan 2005 15:41:04 -0000
> @@ -392,11 +392,9 @@
>      if (pattern == NULL)
>  	return CAIRO_STATUS_NULL_POINTER;
>  
> -    if (gstate->pattern)
> -	cairo_pattern_destroy (gstate->pattern);
> -    
> -    gstate->pattern = pattern;
>      cairo_pattern_reference (pattern);
> +    cairo_pattern_destroy (gstate->pattern);
> +    gstate->pattern = pattern;
>      
>      return CAIRO_STATUS_SUCCESS;
>  }
> @@ -1419,14 +1417,10 @@
>      if (dest->y < src->y)
>  	dest->y = src->y;
>  
> -    if (dest->x + dest->width < src->x + src->width)
> -	dest->width = dest->x + dest->width - dest->x;
> -    else
> +    if (dest->x + dest->width > src->x + src->width)
>  	dest->width = src->x + src->width - dest->x;
>  
> -    if (dest->y + dest->height < src->y + src->height)
> -	dest->height = dest->y + dest->height - dest->y;
> -    else
> +    if (dest->y + dest->height > src->y + src->height)
>  	dest->height = src->y + src->height - dest->y;
>  
>      if (dest->width <= 0 || dest->height == 0) {

I've fixed this in CVS, using a temporary cairo_rectangle_t.

> @@ -1521,8 +1515,6 @@
>  	}
>  	
>  	_cairo_pattern_init_solid (&pattern, 1.0, 1.0, 1.0);
> -	_cairo_gstate_create_pattern (gstate, &pattern);
> -	/* Override the alpha set from gstate. */
>  	_cairo_pattern_set_alpha (&pattern, 1.0);
>  
>  	status = _cairo_surface_composite_trapezoids (CAIRO_OPERATOR_ADD,

Oh, right, makes sense.  I've updated all occurences of this pattern.

> @@ -1989,10 +1980,10 @@
>      cairo_pattern_t pattern;
>      cairo_box_t pattern_extents;
>      cairo_rectangle_t extents;
> +    int xoff, yoff;
>          
>      cairo_surface_get_matrix (surface, &user_to_image);
>      cairo_matrix_multiply (&device_to_image, &gstate->ctm_inverse, &user_to_image);
> -    cairo_surface_set_matrix (surface, &device_to_image);
>  
>      image_to_user = user_to_image;
>      cairo_matrix_invert (&image_to_user);
> @@ -2006,8 +1997,12 @@
>  					  &device_width, &device_height);
>  
>      _cairo_pattern_init_for_surface (&pattern, surface);
> -    _cairo_gstate_create_pattern (gstate, &pattern);
>  
> +    if (!_cairo_matrix_is_integer_translation (&device_to_image, &xoff, &yoff))
> +	pattern.matrix = device_to_image;
> +    
> +    _cairo_pattern_set_alpha (&pattern, gstate->alpha);
> +	

Isn't this an optimization that belongs in the backend?

>      pattern_extents.p1.x = _cairo_fixed_from_double (device_x);
>      pattern_extents.p1.y = _cairo_fixed_from_double (device_y);
>      pattern_extents.p2.x = _cairo_fixed_from_double (device_x + device_width);
> @@ -2017,27 +2012,41 @@
>      if (gstate->clip.surface)
>      {
>  	_cairo_rectangle_intersect (&extents, &gstate->clip.rect);
> -
> -	/* Shortcut if empty */
> -	if (_cairo_rectangle_empty (&extents)) {
> -	    status = CAIRO_STATUS_SUCCESS;
> -	    goto BAIL1;
> +	if (!_cairo_rectangle_empty (&extents))
> +	{
> +	    status = _cairo_surface_composite (gstate->operator,
> +					       &pattern, 
> +					       gstate->clip.surface,
> +					       gstate->surface,
> +					       extents.x + xoff,
> +					       extents.y + yoff,
> +					       0, 0,
> +					       extents.x, extents.y,
> +					       extents.width, extents.height);
>  	}
> -	
> -	status = _cairo_surface_composite (gstate->operator,
> -					   &pattern, 
> -					   gstate->clip.surface,
> -					   gstate->surface,
> -					   extents.x, extents.y,
> -					   0, 0,
> -					   extents.x, extents.y,
> -					   extents.width, extents.height);
> -	
> -    BAIL1:
> -	;
> +	else
> +	    status = CAIRO_STATUS_SUCCESS;
>      }
>      else
>      {
> +	cairo_surface_t *mask = NULL;
> +	
> +	/* alpha as mask surface */
> +	if (pattern.color.alpha != 1.0)
> +	{
> +	    mask = _cairo_surface_create_similar_solid (gstate->surface,
> +							CAIRO_FORMAT_A8, 1, 1,
> +							&pattern.color);
> +	    if (!mask)
> +	    {
> +		_cairo_pattern_fini (&pattern);
> +		return CAIRO_STATUS_NO_MEMORY;
> +	    }
> +
> +	    pattern.color.alpha = 1.0;
> +	    cairo_surface_set_repeat (mask, 1);
> +	}
> +	    

Hmm, this wouldn't handle the case with clipping and alpha != 1.0.  I 
comitted an alternate fix yesterday that should handle both cases.  The 
code is in _cairo_pattern_get_surface in the CAIRO_PATTERN_SURFACE case. 


>  static void
> Index: src/cairo_image_surface.c
> ===================================================================
> RCS file: /cvs/cairo/cairo/src/cairo_image_surface.c,v
> retrieving revision 1.22
> diff -u -r1.22 cairo_image_surface.c
> --- src/cairo_image_surface.c	28 Jan 2005 03:57:32 -0000	1.22
> +++ src/cairo_image_surface.c	28 Jan 2005 15:41:04 -0000
> @@ -395,6 +395,8 @@
>      if (generic_mask && (generic_mask->backend != dst->base.backend))
>  	return CAIRO_INT_STATUS_UNSUPPORTED;
>  
> +    _cairo_pattern_prepare_surface (pattern, &src->base);
> +
>      pixman_composite (_pixman_operator (operator),
>  		      src->pixman_image,
>  		      mask ? mask->pixman_image : NULL,
> @@ -404,6 +406,8 @@
>  		      dst_x, dst_y,
>  		      width, height);
>  
> +    _cairo_pattern_restore_surface (pattern, &src->base);
> +
>      cairo_surface_destroy (&src->base);

Yeah, these are missing a few places, I think, but I was planning on 
moving _cairo_pattern_prepare_surface back into 
_cairo_gstate_create_pattern, which should then be renamed to 
_cairo_gstate_prepare_pattern.  _cairo_pattern_restore_surface should be 
renamed _cairo_gstate_restore_pattern or moved back into 
_cairo_pattern_fini (which is sort of ugly, but works because we always 
use temporary patterns which we fini immediately after use).

...

> Index: src/cairo_pattern.c
> ===================================================================
> RCS file: /cvs/cairo/cairo/src/cairo_pattern.c,v
> retrieving revision 1.15
> diff -u -r1.15 cairo_pattern.c
> --- src/cairo_pattern.c	28 Jan 2005 05:14:06 -0000	1.15
> +++ src/cairo_pattern.c	28 Jan 2005 15:41:04 -0000
> @@ -64,6 +64,9 @@
>  		sizeof (cairo_color_stop_t) * other->n_stops);
>      }
>  
> +    if (pattern->type == CAIRO_PATTERN_SURFACE)
> +	cairo_surface_reference (pattern->u.surface.surface);
> +
>      return CAIRO_STATUS_SUCCESS;
>  }

Comitted this.


> @@ -810,16 +811,17 @@
>  	    }
>  
>  	    image = _cairo_surface_get_image (src);
> -	    
> -	    surface = cairo_surface_create_similar (dst,
> -						    CAIRO_FORMAT_ARGB32,
> -						    width, height);
> +
> +	    surface = cairo_surface_create_similar (dst, CAIRO_FORMAT_ARGB32,
> +						    image->width,
> +						    image->height);

What the difference here?

>  	    if (surface == NULL)
>  		return NULL;
>  
>  	    cairo_surface_set_filter (surface, cairo_surface_get_filter(src));
>  	    _cairo_surface_set_image (surface, image);
>  	    cairo_surface_set_matrix (surface, &(image->base.matrix));
> +	    cairo_surface_set_repeat (surface, image->base.repeat);
>  	    cairo_surface_destroy (&image->base);
>  
>  	    return surface;

Ooh, thanks. Comitted.

> Index: src/cairo_png_surface.c
> ===================================================================
> RCS file: /cvs/cairo/cairo/src/cairo_png_surface.c,v
> retrieving revision 1.12
> diff -u -r1.12 cairo_png_surface.c
> --- src/cairo_png_surface.c	27 Jan 2005 18:46:20 -0000	1.12
> +++ src/cairo_png_surface.c	28 Jan 2005 15:41:05 -0000
> @@ -53,7 +53,7 @@
>  		      int		height)
>  {
>      cairo_surface_t *surface;
> -
> +    
>      surface = cairo_png_surface_create (file, format, 
>  					width, height);
>  
> @@ -239,7 +239,7 @@
>  
>  static cairo_int_status_t
>  _cairo_png_surface_composite (cairo_operator_t	operator,
> -			      cairo_surface_t	*generic_src,
> +			      cairo_pattern_t   *pattern,
>  			      cairo_surface_t	*generic_mask,
>  			      void		*abstract_dst,
>  			      int		src_x,

Check, comitted.

(XCB and Xlib changes omitted)

cheers,
Kristian



More information about the cairo mailing list