[cairo] Re: new clipping change patch
otaylor at redhat.com
Tue May 24 11:48:18 PDT 2005
On Mon, 2005-05-23 at 23:28 -0700, Keith Packard wrote:
> After our IRC discussion today, I went and rewrote the clipping code
> again to pull most of the clipping work into the gstate where it belongs.
> The surface now just reflects state down to the backend, the only thing
> retained is the serial number for the clipping that was set.
> Extents for the clip are kept up in the gstate; the surface will only
> return the extents for the entire object.
> My email is dead until tomorrow sometime; bad things happened to my
> home box this week and my ISP disabled my line as a result...
> Patch available at http:/freedesktop.org/~keithp/cairo-serialclip-2.diff
> I should be back on-line sometime tomorrow; failing that, I'll rework
> email so I can get at it somehow.
Hopefully this mail will get through to you one way or the other.
I like this version of the patch a lot more than the last one.
* I'm not sure the name change cairo_surface_set_clip_region() =>
cairo_surface_clip_region() makes sense. It's still a setter...
there are no plans to make clip_region() intersect with the
current clip region.
* The code goes through various gymnastics to make sure that when
restoring to a previous clip serial, the same serial is used.
I don't see why this is useful. I'd expect
_cairo_gstate_set_clip() to look like:
if (_cairo_surface_get_clip_serial (surface) == gstate->clip.serial)
/* reestablish the clip */
gstate->clip.serial = _cairo_surface_get_clip_serial (surface);
And leave all updating of the serial internal to the surface.
The "no clip gets 0 serial" optimization can still be done this
* The duplication of rectangle intersection code one my time in
_cairo_gstate_get_clip_extents() is painful. That really should
be split out somewhere. (boolean return: FALSE on empty intersection)
Or alternatively, for get_clip_extents(), do the intersection
on regions not rectangles.
* cairo-gstate.c:_get_mask_extents() immediately intersects the
result of get_clip_extents() with the clip region (as a region).
Which is duplication one way or the other.
* I don't understand the logic in _cairo_gstate_set_target_surface()
* _cairo_xlib_surface_ensure_picture (surface, &surface->&src_picture);
Maybe just split that into ensure_src_picture() and
* In _cairo_xlib_surface_set_attributes(), the call to ensure_picture()
is inserted between a call that sets status and the subsequent
check of status. It should be inside the if (CAIRO_OK (status))
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Size: 189 bytes
Desc: This is a digitally signed message part
Url : http://lists.freedesktop.org/archives/cairo/attachments/20050524/d3f6ef6b/attachment.pgp
More information about the cairo