[cairo] Font options patch, first draft

[Owen Taylor]

On Tue, 2005-07-19 at 17:48 -0400, Carl Worth wrote:
> On Mon, 18 Jul 2005 18:59:24 -0400, Owen Taylor wrote:
> > On Tue, 2005-07-19 at 00:50 +0200, Torsten Schoenfeld wrote:
> > > > void
> > > > cairo_surface_get_font_options (cairo_surface_t      *surface,
> > > >                                 cairo_font_options_t *options);
> > > > ===
> > > 
> > > I'm not sure this would map well to languages with namespace support:
> > > 
> > >   $surface->get_font_options ($options);
> > > 
> > > Getters usually don't modify their arguments -- they just return
> > > something.  I'm not really able to come up with a better name right now,
> > > but I think it should be something with 'merge', 'update', 'overwrite'
> > > or something along those lines.
> > 
> > This would map to a straightforward getter in language bindings; just
> > like the matrix functions and the font extents functions.
> > 
> >  $options = $surface->get_font_options ();
> > 
> > It's uglier than the font extents functions and (reformed) matrix
> > options because  cairo_font_options_t can't be stack allocated but
> > otherwise it follows the same pattern.
> 
> Did you consider modeling it after cairo_copy_path ?
> cairo_surface_copy_font_options ?

I'm not that fond of making the name of a function reflect memory
management ... it comes across strange in language bindings. 
For copy_path(), I think "copy" makes sense because there are
all sorts of behavior difference from what you might expect 
from a getter for the current path... we've converted back to
user space, etc.

The model here seems much more like cairo_get_matrix() ... we
are returning a structure "by value". More precisely, it's like
the old Cairo matrix API that I've reviled in the past. But we
don't really have any precedents for opaque value structures
to base off of, so I tried to conform to what we do for 
non-opaque value structures.

> > I didn't make it return a newly allocated object partially for this
> > consistency and partially for performance reasons: to cut down
> > just a tiny bit on the number of places where little blocks of
> > memory are allocated and freed.
> 
> Huh? It's an opaque type so the application still has to allocate it
> with cairo_font_options_create, right? (Though, I suppose the
> application could pool font options, but I don't really see this
> becoming a serious peformance issue).
> 
> To me it looks like the interface requires two functions
> (cairo_font_options_create and cairo_surface_get_font_options) where
> one would do just fine (cairo_surface_get_font_options).
> 
> Unless sharing the object with other calls is a natural part of the
> common expected usage of this function. I'm not totally clear on that
> myself yet.

The performance wasn't a big issue, really... it works out well within
Cairo (the toy API) because inside Cairo my patch stack allocates
cairo_font_options_t. In the one place I used it within Pango, 
it turned out that overwriting was a small win:

  if (!info->surface_options)
    info->surface_options = cairo_font_options_create ();

  target = cairo_get_target (cr);
  cairo_surface_get_font_options (target, info->surface_options);

Would be:

  if (info->surface_options)
    cairo_font_options_free (info->surface_options);

  target = cairo_get_target (cr);
  info->surface_options = cairo_surface_get_font_options (target);

But it is pretty marginal .. I'd be surprised if it showed up on
profiles either way.

The main concerns I had were avoiding the explosion of memory management
models and keeping things natural for language bindings.

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/20050719/ed3d0c08/attachment.pgp