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

[Owen Taylor]

On Fri, 2005-01-28 at 16:45 +0100, 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. 

 if CAIRO_HAS_FT_FONT
-libcairo_ft_headers = cairo-ft.h
+libcairo_ft_headers = cairo-ft.h cairo-ft-private.h
 libcairo_ft_sources = cairo_ft_font.c
 endif
 
That isn't right ... cairo-ft-private.h is a source not a header in
automake terminology ... it will get installed if you put it in
include_HEADERS.

-    if (gstate->pattern)
-       cairo_pattern_destroy (gstate->pattern);
-    
-    gstate->pattern = pattern;
     cairo_pattern_reference (pattern);
+    cairo_pattern_destroy (gstate->pattern);
+    gstate->pattern = pattern;

That looks fine, except that that _cairo_gstate_init() doesn't
check the result of _cairo_pattern_create_solid() so gstate->pattern
*can* be NULL. But that's a bug in the gstate creation not here.

     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;

Still broken with your fix here ... the problem is that 
if dest->x gets clamped dest->width doesn't get reduced.
It's easiest to intersect as with temporary x2,y2 and
then recompute width/height at the end.

     {
+       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);
+       }

This pushes one of the few hw-accelerated fast paths I have in the 
win32 backend through the slow path. If the backend needs to use a mask
for overall-alpha it can create one itself.

(Seems you could use overall alpha for GL as well in a few cases
4-channel-texture * alpha over dest is possible with GL_MODULATE,
right?)

-    } 
+    } else {
+       *itx = 0;
+       *ity = 0;
+    }

I think that's wrong. If it's not an integer translation there is
no meaningful value for itx/ity and the caller must avoid using
the results.

+    /* Two round-trips to check if this fast path is possible, not
pretty. */
     if (operator == CAIRO_OPERATOR_SRC 
        && !mask
        && _cairo_matrix_is_integer_translation(&(src->base.matrix), 
-                                               &src_x_off, &src_y_off)
-       && _cairo_matrix_is_integer_translation(&(dst->base.matrix), 
-                                               &dst_x_off, &dst_y_off))
{
+                                               &src_x_off, &src_y_off)
&&
+       XGetGeometry (dst->dpy, dst->drawable, &dst_root,
+                     &ignore, &ignore, &u_ignore, &u_ignore, &u_ignore,
+                     &dst_depth) == Success &&
+       XGetGeometry (dst->dpy, src->drawable, &src_root,
+                     &ignore, &ignore, &u_ignore, &u_ignore, &u_ignore,
+                     &src_depth) == Success &&
+       dst_root == src_root && dst_depth == src_depth) {
        /* Fast path for copying "raw" areas. */

"not pretty" doesn't begin to describe that. It would be
considerably more efficient to trap errors and just try it ... one 
roundtrip instead of two. Or just assume that the server is going
to be OK at optimizing this case. Or, since we are already have most
of this information when creating the surface save that and use it.

But I don't think the above change is better than just commenting
out the fast path.

Generally, the changes look good ... noticed the problem with
prepare/restore surface and gradients last night but hadn't looked
into fixing it yet.

Regards,
					Owen


-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : http://lists.freedesktop.org/archives/cairo/attachments/20050128/4d498046/attachment.pgp