[cairo] Re: new clipping change patch

[Keith Packard]

Sorry, I hit some key and my previous message was sent before I was
done...

Evolution is not my favorite program today.


On Wed, 2005-05-25 at 09:49 -0400, Owen Taylor wrote:

> What the function does is gets the extents of the mask operation.
> (mask in the name is a verb, not a noun.) The mask argument isn't 
> currently used, but would be needed for the useful optimization:
> 
>  - If the mask is a surface pattern, with extend=NONE and the 
>    operator does nothing for transparent source (OVER, say)
>    then the size of the intermediate surface is bounded by the
>    size of the (transformed mask)

Ok, so the function is just incomplete.
> 
> (This optimization can be done for the source as well)
> 
> Yes, your patch doesn't change the end result of the function, my
> comment was that with the patch we end up doing:

I think the patch just demonstrates how silly the original code was.  If
you have a suggestion for what it should look like, I'd love to see it.
Otherwise, I'll attempt to figure out the intended algorithm and
implemen that.
 
>  
> > The clip serial number is relative to a specific surface; change the
> > surface, and you need to get a number relative to the new surface.  In
> > particular, the clip serial starts at 0 and until a surface is
> > associated, it will remain at 0.  If a clip region is applied before the
> > surface is associated, this 0 clip serial will inadvertantly match the
> > surface clip serial if that surface has no clip.
> 
> Could do with a comment :-). 
> 
>  /* allocate a new serial to represent our current state. Each 
>     surface has its own set of serials */

Thanks!

> say. I think my suggestion above with surface_set_clip_serial() does
> come out a bit better here, since you can just reset to 
> (unsigned int)-1.

See previous message as to why this isn't the best plan.

> > Given that we don't actually support changing surfaces, it might be
> > better to rework the gstate initialization code to include the surface
> > from the start...
> 
> Maybe leave the set_target_surface() code around until we see if we
> need it for groups or not.

Oh, good point.

> > >  * _cairo_xlib_surface_ensure_picture (surface, &surface->&src_picture);
> > > 
> > >   Maybe just split that into ensure_src_picture() and 
> > >   ensure_dst_picture()?
> > 
> > I suppose; this did save a function.  Would you accept a pair of macros?
> > Or is that too 80s?
> 
> I think GCC is going to do better with the pair of functions... if
> inlining makes sense, it has the power to do it.

Ok, I'll just do it right then.

-keith

-------------- 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/20050525/7c802fe5/attachment.pgp