[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