[cairo] updated, hopefully with propr line-wrapping [PATCH] add extents to clone_similar

[Carl Worth]

On Thu, 5 Oct 2006 15:28:16 -0400, "Christopher \"Monty\" Montgomery" wrote:
> Carl said previous attempt got its line wrapping slagged.  One more try...

Still wrapped. But no need to re-send. Just something you can fix for
next time.

> @@ -563,7 +563,16 @@ _cairo_clip_init_deep_copy (cairo_clip_t
>          }
>
>          if (other->surface) {
> -            _cairo_surface_clone_similar (target, clip->surface,
> &clip->surface);
> +            _cairo_surface_clone_similar (target, clip->surface,
> +					  // is this more or less
> +					  // correct than just cloning
> +					  // the entire surface
> +					  // rectangle? --Monty
> +					  clip->surface_rect.x,
> +					  clip->surface_rect.y,
> +					  clip->surface_rect.width,
> +					  clip->surface_rect.height,
> +					  &clip->surface);
>              clip->surface_rect = other->surface_rect;
>          }

It would be better to raise questions like this in the email you send,
rather than burying them in the patch. That way, if we agree with what
you decided anyway, (as I do in this case), then I could just accept
your patch without having to change it at all. (And if I disagree,
then I can just bounce it and have you fix it.) It's all about making
my job easier, right?

Oh, and in cairo we don't use those C++-inspired (and C99-approved) //
comment thingies. (But it looks like I didn't mention that in
cairo/CODING_STYLE).

For now, I've just removed that question and let the code stand. I'm
pretty sure it's correct. Vlad, want to double-check
_cairo_clip_init_copy ?

> @@ -588,6 +588,10 @@ _cairo_xcb_surface_same_screen (cairo_xc
>  static cairo_status_t
>  _cairo_xcb_surface_clone_similar (void			*abstract_surface,
>  				  cairo_surface_t	*src,
> +				  int                    src_x,
> +				  int                    src_y,
> +				  int                    width,
> +				  int                    height,
>  				  cairo_surface_t     **clone_out)
>  {
>      cairo_xcb_surface_t *surface = abstract_surface;
> @@ -614,6 +618,11 @@ _cairo_xcb_surface_clone_similar (void
>  	if (clone->base.status)
>  	    return CAIRO_STATUS_NO_MEMORY;
>
> +	/* can't apply extents; no manpages for XCBPutImage and xcb
> +	source from freedesktop currently won't build.  XCBPutImage is not
> +	referenced in the XCB source from xcb.freedesktop.org/dist
> +	anywhere. */
> +
>  	_draw_image_surface (clone, image_src, 0, 0);

The XCB hackers have been pretty active lately. Jamey and Ian, want to
take a look at fixing this? Until it's fixed there will be a bad
performance bug here, (which cairo-perf should kindly point out for
you with the subimage_copy test case).

So this patch has been pushed out now and can be seen here:

http://gitweb.freedesktop.org/?p=cairo;a=commit;h=8d7a02ed58e06584eb09575e6ca11d0a81094ab6

Note the lovely little performance chart we get in the commit
message. I'd like to make it a requirement for future performance
improvements to have similar little charts, (just use cairo-perf and
cairo-perf-diff to generate them).

Sorry for the delay on applying this, (and yes, part of the reason was
that I wanted good charting like this so that we could document
intended performance improvements in the code history).

-Carl
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://lists.freedesktop.org/archives/cairo/attachments/20061018/d2e66bc3/attachment.pgp